* [PATCH] x86: move declaration of the exception_table to C
@ 2018-01-26 17:37 Roger Pau Monne
2018-01-26 17:46 ` Andrew Cooper
0 siblings, 1 reply; 4+ messages in thread
From: Roger Pau Monne @ 2018-01-26 17:37 UTC (permalink / raw)
To: xen-devel; +Cc: Andrew Cooper, Jan Beulich, Roger Pau Monne
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@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
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,
+ [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,
+};
+
static void show_code(const struct cpu_user_regs *regs)
{
unsigned char insns_before[8] = {}, insns_after[16] = {};
diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
index 710c0616ba..af703f6c06 100644
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -806,36 +806,6 @@ ENTRY(enable_nmis)
GLOBAL(trap_nop)
iretq
-
-
-.section .rodata, "a", @progbits
-
-ENTRY(exception_table)
- .quad do_trap
- .quad do_debug
- .quad do_nmi
- .quad do_int3
- .quad do_trap
- .quad do_trap
- .quad do_invalid_op
- .quad do_device_not_available
- .quad do_reserved_trap /* double_fault - has its own entry. */
- .quad do_reserved_trap /* coproc_seg_overrun - Intel 387 only. */
- .quad do_trap
- .quad do_trap
- .quad do_trap
- .quad do_general_protection
- .quad do_page_fault
- .quad do_reserved_trap /* Default PIC spurious irq - architecturally reserved. */
- .quad do_trap
- .quad do_trap
- .quad do_machine_check
- .quad do_trap
- .rept TRAP_nr - ((. - exception_table) / 8)
- .quad do_reserved_trap /* Architecturally reserved exceptions. */
- .endr
- .size exception_table, . - exception_table
-
/* Table of automatically generated entry points. One per vector. */
.section .init.rodata, "a", @progbits
GLOBAL(autogen_entrypoints)
diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h
index e8c2f02e99..472e138f24 100644
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -501,6 +501,7 @@ DECLARE_TRAP_HANDLER(entry_int82);
void trap_nop(void);
void enable_nmis(void);
+void do_trap(struct cpu_user_regs *regs);
void do_reserved_trap(struct cpu_user_regs *regs);
void sysenter_entry(void);
--
2.15.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] x86: move declaration of the exception_table to C
2018-01-26 17:37 [PATCH] x86: move declaration of the exception_table to C Roger Pau Monne
@ 2018-01-26 17:46 ` Andrew Cooper
2018-01-26 19:02 ` Roger Pau Monné
0 siblings, 1 reply; 4+ messages in thread
From: Andrew Cooper @ 2018-01-26 17:46 UTC (permalink / raw)
To: Roger Pau Monne, xen-devel; +Cc: Jan Beulich
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@citrix.com>
Ooh nice. One less item on my todo list. However...
> ---
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> 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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] x86: move declaration of the exception_table to C
2018-01-26 17:46 ` Andrew Cooper
@ 2018-01-26 19:02 ` Roger Pau Monné
2018-01-26 19:05 ` Andrew Cooper
0 siblings, 1 reply; 4+ messages in thread
From: Roger Pau Monné @ 2018-01-26 19:02 UTC (permalink / raw)
To: Andrew Cooper; +Cc: xen-devel, Jan Beulich
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?
Thanks, Roger.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] x86: move declaration of the exception_table to C
2018-01-26 19:02 ` Roger Pau Monné
@ 2018-01-26 19:05 ` Andrew Cooper
0 siblings, 0 replies; 4+ messages in thread
From: Andrew Cooper @ 2018-01-26 19:05 UTC (permalink / raw)
To: Roger Pau Monné; +Cc: xen-devel, Jan Beulich
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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-01-26 19:05 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-01-26 17:37 [PATCH] x86: move declaration of the exception_table to C Roger Pau Monne
2018-01-26 17:46 ` Andrew Cooper
2018-01-26 19:02 ` Roger Pau Monné
2018-01-26 19:05 ` Andrew Cooper
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).