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
WARNING: multiple messages have this Message-ID (diff)
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: 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 #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --]
next prev parent reply other threads:[~2022-12-12 14:04 UTC|newest] Thread overview: 96+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-11-12 15:16 [PATCH v2 00/14] arm64/nmi: Support for FEAT_NMI Mark Brown 2022-11-12 15:16 ` Mark Brown 2022-11-12 15:16 ` [PATCH v2 01/14] arm64/booting: Document boot requirements " Mark Brown 2022-11-12 15:16 ` Mark Brown 2022-11-12 15:16 ` [PATCH v2 02/14] arm64/sysreg: Add definition for ICC_NMIAR1_EL1 Mark Brown 2022-11-12 15:16 ` Mark Brown 2022-11-12 15:16 ` [PATCH v2 03/14] arm64/sysreg: Add definition of ISR_EL1 Mark Brown 2022-11-12 15:16 ` Mark Brown 2022-12-05 16:45 ` Marc Zyngier 2022-12-05 16:45 ` Marc Zyngier 2022-11-12 15:16 ` [PATCH v2 04/14] arm64/sysreg: Add definitions for immediate versions of MSR ALLINT Mark Brown 2022-11-12 15:16 ` Mark Brown 2022-12-05 16:38 ` Marc Zyngier 2022-12-05 16:38 ` Marc Zyngier 2022-12-05 17:11 ` Mark Brown 2022-12-05 17:11 ` Mark Brown 2022-12-07 19:18 ` Marc Zyngier 2022-12-07 19:18 ` Marc Zyngier 2022-12-07 19:42 ` Mark Brown 2022-12-07 19:42 ` Mark Brown 2022-11-12 15:16 ` [PATCH v2 05/14] arm64/asm: Introduce assembly macros for managing ALLINT Mark Brown 2022-11-12 15:16 ` Mark Brown 2022-12-05 17:29 ` Marc Zyngier 2022-12-05 17:29 ` Marc Zyngier 2022-12-05 18:24 ` Mark Brown 2022-12-05 18:24 ` Mark Brown 2022-12-07 19:14 ` Marc Zyngier 2022-12-07 19:14 ` Marc Zyngier 2022-11-12 15:17 ` [PATCH v2 06/14] arm64/hyp-stub: Enable access to ALLINT Mark Brown 2022-11-12 15:17 ` Mark Brown 2022-12-05 17:50 ` Marc Zyngier 2022-12-05 17:50 ` Marc Zyngier 2022-11-12 15:17 ` [PATCH v2 07/14] arm64/idreg: Add an override for FEAT_NMI Mark Brown 2022-11-12 15:17 ` Mark Brown 2022-11-12 15:17 ` [PATCH v2 08/14] arm64/cpufeature: Detect PE support " Mark Brown 2022-11-12 15:17 ` Mark Brown 2022-12-05 18:03 ` Marc Zyngier 2022-12-05 18:03 ` Marc Zyngier 2022-12-05 19:32 ` Mark Brown 2022-12-05 19:32 ` Mark Brown 2022-12-07 19:06 ` Marc Zyngier 2022-12-07 19:06 ` Marc Zyngier 2022-11-12 15:17 ` [PATCH v2 09/14] KVM: arm64: Hide FEAT_NMI from guests Mark Brown 2022-11-12 15:17 ` Mark Brown 2022-12-05 18:06 ` Marc Zyngier 2022-12-05 18:06 ` Marc Zyngier 2022-12-05 19:03 ` Mark Brown 2022-12-05 19:03 ` Mark Brown 2022-12-07 19:03 ` Marc Zyngier 2022-12-07 19:03 ` Marc Zyngier 2022-12-07 19:33 ` Mark Brown 2022-12-07 19:33 ` Mark Brown 2022-11-12 15:17 ` [PATCH v2 10/14] arm64/nmi: Manage masking for superpriority interrupts along with DAIF Mark Brown 2022-11-12 15:17 ` Mark Brown 2022-12-05 18:47 ` Marc Zyngier 2022-12-05 18:47 ` Marc Zyngier 2022-12-05 20:52 ` Mark Brown 2022-12-05 20:52 ` Mark Brown 2022-12-08 17:19 ` Lorenzo Pieralisi 2022-12-08 17:19 ` Lorenzo Pieralisi 2022-12-12 14:03 ` Mark Brown [this message] 2022-12-12 14:03 ` Mark Brown 2022-12-13 8:37 ` Lorenzo Pieralisi 2022-12-13 8:37 ` Lorenzo Pieralisi 2022-12-13 13:15 ` Mark Brown 2022-12-13 13:15 ` Mark Brown 2022-12-15 13:32 ` Marc Zyngier 2022-12-15 13:32 ` Marc Zyngier 2022-12-12 14:40 ` Mark Rutland 2022-12-12 14:40 ` Mark Rutland 2022-12-15 13:21 ` Mark Brown 2022-12-15 13:21 ` Mark Brown 2022-11-12 15:17 ` [PATCH v2 11/14] arm64/irq: Document handling of FEAT_NMI in irqflags.h Mark Brown 2022-11-12 15:17 ` Mark Brown 2022-11-12 15:17 ` [PATCH v2 12/14] arm64/nmi: Add handling of superpriority interrupts as NMIs Mark Brown 2022-11-12 15:17 ` Mark Brown 2022-12-07 11:03 ` Marc Zyngier 2022-12-07 11:03 ` Marc Zyngier 2022-12-07 13:24 ` Mark Brown 2022-12-07 13:24 ` Mark Brown 2022-12-07 18:57 ` Marc Zyngier 2022-12-07 18:57 ` Marc Zyngier 2022-12-07 19:15 ` Mark Brown 2022-12-07 19:15 ` Mark Brown 2022-11-12 15:17 ` [PATCH v2 13/14] arm64/nmi: Add Kconfig for NMI Mark Brown 2022-11-12 15:17 ` Mark Brown 2022-11-12 15:17 ` [PATCH v2 14/14] irqchip/gic-v3: Implement FEAT_GICv3_NMI support Mark Brown 2022-11-12 15:17 ` Mark Brown 2022-12-07 15:20 ` Marc Zyngier 2022-12-07 15:20 ` Marc Zyngier 2022-12-02 18:42 ` [PATCH v2 00/14] arm64/nmi: Support for FEAT_NMI Marc Zyngier 2022-12-02 18:42 ` Marc Zyngier 2022-12-03 8:25 ` Lorenzo Pieralisi 2022-12-03 8:25 ` Lorenzo Pieralisi 2022-12-03 9:45 ` Marc Zyngier 2022-12-03 9:45 ` Marc Zyngier
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=Y5c0tW7sidPE0oxd@sirena.org.uk \ --to=broonie@kernel.org \ --cc=Sami.Mujawar@arm.com \ --cc=catalin.marinas@arm.com \ --cc=kvmarm@lists.linux.dev \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=lpieralisi@kernel.org \ --cc=mark.rutland@arm.com \ --cc=maz@kernel.org \ --cc=tglx@linutronix.de \ --cc=will@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.