Re: [PATCH 3/3] irqchip/gic-v3: fix priority mask handling

From: Joey Gouly <joey.gouly@arm.com>
To: Mark Rutland <mark.rutland@arm.com>
Cc: <linux-arm-kernel@lists.infradead.org>, <maz@kernel.org>,
	<tglx@linutronix.de>, <will.deacon@arm.com>, <nd@arm.com>
Subject: Re: [PATCH 3/3] irqchip/gic-v3: fix priority mask handling
Date: Fri, 13 May 2022 14:45:52 +0100	[thread overview]
Message-ID: <20220513134552.GA35971@e124191.cambridge.arm.com> (raw)
In-Reply-To: <20220513133038.226182-4-mark.rutland@arm.com>

Hi Mark,

On Fri, May 13, 2022 at 02:30:38PM +0100, Mark Rutland wrote:
> When a kernel is built with CONFIG_ARM64_PSEUDO_NMI=y and pseudo-NMIs
> are enabled at runtime, GICv3's gic_handle_irq() can leave DAIF and
> ICC_PMR_EL1 in an unexpected state in some cases, breaking subsequent
> usage of local_irq_enable() and resulting in softirqs being run with
> IRQs erroneously masked (possibly resulting in deadlocks).
> 
> This can happen when an IRQ exception is taken from a context where
> regular IRQs were unmasked, and either:
> 
> (1) ICC_IAR1_EL1 indicates a special INTID (e.g. as a result of an IRQ
>     being withdrawn since the IRQ exception was taken).
> 
> (2) ICC_IAR1_EL1 and ICC_RPR_EL1 indicate an NMI was acknowledged.
> 
> When an NMI is taken from a context where regular IRQs were masked,
> there is no problem.
> 

[..]

> +/*
> + * An exception has been taken from a context with IRQs enabled, which can only
I think this should be: IRQs *disabled*?

> + * be an NMI.
> + *
> + * The entry code called us with DAIF.IF set to keep NMIs masked. We must leave
> + * DAIF.IF (and ICC_PMR_EL1) unchanged.
> + *
> + * The entry code has performed NMI entry.
> + */
> +static void __gic_handle_irq_from_irqsoff(struct pt_regs *regs)
> +{
> +	u64 pmr;
> +	u32 irqnr;
> +
> +	/*
> +	 * We were in a context with IRQs disabled. However, the
> +	 * entry code has set PMR to a value that allows any
> +	 * interrupt to be acknowledged, and not just NMIs. This can
> +	 * lead to surprising effects if the NMI has been retired in
> +	 * the meantime, and that there is an IRQ pending. The IRQ
> +	 * would then be taken in NMI context, something that nobody
> +	 * wants to debug twice.
> +	 *
> +	 * Until we sort this, drop PMR again to a level that will
> +	 * actually only allow NMIs before reading IAR, and then
> +	 * restore it to what it was.
> +	 */
> +	pmr = gic_read_pmr();
> +	gic_pmr_mask_irqs();
> +	isb();
> +	irqnr = gic_read_iar();
> +	gic_write_pmr(pmr);
> +
> +	__gic_handle_nmi(irqnr, regs);
> +}

Thanks,
Joey

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel