From: Ingo Molnar <mingo@kernel.org>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Stephen Rothwell <sfr@canb.auug.org.au>,
Juergen Gross <jgross@suse.com>,
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
Stefano Stabellini <sstabellini@kernel.org>,
Boris Ostrovsky <boris.ostrovsky@oracle.com>,
Xen Devel <Xen-devel@lists.xensource.com>,
Ingo Molnar <mingo@elte.hu>, "H. Peter Anvin" <hpa@zytor.com>,
Peter Zijlstra <peterz@infradead.org>,
Linux-Next Mailing List <linux-next@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: linux-next: manual merge of the xen-tip tree with the tip tree
Date: Thu, 31 Aug 2017 11:16:56 +0200 [thread overview]
Message-ID: <20170831091656.dmh7lgb7w4cfws6j@gmail.com> (raw)
In-Reply-To: <alpine.DEB.2.20.1708311057260.1874@nanos>
* Thomas Gleixner <tglx@linutronix.de> wrote:
> --- a/arch/x86/xen/enlighten_pv.c
> +++ b/arch/x86/xen/enlighten_pv.c
> @@ -586,6 +586,68 @@ static void xen_write_ldt_entry(struct d
> preempt_enable();
> }
>
> +#ifdef CONFIG_X86_64
> +static struct {
> + void (*orig)(void);
> + void (*xen)(void);
> + bool ist_okay;
> +} trap_array[] = {
> + { debug, xen_xendebug, true },
> + { int3, xen_xenint3, true },
> + { double_fault, xen_double_fault, true },
> +#ifdef CONFIG_X86_MCE
> + { machine_check, xen_machine_check, true },
> +#endif
> + { nmi, xen_nmi, true },
> + { overflow, xen_overflow, false },
> +#ifdef CONFIG_IA32_EMULATION
> + { entry_INT80_compat, xen_entry_INT80_compat, false },
> +#endif
> + { page_fault, xen_page_fault, false },
> + { divide_error, xen_divide_error, false },
> + { bounds, xen_bounds, false },
> + { invalid_op, xen_invalid_op, false },
> + { device_not_available, xen_device_not_available, false },
> + { coprocessor_segment_overrun, xen_coprocessor_segment_overrun, false },
> + { invalid_TSS, xen_invalid_TSS, false },
> + { segment_not_present, xen_segment_not_present, false },
> + { stack_segment, xen_stack_segment, false },
> + { general_protection, xen_general_protection, false },
> + { spurious_interrupt_bug, xen_spurious_interrupt_bug, false },
> + { coprocessor_error, xen_coprocessor_error, false },
> + { alignment_check, xen_alignment_check, false },
> + { simd_coprocessor_error, xen_simd_coprocessor_error, false },
> +#ifdef CONFIG_TRACING
> + { trace_page_fault, xen_trace_page_fault, false },
> +#endif
> +};
Low prio nitpicking, could we please write such table based initializers in a
vertically organized, tabular fashion:
> + { debug, xen_xendebug, true },
> + { int3, xen_xenint3, true },
> + { double_fault, xen_double_fault, true },
> +#ifdef CONFIG_X86_MCE
> + { machine_check, xen_machine_check, true },
> +#endif
> + { nmi, xen_nmi, true },
> + { overflow, xen_overflow, false },
> +#ifdef CONFIG_IA32_EMULATION
> + { entry_INT80_compat, xen_entry_INT80_compat, false },
> +#endif
> + { page_fault, xen_page_fault, false },
> + { divide_error, xen_divide_error, false },
> + { bounds, xen_bounds, false },
> + { invalid_op, xen_invalid_op, false },
> + { device_not_available, xen_device_not_available, false },
> + { coprocessor_segment_overrun, xen_coprocessor_segment_overrun, false },
> + { invalid_TSS, xen_invalid_TSS, false },
> + { segment_not_present, xen_segment_not_present, false },
> + { stack_segment, xen_stack_segment, false },
> + { general_protection, xen_general_protection, false },
> + { spurious_interrupt_bug, xen_spurious_interrupt_bug, false },
> + { coprocessor_error, xen_coprocessor_error, false },
> + { alignment_check, xen_alignment_check, false },
> + { simd_coprocessor_error, xen_simd_coprocessor_error, false },
> +#ifdef CONFIG_TRACING
> + { trace_page_fault, xen_trace_page_fault, false },
> +#endif
... as to me such a table is 100 times more readable - YMMV.
(If checkpatch whinges about col80 then ignore it.)
> +
> +static bool get_trap_addr(unsigned long *addr, unsigned int ist)
> +{
> + unsigned int nr;
> + bool ist_okay = false;
> +
> + /*
> + * Replace trap handler addresses by Xen specific ones.
> + * Check for known traps using IST and whitelist them.
> + * The debugger ones are the only ones we care about.
> + * Xen will handle faults like double_fault, * so we should never see
> + * them. Warn if there's an unexpected IST-using fault handler.
> + */
> + for (nr = 0; nr < ARRAY_SIZE(trap_array); nr++)
> + if (*addr == (unsigned long)trap_array[nr].orig) {
> + *addr = (unsigned long)trap_array[nr].xen;
> + ist_okay = trap_array[nr].ist_okay;
> + break;
> + }
And here I think we could do a more readable variant:
static bool get_trap_addr(void **addr, unsigned int ist)
...
struct trap_array_entry *entry = trap_array + nr;
if (*addr == entry->orig) {
*addr = entry->xen;
ist_okay = entry->ist_okay;
break;
}
I believe 'void **' is the natural type that avoids the type casts, and the
'trap_array_entry' name has to be defined in the array definition further above.
Totally untested though.
Thanks,
Ingo
next prev parent reply other threads:[~2017-08-31 9:16 UTC|newest]
Thread overview: 55+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-08-31 4:26 linux-next: manual merge of the xen-tip tree with the tip tree Stephen Rothwell
2017-08-31 8:10 ` Thomas Gleixner
2017-08-31 9:00 ` Thomas Gleixner
2017-08-31 9:16 ` Ingo Molnar [this message]
2017-08-31 12:36 ` Joe Perches
2017-08-31 10:11 ` Juergen Gross
2017-08-31 12:00 ` Thomas Gleixner
2017-08-31 14:01 ` Boris Ostrovsky
2017-08-31 14:03 ` Juergen Gross
2017-08-31 18:30 ` Thomas Gleixner
-- strict thread matches above, loose matches on Subject: below --
2021-10-28 6:16 Stephen Rothwell
2017-08-31 4:37 Stephen Rothwell
2017-08-28 5:20 Stephen Rothwell
2017-08-17 4:03 Stephen Rothwell
2017-06-28 4:21 Stephen Rothwell
2017-04-12 4:30 Stephen Rothwell
2017-04-26 4:57 ` Stephen Rothwell
2017-04-12 4:20 Stephen Rothwell
2017-03-29 3:36 Stephen Rothwell
2017-03-29 3:37 ` Stephen Rothwell
2017-03-29 3:35 Stephen Rothwell
2017-03-29 8:37 ` Juergen Gross
2017-03-29 8:59 ` Ingo Molnar
2017-03-29 9:28 ` Juergen Gross
2017-03-29 10:06 ` Vitaly Kuznetsov
2017-04-03 18:03 ` Juergen Gross
2017-04-04 16:04 ` Vitaly Kuznetsov
2017-04-04 16:09 ` Juergen Gross
2017-04-05 6:25 ` Juergen Gross
2017-04-03 14:38 ` Ingo Molnar
2017-04-03 14:55 ` Juergen Gross
2017-03-29 9:54 ` Juergen Gross
2017-03-29 10:41 ` Ingo Molnar
2017-03-29 11:13 ` Stephen Rothwell
2016-10-03 1:29 Stephen Rothwell
2016-07-26 4:02 Stephen Rothwell
2016-07-26 4:01 Stephen Rothwell
2016-07-26 13:58 ` Boris Ostrovsky
2016-07-18 6:17 Stephen Rothwell
2016-05-02 4:51 Stephen Rothwell
2016-04-29 4:20 Stephen Rothwell
2015-08-12 5:09 Stephen Rothwell
2015-08-12 13:27 ` Boris Ostrovsky
2015-08-12 17:21 ` Peter Zijlstra
2015-08-12 17:38 ` Peter Zijlstra
2015-08-12 17:46 ` Peter Zijlstra
2015-08-12 18:17 ` Boris Ostrovsky
2015-08-12 18:26 ` Andy Lutomirski
2015-08-12 18:30 ` Boris Ostrovsky
2015-08-12 18:26 ` H. Peter Anvin
2015-08-12 18:36 ` Peter Zijlstra
2015-08-12 18:44 ` Boris Ostrovsky
2015-08-12 19:04 ` Peter Zijlstra
2015-08-13 6:45 ` Ingo Molnar
2014-03-20 4:15 Stephen Rothwell
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170831091656.dmh7lgb7w4cfws6j@gmail.com \
--to=mingo@kernel.org \
--cc=Xen-devel@lists.xensource.com \
--cc=boris.ostrovsky@oracle.com \
--cc=hpa@zytor.com \
--cc=jgross@suse.com \
--cc=konrad.wilk@oracle.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-next@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=peterz@infradead.org \
--cc=sfr@canb.auug.org.au \
--cc=sstabellini@kernel.org \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).