[kvm-unit-tests PATCH v3 0/2] x86/emulator: Add some tests for loading segment descriptor in emulator

[kvm-unit-tests PATCH v3 0/2] x86/emulator: Add some tests for loading segment descriptor in emulator

[Hou Wenlong]

Add some far ret and far jmp tests for the related x86 emulator bugs[*].
Those tests would be tested both on hardware and emulator. Enable
kvm.force_emulation_prefix to test them on emulator.

Changed from v2:
- Fix some complication errors, which are not gotten in my gcc.
- Rename lret as far ret, and rename ljmp as far jmp.

Changed from v1:
- As Sean suggested, refactor the test loop to make the code simple.

v2: https://lore.kernel.org/kvm/cover.1644311445.git.houwenlong.hwl@antgroup.com

[*] https://lore.kernel.org/kvm/cover.1644292363.git.houwenlong.hwl@antgroup.com

Hou Wenlong (3):
  x86/emulator: Add some tests for far ret instruction emulation
  x86/emulator: Rename test_ljmp() as test_far_jmp()
  x86/emulator: Add some tests for far jmp instruction emulation

 x86/emulator.c | 202 +++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 188 insertions(+), 14 deletions(-)

--
2.31.1

[kvm-unit-tests PATCH v3 1/3] x86/emulator: Add some tests for far ret instruction emulation

[Hou Wenlong]

Per Intel's SDM on the "Instruction Set Reference", when
loading segment descriptor for far return, not-present segment
check should be after all type and privilege checks. However,
__load_segment_descriptor() in x86's emulator does not-present
segment check first, so it would trigger #NP instead of #GP
if type or privilege checks fail and the segment is not present.

And if RPL < CPL, it should trigger #GP, but the check is missing
in emulator.

So add some tests for far ret instruction, and it will test
those tests on hardware and emulator. Enable
kvm.force_emulation_prefix when try to test them on emulator.

Signed-off-by: Hou Wenlong <houwenlong.hwl@antgroup.com>
---
 x86/emulator.c | 129 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 129 insertions(+)

diff --git a/x86/emulator.c b/x86/emulator.c
index cd78e3cbbcd7..c56b32be8baa 100644
--- a/x86/emulator.c
+++ b/x86/emulator.c
@@ -21,6 +21,60 @@ static int exceptions;
 #define KVM_FEP "ud2; .byte 'k', 'v', 'm';"
 #define KVM_FEP_LENGTH 5
 static int fep_available = 1;
+static int far_xfer_vector = -1;
+static unsigned int far_xfer_error_code = -1;
+
+struct far_xfer_test_case {
+	uint16_t rpl;
+	uint16_t type;
+	uint16_t dpl;
+	uint16_t p;
+	bool usermode;
+	int vector;
+	const char *msg;
+};
+
+enum far_xfer_insn {
+	FAR_XFER_RET,
+};
+
+struct far_xfer_test {
+	enum far_xfer_insn insn;
+	const char *insn_name;
+	struct far_xfer_test_case *testcases;
+	unsigned int nr_testcases;
+};
+
+#define NON_CONFORM_CS_TYPE	0xb
+#define CONFORM_CS_TYPE		0xf
+#define DS_TYPE			0x3
+
+static struct far_xfer_test_case far_ret_testcases[] = {
+	{0, DS_TYPE,		 0, 0, false, GP_VECTOR, "desc.type!=code && desc.p=0"},
+	{0, NON_CONFORM_CS_TYPE, 3, 0, false, GP_VECTOR, "non-conforming && dpl!=rpl && desc.p=0"},
+	{0, CONFORM_CS_TYPE,	 3, 0, false, GP_VECTOR, "conforming && dpl>rpl && desc.p=0"},
+	{0, NON_CONFORM_CS_TYPE, 0, 0, false, NP_VECTOR, "desc.p=0"},
+	{0, NON_CONFORM_CS_TYPE, 3, 1, true,  GP_VECTOR, "rpl<cpl"},
+};
+
+static struct far_xfer_test far_ret_test = {
+	.insn = FAR_XFER_RET,
+	.insn_name = "far ret",
+	.testcases = &far_ret_testcases[0],
+	.nr_testcases = sizeof(far_ret_testcases) / sizeof(struct far_xfer_test_case),
+};
+
+#define TEST_FAR_RET_ASM(seg, prefix)		\
+({						\
+	asm volatile("lea 1f(%%rip), %%rax\n\t" \
+		     "pushq %[asm_seg]\n\t"	\
+		     "pushq $2f\n\t"		\
+		      prefix "lretq\n\t"	\
+		     "1: addq $16, %%rsp\n\t"	\
+		     "2:"			\
+		     : : [asm_seg]"r"((u64)seg)	\
+		     : "eax", "memory");	\
+})
 
 struct regs {
 	u64 rax, rbx, rcx, rdx;
@@ -895,6 +949,79 @@ static void test_mov_dr(uint64_t *mem)
 		report(rax == DR6_ACTIVE_LOW, "mov_dr6");
 }
 
