Re: [PATCH 0/3] arm64: Fix cpuidle with pseudo-NMI enabled

On Tue, Jun 08, 2021 at 06:27:12PM +0100, Marc Zyngier wrote:

It appears that although cpu_do_idle() is correctly dealing with the
PMR/DAIF duality, the PSCI cpu-suspend code has been left unaware of
it.

On a system that uses PSCI for idle (such as the Ampere Altra I have
access to), the kernel dies as soon as it enters idle (interrupts are
off at the GIC CPU interface level). Boo.

After investigating a bit I realised that this should depend on
ICC_CTLR_EL3.PMHE - if that's clear the PMR should not affect the
GICR->CPU IRQ forwarding (or WakeRequest signal generation when the
GICR_WAKER.ProcessorSleep==1).

IIUC if PMHE == 0, the PMR plays no role in wfi completion (and
WakeSignal generation for a CPU/GICR in quiescent state).

I assume on Ampere Altra PMHE == 1.

This changes almost nothing to the need for this patchset but
at least we clarify this behaviour.

Also, we should not be writing ICC_PMR_EL1 when
GICR_WAKER.ProcessorSleep == 1 (which may be set in
gic_cpu_pm_notifier()), this can hang the system.

I wonder whether this arm_cpuidle_{save,restore}_context() should
be moved into the gic_cpu_pm_notifier() itself - which would
solve also the PSCI suspend issue Sudeep raised - it would be
a bit ugly though (CPU PM notifiers are run in S2R and CPUidle
automatically and this would work for any S2R/CPUidle backend
other than PSCI even though that does not/will never exist on
arm64 ;-))

https://lore.kernel.org/linux-arm-kernel/20210608182044.ayqa6fbab4jyz7kp@bogus (local)

I still believe this series is right - just raised these points
for discussion.

Thanks,
Lorenzo

Instead of spreading more magic code around, I've elected to provide a
pair of helpers (arm_cpuidle_{save,restore}_context()) which do the
heavy lifting.

With that in place, I can finally boot the above system with
irqchip.gicv3_pseudo_nmi=1. I'd welcome feedback from people who may
have experienced similar issues in the past (and on different
machines).

Marc Zyngier (3):
  arm64: Add cpuidle context save/restore helpers
  arm64: Convert cpu_do_idle() to using cpuidle context helpers
  PSCI: Use cpuidle context helpers in psci_cpu_suspend_enter()

 arch/arm/include/asm/cpuidle.h   |  5 ++++
 arch/arm64/include/asm/cpuidle.h | 35 +++++++++++++++++++++++++++
 arch/arm64/kernel/process.c      | 41 +++++++-------------------------
 drivers/firmware/psci/psci.c     |  5 ++++
 4 files changed, 54 insertions(+), 32 deletions(-)

-- 
2.30.2

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