Re: [Xen-devel] [PATCH] x86: move declaration of the exception_table to C

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 26/01/18 17:37, Roger Pau Monne wrote:
> This makes the code cleaner because there's no need to declare the
> exception_table in assembly, and also fixes the following error when
> using clang's integrated assembler:
>
> entry.S:834:15: error: unexpected token in '.rept' directive
>         .rept 32 - ((. - exception_table) / 8)
>               ^
> entry.S:836:14: error: unmatched '.endr' directive
>         .endr
>              ^
>
> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Ooh nice.  One less item on my todo list.  However...

> ---
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
>  xen/arch/x86/traps.c            | 19 +++++++++++++++++++
>  xen/arch/x86/x86_64/entry.S     | 30 ------------------------------
>  xen/include/asm-x86/processor.h |  1 +
>  3 files changed, 20 insertions(+), 30 deletions(-)
>
> diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
> index a3e8f0c9b9..be95baafe1 100644
> --- a/xen/arch/x86/traps.c
> +++ b/xen/arch/x86/traps.c
> @@ -119,6 +119,25 @@ boolean_param("ler", opt_ler);
>  #define stack_words_per_line 4
>  #define ESP_BEFORE_EXCEPTION(regs) ((unsigned long *)regs->rsp)
>  
> +void (* const exception_table[TRAP_nr])(struct cpu_user_regs *regs) = {
> +    [TRAP_divide_error] = do_trap,
> +    [TRAP_debug] = do_debug,
> +    [TRAP_nmi] = (void (*)(struct cpu_user_regs *))do_nmi,

Cast to (void *) is probably better, and certainly shorter.

> +    [TRAP_int3] = do_int3,
> +    [TRAP_overflow ... TRAP_bounds] = do_trap,
> +    [TRAP_invalid_op] = do_invalid_op,
> +    [TRAP_no_device] = do_device_not_available,
> +    [TRAP_double_fault ... TRAP_copro_seg] = do_reserved_trap,
> +    [TRAP_invalid_tss ... TRAP_stack_error] = do_trap,
> +    [TRAP_gp_fault] = do_general_protection,
> +    [TRAP_page_fault] = do_page_fault,
> +    [TRAP_spurious_int] = do_reserved_trap,
> +    [TRAP_copro_error ... TRAP_alignment_check] = do_trap,
> +    [TRAP_machine_check] = (void (*)(struct cpu_user_regs 
> *))do_machine_check,
> +    [TRAP_simd_error] = do_trap,
> +    [TRAP_virtualisation ... (TRAP_nr - 1)] = do_reserved_trap,

Can we see about vertically aligning the assignments for neatness?

Also, I'd drop the do_reserve_trap and have the lookup check for a NULL
pointer.  This is what we do on the hypercall path and is far less error
prone to changes in this table.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel