On Thu, Aug 03, 2023, Weijiang Yang wrote:
> On 8/3/2023 3:43 AM, Sean Christopherson wrote:
> > > diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> > > index 7952ccb..b6d4982 100644
> > > --- a/x86/vmx_tests.c
> > > +++ b/x86/vmx_tests.c
> > > @@ -4173,7 +4173,10 @@ static void test_invalid_event_injection(void)
> > > ent_intr_info);
> > > vmcs_write(GUEST_CR0, guest_cr0_save & ~X86_CR0_PE & ~X86_CR0_PG);
> > > vmcs_write(ENT_INTR_INFO, ent_intr_info);
> > > - test_vmx_invalid_controls();
> > > + if (basic.errcode)
> > > + test_vmx_valid_controls();
> > > + else
> > > + test_vmx_invalid_controls();
> > This is wrong, no? The consistency check is only skipped for PM, the above CR0.PE
> > modification means the target is RM.
> I think this case is executed with !CPU_URG, so RM is "converted" to PM because we
> have below in KVM:
> bool urg = nested_cpu_has2(vmcs12,
> SECONDARY_EXEC_UNRESTRICTED_GUEST);
> bool prot_mode = !urg || vmcs12->guest_cr0 & X86_CR0_PE;
> ...
> 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;
> }
>
> so on platform with basic.errcode == 1, this case passes.
Huh. I get the logic, but IMO based on the SDM, that's a ucode bug that got
propagated into KVM (or an SDM bug, which is my bet for how this gets treated).
I verified HSW at least does indeed generate VM-Fail and not VM-Exit(INVALID_STATE),
so it doesn't appear that KVM is making stuff (for once). Either that or I'm
misreading the SDM (definite possibility), but the only relevant condition I see is:
bit 0 (corresponding to CR0.PE) is set in the CR0 field in the guest-state area
I don't see anything in the SDM that states the CR0.PE is assumed to be '1' for
consistency checks when unrestricted guest is disabled.
Can you bug a VMX architect again to get clarification, e.g. to get an SDM update?
Or just point out where I missed something in the SDM, again...
