On Wed, Apr 03, 2019 at 05:59:05PM -0400, 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>
> ---
> x86/vmx_tests.c | 71 +++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 71 insertions(+)
>
> diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
> index 66a87f6..04b1aee 100644
> --- a/x86/vmx_tests.c
> +++ b/x86/vmx_tests.c
> @@ -4995,6 +4995,75 @@ static void test_sysenter_field(u32 field, const char *name)
> vmcs_write(field, addr_saved);
> }
>
> +/*
> + * 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.
> + */
> +#define PAT_VAL_LIMIT 8
> +
> +static void test_pat(u32 fld, const char * fld_name, u32 ctrl_fld, u64 ctrl_bit)
Please spell out "field" in all cases. Saving a few characters is not
worth the extra mental gymnastics required when reading the code. There
are zero hits for "fld" in KVM or kvm-unit-tests, and hundreds of hits
for "field".
> +{
> + u32 ctrl_saved = vmcs_read(ctrl_fld);
> + u64 pat_saved = vmcs_read(fld);
> + u64 i, val;
> + u32 j;
> + int error;
> +
> + vmcs_write(ctrl_fld, 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(fld, val);
> + report_prefix_pushf("%s %lx", fld_name, val);
> + test_vmx_vmlaunch(0, false);
> + report_prefix_pop();
> + }
> + }
> +
> + vmcs_write(ctrl_fld, 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(fld, val);
> + report_prefix_pushf("%s %lx", fld_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();
> + }
> + }
> +
> + vmcs_write(ctrl_fld, ctrl_saved);
> + vmcs_write(fld, pat_saved);
> +}
> +
> +/*
> + * 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-).
> + *
> + * [Intel SDM]
> + */
> +static void test_load_host_pat(void)
> +{
> + /*
> + * "load IA32_PAT" VM-exit control
> + */
> + if (!(ctrl_exit_rev.clr & EXI_LOAD_PAT)) {
> + printf("\"Load-IA32-PAT\" exit control not supported\n");
> + return;
> + }
> +
> + test_pat(HOST_PAT, "HOST_PAT", EXI_CONTROLS, EXI_LOAD_PAT);
> +}
> +
> /*
> * Check that the virtual CPU checks the VMX Host State Area as
> * documented in the Intel SDM.
> @@ -5010,6 +5079,8 @@ static void vmx_host_state_area_test(void)
>
> test_sysenter_field(HOST_SYSENTER_ESP, "HOST_SYSENTER_ESP");
> test_sysenter_field(HOST_SYSENTER_EIP, "HOST_SYSENTER_EIP");
> +
> + test_load_host_pat();
> }
>
> static bool valid_vmcs_for_vmentry(void)
> --
> 2.17.2
>
