KVM: VMX: Execute WBINVD to keep data consistency with assigned devices
PATCH v6 1/2 KVM: VMX: Execute WBINVD to keep data consistency with assigned devices
Some guest device driver may leverage the "Non-Snoop" I/O, and explicitly WBINVD or CLFLUSH to a RAM space. Since migration may occur before WBINVD or CLFLUSH, we need to maintain data consistency either by:
1: flushing cache (wbinvd) when the guest is scheduled out if there is no wbinvd exit, or
2: execute wbinvd on all dirty physical CPUs when guest wbinvd exits.
Signed-off-by Yaozu (Eddie) Dong <[email protected]>
Signed-off-by Sheng Yang <[email protected]>
--- arch/x86/include/asm/kvm_host.h | 6 +++++ arch/x86/kvm/emulate.c | 5 +++- arch/x86/kvm/svm.c | 7 ++++++ arch/x86/kvm/vmx.c | 10 ++++++++- arch/x86/kvm/x86.c | 41 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 67 insertions(+), 2 deletions(-)
arch/x86/include/asm/kvm_host.h +6 -0
@@ -15,6 +15,7 @@#include <linux/mm.h> #include <linux/mmu_notifier.h> #include <linux/tracepoint.h> +#include <linux/cpumask.h> #include <linux/kvm.h> #include <linux/kvm_para.h>
@@ -358,6 +359,8 @@ struct kvm_vcpu_arch {/* fields used by HYPER-V emulation */ u64 hv_vapic; + +cpumask_var_t wbinvd_dirty_mask; };struct kvm_arch {
@@ -514,6 +517,8 @@ struct kvm_x86_ops {void (*set_supported_cpuid)(u32 func,struct kvm_cpuid_entry2 *entry); +bool (*has_wbinvd_exit)(void ); +const struct trace_print_flags *exit_reasons_str; };
@@ -571,6 +576,7 @@ void kvm_emulate_cpuid(struct kvm_vcpu *vcpu);int kvm_emulate_halt (struct kvm_vcpu *vcpu);int emulate_invlpg (struct kvm_vcpu *vcpu,gva_t address);int emulate_clts (struct kvm_vcpu *vcpu); +int kvm_emulate_wbinvd (struct kvm_vcpu *vcpu);void kvm_get_segment (struct kvm_vcpu *vcpu,struct kvm_segment *var,int seg);int kvm_load_segment_descriptor (struct kvm_vcpu *vcpu,u16 selector,int seg);
arch/x86/kvm/emulate.c +4 -1
@@ -3138,8 +3138,11 @@ twobyte_insn:emulate_clts (ctxt->vcpu); c->dst.type =OP_NONE ;break ; -case 0x08 :/* invd */ case 0x09 :/* wbinvd */ +kvm_emulate_wbinvd (ctxt->vcpu); + c->dst.type =OP_NONE ; +break ; +case 0x08 :/* invd */ case 0x0d :/* GrpP (prefetch) */ case 0x18 :/* Grp16 (prefetch/nop) */ c->dst.type =OP_NONE ;
arch/x86/kvm/svm.c +7 -0
@@ -3424,6 +3424,11 @@ static bool svm_rdtscp_supported(void)return false; } +static bool svm_has_wbinvd_exit (void ) +{ +return true; +} +static void svm_fpu_deactivate (struct kvm_vcpu *vcpu) {struct vcpu_svm *svm =to_svm (vcpu);
@@ -3508,6 +3513,8 @@ static struct kvm_x86_ops svm_x86_ops = { .rdtscp_supported = svm_rdtscp_supported, .set_supported_cpuid = svm_set_supported_cpuid, + + .has_wbinvd_exit = svm_has_wbinvd_exit, };static int __init svm_init (void )
arch/x86/kvm/vmx.c +9 -1
@@ -412,6 +412,12 @@ static inline bool cpu_has_virtual_nmis(void)return vmcs_config.pin_based_exec_ctrl &PIN_BASED_VIRTUAL_NMIS ; } +static inline bool cpu_has_vmx_wbinvd_exit (void ) +{ +return vmcs_config.cpu_based_2nd_exec_ctrl & +SECONDARY_EXEC_WBINVD_EXITING ; +} +static inline bool report_flexpriority (void ) {return flexpriority_enabled;
@@ -3400,7 +3406,7 @@ static int handle_invlpg(struct kvm_vcpu *vcpu)static int handle_wbinvd (struct kvm_vcpu *vcpu) {skip_emulated_instruction (vcpu); -/* TODO: Add support for VT-d/pass-through device */ +kvm_emulate_wbinvd (vcpu);return 1 ; }
@@ -4350,6 +4356,8 @@ static struct kvm_x86_ops vmx_x86_ops = { .rdtscp_supported = vmx_rdtscp_supported, .set_supported_cpuid = vmx_set_supported_cpuid, + + .has_wbinvd_exit = cpu_has_vmx_wbinvd_exit, };static int __init vmx_init (void )
arch/x86/kvm/x86.c +41 -0
68 lines not on this page · raw message
Signature · 1 line
1.7.0.1
PATCH v6 2/2 KVM: Using workqueue for WBINVD
It would buy us the ability to schedule compared to smp_call_function().
Signed-off-by Sheng Yang <[email protected]>
---
But I am not sure if it worth the complexity. Anyway WBINVD itself can't be interrupted, so the benefit should come to the caller cpu I think. And that would extended the waiting time for the caller cpu, for we still need to wait other cpus to complete their works.
arch/x86/include/asm/kvm_host.h | 1 + arch/x86/kvm/x86.c | 35 ++++++++++++++++++++++++++++------- 2 files changed, 29 insertions(+), 7 deletions(-)
arch/x86/include/asm/kvm_host.h +1 -0
@@ -16,6 +16,7 @@#include <linux/mmu_notifier.h> #include <linux/tracepoint.h> #include <linux/cpumask.h> +#include <linux/workqueue.h> #include <linux/kvm.h> #include <linux/kvm_para.h>
arch/x86/kvm/x86.c +28 -7
@@ -153,6 +153,9 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {u64 __read_mostly host_xcr0; +static struct workqueue_struct *wbinvd_wq; +static DEFINE_PER_CPU(struct work_struct, wbinvd_work); +static inline u32 bit (int bitno) {return 1 << (bitno &31 );
@@ -1783,7 +1786,7 @@ out:return r; } -static void wbinvd_ipi (void *garbage) +static void wbinvd_do_work (struct work_struct *work) {wbinvd (); }
@@ -1800,9 +1803,11 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)if (need_emulate_wbinvd (vcpu)) {if (kvm_x86_ops->has_wbinvd_exit ())cpumask_set_cpu (cpu, vcpu->arch.wbinvd_dirty_mask); -else if (vcpu->cpu !=-1 && vcpu->cpu != cpu) -smp_call_function_single (vcpu->cpu, - wbinvd_ipi,NULL ,1 ); +else if (vcpu->cpu !=-1 && vcpu->cpu != cpu) { +queue_work_on (vcpu->cpu, wbinvd_wq, + &per_cpu (wbinvd_work, vcpu->cpu)); +flush_workqueue (wbinvd_wq); + } } kvm_x86_ops->vcpu_load (vcpu, cpu);
@@ -3672,12 +3677,16 @@ int emulate_invlpg(struct kvm_vcpu *vcpu, gva_t address)int kvm_emulate_wbinvd (struct kvm_vcpu *vcpu) { +int cpu; +if (!need_emulate_wbinvd (vcpu))return X86EMUL_CONTINUE ;if (kvm_x86_ops->has_wbinvd_exit ()) { -smp_call_function_many (vcpu->arch.wbinvd_dirty_mask, - wbinvd_ipi,NULL ,1 ); +for_each_cpu (cpu, vcpu->arch.wbinvd_dirty_mask) +queue_work_on (cpu, wbinvd_wq, + &per_cpu (wbinvd_work, cpu)); +flush_workqueue (wbinvd_wq);cpumask_clear (vcpu->arch.wbinvd_dirty_mask); }wbinvd ();
@@ -4179,7 +4188,7 @@ EXPORT_SYMBOL_GPL(kvm_after_handle_nmi);int kvm_arch_init (void *opaque) { -int r; +int r, cpu;struct kvm_x86_ops *ops = (struct kvm_x86_ops *)opaque;if (kvm_x86_ops) {
@@ -4218,6 +4227,16 @@ int kvm_arch_init(void *opaque)if (cpu_has_xsave) host_xcr0 =xgetbv (XCR_XFEATURE_ENABLED_MASK ); +for_each_possible_cpu (cpu) { +INIT_WORK (&per_cpu (wbinvd_work, cpu), wbinvd_do_work); + } + + wbinvd_wq =create_workqueue ("kvm_wbinvd" ); +if (!wbinvd_wq) { + r = -ENOMEM ; + goto out; + } +return 0 ; out:
@@ -4226,6 +4245,8 @@ out:void kvm_arch_exit (void ) { +destroy_workqueue (wbinvd_wq); +perf_unregister_guest_info_callbacks (&kvm_guest_cbs);if (!boot_cpu_has (X86_FEATURE_CONSTANT_TSC ))
Signature · 1 line
1.7.0.1
Re: [PATCH v6 2/2] KVM: Using workqueue for WBINVD
Quoted 9 lines · Sheng Yang
It would buy us the ability to schedule compared to
smp_call_function().Signed-off-by Sheng Yang <[email protected]>---
But I am not sure if it worth the complexity. Anyway WBINVD itself can't be interrupted, so the benefit should come to the caller cpu I think. And that would extended the waiting time for the caller cpu, for we still need to wait other cpus to complete their works.
Alright, can be done later in case it shows to be necessary.
Quoted 41 lines
arch/x86/include/asm/kvm_host.h | 1 + arch/x86/kvm/x86.c | 35 ++++++++++++++++++++++++++++------- 2 files changed, 29 insertions(+), 7 deletions(-)arch/x86/include/asm/kvm_host.h +1
-0@@ -16,6 +16,7 @@#include <linux/mmu_notifier.h> #include <linux/tracepoint.h> #include <linux/cpumask.h> +#include <linux/workqueue.h> #include <linux/kvm.h> #include <linux/kvm_para.h> arch/x86/kvm/x86.c +4
-1@@ -153,6 +153,9 @@ struct kvm_stats_debugfs_item debugfs_entries[] = {u64 __read_mostly host_xcr0; +static struct workqueue_struct *wbinvd_wq; +static DEFINE_PER_CPU(struct work_struct, wbinvd_work); +static inline u32 bit (int bitno) {return 1 << (bitno &31 );@@ -1783,7 +1786,7 @@ out:return r; } -static void wbinvd_ipi (void *garbage) +static void wbinvd_do_work (struct work_struct *work) {wbinvd (); }@@ -1800,9 +1803,11 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)if (need_emulate_wbinvd (vcpu)) {
arch/x86/kvm/x86.c void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu) · line 1810
Quoted 10 lines
arch/x86/kvm/x86.c +5
-3if (kvm_x86_ops->has_wbinvd_exit ())cpumask_set_cpu (cpu, vcpu->arch.wbinvd_dirty_mask); -else if (vcpu->cpu !=-1 && vcpu->cpu != cpu) -smp_call_function_single (vcpu->cpu, - wbinvd_ipi,NULL ,1 ); +else if (vcpu->cpu !=-1 && vcpu->cpu != cpu) { +queue_work_on (vcpu->cpu, wbinvd_wq, + &per_cpu (wbinvd_work, vcpu->cpu)); +flush_workqueue (wbinvd_wq); + }
Can't schedule here since preemption must be disabled during vcpu_load.
Quoted 22 lines
arch/x86/kvm/x86.c +6
-2} kvm_x86_ops->vcpu_load (vcpu, cpu);@@ -3672,12 +3677,16 @@ int emulate_invlpg(struct kvm_vcpu *vcpu, gva_t address)int kvm_emulate_wbinvd (struct kvm_vcpu *vcpu) { +int cpu; +if (!need_emulate_wbinvd (vcpu))return X86EMUL_CONTINUE ;if (kvm_x86_ops->has_wbinvd_exit ()) { -smp_call_function_many (vcpu->arch.wbinvd_dirty_mask, - wbinvd_ipi,NULL ,1 ); +for_each_cpu (cpu, vcpu->arch.wbinvd_dirty_mask) +queue_work_on (cpu, wbinvd_wq, + &per_cpu (wbinvd_work, cpu)); +flush_workqueue (wbinvd_wq);cpumask_clear (vcpu->arch.wbinvd_dirty_mask); }wbinvd ();
Marcelo Tosatti
AppliedQuoted 9 lines · Sheng Yang
Some guest device driver may leverage the "Non-Snoop" I/O, and explicitly WBINVD or CLFLUSH to a RAM space. Since migration may occur before WBINVD or CLFLUSH, we need to maintain data consistency either by:
1: flushing cache (wbinvd) when the guest is scheduled out if there is no wbinvd exit, or
2: execute wbinvd on all dirty physical CPUs when guest wbinvd exits.Signed-off-by Yaozu (Eddie) Dong <[email protected]>Signed-off-by Sheng Yang <[email protected]>
Quoted 7 lines
--- arch/x86/include/asm/kvm_host.h | 6 +++++ arch/x86/kvm/emulate.c | 5 +++- arch/x86/kvm/svm.c | 7 ++++++ arch/x86/kvm/vmx.c | 10 ++++++++- arch/x86/kvm/x86.c | 41 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 67 insertions(+), 2 deletions(-)
Applied, thanks.
.