* [kvm-unit-tests v2 PATCH] vmx: Fix EPT accessed and dirty flag test
@ 2022-02-02 13:55 Cathy Avery
2022-02-07 21:30 ` Sean Christopherson
0 siblings, 1 reply; 2+ messages in thread
From: Cathy Avery @ 2022-02-02 13:55 UTC (permalink / raw)
To: kvm
If ept_ad is not supported by the processor or has been
turned off via kvm module param, test_ept_eptp() will
incorrectly leave EPTP_AD_FLAG set in variable eptp
causing the following failures of subsequent
test_vmx_valid_controls calls:
FAIL: Enable-EPT enabled; reserved bits [11:7] 0: vmlaunch succeeds
FAIL: Enable-EPT enabled; reserved bits [63:N] 0: vmlaunch succeeds
Signed-off-by: Cathy Avery <cavery@redhat.com>
---
* Changes in v2:
- Initialize vmcs EPTP to good values for page walk len
and ept memory type.
- Restore eptp to known good values from eptp_saved
- Cleanup test_vmx_vmlaunch to generate clearer and
more consolidated test reports.
New format suggested by seanjc@google.com
---
x86/vmx_tests.c | 39 +++++++++++++++++++++++----------------
1 file changed, 23 insertions(+), 16 deletions(-)
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 3d57ed6..1269829 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -3392,14 +3392,21 @@ static void test_vmx_vmlaunch(u32 xerror)
bool success = vmlaunch_succeeds();
u32 vmx_inst_err;
- report(success == !xerror, "vmlaunch %s",
- !xerror ? "succeeds" : "fails");
- if (!success && xerror) {
- vmx_inst_err = vmcs_read(VMX_INST_ERROR);
+ if (!success)
+ vmx_inst_err = vmcs_read(VMX_INST_ERROR);
+
+ if (success && !xerror)
+ report_pass("VMLAUNCH succeeded as expected");
+ else if (success && xerror)
+ report_fail("VMLAUNCH succeeded unexpectedly, wanted VM-Fail with error code = %d",
+ xerror);
+ else if (!success && !xerror)
+ report_fail("VMLAUNCH hit unexpected VM-Fail with error code = %d",
+ vmx_inst_err);
+ else
report(vmx_inst_err == xerror,
- "VMX inst error is %d (actual %d)", xerror,
- vmx_inst_err);
- }
+ "VMLAUNCH hit VM-Fail as expected, wanted error code %d, got %d",
+ xerror, vmx_inst_err);
}
/*
@@ -4707,12 +4714,11 @@ static void test_ept_eptp(void)
{
u32 primary_saved = vmcs_read(CPU_EXEC_CTRL0);
u32 secondary_saved = vmcs_read(CPU_EXEC_CTRL1);
- u64 eptp_saved = vmcs_read(EPTP);
u32 primary = primary_saved;
u32 secondary = secondary_saved;
- u64 eptp = eptp_saved;
u32 i, maxphysaddr;
u64 j, resv_bits_mask = 0;
+ u64 eptp_saved, eptp;
if (!((ctrl_cpu_rev[0].clr & CPU_SECONDARY) &&
(ctrl_cpu_rev[1].clr & CPU_EPT))) {
@@ -4720,6 +4726,9 @@ static void test_ept_eptp(void)
return;
}
+ setup_dummy_ept();
+ eptp = eptp_saved = vmcs_read(EPTP);
+
/* Support for 4-level EPT is mandatory. */
report(is_4_level_ept_supported(), "4-level EPT support check");
@@ -4742,8 +4751,7 @@ static void test_ept_eptp(void)
test_vmx_invalid_controls();
report_prefix_pop();
}
-
- eptp = (eptp & ~EPT_MEM_TYPE_MASK) | 6ul;
+ eptp = eptp_saved;
/*
* Page walk length (bits 5:3). Note, the value in VMCS.EPTP "is 1
@@ -4762,9 +4770,7 @@ static void test_ept_eptp(void)
test_vmx_invalid_controls();
report_prefix_pop();
}
-
- eptp = (eptp & ~EPTP_PG_WALK_LEN_MASK) |
- 3ul << EPTP_PG_WALK_LEN_SHIFT;
+ eptp = eptp_saved;
/*
* Accessed and dirty flag (bit 6)
@@ -4784,6 +4790,7 @@ static void test_ept_eptp(void)
eptp |= EPTP_AD_FLAG;
test_eptp_ad_bit(eptp, false);
}
+ eptp = eptp_saved;
/*
* Reserved bits [11:7] and [63:N]
@@ -4802,8 +4809,7 @@ static void test_ept_eptp(void)
test_vmx_invalid_controls();
report_prefix_pop();
}
-
- eptp = (eptp & ~(EPTP_RESERV_BITS_MASK << EPTP_RESERV_BITS_SHIFT));
+ eptp = eptp_saved;
maxphysaddr = cpuid_maxphyaddr();
for (i = 0; i < (63 - maxphysaddr + 1); i++) {
@@ -4822,6 +4828,7 @@ static void test_ept_eptp(void)
test_vmx_invalid_controls();
report_prefix_pop();
}
+ eptp = eptp_saved;
secondary &= ~(CPU_EPT | CPU_URG);
vmcs_write(CPU_EXEC_CTRL1, secondary);
--
2.31.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [kvm-unit-tests v2 PATCH] vmx: Fix EPT accessed and dirty flag test
2022-02-02 13:55 [kvm-unit-tests v2 PATCH] vmx: Fix EPT accessed and dirty flag test Cathy Avery
@ 2022-02-07 21:30 ` Sean Christopherson
0 siblings, 0 replies; 2+ messages in thread
From: Sean Christopherson @ 2022-02-07 21:30 UTC (permalink / raw)
To: Cathy Avery; +Cc: kvm
On Wed, Feb 02, 2022, Cathy Avery wrote:
> If ept_ad is not supported by the processor or has been
> turned off via kvm module param, test_ept_eptp() will
> incorrectly leave EPTP_AD_FLAG set in variable eptp
> causing the following failures of subsequent
> test_vmx_valid_controls calls:
>
> FAIL: Enable-EPT enabled; reserved bits [11:7] 0: vmlaunch succeeds
> FAIL: Enable-EPT enabled; reserved bits [63:N] 0: vmlaunch succeeds
Heh, the changelog never actually provides info on how it fixes things.
Use the saved EPTP to restore the EPTP after each sub-test instead of
manually unwinding what was done by the sub-test, which is error prone
and hard to follow.
Explicitly setup a dummy EPTP, as calling the test in isolation will cause
test failures due to lack a good starting EPTP.
> Signed-off-by: Cathy Avery <cavery@redhat.com>
> ---
>
> * Changes in v2:
>
> - Initialize vmcs EPTP to good values for page walk len
> and ept memory type.
> - Restore eptp to known good values from eptp_saved
> - Cleanup test_vmx_vmlaunch to generate clearer and
> more consolidated test reports.
> New format suggested by seanjc@google.com
> ---
> x86/vmx_tests.c | 39 +++++++++++++++++++++++----------------
> 1 file changed, 23 insertions(+), 16 deletions(-)
>
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index 3d57ed6..1269829 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -3392,14 +3392,21 @@ static void test_vmx_vmlaunch(u32 xerror)
> bool success = vmlaunch_succeeds();
> u32 vmx_inst_err;
>
> - report(success == !xerror, "vmlaunch %s",
> - !xerror ? "succeeds" : "fails");
> - if (!success && xerror) {
> - vmx_inst_err = vmcs_read(VMX_INST_ERROR);
> + if (!success)
> + vmx_inst_err = vmcs_read(VMX_INST_ERROR);
> +
> + if (success && !xerror)
> + report_pass("VMLAUNCH succeeded as expected");
> + else if (success && xerror)
> + report_fail("VMLAUNCH succeeded unexpectedly, wanted VM-Fail with error code = %d",
> + xerror);
> + else if (!success && !xerror)
> + report_fail("VMLAUNCH hit unexpected VM-Fail with error code = %d",
> + vmx_inst_err);
> + else
> report(vmx_inst_err == xerror,
> - "VMX inst error is %d (actual %d)", xerror,
> - vmx_inst_err);
> - }
> + "VMLAUNCH hit VM-Fail as expected, wanted error code %d, got %d",
> + xerror, vmx_inst_err);
> }
The changes to test_vmx_vmlaunch() need to be a separate patch. The addition of
setup_dummy_ept() would ideally be separate as well, though I don't care terribly
about that one.
With this split in two (or three) and an updated changelog,
Reviewed-by: Sean Christopherson <seanjc@google.com>
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2022-02-07 21:30 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-02 13:55 [kvm-unit-tests v2 PATCH] vmx: Fix EPT accessed and dirty flag test Cathy Avery
2022-02-07 21:30 ` Sean Christopherson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.