On Sun, Apr 24, 2022, SU Hang wrote:
Why? As gross as it is, I actually think INTn is a better option because it
doesn't require writing multiple MSRs, and can work for both 64-bit and 32-bit KUT.
The latter is currently a moot point since this code is 64-bit only, but the UMIP
test _does_ support 32-bit, and it's do_ring3() should really be rolled into this
framework.
Furthermore, we really should have a test to verify that KVM correctly emulates
SYSCALL at CPL3 with EFER.SCE=0, and forcing EFER.SCE=1 just to get to CPL3 would
make it impossible to utilize this framework for such a test.
> Signed-off-by: SU Hang <darcy.sh@xxxxxxxxxxxx>
> ---
> lib/x86/usermode.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/lib/x86/usermode.c b/lib/x86/usermode.c
> index 477cb9f..e4cb899 100644
> --- a/lib/x86/usermode.c
> +++ b/lib/x86/usermode.c
> @@ -12,7 +12,6 @@
> #include <stdint.h>
>
> #define USERMODE_STACK_SIZE 0x2000
> -#define RET_TO_KERNEL_IRQ 0x20
>
> static jmp_buf jmpbuf;
>
> @@ -40,9 +39,11 @@ uint64_t run_in_user(usermode_func func, unsigned int fault_vector,
> static unsigned char user_stack[USERMODE_STACK_SIZE];
>
> *raised_vector = 0;
> - set_idt_entry(RET_TO_KERNEL_IRQ, &ret_to_kernel, 3);
> handle_exception(fault_vector,
> restore_exec_to_jmpbuf_exception_handler);
> + wrmsr(MSR_EFER, rdmsr(MSR_EFER) | EFER_SCE);
Tangentially related, KUT should really explicitly initialize EFER.SCE during boot.
cstart64.S leaves it alone, which means KUT has no defined value for EFER.SCE.
One thought would be to set EFER.SCE=1 during boot (in a separate patch) and remove
the code from the syscall test that forces EFER.SCE. AFAICT, no existing test
verifies that KVM injects #UD on SYSCALL without EFER.SCE set, though it would be
nice to add one.
> + wrmsr(MSR_STAR, ((u64)(USER_CS32 << 16) | KERNEL_CS) << 32);
It doesn't matter at this time because this framework doesn't ses SYSRET, but this
should be USER_CS or USER_CS64.
> + wrmsr(MSR_LSTAR, (u64)&ret_to_kernel);
>
> if (setjmp(jmpbuf) != 0) {
> *raised_vector = 1;
> @@ -73,7 +74,7 @@ uint64_t run_in_user(usermode_func func, unsigned int fault_vector,
> "mov %[arg4], %%rcx\n\t"
> "call *%[func]\n\t"
> /* Return to kernel via system call */
> - "int %[kernel_entry_vector]\n\t"
> + "syscall\n\t"
> /* Kernel Mode */
> "ret_to_kernel:\n\t"
> "mov %[rsp0], %%rsp\n\t"
> @@ -89,8 +90,7 @@ uint64_t run_in_user(usermode_func func, unsigned int fault_vector,
> [user_ds]"i"(USER_DS),
> [user_cs]"i"(USER_CS),
> [user_stack_top]"r"(user_stack +
> - sizeof(user_stack)),
> - [kernel_entry_vector]"i"(RET_TO_KERNEL_IRQ)
> + sizeof(user_stack))
> :
> "rsi", "rdi", "rbx", "rcx", "rdx", "r8", "r9", "r10", "r11");
>
> --
> 2.32.0.3.g01195cf9f
>
