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

[Marc Zyngier <maz@kernel.org>]

On Wed, 22 Oct 2025 16:29:55 +0100,
Marek Vasut <marek.vasut@mailbox.org> wrote:
> 
> On 10/22/25 5:02 PM, Marc Zyngier wrote:
> 
> Hello Marc,
> 
> >>> 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 do agree with this, and the update already exists. The upstream TFA
> MR for this platform also has this fixed.
> 
> >> 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 agree.
> 
> > 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.
> 
> For this old firmware, during development, ARM64_HAS_AMU_EXTN is
> disabled in kernel config to avoid triggering the AMU faults.
> 
> Except right now, I still trigger the AMU faults even with
> ARM64_HAS_AMU_EXTN=n , which I think should not happen ?

ARM64_HAS_AMU_EXTN is a *capability*, not a configuration.
CONFIG_ARM64_AMU_EXTN is the configuration. I have the feeling you're
mixing the two.

Irrespective of the configuration, we access the AMU registers
depending on the what is advertised, because we *must* make these
registers inaccessible from EL0, no matter what.

> > 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.
> 
> I would much rather not support the old firmware for this new platform
> in upstream and pollute the kernel with unnecessary workarounds.
>
> I would much rather be able to disable ARM64_HAS_AMU_EXTN in kernel
> config for the old devices with old firmware, without triggering the
> faults ... and say that everything which is going to be upstream will
> always use new firmware that has proper working AMU support.

No, that's the wrong approach. If you leave the AMU accessible to EL0,
you're leaking data to userspace, and that's pretty wrong, no matter
how you look at it.

I also think your hack works by pure luck, because at the point where
your CPUs are booting, the alternatives are yet not in place (the
kernel patching happens much later). In short, this breaks
*everything*.

As I indicated before, you have two options:

- either you update your firmware and leave the kernel alone

- or you implement the workaround as ID register override so that you
  *must* pass something on the kernel command line to boot, and by
  then accept that you will leak critical timing information to
  userspace.

Any other option, including guarding the macro with a config option is
*not* acceptable.

Thanks,

	M.

-- 
Jazz isn't dead. It just smells funny.