Re: [PATCH v6 08/24] arm64: Unmask PMR before going idle
From: Mark Rutland <mark.rutland@arm.com>
Date: 2018-11-29 17:44:43
Also in:
lkml
Subsystem:
arm64 port (aarch64 architecture), the rest · Maintainers:
Catalin Marinas, Will Deacon, Linus Torvalds
On Mon, Nov 12, 2018 at 11:56:59AM +0000, Julien Thierry wrote:
quoted hunk ↗ jump to hunk
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 <redacted> Suggested-by: Daniel Thompson <redacted> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Will Deacon <redacted> --- 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