| Message ID | 20190812211108.17186-1-krish.sadhukhan@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kvm-unit-test: nVMX: Fix 95d6d2c32288 ("nVMX: Test Host Segment Registers and Descriptor Tables on vmentry of nested guests") | expand |
On 12/08/19 23:11, Krish Sadhukhan wrote: > Commit 95d6d2c32288 added a test for the Segment Selector VMCS field. That test > sets the "host address-space size" VM-exit control to zero and as a result, on > VM-exit the guest exits as 32-bit. Since vmx tests are 64-bit, this results in > a hardware error. > This patch also cleans up a few other areas in commit 95d6d2c32288, including > replacing make_non_canonical() with NONCANONICAL. > > Reported-by: Nadav Amit <nadav.amit@gmail.com> > Signed-off-by: Krish Sadhukhan <kris.sadhukhan@oracle.com> > Reviewed-by: Karl Heubaum <karl.heubaum@oracle.com> > --- > lib/x86/processor.h | 5 --- > x86/vmx_tests.c | 76 ++++++++++++++++++++------------------------- > 2 files changed, 33 insertions(+), 48 deletions(-) > > diff --git a/lib/x86/processor.h b/lib/x86/processor.h > index 8b8bb7a..4fef0bc 100644 > --- a/lib/x86/processor.h > +++ b/lib/x86/processor.h > @@ -461,11 +461,6 @@ static inline void write_pkru(u32 pkru) > : : "a" (eax), "c" (ecx), "d" (edx)); > } > > -static inline u64 make_non_canonical(u64 addr) > -{ > - return (addr | 1ull << 48); > -} > - > static inline bool is_canonical(u64 addr) > { > return (s64)(addr << 16) >> 16 == addr; > diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c > index 8ad2674..e115e48 100644 > --- a/x86/vmx_tests.c > +++ b/x86/vmx_tests.c > @@ -6952,15 +6952,14 @@ static void test_load_host_pat(void) > } > > /* > - * Test a value for the given VMCS field. > - * > - * "field" - VMCS field > - * "field_name" - string name of VMCS field > - * "bit_start" - starting bit > - * "bit_end" - ending bit > - * "val" - value that the bit range must or must not contain > - * "valid_val" - whether value given in 'val' must be valid or not > - * "error" - expected VMCS error when vmentry fails for an invalid value > + * test_vmcs_field - test a value for the given VMCS field > + * @field: VMCS field > + * @field_name: string name of VMCS field > + * @bit_start: starting bit > + * @bit_end: ending bit > + * @val: value that the bit range must or must not contain > + * @valid_val: whether value given in 'val' must be valid or not > + * @error: expected VMCS error when vmentry fails for an invalid value > */ > static void test_vmcs_field(u64 field, const char *field_name, u32 bit_start, > u32 bit_end, u64 val, bool valid_val, u32 error) > @@ -7004,16 +7003,14 @@ static void test_vmcs_field(u64 field, const char *field_name, u32 bit_start, > static void test_canonical(u64 field, const char * field_name) > { > u64 addr_saved = vmcs_read(field); > - u64 addr = addr_saved; > > - report_prefix_pushf("%s %lx", field_name, addr); > - if (is_canonical(addr)) { > + report_prefix_pushf("%s %lx", field_name, addr_saved); > + if (is_canonical(addr_saved)) { > test_vmx_vmlaunch(0, false); > report_prefix_pop(); > > - addr = make_non_canonical(addr); > - vmcs_write(field, addr); > - report_prefix_pushf("%s %lx", field_name, addr); > + vmcs_write(field, NONCANONICAL); > + report_prefix_pushf("%s %llx", field_name, NONCANONICAL); > test_vmx_vmlaunch(VMXERR_ENTRY_INVALID_HOST_STATE_FIELD, > false); > > @@ -7025,6 +7022,14 @@ static void test_canonical(u64 field, const char * field_name) > report_prefix_pop(); > } > > +#define TEST_RPL_TI_FLAGS(reg, name) \ > + test_vmcs_field(reg, name, 0, 2, 0x0, true, \ > + VMXERR_ENTRY_INVALID_HOST_STATE_FIELD); > + > +#define TEST_CS_TR_FLAGS(reg, name) \ > + test_vmcs_field(reg, name, 3, 15, 0x0000, false, \ > + VMXERR_ENTRY_INVALID_HOST_STATE_FIELD); > + > /* > * 1. In the selector field for each of CS, SS, DS, ES, FS, GS and TR, the > * RPL (bits 1:0) and the TI flag (bit 2) must be 0. > @@ -7036,34 +7041,24 @@ static void test_canonical(u64 field, const char * field_name) > */ > static void test_host_segment_regs(void) > { > - u32 exit_ctrl_saved = vmcs_read(EXI_CONTROLS); > u16 selector_saved; > > /* > * Test RPL and TI flags > */ > - test_vmcs_field(HOST_SEL_CS, "HOST_SEL_CS", 0, 2, 0x0, true, > - VMXERR_ENTRY_INVALID_HOST_STATE_FIELD); > - test_vmcs_field(HOST_SEL_SS, "HOST_SEL_SS", 0, 2, 0x0, true, > - VMXERR_ENTRY_INVALID_HOST_STATE_FIELD); > - test_vmcs_field(HOST_SEL_DS, "HOST_SEL_DS", 0, 2, 0x0, true, > - VMXERR_ENTRY_INVALID_HOST_STATE_FIELD); > - test_vmcs_field(HOST_SEL_ES, "HOST_SEL_ES", 0, 2, 0x0, true, > - VMXERR_ENTRY_INVALID_HOST_STATE_FIELD); > - test_vmcs_field(HOST_SEL_FS, "HOST_SEL_FS", 0, 2, 0x0, true, > - VMXERR_ENTRY_INVALID_HOST_STATE_FIELD); > - test_vmcs_field(HOST_SEL_GS, "HOST_SEL_GS", 0, 2, 0x0, true, > - VMXERR_ENTRY_INVALID_HOST_STATE_FIELD); > - test_vmcs_field(HOST_SEL_TR, "HOST_SEL_TR", 0, 2, 0x0, true, > - VMXERR_ENTRY_INVALID_HOST_STATE_FIELD); > + TEST_RPL_TI_FLAGS(HOST_SEL_CS, "HOST_SEL_CS"); > + TEST_RPL_TI_FLAGS(HOST_SEL_SS, "HOST_SEL_SS"); > + TEST_RPL_TI_FLAGS(HOST_SEL_DS, "HOST_SEL_DS"); > + TEST_RPL_TI_FLAGS(HOST_SEL_ES, "HOST_SEL_ES"); > + TEST_RPL_TI_FLAGS(HOST_SEL_FS, "HOST_SEL_FS"); > + TEST_RPL_TI_FLAGS(HOST_SEL_GS, "HOST_SEL_GS"); > + TEST_RPL_TI_FLAGS(HOST_SEL_TR, "HOST_SEL_TR"); > > /* > * Test that CS and TR fields can not be 0x0000 > */ > - test_vmcs_field(HOST_SEL_CS, "HOST_SEL_CS", 3, 15, 0x0000, false, > - VMXERR_ENTRY_INVALID_HOST_STATE_FIELD); > - test_vmcs_field(HOST_SEL_TR, "HOST_SEL_TR", 3, 15, 0x0000, false, > - VMXERR_ENTRY_INVALID_HOST_STATE_FIELD); > + TEST_CS_TR_FLAGS(HOST_SEL_CS, "HOST_SEL_CS"); > + TEST_CS_TR_FLAGS(HOST_SEL_TR, "HOST_SEL_TR"); > > /* > * SS field can not be 0x0000 if "host address-space size" VM-exit > @@ -7071,20 +7066,15 @@ static void test_host_segment_regs(void) > */ > selector_saved = vmcs_read(HOST_SEL_SS); > vmcs_write(HOST_SEL_SS, 0); > - if (exit_ctrl_saved & EXI_HOST_64) { > - report_prefix_pushf("HOST_SEL_SS 0"); > + report_prefix_pushf("HOST_SEL_SS 0"); > + if (vmcs_read(EXI_CONTROLS) & EXI_HOST_64) { > test_vmx_vmlaunch(0, false); > - report_prefix_pop(); > - > - vmcs_write(EXI_CONTROLS, exit_ctrl_saved & ~EXI_HOST_64); > + } else { > + test_vmx_vmlaunch(VMXERR_ENTRY_INVALID_HOST_STATE_FIELD, false); > } > - > - report_prefix_pushf("HOST_SEL_SS 0"); > - test_vmx_vmlaunch(VMXERR_ENTRY_INVALID_HOST_STATE_FIELD, false); > report_prefix_pop(); > > vmcs_write(HOST_SEL_SS, selector_saved); > - vmcs_write(EXI_CONTROLS, exit_ctrl_saved); > > #ifdef __x86_64__ > /* > Queued, thanks. Paolo
diff --git a/lib/x86/processor.h b/lib/x86/processor.h index 8b8bb7a..4fef0bc 100644 --- a/lib/x86/processor.h +++ b/lib/x86/processor.h @@ -461,11 +461,6 @@ static inline void write_pkru(u32 pkru) : : "a" (eax), "c" (ecx), "d" (edx)); } -static inline u64 make_non_canonical(u64 addr) -{ - return (addr | 1ull << 48); -} - static inline bool is_canonical(u64 addr) { return (s64)(addr << 16) >> 16 == addr; diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c index 8ad2674..e115e48 100644 --- a/x86/vmx_tests.c +++ b/x86/vmx_tests.c @@ -6952,15 +6952,14 @@ static void test_load_host_pat(void) } /* - * Test a value for the given VMCS field. - * - * "field" - VMCS field - * "field_name" - string name of VMCS field - * "bit_start" - starting bit - * "bit_end" - ending bit - * "val" - value that the bit range must or must not contain - * "valid_val" - whether value given in 'val' must be valid or not - * "error" - expected VMCS error when vmentry fails for an invalid value + * test_vmcs_field - test a value for the given VMCS field + * @field: VMCS field + * @field_name: string name of VMCS field + * @bit_start: starting bit + * @bit_end: ending bit + * @val: value that the bit range must or must not contain + * @valid_val: whether value given in 'val' must be valid or not + * @error: expected VMCS error when vmentry fails for an invalid value */ static void test_vmcs_field(u64 field, const char *field_name, u32 bit_start, u32 bit_end, u64 val, bool valid_val, u32 error) @@ -7004,16 +7003,14 @@ static void test_vmcs_field(u64 field, const char *field_name, u32 bit_start, static void test_canonical(u64 field, const char * field_name) { u64 addr_saved = vmcs_read(field); - u64 addr = addr_saved; - report_prefix_pushf("%s %lx", field_name, addr); - if (is_canonical(addr)) { + report_prefix_pushf("%s %lx", field_name, addr_saved); + if (is_canonical(addr_saved)) { test_vmx_vmlaunch(0, false); report_prefix_pop(); - addr = make_non_canonical(addr); - vmcs_write(field, addr); - report_prefix_pushf("%s %lx", field_name, addr); + vmcs_write(field, NONCANONICAL); + report_prefix_pushf("%s %llx", field_name, NONCANONICAL); test_vmx_vmlaunch(VMXERR_ENTRY_INVALID_HOST_STATE_FIELD, false); @@ -7025,6 +7022,14 @@ static void test_canonical(u64 field, const char * field_name) report_prefix_pop(); } +#define TEST_RPL_TI_FLAGS(reg, name) \ + test_vmcs_field(reg, name, 0, 2, 0x0, true, \ + VMXERR_ENTRY_INVALID_HOST_STATE_FIELD); + +#define TEST_CS_TR_FLAGS(reg, name) \ + test_vmcs_field(reg, name, 3, 15, 0x0000, false, \ + VMXERR_ENTRY_INVALID_HOST_STATE_FIELD); + /* * 1. In the selector field for each of CS, SS, DS, ES, FS, GS and TR, the * RPL (bits 1:0) and the TI flag (bit 2) must be 0. @@ -7036,34 +7041,24 @@ static void test_canonical(u64 field, const char * field_name) */ static void test_host_segment_regs(void) { - u32 exit_ctrl_saved = vmcs_read(EXI_CONTROLS); u16 selector_saved; /* * Test RPL and TI flags */ - test_vmcs_field(HOST_SEL_CS, "HOST_SEL_CS", 0, 2, 0x0, true, - VMXERR_ENTRY_INVALID_HOST_STATE_FIELD); - test_vmcs_field(HOST_SEL_SS, "HOST_SEL_SS", 0, 2, 0x0, true, - VMXERR_ENTRY_INVALID_HOST_STATE_FIELD); - test_vmcs_field(HOST_SEL_DS, "HOST_SEL_DS", 0, 2, 0x0, true, - VMXERR_ENTRY_INVALID_HOST_STATE_FIELD); - test_vmcs_field(HOST_SEL_ES, "HOST_SEL_ES", 0, 2, 0x0, true, - VMXERR_ENTRY_INVALID_HOST_STATE_FIELD); - test_vmcs_field(HOST_SEL_FS, "HOST_SEL_FS", 0, 2, 0x0, true, - VMXERR_ENTRY_INVALID_HOST_STATE_FIELD); - test_vmcs_field(HOST_SEL_GS, "HOST_SEL_GS", 0, 2, 0x0, true, - VMXERR_ENTRY_INVALID_HOST_STATE_FIELD); - test_vmcs_field(HOST_SEL_TR, "HOST_SEL_TR", 0, 2, 0x0, true, - VMXERR_ENTRY_INVALID_HOST_STATE_FIELD); + TEST_RPL_TI_FLAGS(HOST_SEL_CS, "HOST_SEL_CS"); + TEST_RPL_TI_FLAGS(HOST_SEL_SS, "HOST_SEL_SS"); + TEST_RPL_TI_FLAGS(HOST_SEL_DS, "HOST_SEL_DS"); + TEST_RPL_TI_FLAGS(HOST_SEL_ES, "HOST_SEL_ES"); + TEST_RPL_TI_FLAGS(HOST_SEL_FS, "HOST_SEL_FS"); + TEST_RPL_TI_FLAGS(HOST_SEL_GS, "HOST_SEL_GS"); + TEST_RPL_TI_FLAGS(HOST_SEL_TR, "HOST_SEL_TR"); /* * Test that CS and TR fields can not be 0x0000 */ - test_vmcs_field(HOST_SEL_CS, "HOST_SEL_CS", 3, 15, 0x0000, false, - VMXERR_ENTRY_INVALID_HOST_STATE_FIELD); - test_vmcs_field(HOST_SEL_TR, "HOST_SEL_TR", 3, 15, 0x0000, false, - VMXERR_ENTRY_INVALID_HOST_STATE_FIELD); + TEST_CS_TR_FLAGS(HOST_SEL_CS, "HOST_SEL_CS"); + TEST_CS_TR_FLAGS(HOST_SEL_TR, "HOST_SEL_TR"); /* * SS field can not be 0x0000 if "host address-space size" VM-exit @@ -7071,20 +7066,15 @@ static void test_host_segment_regs(void) */ selector_saved = vmcs_read(HOST_SEL_SS); vmcs_write(HOST_SEL_SS, 0); - if (exit_ctrl_saved & EXI_HOST_64) { - report_prefix_pushf("HOST_SEL_SS 0"); + report_prefix_pushf("HOST_SEL_SS 0"); + if (vmcs_read(EXI_CONTROLS) & EXI_HOST_64) { test_vmx_vmlaunch(0, false); - report_prefix_pop(); - - vmcs_write(EXI_CONTROLS, exit_ctrl_saved & ~EXI_HOST_64); + } else { + test_vmx_vmlaunch(VMXERR_ENTRY_INVALID_HOST_STATE_FIELD, false); } - - report_prefix_pushf("HOST_SEL_SS 0"); - test_vmx_vmlaunch(VMXERR_ENTRY_INVALID_HOST_STATE_FIELD, false); report_prefix_pop(); vmcs_write(HOST_SEL_SS, selector_saved); - vmcs_write(EXI_CONTROLS, exit_ctrl_saved); #ifdef __x86_64__ /*