diff mbox series

[2/3] x86/entry: Rename the exception entrypoints

Message ID 20230728194320.3082120-3-andrew.cooper3@citrix.com (mailing list archive)
State New, archived
Headers show
Series x86: Some MISRA Rule 5.3 fixes | expand

Commit Message

Andrew Cooper July 28, 2023, 7:43 p.m. UTC
This makes the names match the architectural short names that we use
elsewhere.  This avoids 'debug' in particular from being a global symbol
shadowed by many local parameter names.

Remove the DECLARE_TRAP_HANDLER{,_CONST}() infrastructure.  Only NMI/#MC are
referenced externally (and NMI will cease to be soon, as part of adding FRED
support).  Move the entrypoint declarations into the respective traps.c where
they're used, rather than keeping them visible across ~all of Xen.

Drop the long-stale comment at the top of init_idt_traps().  It's mostly
discussing a 32bit Xen, and bogus otherwise as it's impossible to use trap
gates correctly for these purposes.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Roberto Bagnara <roberto.bagnara@bugseng.com>
CC: Nicola Vetrini <nicola.vetrini@bugseng.com>

This is half of a previous patch, cut down to just the rename and header file
cleanup.
---
 xen/arch/x86/include/asm/processor.h | 34 +-------------
 xen/arch/x86/pv/traps.c              |  2 +
 xen/arch/x86/traps.c                 | 70 +++++++++++++++++-----------
 xen/arch/x86/x86_64/entry.S          | 36 +++++++-------
 4 files changed, 64 insertions(+), 78 deletions(-)

Comments

Jan Beulich July 31, 2023, 8:57 a.m. UTC | #1
On 28.07.2023 21:43, Andrew Cooper wrote:
> This makes the names match the architectural short names that we use
> elsewhere.  This avoids 'debug' in particular from being a global symbol
> shadowed by many local parameter names.
> 
> Remove the DECLARE_TRAP_HANDLER{,_CONST}() infrastructure.  Only NMI/#MC are
> referenced externally (and NMI will cease to be soon, as part of adding FRED
> support).  Move the entrypoint declarations into the respective traps.c where
> they're used, rather than keeping them visible across ~all of Xen.

Coming back to my Misra concern here: There's no way we can avoid violating
rule 8.7 (and maybe also 8.6) when it comes to symbols defined in assembly
code. Rule 8.5 otoh is a little more relaxed than I recalled when commenting
on the earlier version of the patch, but it still talks of exclusively
"header files". Roberto, could you please clarify whether putting such
declarations in .c files is indeed not a violation of 8.5?

In any event I see you've added "nocall" - I guess that's an easy
identification for (some of) the symbols defined in assembly code, and
hence needing special casing in scans.

> Drop the long-stale comment at the top of init_idt_traps().  It's mostly
> discussing a 32bit Xen, and bogus otherwise as it's impossible to use trap
> gates correctly for these purposes.

I don't think I follow the "impossible", but I'm also not going to make my
(eventual) ack dependent upon you further adjusting this part of the
description.

Jan
Nicola Vetrini July 31, 2023, 9:15 a.m. UTC | #2
On 28/07/2023 21:43, Andrew Cooper wrote:
> This makes the names match the architectural short names that we use
> elsewhere.  This avoids 'debug' in particular from being a global 
> symbol
> shadowed by many local parameter names.
> 
> Remove the DECLARE_TRAP_HANDLER{,_CONST}() infrastructure.  Only 
> NMI/#MC are
> referenced externally (and NMI will cease to be soon, as part of adding 
> FRED
> support).  Move the entrypoint declarations into the respective traps.c 
> where
> they're used, rather than keeping them visible across ~all of Xen.
> 
> Drop the long-stale comment at the top of init_idt_traps().  It's 
> mostly
> discussing a 32bit Xen, and bogus otherwise as it's impossible to use 
> trap
> gates correctly for these purposes.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Wei Liu <wl@xen.org>
> CC: Roberto Bagnara <roberto.bagnara@bugseng.com>
> CC: Nicola Vetrini <nicola.vetrini@bugseng.com>
> 
> This is half of a previous patch, cut down to just the rename and 
> header file
> cleanup.
> ---
>  xen/arch/x86/include/asm/processor.h | 34 +-------------
>  xen/arch/x86/pv/traps.c              |  2 +
>  xen/arch/x86/traps.c                 | 70 +++++++++++++++++-----------
>  xen/arch/x86/x86_64/entry.S          | 36 +++++++-------
>  4 files changed, 64 insertions(+), 78 deletions(-)
> 
With respect to Rule 5.3:
Tested-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
diff mbox series

