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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 26/01/18 19:02, Roger Pau Monné wrote:
> On Fri, Jan 26, 2018 at 05:46:23PM +0000, Andrew Cooper wrote:
>> On 26/01/18 17:37, Roger Pau Monne wrote:
>>> 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?
> Done for both of the above.
>
>> 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.
> I would rather do that in a separate change, since this is
> non-functional. Do you think that's fine?

Hmm ok, so long as the (TRAP_nr - 1) changes to ARRAY_SIZE() - 1 to make
one of the failure modes failsafe.

~Andrew

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