From: Mark Rutland <mark.rutland@arm.com> To: Julien Thierry <julien.thierry@arm.com> Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, daniel.thompson@linaro.org, joel@joelfernandes.org, marc.zyngier@arm.com, christoffer.dall@arm.com, james.morse@arm.com, catalin.marinas@arm.com, will.deacon@arm.com Subject: Re: [PATCH v6 08/24] arm64: Unmask PMR before going idle Date: Thu, 29 Nov 2018 17:44:37 +0000 [thread overview] Message-ID: <20181129174436.avqjydyzvv6ubnjd@lakrids.cambridge.arm.com> (raw) In-Reply-To: <1542023835-21446-9-git-send-email-julien.thierry@arm.com> On Mon, Nov 12, 2018 at 11:56:59AM +0000, Julien Thierry wrote: > CPU does not received signals for interrupts with a priority masked by > ICC_PMR_EL1. This means the CPU might not come back from a WFI > instruction. > > Make sure ICC_PMR_EL1 does not mask interrupts when doing a WFI. > > Signed-off-by: Julien Thierry <julien.thierry@arm.com> > Suggested-by: Daniel Thompson <daniel.thompson@linaro.org> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Will Deacon <will.deacon@arm.com> > --- > arch/arm64/mm/proc.S | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S > index 2c75b0b..3c7064c 100644 > --- a/arch/arm64/mm/proc.S > +++ b/arch/arm64/mm/proc.S > @@ -20,6 +20,7 @@ > > #include <linux/init.h> > #include <linux/linkage.h> > +#include <linux/irqchip/arm-gic-v3.h> > #include <asm/assembler.h> > #include <asm/asm-offsets.h> > #include <asm/hwcap.h> > @@ -53,10 +54,27 @@ > * cpu_do_idle() > * > * Idle the processor (wait for interrupt). > + * > + * If the CPU supports priority masking we must do additional work to > + * ensure that interrupts are not masked at the PMR (because the core will > + * not wake up if we block the wake up signal in the interrupt controller). > */ > ENTRY(cpu_do_idle) > +alternative_if_not ARM64_HAS_IRQ_PRIO_MASKING > + dsb sy // WFI may enter a low-power mode > + wfi > + ret > +alternative_else > + mrs x0, daif // save I bit > + msr daifset, #2 // set I bit > + mrs_s x1, SYS_ICC_PMR_EL1 // save PMR > +alternative_endif > + mov x2, #GIC_PRIO_IRQON > + msr_s SYS_ICC_PMR_EL1, x2 // unmask PMR > dsb sy // WFI may enter a low-power mode Is the DSB SY sufficient and necessary to synchronise the update of SYS_ICC_PMR_EL1? We don't need an ISB too? > wfi > + msr_s SYS_ICC_PMR_EL1, x1 // restore PMR Likewise, we don't need any barriers here before we poke DAIF? > + msr daif, x0 // restore I bit > ret > ENDPROC(cpu_do_idle) If we build without CONFIG_ARM64_PSEUDO_NMI surely we don't want to emit the alternative? How about we move this to C, and have something like the below? For the !CONFIG_ARM64_PSEUDO_NMI case it generates identical assembly to the existing cpu_do_idle(). Note that I've assumed we don't need barriers, which (as above) I'm not certain of. Thanks, Mark. ---->8---- diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c index 7f1628effe6d..ccd2ad8c5e2f 100644 --- a/arch/arm64/kernel/process.c +++ b/arch/arm64/kernel/process.c @@ -73,6 +73,40 @@ EXPORT_SYMBOL_GPL(pm_power_off); void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd); +static inline void __cpu_do_idle(void) +{ + /* WFI may enter a low-power mode */ + dsb(sy); + wfi(); +} + +/* + * When using priority masking we need to take extra care, etc. + */ +static inline void __cpu_do_idle_irqprio(void) +{ + unsigned long flags = arch_local_irq_save(); + unsigned long pmr = gic_read_pmr(); + + gic_write_pmr(GIC_PRIO_IRQON); + + __cpu_do_idle(); + + gic_write_pmr(pmr); + arch_local_irq_enable(); +} + +/* + * Idle the processor (wait for interrupt). + */ +void cpu_do_idle(void) +{ + if (system_uses_irq_prio_masking()) + __cpu_do_idle_irqprio(); + else + __cpu_do_idle(); +} + /* * This is our default idle handler. */ diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S index 03646e6a2ef4..38c0171e52e2 100644 --- a/arch/arm64/mm/proc.S +++ b/arch/arm64/mm/proc.S @@ -49,17 +49,6 @@ #define MAIR(attr, mt) ((attr) << ((mt) * 8)) -/* - * cpu_do_idle() - * - * Idle the processor (wait for interrupt). - */ -ENTRY(cpu_do_idle) - dsb sy // WFI may enter a low-power mode - wfi - ret -ENDPROC(cpu_do_idle) - #ifdef CONFIG_CPU_PM /** * cpu_do_suspend - save CPU registers context
WARNING: multiple messages have this Message-ID (diff)
From: Mark Rutland <mark.rutland@arm.com> To: Julien Thierry <julien.thierry@arm.com> Cc: daniel.thompson@linaro.org, marc.zyngier@arm.com, catalin.marinas@arm.com, will.deacon@arm.com, linux-kernel@vger.kernel.org, christoffer.dall@arm.com, james.morse@arm.com, joel@joelfernandes.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v6 08/24] arm64: Unmask PMR before going idle Date: Thu, 29 Nov 2018 17:44:37 +0000 [thread overview] Message-ID: <20181129174436.avqjydyzvv6ubnjd@lakrids.cambridge.arm.com> (raw) In-Reply-To: <1542023835-21446-9-git-send-email-julien.thierry@arm.com> On Mon, Nov 12, 2018 at 11:56:59AM +0000, Julien Thierry wrote: > CPU does not received signals for interrupts with a priority masked by > ICC_PMR_EL1. This means the CPU might not come back from a WFI > instruction. > > Make sure ICC_PMR_EL1 does not mask interrupts when doing a WFI. > > Signed-off-by: Julien Thierry <julien.thierry@arm.com> > Suggested-by: Daniel Thompson <daniel.thompson@linaro.org> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Will Deacon <will.deacon@arm.com> > --- > arch/arm64/mm/proc.S | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+) > > diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S > index 2c75b0b..3c7064c 100644 > --- a/arch/arm64/mm/proc.S > +++ b/arch/arm64/mm/proc.S > @@ -20,6 +20,7 @@ > > #include <linux/init.h> > #include <linux/linkage.h> > +#include <linux/irqchip/arm-gic-v3.h> > #include <asm/assembler.h> > #include <asm/asm-offsets.h> > #include <asm/hwcap.h> > @@ -53,10 +54,27 @@ > * cpu_do_idle() > * > * Idle the processor (wait for interrupt). > + * > + * If the CPU supports priority masking we must do additional work to > + * ensure that interrupts are not masked at the PMR (because the core will > + * not wake up if we block the wake up signal in the interrupt controller). > */ > ENTRY(cpu_do_idle) > +alternative_if_not ARM64_HAS_IRQ_PRIO_MASKING > + dsb sy // WFI may enter a low-power mode > + wfi > + ret > +alternative_else > + mrs x0, daif // save I bit > + msr daifset, #2 // set I bit > + mrs_s x1, SYS_ICC_PMR_EL1 // save PMR > +alternative_endif > + mov x2, #GIC_PRIO_IRQON > + msr_s SYS_ICC_PMR_EL1, x2 // unmask PMR > dsb sy // WFI may enter a low-power mode Is the DSB SY sufficient and necessary to synchronise the update of SYS_ICC_PMR_EL1? We don't need an ISB too? > wfi > + msr_s SYS_ICC_PMR_EL1, x1 // restore PMR Likewise, we don't need any barriers here before we poke DAIF? > + msr daif, x0 // restore I bit > ret > ENDPROC(cpu_do_idle) If we build without CONFIG_ARM64_PSEUDO_NMI surely we don't want to emit the alternative? How about we move this to C, and have something like the below? For the !CONFIG_ARM64_PSEUDO_NMI case it generates identical assembly to the existing cpu_do_idle(). Note that I've assumed we don't need barriers, which (as above) I'm not certain of. Thanks, Mark. ---->8---- diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c index 7f1628effe6d..ccd2ad8c5e2f 100644 --- a/arch/arm64/kernel/process.c +++ b/arch/arm64/kernel/process.c @@ -73,6 +73,40 @@ EXPORT_SYMBOL_GPL(pm_power_off); void (*arm_pm_restart)(enum reboot_mode reboot_mode, const char *cmd); +static inline void __cpu_do_idle(void) +{ + /* WFI may enter a low-power mode */ + dsb(sy); + wfi(); +} + +/* + * When using priority masking we need to take extra care, etc. + */ +static inline void __cpu_do_idle_irqprio(void) +{ + unsigned long flags = arch_local_irq_save(); + unsigned long pmr = gic_read_pmr(); + + gic_write_pmr(GIC_PRIO_IRQON); + + __cpu_do_idle(); + + gic_write_pmr(pmr); + arch_local_irq_enable(); +} + +/* + * Idle the processor (wait for interrupt). + */ +void cpu_do_idle(void) +{ + if (system_uses_irq_prio_masking()) + __cpu_do_idle_irqprio(); + else + __cpu_do_idle(); +} + /* * This is our default idle handler. */ diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S index 03646e6a2ef4..38c0171e52e2 100644 --- a/arch/arm64/mm/proc.S +++ b/arch/arm64/mm/proc.S @@ -49,17 +49,6 @@ #define MAIR(attr, mt) ((attr) << ((mt) * 8)) -/* - * cpu_do_idle() - * - * Idle the processor (wait for interrupt). - */ -ENTRY(cpu_do_idle) - dsb sy // WFI may enter a low-power mode - wfi - ret -ENDPROC(cpu_do_idle) - #ifdef CONFIG_CPU_PM /** * cpu_do_suspend - save CPU registers context _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2018-11-29 17:44 UTC|newest] Thread overview: 125+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-11-12 11:56 [PATCH v6 00/24] arm64: provide pseudo NMI with GICv3 Julien Thierry 2018-11-12 11:56 ` Julien Thierry 2018-11-12 11:56 ` [PATCH v6 01/24] arm64: Remove unused daif related functions/macros Julien Thierry 2018-11-12 11:56 ` Julien Thierry 2018-11-29 16:26 ` Mark Rutland 2018-11-29 16:26 ` Mark Rutland 2018-11-30 18:03 ` Catalin Marinas 2018-11-30 18:03 ` Catalin Marinas 2018-11-12 11:56 ` [PATCH v6 02/24] arm64: cpufeature: Set SYSREG_GIC_CPUIF as a boot system feature Julien Thierry 2018-11-12 11:56 ` Julien Thierry 2018-11-12 18:00 ` Suzuki K Poulose 2018-11-12 18:00 ` Suzuki K Poulose 2018-11-29 16:27 ` Mark Rutland 2018-11-29 16:27 ` Mark Rutland 2018-11-30 18:07 ` Catalin Marinas 2018-11-30 18:07 ` Catalin Marinas 2018-11-12 11:56 ` [PATCH v6 03/24] arm64: cpufeature: Add cpufeature for IRQ priority masking Julien Thierry 2018-11-12 11:56 ` Julien Thierry 2018-11-12 18:02 ` Suzuki K Poulose 2018-11-12 18:02 ` Suzuki K Poulose 2018-11-29 17:12 ` Mark Rutland 2018-11-29 17:12 ` Mark Rutland 2018-12-03 10:33 ` Julien Thierry 2018-12-03 10:33 ` Julien Thierry 2018-11-30 18:07 ` Catalin Marinas 2018-11-30 18:07 ` Catalin Marinas 2018-11-12 11:56 ` [PATCH v6 04/24] arm/arm64: gic-v3: Add PMR and RPR accessors Julien Thierry 2018-11-12 11:56 ` Julien Thierry 2018-11-29 16:32 ` Mark Rutland 2018-11-29 16:32 ` Mark Rutland 2018-11-30 18:07 ` Catalin Marinas 2018-11-30 18:07 ` Catalin Marinas 2018-11-12 11:56 ` [PATCH v6 05/24] irqchip/gic-v3: Switch to PMR masking before calling IRQ handler Julien Thierry 2018-11-12 11:56 ` Julien Thierry 2018-11-29 18:12 ` Mark Rutland 2018-11-29 18:12 ` Mark Rutland 2018-11-30 9:18 ` Julien Thierry 2018-11-30 9:18 ` Julien Thierry 2018-12-04 16:21 ` Catalin Marinas 2018-12-04 16:21 ` Catalin Marinas 2018-11-12 11:56 ` [PATCH v6 06/24] arm64: ptrace: Provide definitions for PMR values Julien Thierry 2018-11-12 11:56 ` Julien Thierry 2018-11-29 16:40 ` Mark Rutland 2018-11-29 16:40 ` Mark Rutland 2018-11-30 8:53 ` Julien Thierry 2018-11-30 8:53 ` Julien Thierry 2018-11-30 10:38 ` Daniel Thompson 2018-11-30 10:38 ` Daniel Thompson 2018-11-30 11:03 ` Julien Thierry 2018-11-30 11:03 ` Julien Thierry 2018-11-12 11:56 ` [PATCH v6 07/24] arm64: Make PMR part of task context Julien Thierry 2018-11-12 11:56 ` Julien Thierry 2018-11-29 16:46 ` Mark Rutland 2018-11-29 16:46 ` Mark Rutland 2018-11-30 9:25 ` Julien Thierry 2018-11-30 9:25 ` Julien Thierry 2018-12-04 17:09 ` Catalin Marinas 2018-12-04 17:09 ` Catalin Marinas 2018-12-04 17:30 ` Julien Thierry 2018-12-04 17:30 ` Julien Thierry 2018-11-12 11:56 ` [PATCH v6 08/24] arm64: Unmask PMR before going idle Julien Thierry 2018-11-12 11:56 ` Julien Thierry 2018-11-29 17:44 ` Mark Rutland [this message] 2018-11-29 17:44 ` Mark Rutland 2018-11-30 10:55 ` Julien Thierry 2018-11-30 10:55 ` Julien Thierry 2018-11-30 13:37 ` Mark Rutland 2018-11-30 13:37 ` Mark Rutland 2018-12-03 10:38 ` Julien Thierry 2018-12-03 10:38 ` Julien Thierry 2018-11-12 11:57 ` [PATCH v6 09/24] arm64: kvm: Unmask PMR before entering guest Julien Thierry 2018-11-12 11:57 ` Julien Thierry 2018-11-12 11:57 ` Julien Thierry 2018-11-12 11:57 ` [PATCH v6 10/24] arm64: irqflags: Use ICC_PMR_EL1 for interrupt masking Julien Thierry 2018-11-12 11:57 ` Julien Thierry 2018-12-04 17:36 ` Catalin Marinas 2018-12-04 17:36 ` Catalin Marinas 2018-12-05 16:55 ` Julien Thierry 2018-12-05 16:55 ` Julien Thierry 2018-12-05 18:26 ` Catalin Marinas 2018-12-05 18:26 ` Catalin Marinas 2018-12-06 9:50 ` Julien Thierry 2018-12-06 9:50 ` Julien Thierry 2018-12-10 14:39 ` Catalin Marinas 2018-12-10 14:39 ` Catalin Marinas 2018-11-12 11:57 ` [PATCH v6 11/24] arm64: daifflags: Include PMR in daifflags restore operations Julien Thierry 2018-11-12 11:57 ` Julien Thierry 2018-11-12 11:57 ` [PATCH v6 12/24] arm64: alternative: Allow alternative status checking per cpufeature Julien Thierry 2018-11-12 11:57 ` Julien Thierry 2018-11-12 11:57 ` [PATCH v6 13/24] arm64: alternative: Apply alternatives early in boot process Julien Thierry 2018-11-12 11:57 ` Julien Thierry 2018-11-12 11:57 ` [PATCH v6 14/24] irqchip/gic-v3: Factor group0 detection into functions Julien Thierry 2018-11-12 11:57 ` Julien Thierry 2018-11-12 11:57 ` [PATCH v6 15/24] arm64: Switch to PMR masking when starting CPUs Julien Thierry 2018-11-12 11:57 ` Julien Thierry 2018-12-04 17:51 ` Catalin Marinas 2018-12-04 17:51 ` Catalin Marinas 2018-12-04 18:11 ` Julien Thierry 2018-12-04 18:11 ` Julien Thierry 2018-11-12 11:57 ` [PATCH v6 16/24] arm64: gic-v3: Implement arch support for priority masking Julien Thierry 2018-11-12 11:57 ` Julien Thierry 2018-11-12 11:57 ` [PATCH v6 17/24] irqchip/gic-v3: Detect if GIC can support pseudo-NMIs Julien Thierry 2018-11-12 11:57 ` Julien Thierry 2018-11-12 11:57 ` [PATCH v6 18/24] irqchip/gic-v3: Handle pseudo-NMIs Julien Thierry 2018-11-12 11:57 ` Julien Thierry 2018-11-12 11:57 ` [PATCH v6 19/24] irqchip/gic: Add functions to access irq priorities Julien Thierry 2018-11-12 11:57 ` Julien Thierry 2018-11-12 11:57 ` [PATCH v6 20/24] irqchip/gic-v3: Allow interrupts to be set as pseudo-NMI Julien Thierry 2018-11-12 11:57 ` Julien Thierry 2018-11-12 11:57 ` [PATCH v6 21/24] arm64: Handle serror in NMI context Julien Thierry 2018-11-12 11:57 ` Julien Thierry 2018-12-04 18:09 ` Catalin Marinas 2018-12-04 18:09 ` Catalin Marinas 2018-12-05 13:02 ` James Morse 2018-12-05 13:02 ` James Morse 2018-11-12 11:57 ` [PATCH v6 22/24] arm64: Skip preemption when exiting an NMI Julien Thierry 2018-11-12 11:57 ` Julien Thierry 2018-11-12 11:57 ` [PATCH v6 23/24] arm64: Skip irqflags tracing for NMI in IRQs disabled context Julien Thierry 2018-11-12 11:57 ` Julien Thierry 2018-11-12 11:57 ` [PATCH v6 24/24] arm64: Enable the support of pseudo-NMIs Julien Thierry 2018-11-12 11:57 ` Julien Thierry 2018-11-12 12:00 ` [PATCH v6 00/24] arm64: provide pseudo NMI with GICv3 Julien Thierry 2018-11-12 12:00 ` Julien Thierry 2018-11-13 14:43 ` Julien Thierry 2018-11-13 14:43 ` Julien Thierry
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=20181129174436.avqjydyzvv6ubnjd@lakrids.cambridge.arm.com \ --to=mark.rutland@arm.com \ --cc=catalin.marinas@arm.com \ --cc=christoffer.dall@arm.com \ --cc=daniel.thompson@linaro.org \ --cc=james.morse@arm.com \ --cc=joel@joelfernandes.org \ --cc=julien.thierry@arm.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=marc.zyngier@arm.com \ --cc=will.deacon@arm.com \ /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.