Re: [PATCH v7 45/72] x86/entry/64: Add entry code for #VC handler

2021-01-28 Thread Joerg Roedel
Hello Lai,

On Sun, Jan 24, 2021 at 10:11:14PM +0800, Lai Jiangshan wrote:
> > +
> > +   /*
> > +* No need to switch back to the IST stack. The current stack is 
> > either
> > +* identical to the stack in the IRET frame or the VC fall-back 
> > stack,
> > +* so it is definitly mapped even with PTI enabled.
> > +*/
> > +   jmp paranoid_exit
> > +
> >
> 
> Hello
> 
> I know we don't enable PTI on AMD, but the above comment doesn't align to the
> next code.
> 
> We assume PTI is enabled as the comments said "even with PTI enabled".
> 
> When #VC happens after entry_SYSCALL_64 but before it switches to the
> kernel CR3.  vc_switch_off_ist() will switch the stack to the kernel stack
> and paranoid_exit can't work when it switches to user CR3 on the kernel stack.
> 
> The comment above lost information that the current stack is possible to be
> the kernel stack which is mapped not user CR3.
> 
> Maybe I missed something.

You are right, the scenario above would cause problems for the current
#VC entry code. With SEV-ES an #VC exception can't happen in the early
syscall entry code, so I think its the best to update the comment
reflecting this.

In the future this might change and then the #VC entry code needs to
take care of this case too. Thanks for pointing it out.

Regards,

Joerg
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


[PATCH v7 45/72] x86/entry/64: Add entry code for #VC handler

2020-09-07 Thread Joerg Roedel
From: Joerg Roedel 

The #VC handler needs special entry code because:

1. It runs on an IST stack

2. It needs to be able to handle nested #VC exceptions

To make this work the entry code is implemented to pretend it doesn't
use an IST stack. When entered from user-mode or early SYSCALL entry
path it switches to the task stack, if entered from kernel-mode it
tries to switch back to the previous stack in the IRET frame.

The stack found in the IRET frame is validated first, and if it is not
safe to use it for the #VC handler, the code will switch to a
fall-back stack (the #VC2 IST stack). From there it can cause nested
exceptions again.

Signed-off-by: Joerg Roedel 
---
 arch/x86/entry/entry_64.S   | 80 +
 arch/x86/include/asm/idtentry.h | 44 ++
 arch/x86/include/asm/proto.h|  1 +
 arch/x86/include/asm/traps.h|  1 +
 arch/x86/kernel/traps.c | 45 +++
 5 files changed, 171 insertions(+)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 5c5d234d968d..f446e9048d07 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -101,6 +101,8 @@ SYM_CODE_START(entry_SYSCALL_64)
SWITCH_TO_KERNEL_CR3 scratch_reg=%rsp
movqPER_CPU_VAR(cpu_current_top_of_stack), %rsp
 
+SYM_INNER_LABEL(entry_SYSCALL_64_safe_stack, SYM_L_GLOBAL)
+
/* Construct struct pt_regs on stack */
pushq   $__USER_DS  /* pt_regs->ss */
pushq   PER_CPU_VAR(cpu_tss_rw + TSS_sp2)   /* pt_regs->sp */
@@ -446,6 +448,84 @@ _ASM_NOKPROBE(\asmsym)
 SYM_CODE_END(\asmsym)
 .endm
 
+#ifdef CONFIG_AMD_MEM_ENCRYPT
+/**
+ * idtentry_vc - Macro to generate entry stub for #VC
+ * @vector:Vector number
+ * @asmsym:ASM symbol for the entry point
+ * @cfunc: C function to be called
+ *
+ * The macro emits code to set up the kernel context for #VC. The #VC handler
+ * runs on an IST stack and needs to be able to cause nested #VC exceptions.
+ *
+ * To make this work the #VC entry code tries its best to pretend it doesn't 
use
+ * an IST stack by switching to the task stack if coming from user-space (which
+ * includes early SYSCALL entry path) or back to the stack in the IRET frame if
+ * entered from kernel-mode.
+ *
+ * If entered from kernel-mode the return stack is validated first, and if it 
is
+ * not safe to use (e.g. because it points to the entry stack) the #VC handler
+ * will switch to a fall-back stack (VC2) and call a special handler function.
+ *
+ * The macro is only used for one vector, but it is planned to be extended in
+ * the future for the #HV exception.
+ */
+.macro idtentry_vc vector asmsym cfunc
+SYM_CODE_START(\asmsym)
+   UNWIND_HINT_IRET_REGS
+   ASM_CLAC
+
+   /*
+* If the entry is from userspace, switch stacks and treat it as
+* a normal entry.
+*/
+   testb   $3, CS-ORIG_RAX(%rsp)
+   jnz .Lfrom_usermode_switch_stack_\@
+
+   /*
+* paranoid_entry returns SWAPGS flag for paranoid_exit in EBX.
+* EBX == 0 -> SWAPGS, EBX == 1 -> no SWAPGS
+*/
+   callparanoid_entry
+
+   UNWIND_HINT_REGS
+
+   /*
+* Switch off the IST stack to make it free for nested exceptions. The
+* vc_switch_off_ist() function will switch back to the interrupted
+* stack if it is safe to do so. If not it switches to the VC fall-back
+* stack.
+*/
+   movq%rsp, %rdi  /* pt_regs pointer */
+   callvc_switch_off_ist
+   movq%rax, %rsp  /* Switch to new stack */
+
+   UNWIND_HINT_REGS
+
+   /* Update pt_regs */
+   movqORIG_RAX(%rsp), %rsi/* get error code into 2nd argument*/
+   movq$-1, ORIG_RAX(%rsp) /* no syscall to restart */
+
+   movq%rsp, %rdi  /* pt_regs pointer */
+
+   call\cfunc
+
+   /*
+* No need to switch back to the IST stack. The current stack is either
+* identical to the stack in the IRET frame or the VC fall-back stack,
+* so it is definitly mapped even with PTI enabled.
+*/
+   jmp paranoid_exit
+
+   /* Switch to the regular task stack */
+.Lfrom_usermode_switch_stack_\@:
+   idtentry_body safe_stack_\cfunc, has_error_code=1
+
+_ASM_NOKPROBE(\asmsym)
+SYM_CODE_END(\asmsym)
+.endm
+#endif
+
 /*
  * Double fault entry. Straight paranoid. No checks from which context
  * this comes because for the espfix induced #DF this would do the wrong
diff --git a/arch/x86/include/asm/idtentry.h b/arch/x86/include/asm/idtentry.h
index 337dcfd45472..5ce67113a761 100644
--- a/arch/x86/include/asm/idtentry.h
+++ b/arch/x86/include/asm/idtentry.h
@@ -308,6 +308,18 @@ static __always_inline void __##func(struct pt_regs *regs)
DECLARE_IDTENTRY_RAW(vector, func); \
__visible void noist_##func(struct