Pin the hrtimer used for VMX-preemption timer emulation
cover Pin the hrtimer used for VMX-preemption timer emulation
I'm still not entirely convinced that the Linux hrtimer can be used to accurately emulate the VMX-preemption timer, but it definitely doesn't work if the hrtimer is on a different logical processor from the vCPU thread that needs to get kicked out of VMX non-root operation.
With these changes, the kvm-unit-test (sent separately) that verifies that a guest can't actually observe a delayed VMX-preemption timer VM-exit passes 99.999% of the time on a 2GHz Skylake system.
It might be possible to improve that pass rate even more by increasing the scaling factor in the virtual IA32_VMX_MISC[4:0], but you'd have to be even more of a stickler than I to go to that extreme.
By the way, what is the point of migrating the hrtimers for the APIC and the PIT, since they aren't even pinned to begin with?
The subject line of the first patch was crafted for you, Sean. :-D
Jim Mattson (3):
KVM: nVMX: Really make emulated nested preemption timer pinned
KVM: nVMX: Change emulated VMX-preemption timer hrtimer to absolute
KVM: nVMX: Migrate the VMX-preemption timer
arch/x86/include/asm/kvm_host.h | 2 ++ arch/x86/kvm/irq.c | 2 ++ arch/x86/kvm/vmx/nested.c | 5 +++-- arch/x86/kvm/vmx/vmx.c | 11 +++++++++++ 4 files changed, 18 insertions(+), 2 deletions(-)
Signature · 1 line
2.26.2.645.ge9eca65c58-goog
PATCH 1/3 KVM: nVMX: Really make emulated nested preemption timer pinned
The PINNED bit is ignored by hrtimer_init. It is only considered when starting the timer.
When the hrtimer isn't pinned to the same logical processor as the vCPU thread to be interrupted, the emulated VMX-preemption timer often fails to adhere to the architectural specification.
--- arch/x86/kvm/vmx/nested.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/kvm/vmx/nested.c +1 -1
@@ -2041,7 +2041,7 @@ static void vmx_start_preemption_timer(struct kvm_vcpu *vcpu) preemption_timeout *=1000000 ;do_div (preemption_timeout, vcpu->arch.virtual_tsc_khz);hrtimer_start (&vmx->nested.preemption_timer, -ns_to_ktime (preemption_timeout),HRTIMER_MODE_REL ); +ns_to_ktime (preemption_timeout),HRTIMER_MODE_REL_PINNED ); }static u64 nested_vmx_calc_efer (struct vcpu_vmx *vmx,struct vmcs12 *vmcs12)
Signature · 1 line
2.26.2.645.ge9eca65c58-goog
Re: [PATCH 1/3] KVM: nVMX: Really make emulated nested preemption timer pinned
LOL, love the shortlog :-)
Quoted 31 lines · Jim Mattson
The PINNED bit is ignored by
hrtimer_init. It is only considered when starting the timer.When the hrtimer isn't pinned to the same logical processor as the vCPU thread to be interrupted, the emulated VMX-preemption timer often fails to adhere to the architectural specification.
Fixes f15a75eedc18e ("KVM: nVMX: make emulated nested preemption timer pinned")Signed-off-by Jim Mattson <[email protected]>Reviewed-by Peter Shier <[email protected]>Reviewed-by Oliver Upton <[email protected]>--- arch/x86/kvm/vmx/nested.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)arch/x86/kvm/vmx/nested.c +1
-1@@ -2041,7 +2041,7 @@ static void vmx_start_preemption_timer(struct kvm_vcpu *vcpu) preemption_timeout *=1000000 ;do_div (preemption_timeout, vcpu->arch.virtual_tsc_khz);hrtimer_start (&vmx->nested.preemption_timer, -ns_to_ktime (preemption_timeout),HRTIMER_MODE_REL ); +ns_to_ktime (preemption_timeout),HRTIMER_MODE_REL_PINNED ); }static u64 nested_vmx_calc_efer (struct vcpu_vmx *vmx,struct vmcs12 *vmcs12)Signature · 1 line
2.26.2.645.ge9eca65c58-goog
PATCH 2/3 KVM: nVMX: Change emulated VMX-preemption timer hrtimer to absolute
In reply to Jim MattsonPrepare for migration of this hrtimer, by changing it from relative to absolute. (I couldn't get migration to work with a relative timer.)
--- arch/x86/kvm/vmx/nested.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
arch/x86/kvm/vmx/nested.c +3 -2
@@ -2041,7 +2041,8 @@ static void vmx_start_preemption_timer(struct kvm_vcpu *vcpu) preemption_timeout *=1000000 ;do_div (preemption_timeout, vcpu->arch.virtual_tsc_khz);hrtimer_start (&vmx->nested.preemption_timer, -ns_to_ktime (preemption_timeout),HRTIMER_MODE_REL_PINNED ); +ktime_add_ns (ktime_get (), preemption_timeout), +HRTIMER_MODE_ABS_PINNED ); }static u64 nested_vmx_calc_efer (struct vcpu_vmx *vmx,struct vmcs12 *vmcs12)
@@ -4614,7 +4615,7 @@ static int enter_vmx_operation(struct kvm_vcpu *vcpu) goto out_shadow_vmcs;hrtimer_init (&vmx->nested.preemption_timer,CLOCK_MONOTONIC , -HRTIMER_MODE_REL_PINNED ); +HRTIMER_MODE_ABS_PINNED ); vmx->nested.preemption_timer.function = vmx_preemption_timer_fn; vmx->nested.vpid02 =allocate_vpid ();
Signature · 1 line
2.26.2.645.ge9eca65c58-goog
PATCH 3/3 KVM: nVMX: Migrate the VMX-preemption timer
In reply to Jim MattsonThe hrtimer used to emulate the VMX-preemption timer must be pinned to the same logical processor as the vCPU thread to be interrupted if we want to have any hope of adhering to the architectural specification of the VMX-preemption timer. Even with this change, the emulated VMX-preemption timer VM-exit occasionally arrives too late.
--- arch/x86/include/asm/kvm_host.h | 2 ++ arch/x86/kvm/irq.c | 2 ++ arch/x86/kvm/vmx/vmx.c | 11 +++++++++++ 3 files changed, 15 insertions(+)
arch/x86/include/asm/kvm_host.h +2 -0
@@ -1254,6 +1254,8 @@ struct kvm_x86_ops {bool (*apic_init_signal_blocked)(struct kvm_vcpu *vcpu);int (*enable_direct_tlbflush)(struct kvm_vcpu *vcpu); + +void (*migrate_timers)(struct kvm_vcpu *vcpu); };struct kvm_x86_init_ops {
arch/x86/kvm/irq.c +2 -0
@@ -159,6 +159,8 @@ void __kvm_migrate_timers(struct kvm_vcpu *vcpu) {__kvm_migrate_apic_timer (vcpu);__kvm_migrate_pit_timer (vcpu); +if (kvm_x86_ops.migrate_timers) + kvm_x86_ops.migrate_timers (vcpu); }bool kvm_arch_irqfd_allowed (struct kvm *kvm,struct kvm_irqfd *args)
arch/x86/kvm/vmx/vmx.c +11 -0
@@ -7687,6 +7687,16 @@ static bool vmx_apic_init_signal_blocked(struct kvm_vcpu *vcpu)return to_vmx (vcpu)->nested.vmxon; } +static void vmx_migrate_timers (struct kvm_vcpu *vcpu) +{ +if (is_guest_mode (vcpu)) { +struct hrtimer *timer = &to_vmx (vcpu)->nested.preemption_timer; + +if (hrtimer_try_to_cancel (timer) ==1 ) +hrtimer_start_expires (timer,HRTIMER_MODE_ABS_PINNED ); + } +} +static void hardware_unsetup (void ) {if (nested)
@@ -7838,6 +7848,7 @@ static struct kvm_x86_ops vmx_x86_ops __initdata = { .nested_get_evmcs_version = NULL, .need_emulation_on_page_fault = vmx_need_emulation_on_page_fault, .apic_init_signal_blocked = vmx_apic_init_signal_blocked, + .migrate_timers = vmx_migrate_timers, };static __init int hardware_setup (void )
Signature · 1 line
2.26.2.645.ge9eca65c58-goog
Re: [PATCH 3/3] KVM: nVMX: Migrate the VMX-preemption timer
Quoted 64 lines · Jim Mattson
The hrtimer used to emulate the VMX-preemption timer must be pinned to the same logical processor as the vCPU thread to be interrupted if we want to have any hope of adhering to the architectural specification of the VMX-preemption timer. Even with this change, the emulated VMX-preemption timer VM-exit occasionally arrives too late.
Signed-off-by Jim Mattson <[email protected]>Reviewed-by Peter Shier <[email protected]>Reviewed-by Oliver Upton <[email protected]>--- arch/x86/include/asm/kvm_host.h | 2 ++ arch/x86/kvm/irq.c | 2 ++ arch/x86/kvm/vmx/vmx.c | 11 +++++++++++ 3 files changed, 15 insertions(+)arch/x86/include/asm/kvm_host.h +2
-0@@ -1254,6 +1254,8 @@ struct kvm_x86_ops {bool (*apic_init_signal_blocked)(struct kvm_vcpu *vcpu);int (*enable_direct_tlbflush)(struct kvm_vcpu *vcpu); + +void (*migrate_timers)(struct kvm_vcpu *vcpu); };struct kvm_x86_init_ops {arch/x86/kvm/irq.c +2
-0@@ -159,6 +159,8 @@ void __kvm_migrate_timers(struct kvm_vcpu *vcpu) {__kvm_migrate_apic_timer (vcpu);__kvm_migrate_pit_timer (vcpu); +if (kvm_x86_ops.migrate_timers) + kvm_x86_ops.migrate_timers (vcpu); }bool kvm_arch_irqfd_allowed (struct kvm *kvm,struct kvm_irqfd *args)arch/x86/kvm/vmx/vmx.c +10
-0@@ -7687,6 +7687,16 @@ static bool vmx_apic_init_signal_blocked(struct kvm_vcpu *vcpu)return to_vmx (vcpu)->nested.vmxon; } +static void vmx_migrate_timers (struct kvm_vcpu *vcpu) +{ +if (is_guest_mode (vcpu)) { +struct hrtimer *timer = &to_vmx (vcpu)->nested.preemption_timer; + +if (hrtimer_try_to_cancel (timer) ==1 ) +hrtimer_start_expires (timer,HRTIMER_MODE_ABS_PINNED ); + } +} +static void hardware_unsetup (void ) {if (nested)
arch/x86/kvm/vmx/vmx.c static struct kvm_x86_ops vmx_x86_ops __initdata = { · line 7854
Quoted 7 lines
arch/x86/kvm/vmx/vmx.c +1
-0@@ -7838,6 +7848,7 @@ static struct kvm_x86_ops vmx_x86_ops __initdata = { .nested_get_evmcs_version = NULL, .need_emulation_on_page_fault = vmx_need_emulation_on_page_fault, .apic_init_signal_blocked = vmx_apic_init_signal_blocked, + .migrate_timers = vmx_migrate_timers, };static __init int hardware_setup (void )
Queued all, thanks.
Paolo
Paolo Bonzini
maintainerQuoted 7 lines · Jim Mattson
It might be possible to improve that pass rate even more by increasing the scaling factor in the virtual IA32_VMX_MISC[4:0], but you'd have to be even more of a stickler than I to go to that extreme.
By the way, what is the point of migrating the hrtimers for the APIC and the PIT, since they aren't even pinned to begin with?
Unless housekeeping CPUs for timers are configured, the timer will run on the CPU it is started on, even without pinning; see get_nohz_timer_target.
Paolo
Quoted 3 lines · Jim Mattson
I'm still not entirely convinced that the Linux hrtimer can be used to accurately emulate the VMX-preemption timer...
It can't, for several reasons:
1) The conversion between wall-clock time and TSC frequency, based on tsc_khz, isn't precise enough.
2) The base clock for the hrtimer, CLOCK_MONOTONIC, can be slewed, whereas the TSC cannot.
3) The VMX-preemption timer is suspended during MWAIT; the hrtimer is not.
Is there any reason that VMX-preemption timer emulation shouldn't just be a second client of the hv_timer, along with lapic timer emulation?
Paolo Bonzini
maintainerQuoted 7 lines · Jim Mattson
Quoted 3 lines · Jim Mattson
I'm still not entirely convinced that the Linux hrtimer can be used to accurately emulate the VMX-preemption timer...
It can't, for several reasons:
Quoted 8 lines
1) The conversion between wall-clock time and TSC frequency, based on
tsc_khz, isn't precise enough.
2) The base clock for the hrtimer,CLOCK_MONOTONIC, can be slewed, whereas the TSC cannot.
3) The VMX-preemption timer is suspended during MWAIT; the hrtimer is not.Is there any reason that VMX-preemption timer emulation shouldn't just be a second client of the
hv_timer, along with lapic timer emulation?
I don't think so, you'd just need logic to multiplex it.
Paolo