On 8/4/2023 1:11 AM, Sean Christopherson wrote:
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...
Sure, let me throw the ball to the architect, will update here once got reply.
Thanks!
