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@xxxxxxxxxx>
> > ---
> > 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
>