Patch

diff --git a/xen/arch/x86/include/asm/processor.h b/xen/arch/x86/include/asm/processor.h
index c0529cc3d984..0989748be6d5 100644
--- a/xen/arch/x86/include/asm/processor.h
+++ b/xen/arch/x86/include/asm/processor.h
@@ -417,38 +417,8 @@  extern void mtrr_bp_init(void);
 
 void mcheck_init(struct cpuinfo_x86 *c, bool_t bsp);
 
-#define DECLARE_TRAP_HANDLER(_name)                    \
-    void _name(void);                                  \
-    void do_ ## _name(struct cpu_user_regs *regs)
-#define DECLARE_TRAP_HANDLER_CONST(_name)              \
-    void _name(void);                                  \
-    void do_ ## _name(const struct cpu_user_regs *regs)
-
-DECLARE_TRAP_HANDLER(divide_error);
-DECLARE_TRAP_HANDLER(debug);
-DECLARE_TRAP_HANDLER_CONST(nmi);
-DECLARE_TRAP_HANDLER(int3);
-DECLARE_TRAP_HANDLER(overflow);
-DECLARE_TRAP_HANDLER(bounds);
-DECLARE_TRAP_HANDLER(invalid_op);
-DECLARE_TRAP_HANDLER(device_not_available);
-DECLARE_TRAP_HANDLER(double_fault);
-DECLARE_TRAP_HANDLER(invalid_TSS);
-DECLARE_TRAP_HANDLER(segment_not_present);
-DECLARE_TRAP_HANDLER(stack_segment);
-DECLARE_TRAP_HANDLER(general_protection);
-DECLARE_TRAP_HANDLER(page_fault);
-DECLARE_TRAP_HANDLER(early_page_fault);
-DECLARE_TRAP_HANDLER(coprocessor_error);
-DECLARE_TRAP_HANDLER(simd_coprocessor_error);
-DECLARE_TRAP_HANDLER_CONST(machine_check);
-DECLARE_TRAP_HANDLER(alignment_check);
-DECLARE_TRAP_HANDLER(entry_CP);
-
-DECLARE_TRAP_HANDLER(entry_int82);
-
-#undef DECLARE_TRAP_HANDLER_CONST
-#undef DECLARE_TRAP_HANDLER
+void do_nmi(const struct cpu_user_regs *regs);
+void do_machine_check(const struct cpu_user_regs *regs);
 
 void trap_nop(void);
 
diff --git a/xen/arch/x86/pv/traps.c b/xen/arch/x86/pv/traps.c
index 17ca4d1d5142..74f333da7e1c 100644
--- a/xen/arch/x86/pv/traps.c
+++ b/xen/arch/x86/pv/traps.c
@@ -132,6 +132,8 @@  static void cf_check nmi_softirq(void)
     *v_ptr = NULL;
 }
 
+void nocall entry_int82(void);
+
 void __init pv_trap_init(void)
 {
 #ifdef CONFIG_PV32
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index e05e8964482e..8470561cbc27 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -2107,35 +2107,49 @@  void percpu_traps_init(void)
         wrmsrl(MSR_IA32_DEBUGCTLMSR, IA32_DEBUGCTLMSR_LBR);
 }
 
