[PATCH v2] x86: move interrupt entry stubs to entry.S

[PATCH v2] x86: move interrupt entry stubs to entry.S

[Feng Wu]

This patch removes the ugly macros in i8259.c and asm_defns.h, which is
used to build up the interrupt entry stubs. Move the definition of these
stubs to entry.S instead.

Signed-off-by: Feng Wu <feng.wu@intel.com>
---
 xen/arch/x86/i8259.c            | 63 ++---------------------------------------
 xen/arch/x86/traps.c            |  6 ++--
 xen/arch/x86/x86_64/entry.S     | 16 +++++++++++
 xen/include/asm-x86/asm_defns.h |  7 ++---
 xen/include/asm-x86/desc.h      |  2 +-
 5 files changed, 24 insertions(+), 70 deletions(-)

diff --git a/xen/arch/x86/i8259.c b/xen/arch/x86/i8259.c
index 9fec490..f207c2c 100644
--- a/xen/arch/x86/i8259.c
+++ b/xen/arch/x86/i8259.c
@@ -23,65 +23,6 @@
 #include <io_ports.h>
 
 /*
- * Common place to define all x86 IRQ vectors
- *
- * This builds up the IRQ handler stubs using some ugly macros in irq.h
- *
- * These macros create the low-level assembly IRQ routines that save
- * register context and call do_IRQ(). do_IRQ() then does all the
- * operations that are needed to keep the AT (or SMP IOAPIC)
- * interrupt-controller happy.
- */
-
-__asm__(".section .text");
-
-#define IRQ_NAME(nr) VEC##nr##_interrupt
-
-#define BI(nr)                                           \
-void IRQ_NAME(nr)(void);                                 \
-__asm__(                                                 \
-".if " STR(0x##nr) " >= " STR(FIRST_DYNAMIC_VECTOR) "\n" \
-__ALIGN_STR "\n"                                         \
-STR(IRQ_NAME(nr)) ":\n\t"                                \
-BUILD_IRQ(0x##nr) "\n"                                   \
-".else\n"                                                \
-".equ " STR(IRQ_NAME(nr)) ", 0\n"                        \
-".endif\n")
-
-#define BUILD_16_IRQS(x) \
-    BI(x##0); BI(x##1); BI(x##2); BI(x##3); \
-    BI(x##4); BI(x##5); BI(x##6); BI(x##7); \
-    BI(x##8); BI(x##9); BI(x##a); BI(x##b); \
-    BI(x##c); BI(x##d); BI(x##e); BI(x##f)
-
-BUILD_16_IRQS(0); BUILD_16_IRQS(1); BUILD_16_IRQS(2); BUILD_16_IRQS(3);
-BUILD_16_IRQS(4); BUILD_16_IRQS(5); BUILD_16_IRQS(6); BUILD_16_IRQS(7);
-BUILD_16_IRQS(8); BUILD_16_IRQS(9); BUILD_16_IRQS(a); BUILD_16_IRQS(b);
-BUILD_16_IRQS(c); BUILD_16_IRQS(d); BUILD_16_IRQS(e); BUILD_16_IRQS(f);
-
-#undef BUILD_16_IRQS
-#undef BI
-
-
-#define IRQ(x,y) IRQ_NAME(x##y)
-
-#define IRQLIST_16(x) \
-    IRQ(x,0), IRQ(x,1), IRQ(x,2), IRQ(x,3), \
-    IRQ(x,4), IRQ(x,5), IRQ(x,6), IRQ(x,7), \
-    IRQ(x,8), IRQ(x,9), IRQ(x,a), IRQ(x,b), \
-    IRQ(x,c), IRQ(x,d), IRQ(x,e), IRQ(x,f)
-
-static void (*__initdata interrupt[NR_VECTORS])(void) = {
-    IRQLIST_16(0), IRQLIST_16(1), IRQLIST_16(2), IRQLIST_16(3),
-    IRQLIST_16(4), IRQLIST_16(5), IRQLIST_16(6), IRQLIST_16(7),
-    IRQLIST_16(8), IRQLIST_16(9), IRQLIST_16(a), IRQLIST_16(b),
-    IRQLIST_16(c), IRQLIST_16(d), IRQLIST_16(e), IRQLIST_16(f)
-};
-
-#undef IRQ
-#undef IRQLIST_16
-
-/*
  * This is the 'legacy' 8259A Programmable Interrupt Controller,
  * present in the majority of PC/AT boxes.
  * plus some generic x86 specific things if generic specifics makes
@@ -407,8 +348,8 @@ void __init init_IRQ(void)
     {
         if (vector == HYPERCALL_VECTOR || vector == LEGACY_SYSCALL_VECTOR)
             continue;
-        BUG_ON(!interrupt[vector]);
-        set_intr_gate(vector, interrupt[vector]);
+        BUG_ON(!interrupt[vector - FIRST_DYNAMIC_VECTOR]);
+        set_intr_gate(vector, interrupt[vector - FIRST_DYNAMIC_VECTOR]);
     }
 
     for (irq = 0; platform_legacy_irq(irq); irq++) {
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 40366f1..ac9ce0d 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -3425,7 +3425,7 @@ void do_spurious_interrupt_bug(struct cpu_user_regs *regs)
 {
 }
 
-static void __set_intr_gate(unsigned int n, uint32_t dpl, void *addr)
+static void __set_intr_gate(unsigned int n, uint32_t dpl, const void *addr)
 {
     int i;
     /* Keep secondary tables in sync with IRQ updates. */
@@ -3435,12 +3435,12 @@ static void __set_intr_gate(unsigned int n, uint32_t dpl, void *addr)
     _set_gate(&idt_table[n], 14, dpl, addr);
 }
 
-static void set_swint_gate(unsigned int n, void *addr)
+static void set_swint_gate(unsigned int n, const void *addr)
 {
     __set_intr_gate(n, 3, addr);
 }
 
-void set_intr_gate(unsigned int n, void *addr)
+void set_intr_gate(unsigned int n, const void *addr)
 {
     __set_intr_gate(n, 0, addr);
 }
diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
index 4796e65..5b00d1e 100644
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -469,6 +469,22 @@ ENTRY(dom_crash_sync_extable)
         xorl  %edi,%edi
         jmp   asm_domain_crash_synchronous /* Does not return */
 
+        .pushsection .init.rodata,"a"
+ENTRY(interrupt)
+        .popsection
+ENTRY(irq_entries_start)
+        vector=FIRST_DYNAMIC_VECTOR
+        .rept NR_VECTORS-FIRST_DYNAMIC_VECTOR
+        ALIGN
+1:      pushq $0x0
+        movl $vector,0x4(%rsp)
+        jmp common_interrupt
+        .pushsection .init.rodata,"a"
+        .quad 1b
+        .popsection
+        vector=vector+1
+        .endr
+
 ENTRY(common_interrupt)
         SAVE_ALL CLAC
         movq %rsp,%rdi
diff --git a/xen/include/asm-x86/asm_defns.h b/xen/include/asm-x86/asm_defns.h
index df4873b..a228883 100644
--- a/xen/include/asm-x86/asm_defns.h
+++ b/xen/include/asm-x86/asm_defns.h
@@ -13,6 +13,8 @@
 
 #ifndef __ASSEMBLY__
 void ret_from_intr(void);
+typedef void (*interrupt_stub_t)(void);
+extern const interrupt_stub_t interrupt[];
 #endif
 
 #ifdef CONFIG_FRAME_POINTER
@@ -357,9 +359,4 @@ static inline void stac(void)
 #define REX64_PREFIX "rex64/"
 #endif
 
-#define BUILD_IRQ(nr)                           \
-    "pushq $0\n\t"                              \
-    "movl $"#nr",4(%rsp)\n\t"                   \
-    "jmp common_interrupt"
-
 #endif /* __X86_ASM_DEFNS_H__ */
diff --git a/xen/include/asm-x86/desc.h b/xen/include/asm-x86/desc.h
index 4edb834..69f58ae 100644
--- a/xen/include/asm-x86/desc.h
+++ b/xen/include/asm-x86/desc.h
@@ -191,7 +191,7 @@ DECLARE_PER_CPU(struct desc_struct *, gdt_table);
 extern struct desc_struct boot_cpu_compat_gdt_table[];
 DECLARE_PER_CPU(struct desc_struct *, compat_gdt_table);
 
-extern void set_intr_gate(unsigned int irq, void * addr);
+extern void set_intr_gate(unsigned int irq, const void * addr);
 extern void load_TR(void);
 
 #endif /* !__ASSEMBLY__ */
-- 
1.8.3.1

Re: [PATCH v2] x86: move interrupt entry stubs to entry.S

[Jan Beulich]

>>> On 16.05.14 at 04:12, <feng.wu@intel.com> wrote:
> This patch removes the ugly macros in i8259.c and asm_defns.h, which is
> used to build up the interrupt entry stubs. Move the definition of these
> stubs to entry.S instead.

I suppose you saw that Andrew posted a more comprehensive
series with, among other things, the same effect as yours. I'm
clearly tending towards taken his version, unless you can provide
a good explanation of why yours is preferable.

> --- a/xen/include/asm-x86/asm_defns.h
> +++ b/xen/include/asm-x86/asm_defns.h
> @@ -13,6 +13,8 @@
>  
>  #ifndef __ASSEMBLY__
>  void ret_from_intr(void);
> +typedef void (*interrupt_stub_t)(void);
> +extern const interrupt_stub_t interrupt[];

I don't see the point of the typedef, and I don't think the declaration
is well placed in this header.

Jan

Re: [PATCH v2] x86: move interrupt entry stubs to entry.S

[Wu, Feng]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: Friday, May 16, 2014 3:35 PM
> To: Wu, Feng
> Cc: Andrew Cooper; keir.xen@gmail.com; xen-devel@lists.xen.org
> Subject: Re: [PATCH v2] x86: move interrupt entry stubs to entry.S
> 
> >>> On 16.05.14 at 04:12, <feng.wu@intel.com> wrote:
> > This patch removes the ugly macros in i8259.c and asm_defns.h, which is
> > used to build up the interrupt entry stubs. Move the definition of these
> > stubs to entry.S instead.
> 
> I suppose you saw that Andrew posted a more comprehensive
> series with, among other things, the same effect as yours. I'm
> clearly tending towards taken his version, unless you can provide
> a good explanation of why yours is preferable.

I didn't notice Andrew's patch. I am okay with that, I did this because
I think this is a subsequent work of the SMAP patch. If Andrew's patch
can do the same thing, I can just stop. Thanks!

Thanks,
Feng

> 
> > --- a/xen/include/asm-x86/asm_defns.h
> > +++ b/xen/include/asm-x86/asm_defns.h
> > @@ -13,6 +13,8 @@
> >
> >  #ifndef __ASSEMBLY__
> >  void ret_from_intr(void);
> > +typedef void (*interrupt_stub_t)(void);
> > +extern const interrupt_stub_t interrupt[];
> 
> I don't see the point of the typedef, and I don't think the declaration
> is well placed in this header.
> 
> Jan