On 08/04/19 23:35, Krish Sadhukhan wrote:
> .to verify KVM performs the appropriate consistency checks for loading
> IA32_PAT as part of running a nested guest.
>
> According to section "Checks on Host Control Registers and MSRs" in Intel
> SDM vol 3C, the following check is performed on vmentry:
>
> If the “load IA32_PAT” VM-exit control is 1, the value of the field
> for the IA32_PAT MSR must be one that could be written by WRMSR
> without fault at CPL 0. Specifically, each of the 8 bytes in the
> field must have one of the values 0 (UC), 1 (WC), 4 (WT), 5 (WP),
> 6 (WB), or 7 (UC-).
>
> Since a PAT value higher than 8 will yield the same test result as that
> of 8, we want to confine our tests only up to 8 in order to reduce
> redundancy of tests and to avoid too many vmentries.
>
> Signed-off-by: Krish Sadhukhan <krish.sadhukhan@xxxxxxxxxx>
> Reviewed-by: Karl Heubaum <karl.heubaum@xxxxxxxxxx>
> Reviewed-by: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
> ---
> x86/vmx_tests.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 72 insertions(+)
>
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index 66a87f6..380fd85 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -4995,6 +4995,76 @@ static void test_sysenter_field(u32 field, const char *name)
> vmcs_write(field, addr_saved);
> }
>
> +/*
> + * PAT values higher than 8 are uninteresting since they're likely lumped
> + * in with "8". We cap the tests at PAT value of 8 in order to reduce the
> + * number of VM-Entries and keep the runtime reasonable.
> + */
> +#define PAT_VAL_LIMIT 8
> +
> +static void test_pat(u32 field, const char * field_name, u32 ctrl_field,
> + u64 ctrl_bit)
> +{
> + u32 ctrl_saved = vmcs_read(ctrl_field);
> + u64 pat_saved = vmcs_read(field);
> + u64 i, val;
> + u32 j;
> + int error;
> +
> + vmcs_write(ctrl_field, ctrl_saved & ~ctrl_bit);
> + for (i = 0; i <= PAT_VAL_LIMIT; i++) {
> + /* Test PAT0..PAT7 fields */
> + for (j = 0; j < 8; j++) {
> + val = i << j * 8;
> + vmcs_write(field, val);
> + report_prefix_pushf("%s %lx", field_name, val);
> + test_vmx_vmlaunch(0, false);
> + report_prefix_pop();
> + }
> + }
> +
> + vmcs_write(ctrl_field, ctrl_saved | ctrl_bit);
> + for (i = 0; i <= PAT_VAL_LIMIT; i++) {
> + /* Test PAT0..PAT7 fields */
> + for (j = 0; j < 8; j++) {
> + val = i << j * 8;
> + vmcs_write(field, val);
> + report_prefix_pushf("%s %lx", field_name, val);
> + if (i == 0x2 || i == 0x3 || i >= 0x8)
> + error = VMXERR_ENTRY_INVALID_HOST_STATE_FIELD;
> + else
> + error = 0;
> + test_vmx_vmlaunch(error, false);
> + report_prefix_pop();
> + }
Here are some small changes to remove redundant tests and also
improve coverage of values > 8:
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index fd1f483..7adc76a 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -6633,8 +6633,8 @@ static void test_host_ctl_regs(void)
/*
* PAT values higher than 8 are uninteresting since they're likely lumped
- * in with "8". We cap the tests at PAT value of 8 in order to reduce the
- * number of VM-Entries and keep the runtime reasonable.
+ * in with "8". We only test values above 8 one bit at a time,
+ * in order to reduce the number of VM-Entries and keep the runtime reasonable.
*/
#define PAT_VAL_LIMIT 8
@@ -6648,9 +6648,9 @@ static void test_pat(u32 field, const char * field_name, u32 ctrl_field,
int error;
vmcs_write(ctrl_field, ctrl_saved & ~ctrl_bit);
- for (i = 0; i <= PAT_VAL_LIMIT; i++) {
+ for (i = 0; i < 256; i = (i < PAT_VAL_LIMIT) ? i + 1 : i * 2) {
/* Test PAT0..PAT7 fields */
- for (j = 0; j < 8; j++) {
+ for (j = 0; j < (i ? 8 : 1); j++) {
val = i << j * 8;
vmcs_write(field, val);
report_prefix_pushf("%s %lx", field_name, val);
@@ -6660,9 +6660,9 @@ static void test_pat(u32 field, const char * field_name, u32 ctrl_field,
}
vmcs_write(ctrl_field, ctrl_saved | ctrl_bit);
- for (i = 0; i <= PAT_VAL_LIMIT; i++) {
+ for (i = 0; i < 256; i = (i < PAT_VAL_LIMIT) ? i + 1 : i * 2) {
/* Test PAT0..PAT7 fields */
- for (j = 0; j < 8; j++) {
+ for (j = 0; j < (i ? 8 : 1); j++) {
val = i << j * 8;
vmcs_write(field, val);
report_prefix_pushf("%s %lx", field_name, val);
For now I queued the patch with thse changes, holler if you disagree!
Thanks,
Paolo
