| Message ID | 20230803042732.88515-19-weijiang.yang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enable CET Virtualization | expand |
This is not "refinement", this is full on supporting a new nVMX feature. Please phrase the shortlog accordingly, e.g. something like this (it's not very good, but it's a start). KVM: nVMX: Add support for exposing "No PM H/W error code checks" to L1 Regarding shortlog, please update all of them in this series to put a space after the colon, i.e. "KVM: VMX:" and "KVM: x86:", not "KVM:x86:". > static void nested_vmx_setup_cr_fixed(struct nested_vmx_msrs *msrs) > diff --git a/arch/x86/kvm/vmx/nested.h b/arch/x86/kvm/vmx/nested.h > index 96952263b029..1884628294e4 100644 > --- a/arch/x86/kvm/vmx/nested.h > +++ b/arch/x86/kvm/vmx/nested.h > @@ -284,6 +284,13 @@ static inline bool nested_cr4_valid(struct kvm_vcpu *vcpu, unsigned long val) > __kvm_is_valid_cr4(vcpu, val); > } > > +static inline bool nested_cpu_has_no_hw_errcode(struct kvm_vcpu *vcpu) > +{ > + struct vcpu_vmx *vmx = to_vmx(vcpu); > + > + return vmx->nested.msrs.basic & VMX_BASIC_NO_HW_ERROR_CODE; The "CC" part of my suggestion is critical to this being sane. As is, this reads "nested CPU has no hardware error code", which is not even remotely close to the truth. static inline bool nested_cpu_has_no_hw_errcode_cc(struct kvm_vcpu *vcpu) { return to_vmx(vcpu)->nested.msrs.basic & VMX_BASIC_NO_HW_ERROR_CODE_CC; } [*] https://lore.kernel.org/all/ZJ7vyBw1nbTBOfuf@google.com
On 8/5/2023 5:38 AM, Sean Christopherson wrote: > This is not "refinement", this is full on supporting a new nVMX feature. Please > phrase the shortlog accordingly, e.g. something like this (it's not very good, > but it's a start). > > KVM: nVMX: Add support for exposing "No PM H/W error code checks" to L1 > > Regarding shortlog, please update all of them in this series to put a space after > the colon, i.e. "KVM: VMX:" and "KVM: x86:", not "KVM:x86:". OK, will update this part. >> static void nested_vmx_setup_cr_fixed(struct nested_vmx_msrs *msrs) >> diff --git a/arch/x86/kvm/vmx/nested.h b/arch/x86/kvm/vmx/nested.h >> index 96952263b029..1884628294e4 100644 >> --- a/arch/x86/kvm/vmx/nested.h >> +++ b/arch/x86/kvm/vmx/nested.h >> @@ -284,6 +284,13 @@ static inline bool nested_cr4_valid(struct kvm_vcpu *vcpu, unsigned long val) >> __kvm_is_valid_cr4(vcpu, val); >> } >> >> +static inline bool nested_cpu_has_no_hw_errcode(struct kvm_vcpu *vcpu) >> +{ >> + struct vcpu_vmx *vmx = to_vmx(vcpu); >> + >> + return vmx->nested.msrs.basic & VMX_BASIC_NO_HW_ERROR_CODE; > The "CC" part of my suggestion is critical to this being sane. As is, this reads > "nested CPU has no hardware error code", which is not even remotely close to the > truth. Understood, I was not aware of the essence of "CC". > static inline bool nested_cpu_has_no_hw_errcode_cc(struct kvm_vcpu *vcpu) > { > return to_vmx(vcpu)->nested.msrs.basic & VMX_BASIC_NO_HW_ERROR_CODE_CC; > } > > [*] https://lore.kernel.org/all/ZJ7vyBw1nbTBOfuf@google.com
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c index 516391cc0d64..9bcd989252f7 100644 --- a/arch/x86/kvm/vmx/nested.c +++ b/arch/x86/kvm/vmx/nested.c @@ -1205,9 +1205,9 @@ static int vmx_restore_vmx_basic(struct vcpu_vmx *vmx, u64 data) { const u64 feature_and_reserved = /* feature (except bit 48; see below) */ - BIT_ULL(49) | BIT_ULL(54) | BIT_ULL(55) | + BIT_ULL(49) | BIT_ULL(54) | BIT_ULL(55) | BIT_ULL(56) | /* reserved */ - BIT_ULL(31) | GENMASK_ULL(47, 45) | GENMASK_ULL(63, 56); + BIT_ULL(31) | GENMASK_ULL(47, 45) | GENMASK_ULL(63, 57); u64 vmx_basic = vmcs_config.nested.basic; if (!is_bitwise_subset(vmx_basic, data, feature_and_reserved)) @@ -2846,12 +2846,16 @@ static int nested_check_vm_entry_controls(struct kvm_vcpu *vcpu, CC(intr_type == INTR_TYPE_OTHER_EVENT && vector != 0)) return -EINVAL; - /* VM-entry interruption-info field: deliver error code */ - should_have_error_code = - intr_type == INTR_TYPE_HARD_EXCEPTION && prot_mode && - x86_exception_has_error_code(vector); - if (CC(has_error_code != should_have_error_code)) - return -EINVAL; + if (!prot_mode || intr_type != INTR_TYPE_HARD_EXCEPTION || + !nested_cpu_has_no_hw_errcode(vcpu)) { + /* VM-entry interruption-info field: deliver error code */ + should_have_error_code = + intr_type == INTR_TYPE_HARD_EXCEPTION && + prot_mode && + x86_exception_has_error_code(vector); + if (CC(has_error_code != should_have_error_code)) + return -EINVAL; + } /* VM-entry exception error code */ if (CC(has_error_code && @@ -6967,6 +6971,8 @@ static void nested_vmx_setup_basic(struct nested_vmx_msrs *msrs) if (cpu_has_vmx_basic_inout()) msrs->basic |= VMX_BASIC_INOUT; + if (cpu_has_vmx_basic_no_hw_errcode()) + msrs->basic |= VMX_BASIC_NO_HW_ERROR_CODE; } static void nested_vmx_setup_cr_fixed(struct nested_vmx_msrs *msrs) diff --git a/arch/x86/kvm/vmx/nested.h b/arch/x86/kvm/vmx/nested.h index 96952263b029..1884628294e4 100644 --- a/arch/x86/kvm/vmx/nested.h +++ b/arch/x86/kvm/vmx/nested.h @@ -284,6 +284,13 @@ static inline bool nested_cr4_valid(struct kvm_vcpu *vcpu, unsigned long val) __kvm_is_valid_cr4(vcpu, val); } +static inline bool nested_cpu_has_no_hw_errcode(struct kvm_vcpu *vcpu) +{ + struct vcpu_vmx *vmx = to_vmx(vcpu); + + return vmx->nested.msrs.basic & VMX_BASIC_NO_HW_ERROR_CODE; +} + /* No difference in the restrictions on guest and host CR4 in VMX operation. */ #define nested_guest_cr4_valid nested_cr4_valid #define nested_host_cr4_valid nested_cr4_valid
Per SDM description(Vol.3D, Appendix A.1): "If bit 56 is read as 1, software can use VM entry to deliver a hardware exception with or without an error code, regardless of vector" Modify has_error_code check before inject events to nested guest. Only enforce the check when guest is in real mode, the exception is not hard exception and the platform doesn't enumerate bit56 in VMX_BASIC, otherwise ignore it. Signed-off-by: Yang Weijiang <weijiang.yang@intel.com> --- arch/x86/kvm/vmx/nested.c | 22 ++++++++++++++-------- arch/x86/kvm/vmx/nested.h | 7 +++++++ 2 files changed, 21 insertions(+), 8 deletions(-)