>> static u64 __calculate_sltime(struct kvm_vcpu *vcpu)
>> {
>> - u64 now, cputm, sltime = 0;
>> + u64 now, cputm, ckc, sltime = 0;
>> + int64_t ckc_signed, now_signed;
>>
>> if (ckc_interrupts_enabled(vcpu)) {
>> - now = kvm_s390_get_tod_clock_fast(vcpu->kvm);
>> - sltime = tod_to_ns(vcpu->arch.sie_block->ckc - now);
>> - /* already expired or overflow? */
>> - if (!sltime || vcpu->arch.sie_block->ckc <= now)
>> + if (vcpu->arch.sie_block->gcr[0] & 0x0020000000000000ul &&
>> + test_kvm_facility(vcpu->kvm, 139)) {
>> + now = kvm_s390_get_tod_clock_fast(vcpu->kvm);
>> + ckc = vcpu->arch.sie_block->ckc;
>
>
> Shouldn't you be using now_signed and ckc_signed here?
Yes indeed.
>
>
>> + if (ckc < now)
>> + sltime = tod_to_ns(now - ckc);
>> + } else {
>> + now_signed = kvm_s390_get_tod_clock_fast(vcpu->kvm);
>> + ckc_signed = vcpu->arch.sie_block->ckc;
>
> and the unsigned ones here?
>
> Also you could just compare vcpu->arch.sie_block->ckc and
> kvm_s390_get_tod_clock_fast(vcpu->kvm)
That leads to ugly long lines (for the comparison and the calculation)
>
>> + if (ckc_signed < now_signed)
>> + sltime = tod_to_ns(now_signed - ckc_signed);
>
>
> Shouldn't we only calculate sleep time if ckc is greater than now (in
> both cases)?
>
I think you're right!
(this was a 5min hack originally titled RFC, but I screwed up my command
line because I was sending a QEMU patch in between)
Will resend, thanks for having a look! :)
>
>> + }
>> + /* already expired */
>> + if (!sltime)
>> return 0;
>> if (cpu_timer_interrupts_enabled(vcpu)) {
>> cputm = kvm_s390_get_cpu_timer(vcpu);
>
> Other than that, this is a heck of a lot easier to read than what we had
> before.
>
--
Thanks,
David / dhildenb
