Re: [PATCH v2 10/14] arm64/nmi: Manage masking for superpriority interrupts along with DAIF

From: Mark Brown <broonie@kernel.org>
To: Lorenzo Pieralisi <lpieralisi@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>, Marc Zyngier <maz@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Sami Mujawar <Sami.Mujawar@arm.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev
Subject: Re: [PATCH v2 10/14] arm64/nmi: Manage masking for superpriority interrupts along with DAIF
Date: Mon, 12 Dec 2022 14:03:33 +0000	[thread overview]
Message-ID: <Y5c0tW7sidPE0oxd@sirena.org.uk> (raw)
In-Reply-To: <Y5IchpFrIB3y8575@lpieralisi>

[-- Attachment #1.1: Type: text/plain, Size: 1949 bytes --]

On Thu, Dec 08, 2022 at 06:19:02PM +0100, Lorenzo Pieralisi wrote:

> I think I found a nasty spot. We are currently not handling ALLINT in
> arch_local_irq_enable/disable(). The issue I am facing is that we might
> end up preempting in IRQ context with ALLINT set in the exception path
> - arm64_preempt_schedule_irq() - which means we are running with all
> IRQs masked (that's normal; what's not normal is that local_irq_enable()
> does not clear ALLINT, see below).

Right, and handling ALLINT in arch_local_irq_enable/disable() isn't
exactly ideal since it means that whenever we mask interrupts we also
mask NMIs which somewhat reduces the value.

> When we schedule (preempt_schedule_irq()) we do require a
> local_irq_enable() to enable IRQs; ALLINT is still set, so
> local_irq_enable() does not do what is expected so we are calling
> __schedule() with IRQs disabled, which does not seem right.

> Now we need to debate what the fix for this can be but nonetheless
> it is something to be addressed.

A first pass suggests that we should be handling this like we do for
other preemptions and returning early from arm64_preempt_schedule_irq()
if ALLINT is masked.  If we are handling a regular IRQ then ALLINT will
be unmasked and we'll call into preempt_schedule_irq(), if we're
handling a NMI then ALLINT will still be masked so we don't attempt to
schedule.  I've pushed out a change which does this but not yet properly
tested it.

> Clearing and setting ALLINT in arch_local_irq_enable()/disable()
> seems to solve the issue (now I moved on to debugging something
> else, will post the outcome here because this fix does not seem
> to fix the issue completely or I am hitting another bug).

Do you have any specifics on how you're seeing problems?  You did
mention boot stalls offline but I've not been able to to reproduce this
locally in a way that I can identify (based on your mail now I've made
sure I've got preemption enabled).

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

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