| Message ID | 006c75f53539958c1e5d5a0a5073566a5395414e.1644311445.git.houwenlong.hwl@antgroup.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/emulator: Add some tests for loading segment descriptor in emulator | expand |
Nit, "far ret" instead of "lret" in the shortlog. On Tue, Feb 08, 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 lret instruction, and it will test > those tests in hardware and emulator. Enable > kvm.force_emulation_prefix when try to test them in emulator. > > Signed-off-by: Hou Wenlong <houwenlong.hwl@antgroup.com> > --- Was this tested? There are multiple compilation errors with gcc... x86/emulator.c: In function ‘test_far_xfer’: x86/emulator.c:1034:10: error: format not a string literal and no format arguments [-Werror=format-security] 1034 | far_xfer_error_code == t->error_code, t->msg); | ^~~~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors make: *** [<builtin>: x86/emulator.o] Error 1 x86/emulator.c: Assembler messages: x86/emulator.c:81: Error: incorrect register `%si' used with `q' suffix x86/emulator.c:83: Error: incorrect register `%si' used with `q' suffix make: *** [<builtin>: x86/emulator.o] Error 1 > +static struct far_xfer_test_case far_ret_testcases[] = { > + {0, DS_TYPE, 0, 0, false, GP_VECTOR, FIRST_SPARE_SEL, "lret desc.type!=code && desc.p=0"}, > + {0, NON_CONFORM_CS_TYPE, 3, 0, false, GP_VECTOR, FIRST_SPARE_SEL, "lret non-conforming && dpl!=rpl && desc.p=0"}, > + {0, CONFORM_CS_TYPE, 3, 0, false, GP_VECTOR, FIRST_SPARE_SEL, "lret conforming && dpl>rpl && desc.p=0"}, > + {0, NON_CONFORM_CS_TYPE, 0, 0, false, NP_VECTOR, FIRST_SPARE_SEL, "lret desc.p=0"}, > + {0, NON_CONFORM_CS_TYPE, 3, 1, true, GP_VECTOR, FIRST_SPARE_SEL, "lret rpl<cpl"}, Since the framework is responsible for specifying the selector and the expected error code is the same for all subtests that expect an exception, just drop the error code. E.g. if the framework decides to use a different selector than these would need to be updated too, for no real benefit. It'd also be helpful to align everything. s/lret/far ret. Actually to cut down on copy+paste, and because we need formatting anyways (see below), how moving the instruction name into the far_xfer_test? > +}; > + > +static struct far_xfer_test far_ret_test = { > + .insn = FAR_XFER_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"(seg) \ Because @seg is a u16, gcc emites "pushq SI". Kinda dumb, but... @@ -72,7 +71,7 @@ static struct far_xfer_test far_ret_test = { prefix "lretq\n\t" \ "1: addq $16, %%rsp\n\t" \ "2:" \ - : : [asm_seg]"r"(seg) \ + : : [asm_seg]"r"((u64)seg) \ : "eax", "memory"); > + : "eax", "memory"); > + > +static inline void test_far_ret_asm(uint16_t seg, bool force_emulation) > +{ > + if (force_emulation) { No need for braces. Ah, the macro is missing ({ ... }) to force it to be evaluated as a single statement. It should be. Side topic, a ternary operator would be nice, but I don't know how to force string concatentation for this 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"); \ }) > + TEST_FAR_RET_ASM(seg, KVM_FEP); > + } else { > + TEST_FAR_RET_ASM(seg, ""); > + } > +} This only has a single caller, just open code the macros in the case statements of __test_far_xfer(). (My apologies if I suggested this in the last version). > struct regs { > u64 rax, rbx, rcx, rdx; > @@ -891,6 +951,74 @@ static void test_mov_dr(uint64_t *mem) > report(rax == dr6_fixed_1, "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;; Double semi-colon. > +} > + > +static void __test_far_xfer(enum far_xfer_insn insn, uint16_t seg, > + bool force_emulation) > +{ > + switch (insn) { > + case FAR_XFER_RET: > + test_far_ret_asm(seg, force_emulation); > + break; > + default: > + report_fail("unknown instructions"); It's worth spitting out the insn, it might save someone a few seconds/minutes, e.g. 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_error_code == t->error_code, t->msg); To avoid having to specify the error code, just do: (far_xfer_vector < 0 || far_xfer_error_code == FIRST_SPARE_SEL), far_xfer_vector and t->vector need to be signed ints, but that's perfectly ok, vector fits in a u8. Printing out the expecte vs. actual is also helpful for debug (pet peeve of mine in KUT...). I doesn't have to be super fancy formatting, just enough so that the user doesn't have to modify the test to figure out what went wrong. And with the insn_name + msg + target (see below) formatting: 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 ? "emuator" : "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_lret(uint64_t *mem) test_far_ret() > +{ > + printf("test lret in hw\n"); Rather than print this out before, just spit out the "target" in the report. > + test_far_xfer(false, &far_ret_test); > +} All of the above (I think) feeback: --- x86/emulator.c | 57 +++++++++++++++++++++++++------------------------- 1 file changed, 28 insertions(+), 29 deletions(-) diff --git a/x86/emulator.c b/x86/emulator.c index 2a35caf..d28e36c 100644 --- a/x86/emulator.c +++ b/x86/emulator.c @@ -21,7 +21,7 @@ static int exceptions; #define KVM_FEP "ud2; .byte 'k', 'v', 'm';" #define KVM_FEP_LENGTH 5 static int fep_available = 1; -static unsigned int far_xfer_vector = -1; +static int far_xfer_vector = -1; static unsigned int far_xfer_error_code = -1; struct far_xfer_test_case { @@ -30,8 +30,7 @@ struct far_xfer_test_case { uint16_t dpl; uint16_t p; bool usermode; - unsigned int vector; - unsigned int error_code; + int vector; const char *msg; }; @@ -41,6 +40,7 @@ enum far_xfer_insn { struct far_xfer_test { enum far_xfer_insn insn; + const char *insn_name; struct far_xfer_test_case *testcases; unsigned int nr_testcases; }; @@ -50,37 +50,31 @@ struct far_xfer_test { #define DS_TYPE 0x3 static struct far_xfer_test_case far_ret_testcases[] = { - {0, DS_TYPE, 0, 0, false, GP_VECTOR, FIRST_SPARE_SEL, "lret desc.type!=code && desc.p=0"}, - {0, NON_CONFORM_CS_TYPE, 3, 0, false, GP_VECTOR, FIRST_SPARE_SEL, "lret non-conforming && dpl!=rpl && desc.p=0"}, - {0, CONFORM_CS_TYPE, 3, 0, false, GP_VECTOR, FIRST_SPARE_SEL, "lret conforming && dpl>rpl && desc.p=0"}, - {0, NON_CONFORM_CS_TYPE, 0, 0, false, NP_VECTOR, FIRST_SPARE_SEL, "lret desc.p=0"}, - {0, NON_CONFORM_CS_TYPE, 3, 1, true, GP_VECTOR, FIRST_SPARE_SEL, "lret rpl<cpl"}, + {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"(seg) \ - : "eax", "memory"); - -static inline void test_far_ret_asm(uint16_t seg, bool force_emulation) -{ - if (force_emulation) { - TEST_FAR_RET_ASM(seg, KVM_FEP); - } else { - TEST_FAR_RET_ASM(seg, ""); - } -} + : : [asm_seg]"r"((u64)seg) \ + : "eax", "memory"); \ +}) struct regs { u64 rax, rbx, rcx, rdx; @@ -959,7 +953,7 @@ 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;; + regs->rip = regs->rax; } static void __test_far_xfer(enum far_xfer_insn insn, uint16_t seg, @@ -967,10 +961,13 @@ static void __test_far_xfer(enum far_xfer_insn insn, uint16_t seg, { switch (insn) { case FAR_XFER_RET: - test_far_ret_asm(seg, force_emulation); + if (force_emulation) + TEST_FAR_RET_ASM(seg, KVM_FEP); + else + TEST_FAR_RET_ASM(seg, ""); break; default: - report_fail("unknown instructions"); + report_fail("Unexpected insn enum = %d\n", insn); break; } } @@ -1004,22 +1001,24 @@ static void test_far_xfer(bool force_emulation, struct far_xfer_test *test) __test_far_xfer(test->insn, seg, force_emulation); report(far_xfer_vector == t->vector && - far_xfer_error_code == t->error_code, "%s", t->msg); + (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 ? "emuator" : "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_lret(uint64_t *mem) +static void test_far_ret(uint64_t *mem) { - printf("test lret in hw\n"); test_far_xfer(false, &far_ret_test); } -static void test_em_lret(uint64_t *mem) +static void test_em_far_ret(uint64_t *mem) { - printf("test lret in emulator\n"); test_far_xfer(true, &far_ret_test); } @@ -1297,7 +1296,7 @@ int main(void) test_smsw(mem); test_lmsw(); test_ljmp(mem); - test_lret(mem); + test_far_ret(mem); test_stringio(); test_incdecnotneg(mem); test_btc(mem); @@ -1322,7 +1321,7 @@ int main(void) test_smsw_reg(mem); test_nop(mem); test_mov_dr(mem); - test_em_lret(mem); + test_em_far_ret(mem); } else { report_skip("skipping register-only tests, " "use kvm.force_emulation_prefix=1 to enable"); base-commit: b56a1a58abfcc6abc16e782f13505f4495cf59e8 --
Thanks for your kind review. On Thu, Feb 10, 2022 at 06:07:11AM +0800, Sean Christopherson wrote: > Nit, "far ret" instead of "lret" in the shortlog. > > On Tue, Feb 08, 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 lret instruction, and it will test > > those tests in hardware and emulator. Enable > > kvm.force_emulation_prefix when try to test them in emulator. > > > > Signed-off-by: Hou Wenlong <houwenlong.hwl@antgroup.com> > > --- > > Was this tested? There are multiple compilation errors with gcc... > > x86/emulator.c: In function ‘test_far_xfer’: > x86/emulator.c:1034:10: error: format not a string literal and no format arguments [-Werror=format-security] > 1034 | far_xfer_error_code == t->error_code, t->msg); > | ^~~~~~~~~~~~~~~~~~~ > cc1: all warnings being treated as errors > make: *** [<builtin>: x86/emulator.o] Error 1 > > x86/emulator.c: Assembler messages: > x86/emulator.c:81: Error: incorrect register `%si' used with `q' suffix > x86/emulator.c:83: Error: incorrect register `%si' used with `q' suffix > make: *** [<builtin>: x86/emulator.o] Error 1 > The compilation was OK when I tested on my machine. But my gcc is old and customted, -Wformat-security is not enabled by default. After switching to higher version of gcc, I get compilation errors too. > > +static struct far_xfer_test_case far_ret_testcases[] = { > > + {0, DS_TYPE, 0, 0, false, GP_VECTOR, FIRST_SPARE_SEL, "lret desc.type!=code && desc.p=0"}, > > + {0, NON_CONFORM_CS_TYPE, 3, 0, false, GP_VECTOR, FIRST_SPARE_SEL, "lret non-conforming && dpl!=rpl && desc.p=0"}, > > + {0, CONFORM_CS_TYPE, 3, 0, false, GP_VECTOR, FIRST_SPARE_SEL, "lret conforming && dpl>rpl && desc.p=0"}, > > + {0, NON_CONFORM_CS_TYPE, 0, 0, false, NP_VECTOR, FIRST_SPARE_SEL, "lret desc.p=0"}, > > + {0, NON_CONFORM_CS_TYPE, 3, 1, true, GP_VECTOR, FIRST_SPARE_SEL, "lret rpl<cpl"}, > > Since the framework is responsible for specifying the selector and the expected > error code is the same for all subtests that expect an exception, just drop the > error code. E.g. if the framework decides to use a different selector than these > would need to be updated too, for no real benefit. > > It'd also be helpful to align everything. > > s/lret/far ret. Actually to cut down on copy+paste, and because we need formatting > anyways (see below), how moving the instruction name into the far_xfer_test? > > > +}; > > + > > +static struct far_xfer_test far_ret_test = { > > + .insn = FAR_XFER_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"(seg) \ > > Because @seg is a u16, gcc emites "pushq SI". Kinda dumb, but... > > @@ -72,7 +71,7 @@ static struct far_xfer_test far_ret_test = { > prefix "lretq\n\t" \ > "1: addq $16, %%rsp\n\t" \ > "2:" \ > - : : [asm_seg]"r"(seg) \ > + : : [asm_seg]"r"((u64)seg) \ > : "eax", "memory"); > > > + : "eax", "memory"); > > + > > +static inline void test_far_ret_asm(uint16_t seg, bool force_emulation) > > +{ > > + if (force_emulation) { > > No need for braces. Ah, the macro is missing ({ ... }) to force it to be evaluated > as a single statement. It should be. > > Side topic, a ternary operator would be nice, but I don't know how to force string > concatentation for this case... > I don't know too, so I use if/else statement here. > #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"); \ > }) > > > + TEST_FAR_RET_ASM(seg, KVM_FEP); > > + } else { > > + TEST_FAR_RET_ASM(seg, ""); > > + } > > +} > > This only has a single caller, just open code the macros in the case statements > of __test_far_xfer(). (My apologies if I suggested this in the last version). > > > struct regs { > > u64 rax, rbx, rcx, rdx; > > @@ -891,6 +951,74 @@ static void test_mov_dr(uint64_t *mem) > > report(rax == dr6_fixed_1, "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;; > > Double semi-colon. > > > +} > > + > > +static void __test_far_xfer(enum far_xfer_insn insn, uint16_t seg, > > + bool force_emulation) > > +{ > > + switch (insn) { > > + case FAR_XFER_RET: > > + test_far_ret_asm(seg, force_emulation); > > + break; > > + default: > > + report_fail("unknown instructions"); > > It's worth spitting out the insn, it might save someone a few seconds/minutes, e.g. > > 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_error_code == t->error_code, t->msg); > > To avoid having to specify the error code, just do: > > (far_xfer_vector < 0 || far_xfer_error_code == FIRST_SPARE_SEL), > > far_xfer_vector and t->vector need to be signed ints, but that's perfectly ok, > vector fits in a u8. Printing out the expecte vs. actual is also helpful for > debug (pet peeve of mine in KUT...). I doesn't have to be super fancy formatting, > just enough so that the user doesn't have to modify the test to figure out what > went wrong. > > And with the insn_name + msg + target (see below) formatting: > > > 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 ? "emuator" : "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_lret(uint64_t *mem) > > test_far_ret() > > > +{ > > + printf("test lret in hw\n"); > > Rather than print this out before, just spit out the "target" in the report. > > > + test_far_xfer(false, &far_ret_test); > > +} > > All of the above (I think) feeback: > > --- > x86/emulator.c | 57 +++++++++++++++++++++++++------------------------- > 1 file changed, 28 insertions(+), 29 deletions(-) > > diff --git a/x86/emulator.c b/x86/emulator.c > index 2a35caf..d28e36c 100644 > --- a/x86/emulator.c > +++ b/x86/emulator.c > @@ -21,7 +21,7 @@ static int exceptions; > #define KVM_FEP "ud2; .byte 'k', 'v', 'm';" > #define KVM_FEP_LENGTH 5 > static int fep_available = 1; > -static unsigned int far_xfer_vector = -1; > +static int far_xfer_vector = -1; > static unsigned int far_xfer_error_code = -1; > > struct far_xfer_test_case { > @@ -30,8 +30,7 @@ struct far_xfer_test_case { > uint16_t dpl; > uint16_t p; > bool usermode; > - unsigned int vector; > - unsigned int error_code; > + int vector; > const char *msg; > }; > > @@ -41,6 +40,7 @@ enum far_xfer_insn { > > struct far_xfer_test { > enum far_xfer_insn insn; > + const char *insn_name; > struct far_xfer_test_case *testcases; > unsigned int nr_testcases; > }; > @@ -50,37 +50,31 @@ struct far_xfer_test { > #define DS_TYPE 0x3 > > static struct far_xfer_test_case far_ret_testcases[] = { > - {0, DS_TYPE, 0, 0, false, GP_VECTOR, FIRST_SPARE_SEL, "lret desc.type!=code && desc.p=0"}, > - {0, NON_CONFORM_CS_TYPE, 3, 0, false, GP_VECTOR, FIRST_SPARE_SEL, "lret non-conforming && dpl!=rpl && desc.p=0"}, > - {0, CONFORM_CS_TYPE, 3, 0, false, GP_VECTOR, FIRST_SPARE_SEL, "lret conforming && dpl>rpl && desc.p=0"}, > - {0, NON_CONFORM_CS_TYPE, 0, 0, false, NP_VECTOR, FIRST_SPARE_SEL, "lret desc.p=0"}, > - {0, NON_CONFORM_CS_TYPE, 3, 1, true, GP_VECTOR, FIRST_SPARE_SEL, "lret rpl<cpl"}, > + {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"(seg) \ > - : "eax", "memory"); > - > -static inline void test_far_ret_asm(uint16_t seg, bool force_emulation) > -{ > - if (force_emulation) { > - TEST_FAR_RET_ASM(seg, KVM_FEP); > - } else { > - TEST_FAR_RET_ASM(seg, ""); > - } > -} > + : : [asm_seg]"r"((u64)seg) \ > + : "eax", "memory"); \ > +}) > > struct regs { > u64 rax, rbx, rcx, rdx; > @@ -959,7 +953,7 @@ 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;; > + regs->rip = regs->rax; > } > > static void __test_far_xfer(enum far_xfer_insn insn, uint16_t seg, > @@ -967,10 +961,13 @@ static void __test_far_xfer(enum far_xfer_insn insn, uint16_t seg, > { > switch (insn) { > case FAR_XFER_RET: > - test_far_ret_asm(seg, force_emulation); > + if (force_emulation) > + TEST_FAR_RET_ASM(seg, KVM_FEP); > + else > + TEST_FAR_RET_ASM(seg, ""); > break; > default: > - report_fail("unknown instructions"); > + report_fail("Unexpected insn enum = %d\n", insn); > break; > } > } > @@ -1004,22 +1001,24 @@ static void test_far_xfer(bool force_emulation, struct far_xfer_test *test) > __test_far_xfer(test->insn, seg, force_emulation); > > report(far_xfer_vector == t->vector && > - far_xfer_error_code == t->error_code, "%s", t->msg); > + (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 ? "emuator" : "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_lret(uint64_t *mem) > +static void test_far_ret(uint64_t *mem) > { > - printf("test lret in hw\n"); > test_far_xfer(false, &far_ret_test); > } > > -static void test_em_lret(uint64_t *mem) > +static void test_em_far_ret(uint64_t *mem) > { > - printf("test lret in emulator\n"); > test_far_xfer(true, &far_ret_test); > } > > @@ -1297,7 +1296,7 @@ int main(void) > test_smsw(mem); > test_lmsw(); > test_ljmp(mem); > - test_lret(mem); > + test_far_ret(mem); > test_stringio(); > test_incdecnotneg(mem); > test_btc(mem); > @@ -1322,7 +1321,7 @@ int main(void) > test_smsw_reg(mem); > test_nop(mem); > test_mov_dr(mem); > - test_em_lret(mem); > + test_em_far_ret(mem); > } else { > report_skip("skipping register-only tests, " > "use kvm.force_emulation_prefix=1 to enable"); > > base-commit: b56a1a58abfcc6abc16e782f13505f4495cf59e8 > --
diff --git a/x86/emulator.c b/x86/emulator.c index 22a518f4ad65..a68debaabef0 100644 --- a/x86/emulator.c +++ b/x86/emulator.c @@ -19,6 +19,66 @@ static int exceptions; #define KVM_FEP "ud2; .byte 'k', 'v', 'm';" #define KVM_FEP_LENGTH 5 static int fep_available = 1; +static unsigned 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; + unsigned int vector; + unsigned int error_code; + const char *msg; +}; + +enum far_xfer_insn { + FAR_XFER_RET, +}; + +struct far_xfer_test { + enum far_xfer_insn insn; + 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, FIRST_SPARE_SEL, "lret desc.type!=code && desc.p=0"}, + {0, NON_CONFORM_CS_TYPE, 3, 0, false, GP_VECTOR, FIRST_SPARE_SEL, "lret non-conforming && dpl!=rpl && desc.p=0"}, + {0, CONFORM_CS_TYPE, 3, 0, false, GP_VECTOR, FIRST_SPARE_SEL, "lret conforming && dpl>rpl && desc.p=0"}, + {0, NON_CONFORM_CS_TYPE, 0, 0, false, NP_VECTOR, FIRST_SPARE_SEL, "lret desc.p=0"}, + {0, NON_CONFORM_CS_TYPE, 3, 1, true, GP_VECTOR, FIRST_SPARE_SEL, "lret rpl<cpl"}, +}; + +static struct far_xfer_test far_ret_test = { + .insn = FAR_XFER_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"(seg) \ + : "eax", "memory"); + +static inline void test_far_ret_asm(uint16_t seg, bool force_emulation) +{ + if (force_emulation) { + TEST_FAR_RET_ASM(seg, KVM_FEP); + } else { + TEST_FAR_RET_ASM(seg, ""); + } +} struct regs { u64 rax, rbx, rcx, rdx; @@ -891,6 +951,74 @@ static void test_mov_dr(uint64_t *mem) report(rax == dr6_fixed_1, "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: + test_far_ret_asm(seg, force_emulation); + break; + default: + report_fail("unknown instructions"); + 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_error_code == t->error_code, t->msg); + } + + handle_exception(GP_VECTOR, 0); + handle_exception(NP_VECTOR, 0); +} + +static void test_lret(uint64_t *mem) +{ + printf("test lret in hw\n"); + test_far_xfer(false, &far_ret_test); +} + +static void test_em_lret(uint64_t *mem) +{ + printf("test lret in emulator\n"); + test_far_xfer(true, &far_ret_test); +} + static void test_push16(uint64_t *mem) { uint64_t rsp1, rsp2; @@ -1165,6 +1293,7 @@ int main(void) test_smsw(mem); test_lmsw(); test_ljmp(mem); + test_lret(mem); test_stringio(); test_incdecnotneg(mem); test_btc(mem); @@ -1189,6 +1318,7 @@ int main(void) test_smsw_reg(mem); test_nop(mem); test_mov_dr(mem); + test_em_lret(mem); } else { report_skip("skipping register-only tests, " "use kvm.force_emulation_prefix=1 to enable");
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 lret instruction, and it will test those tests in hardware and emulator. Enable kvm.force_emulation_prefix when try to test them in emulator. Signed-off-by: Hou Wenlong <houwenlong.hwl@antgroup.com> --- x86/emulator.c | 130 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 130 insertions(+)