| Message ID | 1440132611-26052-4-git-send-email-guangrong.xiao@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 21/08/2015 06:50, Xiao Guangrong wrote: > Pass PCOMMIT CPU feature to guest to enable PCOMMIT instruction > > Currently we do not catch pcommit instruction for L1 guest and > allow L1 to catch this instruction for L2 > > The specification locates at: > https://software.intel.com/sites/default/files/managed/0d/53/319433-022.pdf > > Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com> > --- > arch/x86/include/asm/vmx.h | 2 +- > arch/x86/include/uapi/asm/vmx.h | 4 +++- > arch/x86/kvm/cpuid.c | 2 +- > arch/x86/kvm/cpuid.h | 8 ++++++++ > arch/x86/kvm/vmx.c | 29 ++++++++++++++++++++++++----- > 5 files changed, 37 insertions(+), 8 deletions(-) > > diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h > index 9299ae5..e2ad08a 100644 > --- a/arch/x86/include/asm/vmx.h > +++ b/arch/x86/include/asm/vmx.h > @@ -72,7 +72,7 @@ > #define SECONDARY_EXEC_SHADOW_VMCS 0x00004000 > #define SECONDARY_EXEC_ENABLE_PML 0x00020000 > #define SECONDARY_EXEC_XSAVES 0x00100000 > - > +#define SECONDARY_EXEC_PCOMMIT 0x00200000 > > #define PIN_BASED_EXT_INTR_MASK 0x00000001 > #define PIN_BASED_NMI_EXITING 0x00000008 > diff --git a/arch/x86/include/uapi/asm/vmx.h b/arch/x86/include/uapi/asm/vmx.h > index 37fee27..5b15d94 100644 > --- a/arch/x86/include/uapi/asm/vmx.h > +++ b/arch/x86/include/uapi/asm/vmx.h > @@ -78,6 +78,7 @@ > #define EXIT_REASON_PML_FULL 62 > #define EXIT_REASON_XSAVES 63 > #define EXIT_REASON_XRSTORS 64 > +#define EXIT_REASON_PCOMMIT 65 > > #define VMX_EXIT_REASONS \ > { EXIT_REASON_EXCEPTION_NMI, "EXCEPTION_NMI" }, \ > @@ -126,7 +127,8 @@ > { EXIT_REASON_INVVPID, "INVVPID" }, \ > { EXIT_REASON_INVPCID, "INVPCID" }, \ > { EXIT_REASON_XSAVES, "XSAVES" }, \ > - { EXIT_REASON_XRSTORS, "XRSTORS" } > + { EXIT_REASON_XRSTORS, "XRSTORS" }, \ > + { EXIT_REASON_PCOMMIT, "PCOMMIT" } > > #define VMX_ABORT_SAVE_GUEST_MSR_FAIL 1 > #define VMX_ABORT_LOAD_HOST_MSR_FAIL 4 > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > index 962fc7d..faeb0b3 100644 > --- a/arch/x86/kvm/cpuid.c > +++ b/arch/x86/kvm/cpuid.c > @@ -348,7 +348,7 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function, > F(FSGSBASE) | F(BMI1) | F(HLE) | F(AVX2) | F(SMEP) | > F(BMI2) | F(ERMS) | f_invpcid | F(RTM) | f_mpx | F(RDSEED) | > F(ADX) | F(SMAP) | F(AVX512F) | F(AVX512PF) | F(AVX512ER) | > - F(AVX512CD) | F(CLFLUSHOPT) | F(CLWB); > + F(AVX512CD) | F(CLFLUSHOPT) | F(CLWB) | F(PCOMMIT); > > /* cpuid 0xD.1.eax */ > const u32 kvm_supported_word10_x86_features = > diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h > index dd05b9c..aed7bfe 100644 > --- a/arch/x86/kvm/cpuid.h > +++ b/arch/x86/kvm/cpuid.h > @@ -133,4 +133,12 @@ static inline bool guest_cpuid_has_mpx(struct kvm_vcpu *vcpu) > best = kvm_find_cpuid_entry(vcpu, 7, 0); > return best && (best->ebx & bit(X86_FEATURE_MPX)); > } > + > +static inline bool guest_cpuid_has_pcommit(struct kvm_vcpu *vcpu) > +{ > + struct kvm_cpuid_entry2 *best; > + > + best = kvm_find_cpuid_entry(vcpu, 7, 0); > + return best && (best->ebx & bit(X86_FEATURE_PCOMMIT)); > +} > #endif > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c > index 4cf25b9..b526c61 100644 > --- a/arch/x86/kvm/vmx.c > +++ b/arch/x86/kvm/vmx.c > @@ -2474,7 +2474,8 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx) > SECONDARY_EXEC_APIC_REGISTER_VIRT | > SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY | > SECONDARY_EXEC_WBINVD_EXITING | > - SECONDARY_EXEC_XSAVES; > + SECONDARY_EXEC_XSAVES | > + SECONDARY_EXEC_PCOMMIT; > > if (enable_ept) { > /* nested EPT: emulate EPT also to L1 */ > @@ -3015,7 +3016,8 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf) > SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY | > SECONDARY_EXEC_SHADOW_VMCS | > SECONDARY_EXEC_XSAVES | > - SECONDARY_EXEC_ENABLE_PML; > + SECONDARY_EXEC_ENABLE_PML | > + SECONDARY_EXEC_PCOMMIT; > if (adjust_vmx_controls(min2, opt2, > MSR_IA32_VMX_PROCBASED_CTLS2, > &_cpu_based_2nd_exec_control) < 0) > @@ -4570,6 +4572,9 @@ static u32 vmx_secondary_exec_control(struct vcpu_vmx *vmx) > /* PML is enabled/disabled in creating/destorying vcpu */ > exec_control &= ~SECONDARY_EXEC_ENABLE_PML; > > + /* Currently, we allow L1 guest to directly run pcommit instruction. */ > + exec_control &= ~SECONDARY_EXEC_PCOMMIT; > + > return exec_control; > } > > @@ -4613,10 +4618,9 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx) > > vmcs_write32(CPU_BASED_VM_EXEC_CONTROL, vmx_exec_control(vmx)); > > - if (cpu_has_secondary_exec_ctrls()) { > + if (cpu_has_secondary_exec_ctrls()) > vmcs_write32(SECONDARY_VM_EXEC_CONTROL, > vmx_secondary_exec_control(vmx)); > - } > > if (vmx_cpu_uses_apicv(&vmx->vcpu)) { > vmcs_write64(EOI_EXIT_BITMAP0, 0); > @@ -7208,6 +7212,13 @@ static int handle_pml_full(struct kvm_vcpu *vcpu) > return 1; > } > > +static int handle_pcommit(struct kvm_vcpu *vcpu) > +{ > + /* we never catch pcommit instruct for L1 guest. */ > + BUG(); Please WARN instead. > + return 1; > +} > + > /* > * The exit handlers return 1 if the exit was handled fully and guest execution > * may resume. Otherwise they set the kvm_run parameter to indicate what needs > @@ -7258,6 +7269,7 @@ static int (*const kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = { > [EXIT_REASON_XSAVES] = handle_xsaves, > [EXIT_REASON_XRSTORS] = handle_xrstors, > [EXIT_REASON_PML_FULL] = handle_pml_full, > + [EXIT_REASON_PCOMMIT] = handle_pcommit, > }; > > static const int kvm_vmx_max_exit_handlers = > @@ -7559,6 +7571,8 @@ static bool nested_vmx_exit_handled(struct kvm_vcpu *vcpu) > * the XSS exit bitmap in vmcs12. > */ > return nested_cpu_has2(vmcs12, SECONDARY_EXEC_XSAVES); > + case EXIT_REASON_PCOMMIT: > + return nested_cpu_has2(vmcs12, SECONDARY_EXEC_PCOMMIT); > default: > return true; > } > @@ -8688,6 +8702,10 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu) > if (best) > best->ebx &= ~bit(X86_FEATURE_INVPCID); > } > + > + if (!guest_cpuid_has_pcommit(vcpu) && nested) > + vmx->nested.nested_vmx_secondary_ctls_high &= > + ~SECONDARY_EXEC_PCOMMIT; Why is this needed? Paolo > } > > static void vmx_set_supported_cpuid(u32 func, struct kvm_cpuid_entry2 *entry) > @@ -9301,7 +9319,8 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12) > exec_control &= ~(SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES | > SECONDARY_EXEC_RDTSCP | > SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY | > - SECONDARY_EXEC_APIC_REGISTER_VIRT); > + SECONDARY_EXEC_APIC_REGISTER_VIRT | > + SECONDARY_EXEC_PCOMMIT); > if (nested_cpu_has(vmcs12, > CPU_BASED_ACTIVATE_SECONDARY_CONTROLS)) > exec_control |= vmcs12->secondary_vm_exec_control; > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 09/07/2015 07:18 PM, Paolo Bonzini wrote: >> >> +static int handle_pcommit(struct kvm_vcpu *vcpu) >> +{ >> + /* we never catch pcommit instruct for L1 guest. */ >> + BUG(); > > Please WARN instead. > Okay. >> + return 1; >> +} >> + >> /* >> * The exit handlers return 1 if the exit was handled fully and guest execution >> * may resume. Otherwise they set the kvm_run parameter to indicate what needs >> @@ -7258,6 +7269,7 @@ static int (*const kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = { >> [EXIT_REASON_XSAVES] = handle_xsaves, >> [EXIT_REASON_XRSTORS] = handle_xrstors, >> [EXIT_REASON_PML_FULL] = handle_pml_full, >> + [EXIT_REASON_PCOMMIT] = handle_pcommit, >> }; >> >> static const int kvm_vmx_max_exit_handlers = >> @@ -7559,6 +7571,8 @@ static bool nested_vmx_exit_handled(struct kvm_vcpu *vcpu) >> * the XSS exit bitmap in vmcs12. >> */ >> return nested_cpu_has2(vmcs12, SECONDARY_EXEC_XSAVES); >> + case EXIT_REASON_PCOMMIT: >> + return nested_cpu_has2(vmcs12, SECONDARY_EXEC_PCOMMIT); >> default: >> return true; >> } >> @@ -8688,6 +8702,10 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu) >> if (best) >> best->ebx &= ~bit(X86_FEATURE_INVPCID); >> } >> + >> + if (!guest_cpuid_has_pcommit(vcpu) && nested) >> + vmx->nested.nested_vmx_secondary_ctls_high &= >> + ~SECONDARY_EXEC_PCOMMIT; > > Why is this needed? > If pcommit is not allowed in L1 guest, L1 is not allowed to intercept pcommit for L2. BTW, the spec saied: | IA32_VMX_PROCBASED_CTLS2[53] (which enumerates support for the 1-setting of “PCOMMIT exiting”) is | always the same as CPUID.07H:EBX.PCOMMIT[bit 22]. Thus, software can set “PCOMMIT exiting” to 1 | if and only if the PCOMMIT instruction is enumerated via CPUID -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 08/09/2015 16:17, Xiao Guangrong wrote: > > BTW, the spec saied: > > | IA32_VMX_PROCBASED_CTLS2[53] (which enumerates support for the > 1-setting of “PCOMMIT exiting”) is > | always the same as CPUID.07H:EBX.PCOMMIT[bit 22]. Thus, software can > set “PCOMMIT exiting” to 1 > | if and only if the PCOMMIT instruction is enumerated via CPUID Thanks. Can you add it to the commit message ("allow L1 to catch this instruction for L2 if, as required by the spec, L1 can enumerate the PCOMMIT instruction via CPUID"). Paolo -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h index 9299ae5..e2ad08a 100644 --- a/arch/x86/include/asm/vmx.h +++ b/arch/x86/include/asm/vmx.h @@ -72,7 +72,7 @@ #define SECONDARY_EXEC_SHADOW_VMCS 0x00004000 #define SECONDARY_EXEC_ENABLE_PML 0x00020000 #define SECONDARY_EXEC_XSAVES 0x00100000 - +#define SECONDARY_EXEC_PCOMMIT 0x00200000 #define PIN_BASED_EXT_INTR_MASK 0x00000001 #define PIN_BASED_NMI_EXITING 0x00000008 diff --git a/arch/x86/include/uapi/asm/vmx.h b/arch/x86/include/uapi/asm/vmx.h index 37fee27..5b15d94 100644 --- a/arch/x86/include/uapi/asm/vmx.h +++ b/arch/x86/include/uapi/asm/vmx.h @@ -78,6 +78,7 @@ #define EXIT_REASON_PML_FULL 62 #define EXIT_REASON_XSAVES 63 #define EXIT_REASON_XRSTORS 64 +#define EXIT_REASON_PCOMMIT 65 #define VMX_EXIT_REASONS \ { EXIT_REASON_EXCEPTION_NMI, "EXCEPTION_NMI" }, \ @@ -126,7 +127,8 @@ { EXIT_REASON_INVVPID, "INVVPID" }, \ { EXIT_REASON_INVPCID, "INVPCID" }, \ { EXIT_REASON_XSAVES, "XSAVES" }, \ - { EXIT_REASON_XRSTORS, "XRSTORS" } + { EXIT_REASON_XRSTORS, "XRSTORS" }, \ + { EXIT_REASON_PCOMMIT, "PCOMMIT" } #define VMX_ABORT_SAVE_GUEST_MSR_FAIL 1 #define VMX_ABORT_LOAD_HOST_MSR_FAIL 4 diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c index 962fc7d..faeb0b3 100644 --- a/arch/x86/kvm/cpuid.c +++ b/arch/x86/kvm/cpuid.c @@ -348,7 +348,7 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function, F(FSGSBASE) | F(BMI1) | F(HLE) | F(AVX2) | F(SMEP) | F(BMI2) | F(ERMS) | f_invpcid | F(RTM) | f_mpx | F(RDSEED) | F(ADX) | F(SMAP) | F(AVX512F) | F(AVX512PF) | F(AVX512ER) | - F(AVX512CD) | F(CLFLUSHOPT) | F(CLWB); + F(AVX512CD) | F(CLFLUSHOPT) | F(CLWB) | F(PCOMMIT); /* cpuid 0xD.1.eax */ const u32 kvm_supported_word10_x86_features = diff --git a/arch/x86/kvm/cpuid.h b/arch/x86/kvm/cpuid.h index dd05b9c..aed7bfe 100644 --- a/arch/x86/kvm/cpuid.h +++ b/arch/x86/kvm/cpuid.h @@ -133,4 +133,12 @@ static inline bool guest_cpuid_has_mpx(struct kvm_vcpu *vcpu) best = kvm_find_cpuid_entry(vcpu, 7, 0); return best && (best->ebx & bit(X86_FEATURE_MPX)); } + +static inline bool guest_cpuid_has_pcommit(struct kvm_vcpu *vcpu) +{ + struct kvm_cpuid_entry2 *best; + + best = kvm_find_cpuid_entry(vcpu, 7, 0); + return best && (best->ebx & bit(X86_FEATURE_PCOMMIT)); +} #endif diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index 4cf25b9..b526c61 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -2474,7 +2474,8 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx *vmx) SECONDARY_EXEC_APIC_REGISTER_VIRT | SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY | SECONDARY_EXEC_WBINVD_EXITING | - SECONDARY_EXEC_XSAVES; + SECONDARY_EXEC_XSAVES | + SECONDARY_EXEC_PCOMMIT; if (enable_ept) { /* nested EPT: emulate EPT also to L1 */ @@ -3015,7 +3016,8 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf) SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY | SECONDARY_EXEC_SHADOW_VMCS | SECONDARY_EXEC_XSAVES | - SECONDARY_EXEC_ENABLE_PML; + SECONDARY_EXEC_ENABLE_PML | + SECONDARY_EXEC_PCOMMIT; if (adjust_vmx_controls(min2, opt2, MSR_IA32_VMX_PROCBASED_CTLS2, &_cpu_based_2nd_exec_control) < 0) @@ -4570,6 +4572,9 @@ static u32 vmx_secondary_exec_control(struct vcpu_vmx *vmx) /* PML is enabled/disabled in creating/destorying vcpu */ exec_control &= ~SECONDARY_EXEC_ENABLE_PML; + /* Currently, we allow L1 guest to directly run pcommit instruction. */ + exec_control &= ~SECONDARY_EXEC_PCOMMIT; + return exec_control; } @@ -4613,10 +4618,9 @@ static int vmx_vcpu_setup(struct vcpu_vmx *vmx) vmcs_write32(CPU_BASED_VM_EXEC_CONTROL, vmx_exec_control(vmx)); - if (cpu_has_secondary_exec_ctrls()) { + if (cpu_has_secondary_exec_ctrls()) vmcs_write32(SECONDARY_VM_EXEC_CONTROL, vmx_secondary_exec_control(vmx)); - } if (vmx_cpu_uses_apicv(&vmx->vcpu)) { vmcs_write64(EOI_EXIT_BITMAP0, 0); @@ -7208,6 +7212,13 @@ static int handle_pml_full(struct kvm_vcpu *vcpu) return 1; } +static int handle_pcommit(struct kvm_vcpu *vcpu) +{ + /* we never catch pcommit instruct for L1 guest. */ + BUG(); + return 1; +} + /* * The exit handlers return 1 if the exit was handled fully and guest execution * may resume. Otherwise they set the kvm_run parameter to indicate what needs @@ -7258,6 +7269,7 @@ static int (*const kvm_vmx_exit_handlers[])(struct kvm_vcpu *vcpu) = { [EXIT_REASON_XSAVES] = handle_xsaves, [EXIT_REASON_XRSTORS] = handle_xrstors, [EXIT_REASON_PML_FULL] = handle_pml_full, + [EXIT_REASON_PCOMMIT] = handle_pcommit, }; static const int kvm_vmx_max_exit_handlers = @@ -7559,6 +7571,8 @@ static bool nested_vmx_exit_handled(struct kvm_vcpu *vcpu) * the XSS exit bitmap in vmcs12. */ return nested_cpu_has2(vmcs12, SECONDARY_EXEC_XSAVES); + case EXIT_REASON_PCOMMIT: + return nested_cpu_has2(vmcs12, SECONDARY_EXEC_PCOMMIT); default: return true; } @@ -8688,6 +8702,10 @@ static void vmx_cpuid_update(struct kvm_vcpu *vcpu) if (best) best->ebx &= ~bit(X86_FEATURE_INVPCID); } + + if (!guest_cpuid_has_pcommit(vcpu) && nested) + vmx->nested.nested_vmx_secondary_ctls_high &= + ~SECONDARY_EXEC_PCOMMIT; } static void vmx_set_supported_cpuid(u32 func, struct kvm_cpuid_entry2 *entry) @@ -9301,7 +9319,8 @@ static void prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12) exec_control &= ~(SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES | SECONDARY_EXEC_RDTSCP | SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY | - SECONDARY_EXEC_APIC_REGISTER_VIRT); + SECONDARY_EXEC_APIC_REGISTER_VIRT | + SECONDARY_EXEC_PCOMMIT); if (nested_cpu_has(vmcs12, CPU_BASED_ACTIVATE_SECONDARY_CONTROLS)) exec_control |= vmcs12->secondary_vm_exec_control;
Pass PCOMMIT CPU feature to guest to enable PCOMMIT instruction Currently we do not catch pcommit instruction for L1 guest and allow L1 to catch this instruction for L2 The specification locates at: https://software.intel.com/sites/default/files/managed/0d/53/319433-022.pdf Signed-off-by: Xiao Guangrong <guangrong.xiao@linux.intel.com> --- arch/x86/include/asm/vmx.h | 2 +- arch/x86/include/uapi/asm/vmx.h | 4 +++- arch/x86/kvm/cpuid.c | 2 +- arch/x86/kvm/cpuid.h | 8 ++++++++ arch/x86/kvm/vmx.c | 29 ++++++++++++++++++++++++----- 5 files changed, 37 insertions(+), 8 deletions(-)