+/* Exception entries */
+void nocall entry_DE(void);
+void nocall entry_DB(void);
+void nocall entry_NMI(void);
+void nocall entry_BP(void);
+void nocall entry_OF(void);
+void nocall entry_BR(void);
+void nocall entry_UD(void);
+void nocall entry_NM(void);
+void nocall entry_DF(void);
+void nocall entry_TS(void);
+void nocall entry_NP(void);
+void nocall entry_SS(void);
+void nocall entry_GP(void);
+void nocall early_page_fault(void);
+void nocall entry_PF(void);
+void nocall entry_MF(void);
+void nocall entry_AC(void);
+void nocall entry_MC(void);
+void nocall entry_XM(void);
+void nocall entry_CP(void);
+
 void __init init_idt_traps(void)
 {
-    /*
-     * Note that interrupt gates are always used, rather than trap gates. We
-     * must have interrupts disabled until DS/ES/FS/GS are saved because the
-     * first activation must have the "bad" value(s) for these registers and
-     * we may lose them if another activation is installed before they are
-     * saved. The page-fault handler also needs interrupts disabled until %cr2
-     * has been read and saved on the stack.
-     */
-    set_intr_gate(X86_EXC_DE,  divide_error);
-    set_intr_gate(X86_EXC_DB,  debug);
-    set_intr_gate(X86_EXC_NMI, nmi);
-    set_swint_gate(X86_EXC_BP, int3);     /* usable from all privileges */
-    set_swint_gate(X86_EXC_OF, overflow); /* usable from all privileges */
-    set_intr_gate(X86_EXC_BR,  bounds);
-    set_intr_gate(X86_EXC_UD,  invalid_op);
-    set_intr_gate(X86_EXC_NM,  device_not_available);
-    set_intr_gate(X86_EXC_DF,  double_fault);
-    set_intr_gate(X86_EXC_TS,  invalid_TSS);
-    set_intr_gate(X86_EXC_NP,  segment_not_present);
-    set_intr_gate(X86_EXC_SS,  stack_segment);
-    set_intr_gate(X86_EXC_GP,  general_protection);
-    set_intr_gate(X86_EXC_PF,  early_page_fault);
-    set_intr_gate(X86_EXC_MF,  coprocessor_error);
-    set_intr_gate(X86_EXC_AC,  alignment_check);
-    set_intr_gate(X86_EXC_MC,  machine_check);
-    set_intr_gate(X86_EXC_XM,  simd_coprocessor_error);
-    set_intr_gate(X86_EXC_CP,  entry_CP);
+    set_intr_gate (X86_EXC_DE,  entry_DE);
+    set_intr_gate (X86_EXC_DB,  entry_DB);
+    set_intr_gate (X86_EXC_NMI, entry_NMI);
+    set_swint_gate(X86_EXC_BP,  entry_BP);
+    set_swint_gate(X86_EXC_OF,  entry_OF);
+    set_intr_gate (X86_EXC_BR,  entry_BR);
+    set_intr_gate (X86_EXC_UD,  entry_UD);
+    set_intr_gate (X86_EXC_NM,  entry_NM);
+    set_intr_gate (X86_EXC_DF,  entry_DF);
+    set_intr_gate (X86_EXC_TS,  entry_TS);
+    set_intr_gate (X86_EXC_NP,  entry_NP);
+    set_intr_gate (X86_EXC_SS,  entry_SS);
+    set_intr_gate (X86_EXC_GP,  entry_GP);
+    set_intr_gate (X86_EXC_PF,  early_page_fault);
+    set_intr_gate (X86_EXC_MF,  entry_MF);
+    set_intr_gate (X86_EXC_AC,  entry_AC);
+    set_intr_gate (X86_EXC_MC,  entry_MC);
+    set_intr_gate (X86_EXC_XM,  entry_XM);
+    set_intr_gate (X86_EXC_CP,  entry_CP);
 
     /* Specify dedicated interrupt stacks for NMI, #DF, and #MC. */
     enable_each_ist(idt_table);
@@ -2154,7 +2168,7 @@  void __init trap_init(void)
     unsigned int vector;
 
     /* Replace early pagefault with real pagefault handler. */
-    set_intr_gate(X86_EXC_PF, &page_fault);
+    set_intr_gate(X86_EXC_PF, entry_PF);
 
     pv_trap_init();
 
diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
index bca1500e2b45..81dd2c74b876 100644
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -708,7 +708,7 @@  ENTRY(common_interrupt)
         mov   %bl, STACK_CPUINFO_FIELD(use_pv_cr3)(%r14)
         jmp ret_from_intr
 
-ENTRY(page_fault)
+ENTRY(entry_PF)
         ENDBR64
         movl  $X86_EXC_PF, 4(%rsp)
 /* No special register assumptions. */
@@ -881,81 +881,81 @@  FATAL_exception_with_ints_disabled:
         movq  %rsp,%rdi
         tailcall fatal_trap
 
-ENTRY(divide_error)
+ENTRY(entry_DE)
         ENDBR64
         pushq $0
         movl  $X86_EXC_DE, 4(%rsp)
         jmp   handle_exception
 
-ENTRY(coprocessor_error)
+ENTRY(entry_MF)
         ENDBR64
         pushq $0
         movl  $X86_EXC_MF, 4(%rsp)
         jmp   handle_exception
 
-ENTRY(simd_coprocessor_error)
+ENTRY(entry_XM)
         ENDBR64
         pushq $0
         movl  $X86_EXC_XM, 4(%rsp)
         jmp   handle_exception
 
-ENTRY(device_not_available)
+ENTRY(entry_NM)
         ENDBR64
         pushq $0
         movl  $X86_EXC_NM, 4(%rsp)
         jmp   handle_exception
 
-ENTRY(debug)
+ENTRY(entry_DB)
         ENDBR64
         pushq $0
         movl  $X86_EXC_DB, 4(%rsp)
         jmp   handle_ist_exception
 
-ENTRY(int3)
+ENTRY(entry_BP)
         ENDBR64
         pushq $0
         movl  $X86_EXC_BP, 4(%rsp)
         jmp   handle_exception
 
-ENTRY(overflow)
+ENTRY(entry_OF)
         ENDBR64
         pushq $0
         movl  $X86_EXC_OF, 4(%rsp)
         jmp   handle_exception
 
-ENTRY(bounds)
+ENTRY(entry_BR)
         ENDBR64
         pushq $0
         movl  $X86_EXC_BR, 4(%rsp)
         jmp   handle_exception
 
-ENTRY(invalid_op)
+ENTRY(entry_UD)
         ENDBR64
         pushq $0
         movl  $X86_EXC_UD, 4(%rsp)
         jmp   handle_exception
 
-ENTRY(invalid_TSS)
+ENTRY(entry_TS)
         ENDBR64
         movl  $X86_EXC_TS, 4(%rsp)
         jmp   handle_exception
 
-ENTRY(segment_not_present)
+ENTRY(entry_NP)
         ENDBR64
         movl  $X86_EXC_NP, 4(%rsp)
         jmp   handle_exception
 
-ENTRY(stack_segment)
+ENTRY(entry_SS)
         ENDBR64
         movl  $X86_EXC_SS, 4(%rsp)
         jmp   handle_exception
 
-ENTRY(general_protection)
+ENTRY(entry_GP)
         ENDBR64
         movl  $X86_EXC_GP, 4(%rsp)
         jmp   handle_exception
 
-ENTRY(alignment_check)
+ENTRY(entry_AC)
         ENDBR64
         movl  $X86_EXC_AC, 4(%rsp)
         jmp   handle_exception
@@ -965,7 +965,7 @@  ENTRY(entry_CP)
         movl  $X86_EXC_CP, 4(%rsp)
         jmp   handle_exception
 
-ENTRY(double_fault)
+ENTRY(entry_DF)
         ENDBR64
         movl  $X86_EXC_DF, 4(%rsp)
         /* Set AC to reduce chance of further SMAP faults */
@@ -989,7 +989,7 @@  ENTRY(double_fault)
         movq  %rsp,%rdi
         tailcall do_double_fault
 
-ENTRY(nmi)
+ENTRY(entry_NMI)
         ENDBR64
         pushq $0
         movl  $X86_EXC_NMI, 4(%rsp)
@@ -1117,7 +1117,7 @@  handle_ist_exception:
         jmp   restore_all_xen
 #endif
 
-ENTRY(machine_check)
+ENTRY(entry_MC)
         ENDBR64
         pushq $0
         movl  $X86_EXC_MC, 4(%rsp)