| Message ID | 20200217184324.73762-4-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86: fixes/improvements for scratch cpumask | expand |
Hi Roger, On 17/02/2020 18:43, Roger Pau Monne wrote: > Add helpers to track when executing in #MC context. This is modeled > after the in_irq helpers. > > Note that there are no users of in_mc() introduced by the change, > further users will be added by followup changes. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > --- > xen/arch/x86/cpu/mcheck/mce.c | 2 ++ > xen/include/asm-x86/hardirq.h | 5 +++++ > xen/include/xen/irq_cpustat.h | 1 + > 3 files changed, 8 insertions(+) > > diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c > index d61e582af3..93ed5752ac 100644 > --- a/xen/arch/x86/cpu/mcheck/mce.c > +++ b/xen/arch/x86/cpu/mcheck/mce.c > @@ -93,7 +93,9 @@ void x86_mce_vector_register(x86_mce_vector_t hdlr) > > void do_machine_check(const struct cpu_user_regs *regs) > { > + mc_enter(); > _machine_check_vector(regs); > + mc_exit(); > } > > /* > diff --git a/xen/include/asm-x86/hardirq.h b/xen/include/asm-x86/hardirq.h > index 34e1b49260..af3eab6a4d 100644 > --- a/xen/include/asm-x86/hardirq.h > +++ b/xen/include/asm-x86/hardirq.h > @@ -8,6 +8,7 @@ typedef struct { > unsigned int __softirq_pending; > unsigned int __local_irq_count; > unsigned int __nmi_count; > + unsigned int mc_count; > bool_t __mwait_wakeup; > } __cacheline_aligned irq_cpustat_t; > > @@ -18,6 +19,10 @@ typedef struct { > #define irq_enter() (local_irq_count(smp_processor_id())++) > #define irq_exit() (local_irq_count(smp_processor_id())--) > > +#define in_mc() (mc_count(smp_processor_id()) != 0) > +#define mc_enter() (mc_count(smp_processor_id())++) > +#define mc_exit() (mc_count(smp_processor_id())--) > + > void ack_bad_irq(unsigned int irq); > > extern void apic_intr_init(void); > diff --git a/xen/include/xen/irq_cpustat.h b/xen/include/xen/irq_cpustat.h > index 73629f6ec8..12b932fc39 100644 > --- a/xen/include/xen/irq_cpustat.h > +++ b/xen/include/xen/irq_cpustat.h > @@ -26,5 +26,6 @@ extern irq_cpustat_t irq_stat[]; > #define local_irq_count(cpu) __IRQ_STAT((cpu), __local_irq_count) > #define nmi_count(cpu) __IRQ_STAT((cpu), __nmi_count) > #define mwait_wakeup(cpu) __IRQ_STAT((cpu), __mwait_wakeup) > +#define mc_count(cpu) __IRQ_STAT((cpu), mc_count) The header is only meant to contain arch-independent IRQ stats (see comment a few lines above). This is unlikely to be used on Arm, so can you move this into an x86 specific header? Cheers,
On Mon, Feb 17, 2020 at 07:29:29PM +0000, Julien Grall wrote: > Hi Roger, > > On 17/02/2020 18:43, Roger Pau Monne wrote: > > Add helpers to track when executing in #MC context. This is modeled > > after the in_irq helpers. > > > > Note that there are no users of in_mc() introduced by the change, > > further users will be added by followup changes. > > > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > > --- > > xen/arch/x86/cpu/mcheck/mce.c | 2 ++ > > xen/include/asm-x86/hardirq.h | 5 +++++ > > xen/include/xen/irq_cpustat.h | 1 + > > 3 files changed, 8 insertions(+) > > > > diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c > > index d61e582af3..93ed5752ac 100644 > > --- a/xen/arch/x86/cpu/mcheck/mce.c > > +++ b/xen/arch/x86/cpu/mcheck/mce.c > > @@ -93,7 +93,9 @@ void x86_mce_vector_register(x86_mce_vector_t hdlr) > > void do_machine_check(const struct cpu_user_regs *regs) > > { > > + mc_enter(); > > _machine_check_vector(regs); > > + mc_exit(); > > } > > /* > > diff --git a/xen/include/asm-x86/hardirq.h b/xen/include/asm-x86/hardirq.h > > index 34e1b49260..af3eab6a4d 100644 > > --- a/xen/include/asm-x86/hardirq.h > > +++ b/xen/include/asm-x86/hardirq.h > > @@ -8,6 +8,7 @@ typedef struct { > > unsigned int __softirq_pending; > > unsigned int __local_irq_count; > > unsigned int __nmi_count; > > + unsigned int mc_count; > > bool_t __mwait_wakeup; > > } __cacheline_aligned irq_cpustat_t; > > @@ -18,6 +19,10 @@ typedef struct { > > #define irq_enter() (local_irq_count(smp_processor_id())++) > > #define irq_exit() (local_irq_count(smp_processor_id())--) > > +#define in_mc() (mc_count(smp_processor_id()) != 0) > > +#define mc_enter() (mc_count(smp_processor_id())++) > > +#define mc_exit() (mc_count(smp_processor_id())--) > > + > > void ack_bad_irq(unsigned int irq); > > extern void apic_intr_init(void); > > diff --git a/xen/include/xen/irq_cpustat.h b/xen/include/xen/irq_cpustat.h > > index 73629f6ec8..12b932fc39 100644 > > --- a/xen/include/xen/irq_cpustat.h > > +++ b/xen/include/xen/irq_cpustat.h > > @@ -26,5 +26,6 @@ extern irq_cpustat_t irq_stat[]; > > #define local_irq_count(cpu) __IRQ_STAT((cpu), __local_irq_count) > > #define nmi_count(cpu) __IRQ_STAT((cpu), __nmi_count) > > #define mwait_wakeup(cpu) __IRQ_STAT((cpu), __mwait_wakeup) > > +#define mc_count(cpu) __IRQ_STAT((cpu), mc_count) > > The header is only meant to contain arch-independent IRQ stats (see comment > a few lines above). This is unlikely to be used on Arm, so can you move this > into an x86 specific header? Uh, sure. Sorry for not realizing this. Thanks, Roger.
On Mon, Feb 17, 2020 at 07:29:29PM +0000, Julien Grall wrote: > Hi Roger, > > On 17/02/2020 18:43, Roger Pau Monne wrote: > > Add helpers to track when executing in #MC context. This is modeled > > after the in_irq helpers. > > > > Note that there are no users of in_mc() introduced by the change, > > further users will be added by followup changes. > > > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > > --- > > xen/arch/x86/cpu/mcheck/mce.c | 2 ++ > > xen/include/asm-x86/hardirq.h | 5 +++++ > > xen/include/xen/irq_cpustat.h | 1 + > > 3 files changed, 8 insertions(+) > > > > diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c > > index d61e582af3..93ed5752ac 100644 > > --- a/xen/arch/x86/cpu/mcheck/mce.c > > +++ b/xen/arch/x86/cpu/mcheck/mce.c > > @@ -93,7 +93,9 @@ void x86_mce_vector_register(x86_mce_vector_t hdlr) > > void do_machine_check(const struct cpu_user_regs *regs) > > { > > + mc_enter(); > > _machine_check_vector(regs); > > + mc_exit(); > > } > > /* > > diff --git a/xen/include/asm-x86/hardirq.h b/xen/include/asm-x86/hardirq.h > > index 34e1b49260..af3eab6a4d 100644 > > --- a/xen/include/asm-x86/hardirq.h > > +++ b/xen/include/asm-x86/hardirq.h > > @@ -8,6 +8,7 @@ typedef struct { > > unsigned int __softirq_pending; > > unsigned int __local_irq_count; > > unsigned int __nmi_count; > > + unsigned int mc_count; > > bool_t __mwait_wakeup; > > } __cacheline_aligned irq_cpustat_t; > > @@ -18,6 +19,10 @@ typedef struct { > > #define irq_enter() (local_irq_count(smp_processor_id())++) > > #define irq_exit() (local_irq_count(smp_processor_id())--) > > +#define in_mc() (mc_count(smp_processor_id()) != 0) > > +#define mc_enter() (mc_count(smp_processor_id())++) > > +#define mc_exit() (mc_count(smp_processor_id())--) > > + > > void ack_bad_irq(unsigned int irq); > > extern void apic_intr_init(void); > > diff --git a/xen/include/xen/irq_cpustat.h b/xen/include/xen/irq_cpustat.h > > index 73629f6ec8..12b932fc39 100644 > > --- a/xen/include/xen/irq_cpustat.h > > +++ b/xen/include/xen/irq_cpustat.h > > @@ -26,5 +26,6 @@ extern irq_cpustat_t irq_stat[]; > > #define local_irq_count(cpu) __IRQ_STAT((cpu), __local_irq_count) > > #define nmi_count(cpu) __IRQ_STAT((cpu), __nmi_count) > > #define mwait_wakeup(cpu) __IRQ_STAT((cpu), __mwait_wakeup) > > +#define mc_count(cpu) __IRQ_STAT((cpu), mc_count) > > The header is only meant to contain arch-independent IRQ stats (see comment > a few lines above). This is unlikely to be used on Arm, so can you move this > into an x86 specific header? Now that I look at it, there's also nmi_count and mwait_wakeup defined in irq_cpustat.h which won't build if used on Arm, since the fields don't exist in the Arm version of irq_cpustat_t. Roger.
Hi Roger, On 18/02/2020 09:48, Roger Pau Monné wrote: > On Mon, Feb 17, 2020 at 07:29:29PM +0000, Julien Grall wrote: >> Hi Roger, >> >> On 17/02/2020 18:43, Roger Pau Monne wrote: >>> Add helpers to track when executing in #MC context. This is modeled >>> after the in_irq helpers. >>> >>> Note that there are no users of in_mc() introduced by the change, >>> further users will be added by followup changes. >>> >>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> >>> --- >>> xen/arch/x86/cpu/mcheck/mce.c | 2 ++ >>> xen/include/asm-x86/hardirq.h | 5 +++++ >>> xen/include/xen/irq_cpustat.h | 1 + >>> 3 files changed, 8 insertions(+) >>> >>> diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c >>> index d61e582af3..93ed5752ac 100644 >>> --- a/xen/arch/x86/cpu/mcheck/mce.c >>> +++ b/xen/arch/x86/cpu/mcheck/mce.c >>> @@ -93,7 +93,9 @@ void x86_mce_vector_register(x86_mce_vector_t hdlr) >>> void do_machine_check(const struct cpu_user_regs *regs) >>> { >>> + mc_enter(); >>> _machine_check_vector(regs); >>> + mc_exit(); >>> } >>> /* >>> diff --git a/xen/include/asm-x86/hardirq.h b/xen/include/asm-x86/hardirq.h >>> index 34e1b49260..af3eab6a4d 100644 >>> --- a/xen/include/asm-x86/hardirq.h >>> +++ b/xen/include/asm-x86/hardirq.h >>> @@ -8,6 +8,7 @@ typedef struct { >>> unsigned int __softirq_pending; >>> unsigned int __local_irq_count; >>> unsigned int __nmi_count; >>> + unsigned int mc_count; >>> bool_t __mwait_wakeup; >>> } __cacheline_aligned irq_cpustat_t; >>> @@ -18,6 +19,10 @@ typedef struct { >>> #define irq_enter() (local_irq_count(smp_processor_id())++) >>> #define irq_exit() (local_irq_count(smp_processor_id())--) >>> +#define in_mc() (mc_count(smp_processor_id()) != 0) >>> +#define mc_enter() (mc_count(smp_processor_id())++) >>> +#define mc_exit() (mc_count(smp_processor_id())--) >>> + >>> void ack_bad_irq(unsigned int irq); >>> extern void apic_intr_init(void); >>> diff --git a/xen/include/xen/irq_cpustat.h b/xen/include/xen/irq_cpustat.h >>> index 73629f6ec8..12b932fc39 100644 >>> --- a/xen/include/xen/irq_cpustat.h >>> +++ b/xen/include/xen/irq_cpustat.h >>> @@ -26,5 +26,6 @@ extern irq_cpustat_t irq_stat[]; >>> #define local_irq_count(cpu) __IRQ_STAT((cpu), __local_irq_count) >>> #define nmi_count(cpu) __IRQ_STAT((cpu), __nmi_count) >>> #define mwait_wakeup(cpu) __IRQ_STAT((cpu), __mwait_wakeup) >>> +#define mc_count(cpu) __IRQ_STAT((cpu), mc_count) >> >> The header is only meant to contain arch-independent IRQ stats (see comment >> a few lines above). This is unlikely to be used on Arm, so can you move this >> into an x86 specific header? > > Now that I look at it, there's also nmi_count and mwait_wakeup defined > in irq_cpustat.h which won't build if used on Arm, since the fields > don't exist in the Arm version of irq_cpustat_t. I would prefer if we don't introduce more cases in xen/irq_cpustat.h. It would be good to remove nmi_count() and mwait_wakeup() from the common header, but that's a separate cleanup. Cheers,
diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c index d61e582af3..93ed5752ac 100644 --- a/xen/arch/x86/cpu/mcheck/mce.c +++ b/xen/arch/x86/cpu/mcheck/mce.c @@ -93,7 +93,9 @@ void x86_mce_vector_register(x86_mce_vector_t hdlr) void do_machine_check(const struct cpu_user_regs *regs) { + mc_enter(); _machine_check_vector(regs); + mc_exit(); } /* diff --git a/xen/include/asm-x86/hardirq.h b/xen/include/asm-x86/hardirq.h index 34e1b49260..af3eab6a4d 100644 --- a/xen/include/asm-x86/hardirq.h +++ b/xen/include/asm-x86/hardirq.h @@ -8,6 +8,7 @@ typedef struct { unsigned int __softirq_pending; unsigned int __local_irq_count; unsigned int __nmi_count; + unsigned int mc_count; bool_t __mwait_wakeup; } __cacheline_aligned irq_cpustat_t; @@ -18,6 +19,10 @@ typedef struct { #define irq_enter() (local_irq_count(smp_processor_id())++) #define irq_exit() (local_irq_count(smp_processor_id())--) +#define in_mc() (mc_count(smp_processor_id()) != 0) +#define mc_enter() (mc_count(smp_processor_id())++) +#define mc_exit() (mc_count(smp_processor_id())--) + void ack_bad_irq(unsigned int irq); extern void apic_intr_init(void); diff --git a/xen/include/xen/irq_cpustat.h b/xen/include/xen/irq_cpustat.h index 73629f6ec8..12b932fc39 100644 --- a/xen/include/xen/irq_cpustat.h +++ b/xen/include/xen/irq_cpustat.h @@ -26,5 +26,6 @@ extern irq_cpustat_t irq_stat[]; #define local_irq_count(cpu) __IRQ_STAT((cpu), __local_irq_count) #define nmi_count(cpu) __IRQ_STAT((cpu), __nmi_count) #define mwait_wakeup(cpu) __IRQ_STAT((cpu), __mwait_wakeup) +#define mc_count(cpu) __IRQ_STAT((cpu), mc_count) #endif /* __irq_cpustat_h */
Add helpers to track when executing in #MC context. This is modeled after the in_irq helpers. Note that there are no users of in_mc() introduced by the change, further users will be added by followup changes. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- xen/arch/x86/cpu/mcheck/mce.c | 2 ++ xen/include/asm-x86/hardirq.h | 5 +++++ xen/include/xen/irq_cpustat.h | 1 + 3 files changed, 8 insertions(+)