linux-kernel - Re: [RFC Patch v1 2/4] irqchip/gic-v3: Add support to handle SGI as pseudo NMI

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200425112950.3a4815b6@why>
Date:   Sat, 25 Apr 2020 11:29:50 +0100
From:   Marc Zyngier <maz@...nel.org>
To:     Sumit Garg <sumit.garg@...aro.org>
Cc:     linux-arm-kernel@...ts.infradead.org, catalin.marinas@....com,
        will@...nel.org, tglx@...utronix.de, jason@...edaemon.net,
        julien.thierry.kdev@...il.com, dianders@...omium.org,
        daniel.thompson@...aro.org, jason.wessel@...driver.com,
        kgdb-bugreport@...ts.sourceforge.net, linux-kernel@...r.kernel.org
Subject: Re: [RFC Patch v1 2/4] irqchip/gic-v3: Add support to handle SGI as
 pseudo NMI

On Fri, 24 Apr 2020 16:39:12 +0530
Sumit Garg <sumit.garg@...aro.org> wrote:

Hi Sumit,

> With pseudo NMIs enabled, interrupt controller can be configured to
> deliver SGI as a pseudo NMI. So add corresponding handling for SGIs.
> 
> Signed-off-by: Sumit Garg <sumit.garg@...aro.org>
> ---
>  drivers/irqchip/irq-gic-v3.c | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
> index d7006ef..be361bf 100644
> --- a/drivers/irqchip/irq-gic-v3.c
> +++ b/drivers/irqchip/irq-gic-v3.c
> @@ -609,17 +609,29 @@ static inline void gic_handle_nmi(u32 irqnr, struct pt_regs *regs)
>  	if (irqs_enabled)
>  		nmi_enter();
>  
> -	if (static_branch_likely(&supports_deactivate_key))
> -		gic_write_eoir(irqnr);
>  	/*
>  	 * Leave the PSR.I bit set to prevent other NMIs to be
>  	 * received while handling this one.
>  	 * PSR.I will be restored when we ERET to the
>  	 * interrupted context.
>  	 */
> -	err = handle_domain_nmi(gic_data.domain, irqnr, regs);
> -	if (err)
> -		gic_deactivate_unhandled(irqnr);
> +	if (likely(irqnr > 15)) {
> +		if (static_branch_likely(&supports_deactivate_key))
> +			gic_write_eoir(irqnr);
> +
> +		err = handle_domain_nmi(gic_data.domain, irqnr, regs);
> +		if (err)
> +			gic_deactivate_unhandled(irqnr);
> +	} else {
> +		gic_write_eoir(irqnr);
> +		if (static_branch_likely(&supports_deactivate_key))
> +			gic_write_dir(irqnr);
> +#ifdef CONFIG_SMP
> +		handle_IPI(irqnr, regs);
> +#else
> +		WARN_ONCE(true, "Unexpected SGI received!\n");
> +#endif
> +	}
>  
>  	if (irqs_enabled)
>  		nmi_exit();

If there is one thing I would like to avoid, it is to add more ugly
hacks to the way we handle SGIs. There is very little reason why SGIs
should be handled differently from all other interrupts. They have the
same properties, and it is only because of the 32bit legacy that we deal
with them in such a cumbersome way. Nothing that we cannot fix though.

What I would really like to see is first a conversion of the SGIs to
normal, full fat interrupts. These interrupts can then be configured as
NMI using the normal API.

I think Julien had something along these lines (or was that limited to
the PMU?). Otherwise, I'll happily help you with that.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...