Re: [Xen-devel] [PATCH] x86: move interrupt entry stubs to entry.S

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.05.14 at 07:34, <feng.wu@xxxxxxxxx> 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.

Fundamentally fine, but a couple of mechanical adjustments are still
needed:

> --- a/xen/arch/x86/x86_64/entry.S
> +++ b/xen/arch/x86/x86_64/entry.S
> @@ -469,6 +469,26 @@ ENTRY(dom_crash_sync_extable)
>          xorl  %edi,%edi
>          jmp   asm_domain_crash_synchronous /* Does not return */
>  
> +        .section .init.data,"a"

Especially with you using "a", this ought to be .init.rodata.

> +ENTRY(interrupt)
> +        .section .text
> +ENTRY(irq_entries_start)
> +        vector=FIRST_DYNAMIC_VECTOR
> +        .rept NR_VECTORS-FIRST_DYNAMIC_VECTOR
> +        ALIGN
> +        .if vector < NR_VECTORS

With the .rept operand above, what is this .if good for?

> +1:      pushq $0x0
> +        movl $vector,0x4(%rsp)
> +        jmp common_interrupt
> +        .previous
> +        .quad 1b
> +        .section .text
> +        vector=vector+1
> +        .endif
> +        .endr
> +        .previous
> +        .previous

I don't think a double previous really does what you expect. Please
use .pushsection/.popsection instead for nested section handling.

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

With the above this is then also missing a const qualifier.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel