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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200228103234.GA3904776@arrakis.emea.arm.com>
Date:   Fri, 28 Feb 2020 10:32:34 +0000
From:   Catalin Marinas <catalin.marinas@....com>
To:     Ionela Voinescu <ionela.voinescu@....com>
Cc:     will@...nel.org, mark.rutland@....com, maz@...nel.org,
        suzuki.poulose@....com, sudeep.holla@....com, lukasz.luba@....com,
        valentin.schneider@....com, dietmar.eggemann@....com,
        rjw@...ysocki.net, pkondeti@...eaurora.org, peterz@...radead.org,
        mingo@...hat.com, vincent.guittot@...aro.org,
        viresh.kumar@...aro.org, linux-arm-kernel@...ts.infradead.org,
        linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-pm@...r.kernel.org
Subject: Re: [PATCH v5 1/7] arm64: add support for the AMU extension v1

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