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

[Marc Zyngier <maz@kernel.org>]

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...

> 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.

In any case, this sort of terminally broken stuff should be handled as
an IDreg override, for which we have a whole infrastructure already.
There are countless examples in the tree already for similar purposes.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.