>> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-s390" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html