* [kvm-unit-tests PATCH 1/2] x86: replace `push` `pop` with callee-clobbered list
@ 2022-04-24 7:09 SU Hang
2022-04-24 7:09 ` [kvm-unit-tests PATCH 2/2] x86: replace `int 0x20` with `syscall` SU Hang
0 siblings, 1 reply; 6+ messages in thread
From: SU Hang @ 2022-04-24 7:09 UTC (permalink / raw)
To: kvm, pbonzini, thuth, drjones, SU Hang
Stopping mess up asm callee-clobbered list with `push` `pop`,
clean up code to make it more readable.
Signed-off-by: SU Hang <darcy.sh@antgroup.com>
---
lib/x86/usermode.c | 22 +---------------------
1 file changed, 1 insertion(+), 21 deletions(-)
diff --git a/lib/x86/usermode.c b/lib/x86/usermode.c
index e22fb8f..477cb9f 100644
--- a/lib/x86/usermode.c
+++ b/lib/x86/usermode.c
@@ -66,32 +66,12 @@ uint64_t run_in_user(usermode_func func, unsigned int fault_vector,
"iretq\n"
"user_mode:\n\t"
- /* Back up registers before invoking func */
- "push %%rbx\n\t"
- "push %%rcx\n\t"
- "push %%rdx\n\t"
- "push %%r8\n\t"
- "push %%r9\n\t"
- "push %%r10\n\t"
- "push %%r11\n\t"
- "push %%rdi\n\t"
- "push %%rsi\n\t"
/* Call user mode function */
"mov %[arg1], %%rdi\n\t"
"mov %[arg2], %%rsi\n\t"
"mov %[arg3], %%rdx\n\t"
"mov %[arg4], %%rcx\n\t"
"call *%[func]\n\t"
- /* Restore registers */
- "pop %%rsi\n\t"
- "pop %%rdi\n\t"
- "pop %%r11\n\t"
- "pop %%r10\n\t"
- "pop %%r9\n\t"
- "pop %%r8\n\t"
- "pop %%rdx\n\t"
- "pop %%rcx\n\t"
- "pop %%rbx\n\t"
/* Return to kernel via system call */
"int %[kernel_entry_vector]\n\t"
/* Kernel Mode */
@@ -112,7 +92,7 @@ uint64_t run_in_user(usermode_func func, unsigned int fault_vector,
sizeof(user_stack)),
[kernel_entry_vector]"i"(RET_TO_KERNEL_IRQ)
:
- "rsi", "rdi", "rcx", "rdx");
+ "rsi", "rdi", "rbx", "rcx", "rdx", "r8", "r9", "r10", "r11");
return rax;
}
--
2.32.0.3.g01195cf9f
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [kvm-unit-tests PATCH 2/2] x86: replace `int 0x20` with `syscall`
2022-04-24 7:09 [kvm-unit-tests PATCH 1/2] x86: replace `push` `pop` with callee-clobbered list SU Hang
@ 2022-04-24 7:09 ` SU Hang
2022-04-25 15:58 ` Sean Christopherson
0 siblings, 1 reply; 6+ messages in thread
From: SU Hang @ 2022-04-24 7:09 UTC (permalink / raw)
To: kvm, pbonzini, thuth, drjones, SU Hang
Signed-off-by: SU Hang <darcy.sh@antgroup.com>
---
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);
+ wrmsr(MSR_STAR, ((u64)(USER_CS32 << 16) | KERNEL_CS) << 32);
+ 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [kvm-unit-tests PATCH 2/2] x86: replace `int 0x20` with `syscall`
2022-04-24 7:09 ` [kvm-unit-tests PATCH 2/2] x86: replace `int 0x20` with `syscall` SU Hang
@ 2022-04-25 15:58 ` Sean Christopherson
2022-04-27 1:07 ` Sean Christopherson
2022-04-27 9:27 ` SU Hang
0 siblings, 2 replies; 6+ messages in thread
From: Sean Christopherson @ 2022-04-25 15:58 UTC (permalink / raw)
To: SU Hang; +Cc: kvm, pbonzini, thuth, drjones
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@antgroup.com>
> ---
> 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
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [kvm-unit-tests PATCH 2/2] x86: replace `int 0x20` with `syscall`
2022-04-25 15:58 ` Sean Christopherson
@ 2022-04-27 1:07 ` Sean Christopherson
2022-04-27 9:27 ` SU Hang
1 sibling, 0 replies; 6+ messages in thread
From: Sean Christopherson @ 2022-04-27 1:07 UTC (permalink / raw)
To: SU Hang; +Cc: kvm, pbonzini, thuth, drjones
On Mon, Apr 25, 2022, Sean Christopherson wrote:
> 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.
And a concrete reason not to apply this patch: it causes the nVMX #AC test to fail:
FAIL: #AC handled by L2
FAIL: #AC handled by L1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [kvm-unit-tests PATCH 2/2] x86: replace `int 0x20` with `syscall`
2022-04-25 15:58 ` Sean Christopherson
2022-04-27 1:07 ` Sean Christopherson
@ 2022-04-27 9:27 ` SU Hang
2022-04-28 15:58 ` Sean Christopherson
1 sibling, 1 reply; 6+ messages in thread
From: SU Hang @ 2022-04-27 9:27 UTC (permalink / raw)
To: seanjc; +Cc: SU Hang, drjones, kvm, pbonzini, thuth
> Why?
We are implementing a para-virtualization hypervisor, which doesn't allow guest
to trigger soft interrupt > 0x20(but `int 0x80` works fine), so I want to
replace `int 0x20` with a more common `syscall`.
> it's do_ring3() should really be rolled into this framework.
Yes, it is worth working on it, I'll do it in my part-time.
> no existing test verifies that KVM injects #UD on SYSCALL without EFER.SCE
> set, though it would be nice to add one.
I am also interested in it, maybe do it later.
> > + 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.
Oops, intel SDM vol.3 <chap 5.8.8> says:
"""
When SYSRET transfers control to 64-bit mode user code using REX.W, the
processor gets the privilege level 3 target code segment, instruction pointer,
stack segment, and flags as follows:
• Target code segment — Reads a non-NULL selector from IA32_STAR[63:48] + 16.
• Stack segment — IA32_STAR[63:48] + 8.
"""
Since the value of USER_CS is 0x4b in 64 bit mode, SS register points to 0x53 =
0x4b + 8, (offset is 0x50) But `gdt + offset(0x50)` hasn't been setup(so does DS
register).
> refs: https://gitlab.com/kvm-unit-tests/kvm-unit-tests/-/blob/master/lib/x86/desc.c#L34
> refs: https://gitlab.com/kvm-unit-tests/kvm-unit-tests/-/blob/master/x86/syscall.c#L68
Linux also does so, the reason is to reuse user segment descriptor in both 32/64 bit.
> refs1: https://github.com/torvalds/linux/blob/46cf2c613f4b10eb12f749207b0fd2c1bfae3088/arch/x86/kernel/cpu/common.c#L1942
> refs2: https://github.com/torvalds/linux/blob/46cf2c613f4b10eb12f749207b0fd2c1bfae3088/arch/x86/include/asm/segment.h#L211
> refs3: https://github.com/torvalds/linux/blob/46cf2c613f4b10eb12f749207b0fd2c1bfae3088/arch/x86/kernel/cpu/common.c#L216
> And a concrete reason not to apply this patch: it causes the nVMX #AC test to fail:
It's awkward, some KUT test cases results diffs on my different machines, which
makes me don't know which result I could trust, so I only pay attention to the
test cases that I care about. I'll keep an eye on the rest cases in the
future.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [kvm-unit-tests PATCH 2/2] x86: replace `int 0x20` with `syscall`
2022-04-27 9:27 ` SU Hang
@ 2022-04-28 15:58 ` Sean Christopherson
0 siblings, 0 replies; 6+ messages in thread
From: Sean Christopherson @ 2022-04-28 15:58 UTC (permalink / raw)
To: SU Hang; +Cc: drjones, kvm, pbonzini, thuth
On Wed, Apr 27, 2022, SU Hang wrote:
> > It doesn't matter at this time because this framework doesn't ses SYSRET, but
> > this should be USER_CS or USER_CS64.
> Oops, intel SDM vol.3 <chap 5.8.8> says:
> """
> When SYSRET transfers control to 64-bit mode user code using REX.W, the
> processor gets the privilege level 3 target code segment, instruction pointer,
> stack segment, and flags as follows:
> • Target code segment — Reads a non-NULL selector from IA32_STAR[63:48] + 16.
Ugh, missed that detail.
> • Stack segment — IA32_STAR[63:48] + 8.
> """
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-04-28 15:58 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-24 7:09 [kvm-unit-tests PATCH 1/2] x86: replace `push` `pop` with callee-clobbered list SU Hang
2022-04-24 7:09 ` [kvm-unit-tests PATCH 2/2] x86: replace `int 0x20` with `syscall` SU Hang
2022-04-25 15:58 ` Sean Christopherson
2022-04-27 1:07 ` Sean Christopherson
2022-04-27 9:27 ` SU Hang
2022-04-28 15:58 ` Sean Christopherson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.