diff mbox series

x86/traps: More use of nocall

Message ID 20231120194537.1341452-1-andrew.cooper3@citrix.com (mailing list archive)
State New, archived
Headers show
Series x86/traps: More use of nocall | expand

Commit Message

Andrew Cooper Nov. 20, 2023, 7:45 p.m. UTC
sysenter_eflags_saved() and int80_direct_trap() are now only used by a single
translation unit.  Move the declarations into the respective traps.c, renaming
int80_direct_trap() to entry_int80() to match the style elsewhere.

Annotate all 3 with nocall like all other entry paths.

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>

Followup from XSA-446
---
 xen/arch/x86/include/asm/processor.h | 4 +---
 xen/arch/x86/pv/traps.c              | 3 ++-
 xen/arch/x86/traps.c                 | 2 ++
 xen/arch/x86/x86_64/entry.S          | 2 +-
 4 files changed, 6 insertions(+), 5 deletions(-)

Comments

Jan Beulich Nov. 21, 2023, 8:52 a.m. UTC | #1
On 20.11.2023 20:45, Andrew Cooper wrote:
> sysenter_eflags_saved() and int80_direct_trap() are now only used by a single
> translation unit.  Move the declarations into the respective traps.c, renaming
> int80_direct_trap() to entry_int80() to match the style elsewhere.
> 
> Annotate all 3 with nocall like all other entry paths.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Nevertheless I wonder if this wouldn't better be deferred until asmlinkage
has appeared in the code base, such that along with nocall that could be
added at the same time. (Even then it's not entirely clear to me whether
Misra is going to be happy with declarations not living in headers. But
imo limiting exposure trumps Misra's possible pickiness.)

Jan
Andrew Cooper Nov. 21, 2023, 12:39 p.m. UTC | #2
On 21/11/2023 8:52 am, Jan Beulich wrote:
> On 20.11.2023 20:45, Andrew Cooper wrote:
>> sysenter_eflags_saved() and int80_direct_trap() are now only used by a single
>> translation unit.  Move the declarations into the respective traps.c, renaming
>> int80_direct_trap() to entry_int80() to match the style elsewhere.
>>
>> Annotate all 3 with nocall like all other entry paths.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Thanks.

> Nevertheless I wonder if this wouldn't better be deferred until asmlinkage
> has appeared in the code base, such that along with nocall that could be
> added at the same time. (Even then it's not entirely clear to me whether
> Misra is going to be happy with declarations not living in headers. But
> imo limiting exposure trumps Misra's possible pickiness.)

We've been over this many times.

MISRA demands that it is declared only once.  It *does not* demand that
the declaration is in a header file.

And this has been confirmed by our MISRA experts previously.

~Andrew
diff mbox series

Patch

diff --git a/xen/arch/x86/include/asm/processor.h b/xen/arch/x86/include/asm/processor.h
index b0d2a62c075f..ff62b080afbf 100644
--- a/xen/arch/x86/include/asm/processor.h
+++ b/xen/arch/x86/include/asm/processor.h
@@ -457,9 +457,7 @@  static inline void enable_nmis(void)
                      [cs] "r" (__HYPERVISOR_CS) );
 }
 
-void sysenter_entry(void);
-void sysenter_eflags_saved(void);
-void int80_direct_trap(void);
+void nocall sysenter_entry(void);
 
 struct stubs {
     union {
diff --git a/xen/arch/x86/pv/traps.c b/xen/arch/x86/pv/traps.c
index 240d1a2db7a3..83e84e276233 100644
--- a/xen/arch/x86/pv/traps.c
+++ b/xen/arch/x86/pv/traps.c
@@ -132,6 +132,7 @@  static void cf_check nmi_softirq(void)
     *v_ptr = NULL;
 }
 
+void nocall entry_int80(void);
 void nocall entry_int82(void);
 
 void __init pv_trap_init(void)
@@ -144,7 +145,7 @@  void __init pv_trap_init(void)
 
     /* Fast trap for int80 (faster than taking the #GP-fixup path). */
     _set_gate(idt_table + LEGACY_SYSCALL_VECTOR, SYS_DESC_irq_gate, 3,
-              &int80_direct_trap);
+              &entry_int80);
 
     open_softirq(NMI_SOFTIRQ, nmi_softirq);
 }
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index e1356f696aba..9a6d29f24ae1 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -1884,6 +1884,8 @@  void do_device_not_available(struct cpu_user_regs *regs)
 #endif
 }
 
+void nocall sysenter_eflags_saved(void);
+
 /* SAF-1-safe */
 void do_debug(struct cpu_user_regs *regs)
 {
diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
index 9a7b129aa7e4..bf654fe27ec3 100644
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -385,7 +385,7 @@  UNLIKELY_END(sysenter_gpf)
 #endif
         jmp   .Lbounce_exception
 
-ENTRY(int80_direct_trap)
+ENTRY(entry_int80)
         ENDBR64
         ALTERNATIVE "", clac, X86_FEATURE_XEN_SMAP
         pushq $0