[v2,3/6] x86: track when in #MC context
diff mbox series

Message ID 20200217184324.73762-4-roger.pau@citrix.com
State New
Headers show
Series
  • x86: fixes/improvements for scratch cpumask
Related show

Commit Message

Roger Pau Monné Feb. 17, 2020, 6:43 p.m. UTC
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(+)

Comments

Julien Grall Feb. 17, 2020, 7:29 p.m. UTC | #1
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,
Roger Pau Monné Feb. 18, 2020, 9:45 a.m. UTC | #2
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.
Roger Pau Monné Feb. 18, 2020, 9:48 a.m. UTC | #3
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.
Julien Grall Feb. 18, 2020, 11:20 a.m. UTC | #4
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,

Patch
diff mbox series

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 */