On Thu, Sep 14, 2023 at 02:33:24AM -0400, Yang Weijiang wrote:
>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, in all
>other case ignore the check to make the logic consistent with SDM.
>
>Signed-off-by: Yang Weijiang <weijiang.yang@xxxxxxxxx>
>---
> arch/x86/kvm/vmx/nested.c | 22 ++++++++++++++--------
> arch/x86/kvm/vmx/nested.h | 5 +++++
> 2 files changed, 19 insertions(+), 8 deletions(-)
>
>diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
>index c5ec0ef51ff7..78a3be394d00 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_cc(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;
>+ }
prot_mode and intr_type are used twice, making the code a little hard to read.
how about:
/*
* Cannot deliver error code in real mode or if the
* interruption type is not hardware exception. For other
* cases, do the consistency check only if the vCPU doesn't
* enumerate VMX_BASIC_NO_HW_ERROR_CODE_CC.
*/
if (!prot_mode || intr_type != INTR_TYPE_HARD_EXCEPTION) {
if (CC(has_error_code))
return -EINVAL;
} else if (!nested_cpu_has_no_hw_errcode_cc(vcpu)) {
if (CC(has_error_code != x86_exception_has_error_code(vector)))
return -EINVAL;
}
and drop should_have_error_code.