+static void far_xfer_exception_handler(struct ex_regs *regs)
+{
+	far_xfer_vector = regs->vector;
+	far_xfer_error_code = regs->error_code;
+	regs->rip = regs->rax;
+}
+
+static void __test_far_xfer(enum far_xfer_insn insn, uint16_t seg,
+			    bool force_emulation)
+{
+	switch (insn) {
+	case FAR_XFER_RET:
+		if (force_emulation)
+			TEST_FAR_RET_ASM(seg, KVM_FEP);
+		else
+			TEST_FAR_RET_ASM(seg, "");
+		break;
+	default:
+		report_fail("Unexpected insn enum = %d\n", insn);
+		break;
+	}
+}
+
+static void test_far_xfer(bool force_emulation, struct far_xfer_test *test)
+{
+	struct far_xfer_test_case *t;
+	uint16_t seg;
+	bool ign;
+	int i;
+
+	handle_exception(GP_VECTOR, far_xfer_exception_handler);
+	handle_exception(NP_VECTOR, far_xfer_exception_handler);
+
+	for (i = 0; i < test->nr_testcases; i++) {
+		t = &test->testcases[i];
+
+		seg = FIRST_SPARE_SEL | t->rpl;
+		gdt[seg / 8] = gdt[(t->usermode ? USER_CS64 : KERNEL_CS) / 8];
+		gdt[seg / 8].type = t->type;
+		gdt[seg / 8].dpl = t->dpl;
+		gdt[seg / 8].p = t->p;
+
+		far_xfer_vector = -1;
+		far_xfer_error_code = -1;
+
+		if (t->usermode)
+			run_in_user((usermode_func)__test_far_xfer, UD_VECTOR,
+				    test->insn, seg, force_emulation, 0, &ign);
+		else
+			__test_far_xfer(test->insn, seg, force_emulation);
+
+		report(far_xfer_vector == t->vector &&
+		       (far_xfer_vector < 0 || far_xfer_error_code == FIRST_SPARE_SEL),
+		       "%s on %s, %s: wanted %d (%d), got %d (%d)",
+		       test->insn_name, force_emulation ? "emulator" : "hardware", t->msg,
+		       t->vector, t->vector < 0 ? -1 : FIRST_SPARE_SEL,
+		       far_xfer_vector, far_xfer_error_code);
+	}
+
+	handle_exception(GP_VECTOR, 0);
+	handle_exception(NP_VECTOR, 0);
+}
+
+static void test_far_ret(uint64_t *mem)
+{
+	test_far_xfer(false, &far_ret_test);
+}
+
+static void test_em_far_ret(uint64_t *mem)
+{
+	test_far_xfer(true, &far_ret_test);
+}
+
 static void test_push16(uint64_t *mem)
 {
 	uint64_t rsp1, rsp2;
@@ -1169,6 +1296,7 @@ int main(void)
 	test_smsw(mem);
 	test_lmsw();
 	test_ljmp(mem);
+	test_far_ret(mem);
 	test_stringio();
 	test_incdecnotneg(mem);
 	test_btc(mem);
@@ -1193,6 +1321,7 @@ int main(void)
 		test_smsw_reg(mem);
 		test_nop(mem);
 		test_mov_dr(mem);
+		test_em_far_ret(mem);
 	} else {
 		report_skip("skipping register-only tests, "
 			    "use kvm.force_emulation_prefix=1 to enable");
-- 
2.31.1

Re: [kvm-unit-tests PATCH v3 1/3] x86/emulator: Add some tests for far ret instruction emulation

[Sean Christopherson]

On Thu, Feb 10, 2022, Hou Wenlong wrote:
> Per Intel's SDM on the "Instruction Set Reference", when
> loading segment descriptor for far return, not-present segment
> check should be after all type and privilege checks. However,
> __load_segment_descriptor() in x86's emulator does not-present
> segment check first, so it would trigger #NP instead of #GP
> if type or privilege checks fail and the segment is not present.
> 
> And if RPL < CPL, it should trigger #GP, but the check is missing
> in emulator.
> 
> So add some tests for far ret instruction, and it will test
> those tests on hardware and emulator. Enable
> kvm.force_emulation_prefix when try to test them on emulator.
> 
> Signed-off-by: Hou Wenlong <houwenlong.hwl@antgroup.com>
> ---

With fixup for the -fPIC issue...

Reviewed-and-tested-by: Sean Christopherson <seanjc@google.com>

> +#define TEST_FAR_RET_ASM(seg, prefix)		\
> +({						\
> +	asm volatile("lea 1f(%%rip), %%rax\n\t" \
> +		     "pushq %[asm_seg]\n\t"	\
> +		     "pushq $2f\n\t"		\
> +		      prefix "lretq\n\t"	\
> +		     "1: addq $16, %%rsp\n\t"	\
> +		     "2:"			\
> +		     : : [asm_seg]"r"((u64)seg)	\
> +		     : "eax", "memory");	\
> +})


The "push $2f" generates an absolute address and fails to build with --target-efi,
which requires -fPIC.  The easiest thing that comes to mind is to load the address
into RAX and then push RAX.  The lea to get the exception IRET target into RAX needs
to be moved down, but that's ok.

---
 x86/emulator.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/x86/emulator.c b/x86/emulator.c
index c56b32b..c62dced 100644
--- a/x86/emulator.c
+++ b/x86/emulator.c
@@ -66,9 +66,10 @@ static struct far_xfer_test far_ret_test = {

 #define TEST_FAR_RET_ASM(seg, prefix)		\
 ({						\
-	asm volatile("lea 1f(%%rip), %%rax\n\t" \
-		     "pushq %[asm_seg]\n\t"	\
-		     "pushq $2f\n\t"		\
+	asm volatile("pushq %[asm_seg]\n\t"	\
+		     "lea 2f(%%rip), %%rax\n\t" \
+		     "pushq %%rax\n\t"		\
+		     "lea 1f(%%rip), %%rax\n\t" \
 		      prefix "lretq\n\t"	\
 		     "1: addq $16, %%rsp\n\t"	\
 		     "2:"			\

base-commit: 41d3306e19784478679910ee0afa55de05279b42
--

[kvm-unit-tests PATCH v3 2/3] x86/emulator: Rename test_ljmp() as test_far_jmp()

[Hou Wenlong]

Rename test_ljmp() as test_far_jmp() to better match the SDM. Also
change the output of test to explain what it is doing.

Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: Hou Wenlong <houwenlong.hwl@antgroup.com>
---
 x86/emulator.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/x86/emulator.c b/x86/emulator.c
index c56b32be8baa..45972c2fe940 100644
--- a/x86/emulator.c
+++ b/x86/emulator.c
@@ -358,7 +358,7 @@ static void test_pop(void *mem)
 	       "enter");
 }
 
-static void test_ljmp(void *mem)
+static void test_far_jmp(void *mem)
 {
     unsigned char *m = mem;
     volatile int res = 1;
@@ -368,7 +368,7 @@ static void test_ljmp(void *mem)
     asm volatile ("rex64 ljmp *%0"::"m"(*m));
     res = 0;
 jmpf:
-    report(res, "ljmp");
+    report(res, "far jmp, via emulated MMIO");
 }
 
 static void test_incdecnotneg(void *mem)
@@ -1295,7 +1295,7 @@ int main(void)
 
 	test_smsw(mem);
 	test_lmsw();
-	test_ljmp(mem);
+	test_far_jmp(mem);
 	test_far_ret(mem);
 	test_stringio();
 	test_incdecnotneg(mem);
-- 
2.31.1

Re: [kvm-unit-tests PATCH v3 2/3] x86/emulator: Rename test_ljmp() as test_far_jmp()

[Sean Christopherson]

On Thu, Feb 10, 2022, Hou Wenlong wrote:
> Rename test_ljmp() as test_far_jmp() to better match the SDM. Also
> change the output of test to explain what it is doing.
> 
> Suggested-by: Sean Christopherson <seanjc@google.com>
> Signed-off-by: Hou Wenlong <houwenlong.hwl@antgroup.com>
> ---

Reviewed-and-tested-by: Sean Christopherson <seanjc@google.com>

[kvm-unit-tests PATCH v3 3/3] x86/emulator: Add some tests for far jmp instruction emulation

[Hou Wenlong]

Per Intel's SDM on the "Instruction Set Reference", when
loading segment descriptor for far jmp, not-present segment
check should be after all type and privilege checks. However,
__load_segment_descriptor() in x86's emulator does not-present
segment check first, so it would trigger #NP instead of #GP
if type or privilege checks fail and the segment is not present.

So add some tests for far jmp instruction, and it will test
those tests on hardware and emulator. Enable
kvm.force_emulation_prefix when try to test them on emulator.

Signed-off-by: Hou Wenlong <houwenlong.hwl@antgroup.com>
---
 x86/emulator.c | 71 +++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 58 insertions(+), 13 deletions(-)

diff --git a/x86/emulator.c b/x86/emulator.c
index 45972c2fe940..7e98bacd714a 100644
--- a/x86/emulator.c
+++ b/x86/emulator.c
@@ -36,6 +36,7 @@ struct far_xfer_test_case {
 
 enum far_xfer_insn {
 	FAR_XFER_RET,
+	FAR_XFER_JMP,
 };
 
 struct far_xfer_test {
@@ -64,6 +65,25 @@ static struct far_xfer_test far_ret_test = {
 	.nr_testcases = sizeof(far_ret_testcases) / sizeof(struct far_xfer_test_case),
 };
 
+static struct far_xfer_test_case far_jmp_testcases[] = {
+	{0, DS_TYPE,		 0, 0, false, GP_VECTOR, "desc.type!=code && desc.p=0"},
+	{0, NON_CONFORM_CS_TYPE, 3, 0, false, GP_VECTOR, "non-conforming && dpl!=cpl && desc.p=0"},
+	{3, NON_CONFORM_CS_TYPE, 0, 0, false, GP_VECTOR, "conforming && rpl>cpl && desc.p=0"},
+	{0, CONFORM_CS_TYPE,	 3, 0, false, GP_VECTOR, "conforming && dpl>cpl && desc.p=0"},
+	{0, NON_CONFORM_CS_TYPE, 0, 0, false, NP_VECTOR, "desc.p=0"},
+	{3, CONFORM_CS_TYPE,	 0, 1, true,  -1,	 "dpl<cpl"},
+};
+
+static struct far_xfer_test far_jmp_test = {
+	.insn = FAR_XFER_JMP,
+	.insn_name = "far jmp",
+	.testcases = &far_jmp_testcases[0],
+	.nr_testcases = sizeof(far_jmp_testcases) / sizeof(struct far_xfer_test_case),
+};
+
+static unsigned long fep_jmp_buf[2];
+static unsigned long *fep_jmp_buf_ptr = &fep_jmp_buf[0];
+
 #define TEST_FAR_RET_ASM(seg, prefix)		\
 ({						\
 	asm volatile("lea 1f(%%rip), %%rax\n\t" \
@@ -76,6 +96,17 @@ static struct far_xfer_test far_ret_test = {
 		     : "eax", "memory");	\
 })
 
+#define TEST_FAR_JMP_ASM(seg, prefix)		\
+({						\
+	*(uint16_t *)(&fep_jmp_buf[1]) = seg;	\
+	asm volatile("lea 1f(%%rip), %%rax\n\t" \
+		     "movq $1f, (%[mem])\n\t"	\
+		      prefix "rex64 ljmp *(%[mem])\n\t"\
+		     "1:"			\
+		     : : [mem]"r"(fep_jmp_buf_ptr)\
+		     : "eax", "memory");	\
+})
+
 struct regs {
 	u64 rax, rbx, rcx, rdx;
 	u64 rsi, rdi, rsp, rbp;
@@ -358,19 +389,6 @@ static void test_pop(void *mem)
 	       "enter");
 }
 
-static void test_far_jmp(void *mem)
-{
-    unsigned char *m = mem;
-    volatile int res = 1;
-
-    *(unsigned long**)m = &&jmpf;
-    asm volatile ("data16 mov %%cs, %0":"=m"(*(m + sizeof(unsigned long))));
-    asm volatile ("rex64 ljmp *%0"::"m"(*m));
-    res = 0;
-jmpf:
-    report(res, "far jmp, via emulated MMIO");
-}
-
 static void test_incdecnotneg(void *mem)
 {
     unsigned long *m = mem, v = 1234;
@@ -966,6 +984,12 @@ static void __test_far_xfer(enum far_xfer_insn insn, uint16_t seg,
 		else
 			TEST_FAR_RET_ASM(seg, "");
 		break;
+	case FAR_XFER_JMP:
+		if (force_emulation)
+			TEST_FAR_JMP_ASM(seg, KVM_FEP);
+		else
+			TEST_FAR_JMP_ASM(seg, "");
+		break;
 	default:
 		report_fail("Unexpected insn enum = %d\n", insn);
 		break;
@@ -1012,6 +1036,26 @@ static void test_far_xfer(bool force_emulation, struct far_xfer_test *test)
 	handle_exception(NP_VECTOR, 0);
 }
 
+static void test_far_jmp(uint64_t *mem)
+{
+	unsigned char *m = (unsigned char *)mem;
+	volatile int res = 1;
+
+	*(unsigned long**)m = &&jmpf;
+	asm volatile ("data16 mov %%cs, %0":"=m"(*(m + sizeof(unsigned long))));
+	asm volatile ("rex64 ljmp *%0"::"m"(*m));
+	res = 0;
+jmpf:
+	report(res, "far jmp, via emulated MMIO");
+
+	test_far_xfer(false, &far_jmp_test);
+}
+
+static void test_em_far_jmp(uint64_t *mem)
+{
+	test_far_xfer(true, &far_jmp_test);
+}
+
 static void test_far_ret(uint64_t *mem)
 {
 	test_far_xfer(false, &far_ret_test);
@@ -1321,6 +1365,7 @@ int main(void)
 		test_smsw_reg(mem);
 		test_nop(mem);
 		test_mov_dr(mem);
+		test_em_far_jmp(mem);
 		test_em_far_ret(mem);
 	} else {
 		report_skip("skipping register-only tests, "
-- 
2.31.1

Re: [kvm-unit-tests PATCH v3 3/3] x86/emulator: Add some tests for far jmp instruction emulation

[Sean Christopherson]

On Thu, Feb 10, 2022, Hou Wenlong wrote:
> @@ -76,6 +96,17 @@ static struct far_xfer_test far_ret_test = {
>  		     : "eax", "memory");	\
>  })
>  
> +#define TEST_FAR_JMP_ASM(seg, prefix)		\
> +({						\
> +	*(uint16_t *)(&fep_jmp_buf[1]) = seg;	\
> +	asm volatile("lea 1f(%%rip), %%rax\n\t" \
> +		     "movq $1f, (%[mem])\n\t"	\
> +		      prefix "rex64 ljmp *(%[mem])\n\t"\
> +		     "1:"			\
> +		     : : [mem]"r"(fep_jmp_buf_ptr)\
> +		     : "eax", "memory");	\

Align the backslashes for a given macro, even though it means the two TEST_FAR_*_ASM
macros won't share alignment.  This needs an -fPIC tweak for the movq too, but this
one is easy since RAX already holds what we want.

With the below fixup...

Reviewed-and-tested-by: Sean Christopherson <seanjc@google.com>

---
 x86/emulator.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/x86/emulator.c b/x86/emulator.c
index 22a5c9d..56a263e 100644
--- a/x86/emulator.c
+++ b/x86/emulator.c
@@ -97,15 +97,15 @@ static unsigned long *fep_jmp_buf_ptr = &fep_jmp_buf[0];
 		     : "eax", "memory");	\
 })

-#define TEST_FAR_JMP_ASM(seg, prefix)		\
-({						\
-	*(uint16_t *)(&fep_jmp_buf[1]) = seg;	\
-	asm volatile("lea 1f(%%rip), %%rax\n\t" \
-		     "movq $1f, (%[mem])\n\t"	\
-		      prefix "rex64 ljmp *(%[mem])\n\t"\
-		     "1:"			\
-		     : : [mem]"r"(fep_jmp_buf_ptr)\
-		     : "eax", "memory");	\
+#define TEST_FAR_JMP_ASM(seg, prefix)			\
+({							\
+	*(uint16_t *)(&fep_jmp_buf[1]) = seg;		\
+	asm volatile("lea 1f(%%rip), %%rax\n\t"		\
+		     "movq %%rax, (%[mem])\n\t"		\
+		      prefix "rex64 ljmp *(%[mem])\n\t"	\
+		     "1:"				\
+		     : : [mem]"r"(fep_jmp_buf_ptr)	\
+		     : "eax", "memory");		\
 })

 struct regs {

base-commit: 6bd9c4b6a79ed51c0e3e6a997654f4a9feb9c377
--