Re: [PATCH v5 1/7] arm64: add support for the AMU extension v1

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

Hi Ionela,

On Wed, Feb 26, 2020 at 01:29:41PM +0000, Ionela Voinescu wrote:
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index dbc22d684627..49f0c436928f 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -318,6 +318,15 @@
>  			Format: <a>,<b>
>  			See also Documentation/input/joydev/joystick.rst
>  
> +	amu=		[ARM64]
> +			Enables or disables detection, enablement and access to
> +			counter registers of the Activity Monitors Unit (AMU).
> +			Format: amu=[0/1/on/off/y/n]
> +			amu=[0/off/n] ensures access to AMU's counter registers
> +				      is not attempted.
> +			amu=[1/on/y] (default) enables detection and access to
> +				     AMU's counter registers.

Is the only reason for this parameter to be able to disable the feature
if the firmware doesn't support it? According to the Kconfig entry, you
may see weird behaviour, firmware lock-up. Is the user supposed to try
again with amu=0?

I'm not particularly fond of adding kernel parameters to work around
broken firmware. We have other architecture features (e.g. PtrAuth) that
need enabling at EL3 but we don't have such parameters. If it's likely
that we hit this issue in practice, I'd rather have the firmware
describing the presence of AMU via some DT entry. But I'd rather not
bother at all, just get the vendors to update their firmware.

If we drop this parameter, patch 1 would need to change. Otherwise the
patches look fine.

Thanks.

-- 
Catalin