* Re: [kvm-unit-tests PATCH] vmx: Fix EPT accessed and dirty flag test
2022-01-21 15:34 [kvm-unit-tests PATCH] vmx: Fix EPT accessed and dirty flag test Cathy Avery
@ 2022-01-21 15:50 ` Sean Christopherson
0 siblings, 0 replies; 2+ messages in thread
From: Sean Christopherson @ 2022-01-21 15:50 UTC (permalink / raw)
To: Cathy Avery; +Cc: kvm
On Fri, Jan 21, 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
>
> Signed-off-by: Cathy Avery <cavery@redhat.com>
> ---
> x86/vmx_tests.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index 3d57ed6..54f2aaa 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -4783,6 +4783,7 @@ static void test_ept_eptp(void)
>
> eptp |= EPTP_AD_FLAG;
> test_eptp_ad_bit(eptp, false);
> + eptp &= ~EPTP_AD_FLAG;
> }
Heh, or we could get cute and do:
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index e67eaea..9a8f7c2 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -4785,11 +4785,11 @@ static void test_ept_eptp(void)
test_eptp_ad_bit(eptp, true);
} else {
report_info("Processor does not supports accessed and dirty flag");
- eptp &= ~EPTP_AD_FLAG;
- test_eptp_ad_bit(eptp, true);
-
eptp |= EPTP_AD_FLAG;
test_eptp_ad_bit(eptp, false);
+
+ eptp &= ~EPTP_AD_FLAG;
+ test_eptp_ad_bit(eptp, true);
}
/*
More seriously, I would much prefer we use eptp_saved to restore the known good
eptp instead of manually clearing the bits that were set. Does this work?
---
x86/vmx_tests.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index e67eaea..8116b0c 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -4749,8 +4749,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
@@ -4769,9 +4768,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)
@@ -4791,6 +4788,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]
@@ -4809,8 +4807,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++) {
@@ -4829,6 +4826,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);
--
^ permalink raw reply related [flat|nested] 2+ messages in thread