On Tue, Mar 10, 2020 at 06:51:49PM -0400, Krish Sadhukhan wrote:
According to section "Checks on Guest Segment Registers" in Intel SDM vol 3C,
the following checks are performed on the Guest Segment Registers on vmentry
of nested guests:
Selector fields:
— TR. The TI flag (bit 2) must be 0.
— LDTR. If LDTR is usable, the TI flag (bit 2) must be 0.
— SS. If the guest will not be virtual-8086 and the "unrestricted
guest" VM-execution control is 0, the RPL (bits 1:0) must equal
the RPL of the selector field for CS.1
Base-address fields:
— CS, SS, DS, ES, FS, GS. If the guest will be virtual-8086, the
address must be the selector field shifted left 4 bits (multiplied
by 16).
— The following checks are performed on processors that support Intel
64 architecture:
TR, FS, GS. The address must be canonical.
LDTR. If LDTR is usable, the address must be canonical.
CS. Bits 63:32 of the address must be zero.
SS, DS, ES. If the register is usable, bits 63:32 of the
address must be zero.
Signed-off-by: Krish Sadhukhan <krish.sadhukhan@xxxxxxxxxx>
---
lib/x86/processor.h | 1 +
x86/vmx_tests.c | 109 ++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 110 insertions(+)
diff --git a/lib/x86/processor.h b/lib/x86/processor.h
index 03fdf64..3642212 100644
--- a/lib/x86/processor.h
+++ b/lib/x86/processor.h
@@ -57,6 +57,7 @@
#define X86_EFLAGS_OF 0x00000800
#define X86_EFLAGS_IOPL 0x00003000
#define X86_EFLAGS_NT 0x00004000
+#define X86_EFLAGS_VM 0x00020000
#define X86_EFLAGS_AC 0x00040000
#define X86_EFLAGS_ALU (X86_EFLAGS_CF | X86_EFLAGS_PF | X86_EFLAGS_AF | \
diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index a7abd63..5e96dfa 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -7681,6 +7681,113 @@ static void test_load_guest_pat(void)
test_pat(GUEST_PAT, "GUEST_PAT", ENT_CONTROLS, ENT_LOAD_PAT);
}
+#define GUEST_SEG_USABLE_MASK 1u << 16
s/USABLE/UNUSABLE
The usage below looks correct, just the name is inverted.
+#define TEST_SEGMENT_SEL(seg_sel, seg_sel_name, val, val_saved) \
+ vmcs_write(seg_sel, val); \
+ enter_guest_with_invalid_guest_state(); \
+ report_guest_state_test(seg_sel_name, \
+ VMX_ENTRY_FAILURE | \
+ VMX_FAIL_STATE, \
+ val, seg_sel_name); \
+ vmcs_write(seg_sel, val_saved);
+
+/*
+ * The following checks are done on the Selector field of the Guest Segment
+ * Registers:
+ * — TR. The TI flag (bit 2) must be 0.
+ * — LDTR. If LDTR is usable, the TI flag (bit 2) must be 0.
+ * — SS. If the guest will not be virtual-8086 and the "unrestricted
+ * guest" VM-execution control is 0, the RPL (bits 1:0) must equal
+ * the RPL of the selector field for CS.
+ *
+ * [Intel SDM]
+ */
+static void test_guest_segment_sel_fields(void)
+{
+ u16 sel_saved;
+ u16 sel;
+
+ sel_saved = vmcs_read(GUEST_SEL_TR);
+ sel = sel_saved | 0x4;
+ TEST_SEGMENT_SEL(GUEST_SEL_TR, "GUEST_SEL_TR", sel, sel_saved);
+
+ sel_saved = vmcs_read(GUEST_SEL_LDTR);
+ sel = sel_saved | 0x4;
+ TEST_SEGMENT_SEL(GUEST_SEL_LDTR, "GUEST_SEL_LDTR", sel, sel_saved);
+
+ if (!(vmcs_read(GUEST_RFLAGS) & X86_EFLAGS_VM) &&
+ !(vmcs_read(CPU_SECONDARY) & CPU_URG)) {
Rather than react to the environment, these tests should configure every
relevant aspect and ignore the ones it can't change. E.g. the unit tests
aren't going to randomly launch a vm86 guest. Ditto for the unusuable bit,
it's unlikely to be set for most segments and would be something to test
explicitly.
+ u16 cs_rpl_bits = vmcs_read(GUEST_SEL_CS) & 0x3;
+ sel_saved = vmcs_read(GUEST_SEL_SS);
+ sel = sel_saved | (~cs_rpl_bits & 0x3);
+ TEST_SEGMENT_SEL(GUEST_SEL_SS, "GUEST_SEL_SS", sel, sel_saved);
+ }
+}
+
+#define TEST_SEGMENT_BASE_ADDR_UPPER_BITS(seg_base, seg_base_name) \
+ addr_saved = vmcs_read(seg_base); \
+ for (i = 32; i < 63; i = i + 4) { \
+ addr = addr_saved | 1ull << i; \
+ vmcs_write(seg_base, addr); \
+ enter_guest_with_invalid_guest_state(); \
+ report_guest_state_test(seg_base_name, \
+ VMX_ENTRY_FAILURE | \
+ VMX_FAIL_STATE, \
+ addr, seg_base_name); \
+ } \
+ \
+ vmcs_write(seg_base, addr_saved);
+
+#define TEST_SEGMENT_BASE_ADDR_CANONICAL(seg_base, seg_base_name) \
+ addr_saved = vmcs_read(seg_base); \
+ vmcs_write(seg_base, NONCANONICAL); \
+ enter_guest_with_invalid_guest_state(); \
+ report_guest_state_test(seg_base_name, \
+ VMX_ENTRY_FAILURE | VMX_FAIL_STATE, \
+ NONCANONICAL, seg_base_name); \
+ vmcs_write(seg_base, addr_saved);
+
+/*
+ * The following checks are done on the Base Address field of the Guest
+ * Segment Registers on processors that support Intel 64 architecture:
+ * - TR, FS, GS : The address must be canonical.
+ * - LDTR : If LDTR is usable, the address must be canonical.
+ * - CS : Bits 63:32 of the address must be zero.
+ * - SS, DS, ES : If the register is usable, bits 63:32 of the address
+ * must be zero.
+ *
+ * [Intel SDM]
+ */
+static void test_guest_segment_base_addr_fields(void)
+{
+ u64 addr_saved, addr;
+ int i;
+
+ /*
+ * The address of TR, FS, GS and LDTR must be canonical.
+ */
+ TEST_SEGMENT_BASE_ADDR_CANONICAL(GUEST_BASE_TR, "GUEST_BASE_TR");
+ TEST_SEGMENT_BASE_ADDR_CANONICAL(GUEST_BASE_FS, "GUEST_BASE_FS");
+ TEST_SEGMENT_BASE_ADDR_CANONICAL(GUEST_BASE_GS, "GUEST_BASE_GS");
FS/GS bases aren't checked if the segment is unusable.
+ if (!(vmcs_read(GUEST_AR_LDTR) & GUEST_SEG_USABLE_MASK))
+ TEST_SEGMENT_BASE_ADDR_CANONICAL(GUEST_BASE_LDTR,
+ "GUEST_BASE_LDTR");
+
+ /*
+ * Bits 63:32 in CS, SS, DS and ES base address must be zero
+ */
+ TEST_SEGMENT_BASE_ADDR_UPPER_BITS(GUEST_BASE_CS, "GUEST_BASE_CS");
+ if (!(vmcs_read(GUEST_AR_SS) & GUEST_SEG_USABLE_MASK))
+ TEST_SEGMENT_BASE_ADDR_UPPER_BITS(GUEST_BASE_SS,
+ "GUEST_BASE_SS");
+ if (!(vmcs_read(GUEST_AR_DS) & GUEST_SEG_USABLE_MASK))
+ TEST_SEGMENT_BASE_ADDR_UPPER_BITS(GUEST_BASE_DS,
+ "GUEST_BASE_DS");
+ if (!(vmcs_read(GUEST_AR_ES) & GUEST_SEG_USABLE_MASK))
+ TEST_SEGMENT_BASE_ADDR_UPPER_BITS(GUEST_BASE_ES,
+ "GUEST_BASE_ES");
+}
+
/*
* Check that the virtual CPU checks the VMX Guest State Area as
* documented in the Intel SDM.
@@ -7701,6 +7808,8 @@ static void vmx_guest_state_area_test(void)
test_load_guest_pat();
test_guest_efer();
test_load_guest_perf_global_ctrl();
+ test_guest_segment_sel_fields();
+ test_guest_segment_base_addr_fields();
/*
* Let the guest finish execution
--
1.8.3.1