Re: [PATCH] arm64: guard AMU register access with ARM64_HAS_AMU_EXTN

[Catalin Marinas <catalin.marinas@arm.com>]

On Wed, Oct 22, 2025 at 04:02:15PM +0100, Marc Zyngier wrote:
> On Wed, 22 Oct 2025 15:33:38 +0100,
> Marek Vasut <marek.vasut@mailbox.org> wrote:
> > 
> > On 10/22/25 4:20 PM, Marc Zyngier wrote:
> > > On Wed, 22 Oct 2025 14:35:28 +0100,
> > > Marek Vasut <marek.vasut+renesas@mailbox.org> wrote:
> > >> 
> > >> The AMU configuration register access may fault and prevent successful
> > >> kernel boot. This can occur for example in case the firmware does not
> > >> allow AMU counter access from EL1. Guard the AMU configuration register
> > >> access with ARM64_HAS_AMU_EXTN to prevent this fault if ARM64_HAS_AMU_EXTN
> > >> Kconfig option is explicitly disabled. Other interaction with the AMU is
> > >> already guarded by similar ifdeffery.
> > >> 
> > >> Fixes: 87a1f063464a ("arm64: trap to EL1 accesses to AMU counters from EL0")
> > >> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> > >> ---
> > >> Cc: Anshuman Khandual <anshuman.khandual@arm.com>
> > >> Cc: Catalin Marinas <catalin.marinas@arm.com>
> > >> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
> > >> Cc: Marc Zyngier <maz@kernel.org>
> > >> Cc: Ryan Roberts <ryan.roberts@arm.com>
> > >> Cc: Will Deacon <will@kernel.org>
> > >> Cc: Yicong Yang <yangyicong@hisilicon.com>
> > >> Cc: linux-arm-kernel@lists.infradead.org
> > >> Cc: linux-renesas-soc@vger.kernel.org
> > >> ---
> > >>   arch/arm64/mm/proc.S | 4 ++++
> > >>   1 file changed, 4 insertions(+)
> > >> 
> > >> diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S
> > >> index 86818511962b6..123538ffeda6b 100644
> > >> --- a/arch/arm64/mm/proc.S
> > >> +++ b/arch/arm64/mm/proc.S
> > >> @@ -145,7 +145,9 @@ SYM_FUNC_START(cpu_do_resume)
> > >>   	ubfx	x11, x11, #1, #1
> > >>   	msr	oslar_el1, x11
> > >>   	reset_pmuserenr_el0 x0			// Disable PMU access from EL0
> > >> +alternative_if ARM64_HAS_AMU_EXTN
> > >>   	reset_amuserenr_el0 x0			// Disable AMU access from EL0
> > >> +alternative_else_nop_endif
> > > 
> > > Why?
> > > 
> > > We ensure that the AMU is available in the macro itself by checking
> > > for ID_AA64PFR0_EL1.AMU. If the AMu isn't present on this CPU, we skip
> > > the offending sysreg access. This is similar to what we do for the
> > > PMU.
> > > 
> > > Does your HW advertise an AMU, but doesn't actually have one?
> >
> > The hardware does have AMU, but it is currently blocked in old TFA
> > version and access to this AMU register here causes a fault. That's
> > why I have to disable ARM64_HAS_AMU_EXTN until the TFA is updated and
> > the AMU access is made available on this hardware. But even if I do
> > disable ARM64_HAS_AMU_EXTN=n , I get a fault.
> 
> Well, I would tend to say that you are trying to update the wrong
> piece of SW here. Crashing kernels should be a good incentive for the
> board manufacturer to update their firmware pronto, specially when we
> are talking of code that has been in the tree for over 5 years...

I agree.

> > This patch is mainly meant to prevent a surprise in case someone does
> > set ARM64_HAS_AMU_EXTN=n , and the system still faults on AMU register
> > access.
> 
> But that doesn't really fix anything if you have a buggy firmware,
> because you can't tell which CPUs have been correctly configured, and
> which have not. I also don't really get why this hack works for you,
> because the feature will be set as soon as one CPU advertises the
> feature.

I think Marek also disables the config option and the feature won't be
turned on.

-- 
Catalin