[v5,08/12] powerpc: Add support to mask perf interrupts and replay them

Two new bit mask field "IRQ_DISABLE_MASK_PMU" is introduced to support
the masking of PMI and "IRQ_DISABLE_MASK_ALL" to aid interrupt masking checking.

Couple of new irq #defs "PACA_IRQ_PMI" and "SOFTEN_VALUE_0xf0*" added to
use in the exception code to check for PMI interrupts.

In the masked_interrupt handler, for PMIs we reset the MSR[EE]
and return. In the __check_irq_replay(), replay the PMI interrupt
by calling performance_monitor_common handler.

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
Signed-off-by: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/exception-64s.h |  5 +++++
 arch/powerpc/include/asm/hw_irq.h        |  5 ++++-
 arch/powerpc/kernel/entry_64.S           |  5 +++++
 arch/powerpc/kernel/exceptions-64s.S     |  6 ++++--
 arch/powerpc/kernel/irq.c                | 25 ++++++++++++++++++++++++-
 5 files changed, 42 insertions(+), 4 deletions(-)

On Wed,  4 Jan 2017 17:19:46 +0530
Madhavan Srinivasan <maddy@linux.vnet.ibm.com> wrote:

> @@ -134,7 +137,7 @@ static inline bool arch_irqs_disabled(void)
>  	_was_enabled = local_paca->soft_enabled;	\
>  	local_paca->soft_enabled = IRQ_DISABLE_MASK_LINUX;\
>  	local_paca->irq_happened |= PACA_IRQ_HARD_DIS;	\
> -	if (!(_was_enabled & IRQ_DISABLE_MASK_LINUX))	\
> +	if (!(_was_enabled & IRQ_DISABLE_MASK_ALL))	\
>  		trace_hardirqs_off();			\
>  } while(0)

Hang on, maybe there's some confusion about this. trace_hardirqs_off() is
for Linux irqs (i.e., local_irq_disable()), so that should continue to
test just the LINUX mask I think. Otherwise this

    powerpc_local_pmu_disable();
    hard_irq_disable();

Will miss calling trace_hardirqs_off(). You don't have a function that
disables PMU irqs without Linux irqs, but one might exist.

What I was concerned about is actually setting the disable mask to ALL

  	local_paca->soft_enabled = IRQ_DISABLE_MASK_ALL;

No? Otherwise if you did

    powerpc_local_irq_pmu_disable();
    hard_irq_disable();

Then you would lose the PMU bit out of the mask.

Thanks,
Nick

On Wednesday 04 January 2017 06:08 PM, Nicholas Piggin wrote:
> On Wed,  4 Jan 2017 17:19:46 +0530
> Madhavan Srinivasan <maddy@linux.vnet.ibm.com> wrote:
>
>> @@ -134,7 +137,7 @@ static inline bool arch_irqs_disabled(void)
>>   	_was_enabled = local_paca->soft_enabled;	\
>>   	local_paca->soft_enabled = IRQ_DISABLE_MASK_LINUX;\
>>   	local_paca->irq_happened |= PACA_IRQ_HARD_DIS;	\
>> -	if (!(_was_enabled & IRQ_DISABLE_MASK_LINUX))	\
>> +	if (!(_was_enabled & IRQ_DISABLE_MASK_ALL))	\
>>   		trace_hardirqs_off();			\
>>   } while(0)
> Hang on, maybe there's some confusion about this. trace_hardirqs_off() is
> for Linux irqs (i.e., local_irq_disable()), so that should continue to
> test just the LINUX mask I think. Otherwise this
>
>      powerpc_local_pmu_disable();
>      hard_irq_disable();

Currently we set both bits for pmu soft disable

                 flags = 
soft_disabled_mask_or_return(IRQ_DISABLE_MASK_LINUX | \
IRQ_DISABLE_MASK_PMU);                  \

So yes in the above seq, we will miss the pmu bit. But since 
trace_hardirqs_off()
is for _LINUX, instead will it not be safer to OR it?

         local_paca->soft_disabled_mask |= IRQ_DISABLE_MASK_LINUX;\

Maddy

> Will miss calling trace_hardirqs_off(). You don't have a function that
> disables PMU irqs without Linux irqs, but one might exist.
>
> What I was concerned about is actually setting the disable mask to ALL
>
>    	local_paca->soft_enabled = IRQ_DISABLE_MASK_ALL;
>
> No? Otherwise if you did
>
>      powerpc_local_irq_pmu_disable();
>      hard_irq_disable();
>
> Then you would lose the PMU bit out of the mask.
>
> Thanks,
> Nick
>

On Wed, 4 Jan 2017 22:21:05 +0530
Madhavan Srinivasan <maddy@linux.vnet.ibm.com> wrote:

> On Wednesday 04 January 2017 06:08 PM, Nicholas Piggin wrote:
> > On Wed,  4 Jan 2017 17:19:46 +0530
> > Madhavan Srinivasan <maddy@linux.vnet.ibm.com> wrote:
> >  
> >> @@ -134,7 +137,7 @@ static inline bool arch_irqs_disabled(void)
> >>   	_was_enabled = local_paca->soft_enabled;	\
> >>   	local_paca->soft_enabled = IRQ_DISABLE_MASK_LINUX;\
> >>   	local_paca->irq_happened |= PACA_IRQ_HARD_DIS;	\
> >> -	if (!(_was_enabled & IRQ_DISABLE_MASK_LINUX))	\
> >> +	if (!(_was_enabled & IRQ_DISABLE_MASK_ALL))	\
> >>   		trace_hardirqs_off();			\
> >>   } while(0)  
> > Hang on, maybe there's some confusion about this. trace_hardirqs_off() is
> > for Linux irqs (i.e., local_irq_disable()), so that should continue to
> > test just the LINUX mask I think. Otherwise this
> >
> >      powerpc_local_pmu_disable();
> >      hard_irq_disable();  
> 
> Currently we set both bits for pmu soft disable
> 
>                  flags = 
> soft_disabled_mask_or_return(IRQ_DISABLE_MASK_LINUX | \
> IRQ_DISABLE_MASK_PMU);                  \
> 
> So yes in the above seq, we will miss the pmu bit. But since 
> trace_hardirqs_off()
> is for _LINUX, instead will it not be safer to OR it?
> 
>          local_paca->soft_disabled_mask |= IRQ_DISABLE_MASK_LINUX;\

I would just say set all unconditionally. Despite "Linux" IRQs being the
special case, I would much prefer it to be written as much as possible
with all mask bits being equal, and then cases where LINUX bits need to
be handled differently built on that.

We might end up adding more bits, or even splitting the current single
LINUX bit into several others.

hard disable effectively masks all the interrupts, so then it seems we
should set them all in the disabled mask rather than just one. The
special case would then be just the test for the LINUX bit when deciding
whether to call trace_hardirqs_off().

Thanks,
Nick