On 04/06/2016 02:42, Yunhong Jiang wrote:
> +void kvm_lapic_expired_hv_timer(struct kvm_vcpu *vcpu)
> +{
> + struct kvm_lapic *apic = vcpu->arch.apic;
> +
> + WARN_ON(!apic->lapic_timer.hv_timer_in_use);
> + WARN_ON(swait_active(&vcpu->wq));
> + kvm_x86_ops->cancel_hv_timer(vcpu);
> + apic->lapic_timer.hv_timer_in_use = 0;
Please use "false" here instead of 0.
> + apic_timer_expired(apic);
> +}
> +EXPORT_SYMBOL_GPL(kvm_lapic_expired_hv_timer);
> +
>
> + if (!kvm_x86_ops->set_hv_timer(apic->vcpu, tscdeadline)) {
> + apic->lapic_timer.hv_timer_in_use = true;
> + hrtimer_cancel(&apic->lapic_timer.timer);
> + }
> +
> + /* In case the sw timer triggered in above small window */
> + if (atomic_read(&apic->lapic_timer.pending) &&
> + apic->lapic_timer.hv_timer_in_use)
> + kvm_x86_ops->cancel_hv_timer(apic->vcpu);
Put this "if" after hrtimer_cancel, and you can avoid the "&&
apic->lapic_timer.hv_timer_in_use". Also, I think it's clear to add an
extra
apic->lapic_timer.hv_timer_in_use = false;
here, since you have it in kvm_lapic_expired_hv_timer.
> +void vmx_arm_hv_timer(struct kvm_vcpu *vcpu)
> +{
> + struct kvm_lapic *apic = vcpu->arch.apic;
> + u64 tscl;
> + u32 delta_tsc;
> +
> + if (!apic->lapic_timer.hv_timer_in_use)
Can you change this to something like vcpu->arch.hv_deadline_tsc != -1
(by adjusting set_hv_timer and cancel_hv_timer)?
> + return;
> +
> + tscl = rdtsc();
> + if (vcpu->arch.hv_deadline_tsc > tscl)
> + /* sure to be 32 bit only because checked on set_hv_timer */
/* vmx_set_hv_timer checks that the delta fits in 32 bits */
> + delta_tsc = (u32)((vcpu->arch.hv_deadline_tsc - tscl) >>
> + cpu_preemption_timer_multi);
> + else
> + delta_tsc = 0;
> +
> + vmcs_write32(VMX_PREEMPTION_TIMER_VALUE, delta_tsc);
> +}
> +
...
> + u64 tscl = rdtsc();
> s64 tsc_delta = !vcpu->arch.last_host_tsc ? 0 :
> - rdtsc() - vcpu->arch.last_host_tsc;
> + tscl - vcpu->arch.last_host_tsc;
> if (tsc_delta < 0)
> mark_tsc_unstable("KVM discovered backwards TSC");
> +
> + /*
> + * If tsc backwrap, we need update the hv_deadline_tsc,
> + * otherwise, the ((hv_deadline_tsc-tsc) >>
> + * cpu_preemption_timer_multi) may >32bit on vcpu_run().
> + * This may cause deadline_tsc not so accurate, but that's
> + * inevitable anyway.
> + */
> + if (tscl < vcpu->arch.hv_orig_tsc) {
> + vcpu->arch.hv_orig_tsc = tscl;
> + vcpu->arch.hv_deadline_tsc -=
> + vcpu->arch.hv_orig_tsc - tscl;
> + }
Can you instead call vmx_set_hv_timer again (and if it returns an error,
call kvm_lapic_switch_to_sw_timer)? Perhaps it should even be done
unconditionally on vmx_vcpu_load.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
