| Message ID | 20221020152404.283980-8-mlevitsk@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kvm-unit-tests: set of fixes and new tests | expand |
On Thu, Oct 20, 2022, Maxim Levitsky wrote: > Run the test with Intel's vendor ID and in the long mode, > to test the emulation of this instruction on AMD. > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > --- > x86/Makefile.x86_64 | 2 + > x86/sysenter.c | 127 ++++++++++++++++++++++++++++++++++++++++++++ > x86/unittests.cfg | 5 ++ > 3 files changed, 134 insertions(+) > create mode 100644 x86/sysenter.c > > diff --git a/x86/Makefile.x86_64 b/x86/Makefile.x86_64 > index 865da07d..8ce53650 100644 > --- a/x86/Makefile.x86_64 > +++ b/x86/Makefile.x86_64 > @@ -33,6 +33,7 @@ tests += $(TEST_DIR)/vmware_backdoors.$(exe) > tests += $(TEST_DIR)/rdpru.$(exe) > tests += $(TEST_DIR)/pks.$(exe) > tests += $(TEST_DIR)/pmu_lbr.$(exe) > +tests += $(TEST_DIR)/sysenter.$(exe) > > > ifeq ($(CONFIG_EFI),y) > @@ -60,3 +61,4 @@ $(TEST_DIR)/hyperv_clock.$(bin): $(TEST_DIR)/hyperv_clock.o > $(TEST_DIR)/vmx.$(bin): $(TEST_DIR)/vmx_tests.o > $(TEST_DIR)/svm.$(bin): $(TEST_DIR)/svm_tests.o > $(TEST_DIR)/svm_npt.$(bin): $(TEST_DIR)/svm_npt.o > +$(TEST_DIR)/sysenter.o: CFLAGS += -Wa,-mintel64 > diff --git a/x86/sysenter.c b/x86/sysenter.c > new file mode 100644 > index 00000000..6c32fea4 > --- /dev/null > +++ b/x86/sysenter.c > @@ -0,0 +1,127 @@ > +#include "alloc.h" > +#include "libcflat.h" > +#include "processor.h" > +#include "msr.h" > +#include "desc.h" > + > + > +// undefine this to run the syscall instruction in 64 bit mode. > +// this won't work on AMD due to disabled code in the emulator. > +#define COMP32 Why not run the test in both 32-bit and 64-bit mode, and skip the 64-bit mode version if the vCPU model is AMD? > + > +int main(int ac, char **av) > +{ > + extern void sysenter_target(void); > + extern void test_done(void); Tabs instead of spaces. > + > + setup_vm(); > + > + int gdt_index = 0x50 >> 3; > + ulong rax = 0xDEAD; > + > + /* init the sysenter GDT block */ > + /*gdt64[gdt_index+0] = gdt64[KERNEL_CS >> 3]; > + gdt64[gdt_index+1] = gdt64[KERNEL_DS >> 3]; > + gdt64[gdt_index+2] = gdt64[USER_CS >> 3]; > + gdt64[gdt_index+3] = gdt64[USER_DS >> 3];*/ > + > + /* init the sysenter msrs*/ > + wrmsr(MSR_IA32_SYSENTER_CS, gdt_index << 3); > + wrmsr(MSR_IA32_SYSENTER_ESP, 0xAAFFFFFFFF); > + wrmsr(MSR_IA32_SYSENTER_EIP, (uint64_t)sysenter_target); > + > + u8 *thunk = (u8*)malloc(50); > + u8 *tmp = thunk; > + > + printf("Thunk at 0x%lx\n", (u64)thunk); > + > + /* movabs test_done, %rdx*/ > + *tmp++ = 0x48; *tmp++ = 0xBA; > + *(u64 *)tmp = (uint64_t)test_done; tmp += 8; > + /* jmp %%rdx*/ > + *tmp++ = 0xFF; *tmp++ = 0xe2; > + > + asm volatile ( Can we add a helper sysenter_asm.S or whatever instead of making this a gigantic inline asm blob? And then have separate routines for 32-bit vs. 64-bit? That'd require a bit of code duplication, but macros could be used to dedup the common parts if necessary. And with a .S file, I believe there's no need to dynamically generate the thunk, e.g. pass the jump target through a GPR that's not modified/used by SYSENTER. > +#ifdef COMP32 > + "# switch to comp32, mode prior to running the test\n" > + "ljmpl *1f\n" > + "1:\n" > + ".long 1f\n" > + ".long " xstr(KERNEL_CS32) "\n" > + "1:\n" > + ".code32\n" > +#else > + "# store the 64 bit thunk address to rdx\n" > + "mov %[thunk], %%rdx\n" > +#endif
On Thu, 2022-10-20 at 19:25 +0000, Sean Christopherson wrote: > On Thu, Oct 20, 2022, Maxim Levitsky wrote: > > Run the test with Intel's vendor ID and in the long mode, > > to test the emulation of this instruction on AMD. > > > > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > > --- > > x86/Makefile.x86_64 | 2 + > > x86/sysenter.c | 127 ++++++++++++++++++++++++++++++++++++++++++++ > > x86/unittests.cfg | 5 ++ > > 3 files changed, 134 insertions(+) > > create mode 100644 x86/sysenter.c > > > > diff --git a/x86/Makefile.x86_64 b/x86/Makefile.x86_64 > > index 865da07d..8ce53650 100644 > > --- a/x86/Makefile.x86_64 > > +++ b/x86/Makefile.x86_64 > > @@ -33,6 +33,7 @@ tests += $(TEST_DIR)/vmware_backdoors.$(exe) > > tests += $(TEST_DIR)/rdpru.$(exe) > > tests += $(TEST_DIR)/pks.$(exe) > > tests += $(TEST_DIR)/pmu_lbr.$(exe) > > +tests += $(TEST_DIR)/sysenter.$(exe) > > > > > > ifeq ($(CONFIG_EFI),y) > > @@ -60,3 +61,4 @@ $(TEST_DIR)/hyperv_clock.$(bin): $(TEST_DIR)/hyperv_clock.o > > $(TEST_DIR)/vmx.$(bin): $(TEST_DIR)/vmx_tests.o > > $(TEST_DIR)/svm.$(bin): $(TEST_DIR)/svm_tests.o > > $(TEST_DIR)/svm_npt.$(bin): $(TEST_DIR)/svm_npt.o > > +$(TEST_DIR)/sysenter.o: CFLAGS += -Wa,-mintel64 > > diff --git a/x86/sysenter.c b/x86/sysenter.c > > new file mode 100644 > > index 00000000..6c32fea4 > > --- /dev/null > > +++ b/x86/sysenter.c > > @@ -0,0 +1,127 @@ > > +#include "alloc.h" > > +#include "libcflat.h" > > +#include "processor.h" > > +#include "msr.h" > > +#include "desc.h" > > + > > + > > +// undefine this to run the syscall instruction in 64 bit mode. > > +// this won't work on AMD due to disabled code in the emulator. > > +#define COMP32 > > Why not run the test in both 32-bit and 64-bit mode, and skip the 64-bit mode > version if the vCPU model is AMD? True, but on Intel the test won't test much since the instruction is not emulated there. It is also possible to enable the emulation on x86 on AMD as well, there doesn't seem anything special and/or dangerous in the KVM emulator. > > > + > > +int main(int ac, char **av) > > +{ > > + extern void sysenter_target(void); > > + extern void test_done(void); > > Tabs instead of spaces. OK, I'll take a note. > > > + > > + setup_vm(); > > + > > + int gdt_index = 0x50 >> 3; > > + ulong rax = 0xDEAD; > > + > > + /* init the sysenter GDT block */ > > + /*gdt64[gdt_index+0] = gdt64[KERNEL_CS >> 3]; > > + gdt64[gdt_index+1] = gdt64[KERNEL_DS >> 3]; > > + gdt64[gdt_index+2] = gdt64[USER_CS >> 3]; > > + gdt64[gdt_index+3] = gdt64[USER_DS >> 3];*/ > > + > > + /* init the sysenter msrs*/ > > + wrmsr(MSR_IA32_SYSENTER_CS, gdt_index << 3); > > + wrmsr(MSR_IA32_SYSENTER_ESP, 0xAAFFFFFFFF); > > + wrmsr(MSR_IA32_SYSENTER_EIP, (uint64_t)sysenter_target); > > + > > + u8 *thunk = (u8*)malloc(50); > > + u8 *tmp = thunk; > > + > > + printf("Thunk at 0x%lx\n", (u64)thunk); > > + > > + /* movabs test_done, %rdx*/ > > + *tmp++ = 0x48; *tmp++ = 0xBA; > > + *(u64 *)tmp = (uint64_t)test_done; tmp += 8; > > + /* jmp %%rdx*/ > > + *tmp++ = 0xFF; *tmp++ = 0xe2; > > + > > + asm volatile ( > > Can we add a helper sysenter_asm.S or whatever instead of making this a gigantic > inline asm blob? And then have separate routines for 32-bit vs. 64-bit? That'd > require a bit of code duplication, but macros could be used to dedup the common > parts if necessary. > > And with a .S file, I believe there's no need to dynamically generate the thunk, > e.g. pass the jump target through a GPR that's not modified/used by SYSENTER. I'll take a look, however since I wrote this test long ago and I am kind of short on time, I prefer to merge it as is and then improve it as you suggested. Best regards, Maxim Levitsky > > > +#ifdef COMP32 > > + "# switch to comp32, mode prior to running the test\n" > > + "ljmpl *1f\n" > > + "1:\n" > > + ".long 1f\n" > > + ".long " xstr(KERNEL_CS32) "\n" > > + "1:\n" > > + ".code32\n" > > +#else > > + "# store the 64 bit thunk address to rdx\n" > > + "mov %[thunk], %%rdx\n" > > +#endif >
diff --git a/x86/Makefile.x86_64 b/x86/Makefile.x86_64 index 865da07d..8ce53650 100644 --- a/x86/Makefile.x86_64 +++ b/x86/Makefile.x86_64 @@ -33,6 +33,7 @@ tests += $(TEST_DIR)/vmware_backdoors.$(exe) tests += $(TEST_DIR)/rdpru.$(exe) tests += $(TEST_DIR)/pks.$(exe) tests += $(TEST_DIR)/pmu_lbr.$(exe) +tests += $(TEST_DIR)/sysenter.$(exe) ifeq ($(CONFIG_EFI),y) @@ -60,3 +61,4 @@ $(TEST_DIR)/hyperv_clock.$(bin): $(TEST_DIR)/hyperv_clock.o $(TEST_DIR)/vmx.$(bin): $(TEST_DIR)/vmx_tests.o $(TEST_DIR)/svm.$(bin): $(TEST_DIR)/svm_tests.o $(TEST_DIR)/svm_npt.$(bin): $(TEST_DIR)/svm_npt.o +$(TEST_DIR)/sysenter.o: CFLAGS += -Wa,-mintel64 diff --git a/x86/sysenter.c b/x86/sysenter.c new file mode 100644 index 00000000..6c32fea4 --- /dev/null +++ b/x86/sysenter.c @@ -0,0 +1,127 @@ +#include "alloc.h" +#include "libcflat.h" +#include "processor.h" +#include "msr.h" +#include "desc.h" + + +// undefine this to run the syscall instruction in 64 bit mode. +// this won't work on AMD due to disabled code in the emulator. +#define COMP32 + +int main(int ac, char **av) +{ + extern void sysenter_target(void); + extern void test_done(void); + + setup_vm(); + + int gdt_index = 0x50 >> 3; + ulong rax = 0xDEAD; + + /* init the sysenter GDT block */ + /*gdt64[gdt_index+0] = gdt64[KERNEL_CS >> 3]; + gdt64[gdt_index+1] = gdt64[KERNEL_DS >> 3]; + gdt64[gdt_index+2] = gdt64[USER_CS >> 3]; + gdt64[gdt_index+3] = gdt64[USER_DS >> 3];*/ + + /* init the sysenter msrs*/ + wrmsr(MSR_IA32_SYSENTER_CS, gdt_index << 3); + wrmsr(MSR_IA32_SYSENTER_ESP, 0xAAFFFFFFFF); + wrmsr(MSR_IA32_SYSENTER_EIP, (uint64_t)sysenter_target); + + u8 *thunk = (u8*)malloc(50); + u8 *tmp = thunk; + + printf("Thunk at 0x%lx\n", (u64)thunk); + + /* movabs test_done, %rdx*/ + *tmp++ = 0x48; *tmp++ = 0xBA; + *(u64 *)tmp = (uint64_t)test_done; tmp += 8; + /* jmp %%rdx*/ + *tmp++ = 0xFF; *tmp++ = 0xe2; + + asm volatile ( +#ifdef COMP32 + "# switch to comp32, mode prior to running the test\n" + "ljmpl *1f\n" + "1:\n" + ".long 1f\n" + ".long " xstr(KERNEL_CS32) "\n" + "1:\n" + ".code32\n" +#else + "# store the 64 bit thunk address to rdx\n" + "mov %[thunk], %%rdx\n" +#endif + "#+++++++++++++++++++++++++++++++++++++++++++++++++++" + "# user code (64 bit or comp32)" + "#+++++++++++++++++++++++++++++++++++++++++++++++++++" + + "# use sysenter to enter 64 bit system code\n" + "mov %%esp, %%ecx #stash rsp value\n" + "mov $1, %%ebx\n" + "sysenter\n" + "ud2\n" + + "#+++++++++++++++++++++++++++++++++++++++++++++++++++\n" + "# 64 bit cpl=0 code" + "#+++++++++++++++++++++++++++++++++++++++++++++++++++\n" + + ".code64\n" + "sysenter_target:\n" + +#ifdef COMP32 + "test %%rbx, %%rbx # check if we are here for second time \n" + "jne 1f\n" + "movq %%rcx, %%rsp # restore stack pointer manually\n" + "jmp test_done\n" + "1:\n" +#endif + + "# test that MSR_IA32_SYSENTER_ESP is correct\n" + "movq $0xAAFFFFFFFF, %%rbx\n" + "movq $0xDEAD, %%rax\n" + "cmpq %%rsp, %%rbx \n" + "jne 1f\n" + "movq $0xACED, %%rax\n" + + "# use sysexit to exit back\n" + "1:\n" +#ifdef COMP32 + "leaq sysexit_target, %%rdx\n" + "sysexit\n" + "sysexit_target:\n" + "# second sysenter to return to CPL=0 and 64 bit\n" + "# the sysenter handler will jump back to here without sysexit due to ebx=0\n" + ".code32\n" + "mov $0, %%ebx\n" + "sysenter\n" +#else + "# this will go through thunk to test_done, which tests,\n" + "# that we can sysexit to high addresses\n" + ".byte 0x48\n" + "sysexit\n" + "ud2\n" +#endif + + ".code64\n" + "test_done:\n" + "nop\n" + + : /*outputs*/ + "=a" (rax) + : /* inputs*/ + [thunk] "r" (thunk) + : /*clobbers*/ + "rbx", /* action flag for sysenter_target */ + "rcx", /* saved RSP */ + "rdx", /* used for SYSEXIT*/ + "flags" + ); + + report(rax == 0xACED, "MSR_IA32_SYSENTER_ESP has correct value"); + return report_summary(); +} + + diff --git a/x86/unittests.cfg b/x86/unittests.cfg index db9bb3ac..ebb3fdfc 100644 --- a/x86/unittests.cfg +++ b/x86/unittests.cfg @@ -223,6 +223,11 @@ file = syscall.flat arch = x86_64 extra_params = -cpu Opteron_G1,vendor=AuthenticAMD +[sysenter] +file = sysenter.flat +arch = x86_64 +extra_params = -cpu host,vendor=GenuineIntel + [tsc] file = tsc.flat extra_params = -cpu kvm64,+rdtscp
Run the test with Intel's vendor ID and in the long mode, to test the emulation of this instruction on AMD. Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> --- x86/Makefile.x86_64 | 2 + x86/sysenter.c | 127 ++++++++++++++++++++++++++++++++++++++++++++ x86/unittests.cfg | 5 ++ 3 files changed, 134 insertions(+) create mode 100644 x86/sysenter.c