Re: [PATCHv3 3/4] irqchip: GICv3: expose pNMI discriminator

[Marc Zyngier <maz@kernel.org>]

Hi Pingfan,

On Tue, 16 Nov 2021 08:24:49 +0000,
Pingfan Liu <kernelfans@gmail.com> wrote:
> 
> Arch level code is ready to take over the nmi_enter()/nmi_exit()
> housekeeping.
> 
> GICv3 can expose the pNMI discriminator, then simply remove the
> housekeeping.
> 
> Signed-off-by: Pingfan Liu <kernelfans@gmail.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Joey Gouly <joey.gouly@arm.com>
> Cc: Sami Tolvanen <samitolvanen@google.com>
> Cc: Julien Thierry <julien.thierry@arm.com>
> Cc: Yuichi Ito <ito-yuichi@fujitsu.com>
> Cc: rcu@vger.kernel.org
> To: linux-arm-kernel@lists.infradead.org
> ---
>  drivers/irqchip/irq-gic-v3.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index daec3309b014..aa2bcb47b47e 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -646,12 +646,8 @@ static void gic_deactivate_unhandled(u32 irqnr)
>  
>  static inline void gic_handle_nmi(u32 irqnr, struct pt_regs *regs)
>  {
> -	bool irqs_enabled = interrupts_enabled(regs);
>  	int err;
>  
> -	if (irqs_enabled)
> -		nmi_enter();
> -
>  	if (static_branch_likely(&supports_deactivate_key))
>  		gic_write_eoir(irqnr);
>  	/*
> @@ -664,8 +660,6 @@ static inline void gic_handle_nmi(u32 irqnr, struct pt_regs *regs)
>  	if (err)
>  		gic_deactivate_unhandled(irqnr);
>  
> -	if (irqs_enabled)
> -		nmi_exit();
>  }
>  
>  static u32 do_read_iar(struct pt_regs *regs)
> @@ -702,6 +696,15 @@ static u32 do_read_iar(struct pt_regs *regs)
>  	return iar;
>  }
>  
> +static bool gic_is_in_nmi(void)
> +{
> +	if (gic_supports_nmi() &&
> +	    unlikely(gic_read_rpr() == GICD_INT_RPR_PRI(GICD_INT_NMI_PRI)))
> +		return true;

I don't think this fixes anything.

RPR stands for 'Running Priority Register', which in GIC speak reports
the priority of the most recently Ack'ed interrupt.

You cannot use this to find out whether the interrupt that you /will/
ack is a NMI or not. Actually, you cannot find out about *any*
priority until you actually ack the interrupt. What you are asking for
is the equivalent of a crystal ball, and we're in short supply... ;-)

The only case where ICC_RPR_EL1 will return something that is equal to
GICD_INT_NMI_PRI is when you are *already* in an NMI context. So
unless I have completely misunderstood your approach (which is always
possible), I don't see how this can work.

If you want to distinguish between NMI and IRQ early on (before
acknowledging the interrupt), the only solution is to turn the NMI
into a Group-0 interrupt so that it is presented to the CPU as a
FIQ. At which point, you have the information by construction.

Unfortunately, this will only work in VMs, as Group-0 interrupts are
usually routed to EL3 on bare metal systems.

	M.

-- 
Without deviation from the norm, progress is not possible.