lkml.org 
[lkml]   [2019]   [Jul]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v2 3/7] x86/entry/64: Simplify idtentry a little
On Thu, Jul 4, 2019 at 1:03 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> There's a bunch of duplication in idtentry, namely the
> .Lfrom_usermode_switch_stack is a paranoid=0 copy of the normal flow.
>
> Make this explicit by creating a idtentry_part helper macro.
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
> arch/x86/entry/entry_64.S | 100 +++++++++++++++++++++-------------------------
> 1 file changed, 47 insertions(+), 53 deletions(-)
>
> --- a/arch/x86/entry/entry_64.S
> +++ b/arch/x86/entry/entry_64.S
> @@ -865,6 +865,51 @@ apicinterrupt IRQ_WORK_VECTOR irq_work
> */
> #define CPU_TSS_IST(x) PER_CPU_VAR(cpu_tss_rw) + (TSS_ist + (x) * 8)
>
> +.macro idtentry_part do_sym, has_error_code:req, paranoid:req, shift_ist=-1, ist_offset=0
> +
> + .if \paranoid
> + call paranoid_entry
> + /* returned flag: ebx=0: need swapgs on exit, ebx=1: don't need it */
> + .else
> + call error_entry
> + .endif
> + UNWIND_HINT_REGS
> +
> + .if \paranoid
> + .if \shift_ist != -1
> + TRACE_IRQS_OFF_DEBUG /* reload IDT in case of recursion */
> + .else
> + TRACE_IRQS_OFF
> + .endif
> + .endif
> +
> + movq %rsp, %rdi /* pt_regs pointer */
> +
> + .if \has_error_code
> + movq ORIG_RAX(%rsp), %rsi /* get error code */
> + movq $-1, ORIG_RAX(%rsp) /* no syscall to restart */
> + .else
> + xorl %esi, %esi /* no error code */
> + .endif
> +
> + .if \shift_ist != -1
> + subq $\ist_offset, CPU_TSS_IST(\shift_ist)
> + .endif
> +
> + call \do_sym
> +
> + .if \shift_ist != -1
> + addq $\ist_offset, CPU_TSS_IST(\shift_ist)
> + .endif
> +
> + .if \paranoid
> + jmp paranoid_exit
> + .else
> + jmp error_exit
> + .endif
> +
> +.endm
> +
> /**
> * idtentry - Generate an IDT entry stub
> * @sym: Name of the generated entry point
> @@ -935,46 +980,7 @@ ENTRY(\sym)
> .Lfrom_usermode_no_gap_\@:
> .endif
>
> - .if \paranoid
> - call paranoid_entry
> - .else
> - call error_entry
> - .endif
> - UNWIND_HINT_REGS
> - /* returned flag: ebx=0: need swapgs on exit, ebx=1: don't need it */
> -
> - .if \paranoid
> - .if \shift_ist != -1
> - TRACE_IRQS_OFF_DEBUG /* reload IDT in case of recursion */
> - .else
> - TRACE_IRQS_OFF
> - .endif
> - .endif
> -
> - movq %rsp, %rdi /* pt_regs pointer */
> -
> - .if \has_error_code
> - movq ORIG_RAX(%rsp), %rsi /* get error code */
> - movq $-1, ORIG_RAX(%rsp) /* no syscall to restart */
> - .else
> - xorl %esi, %esi /* no error code */
> - .endif
> -
> - .if \shift_ist != -1
> - subq $\ist_offset, CPU_TSS_IST(\shift_ist)
> - .endif
> -
> - call \do_sym
> -
> - .if \shift_ist != -1
> - addq $\ist_offset, CPU_TSS_IST(\shift_ist)
> - .endif
> -
> - .if \paranoid
> - jmp paranoid_exit
> - .else
> - jmp error_exit
> - .endif
> + idtentry_part \do_sym, \has_error_code, \paranoid, \shift_ist, \ist_offset
>
> .if \paranoid == 1
> /*
> @@ -983,21 +989,9 @@ ENTRY(\sym)
> * run in real process context if user_mode(regs).
> */
> .Lfrom_usermode_switch_stack_\@:
> - call error_entry
> -
> - movq %rsp, %rdi /* pt_regs pointer */
> -
> - .if \has_error_code
> - movq ORIG_RAX(%rsp), %rsi /* get error code */
> - movq $-1, ORIG_RAX(%rsp) /* no syscall to restart */
> - .else
> - xorl %esi, %esi /* no error code */
> + idtentry_part \do_sym, \has_error_code, 0

Nice! You are adding an extra UNWIND_HINT_REGS that wasn't here
before, but I think that's fine. However, can you pleace make it
paranoid=0 instead of just 0? You could go all the way verbose and
say do_sym=\do_sym, etc, but that seems like overkill.

Other than that nitpick, Acked-by: Andy Lutomirski <luto@kernel.org>

--Andy

\
 
 \ /
  Last update: 2019-07-04 23:55    [W:0.200 / U:12.324 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site