Re: [PATCH v2 1/6] arm64: add support for the AMU extension v1
From: Ionela Voinescu <hidden>
Date: 2020-01-28 11:00:10
Also in:
linux-doc, lkml
Hi Valentin, On Friday 24 Jan 2020 at 12:00:25 (+0000), Valentin Schneider wrote:
On 23/01/2020 18:32, Ionela Voinescu wrote: [...]quoted
and later we can use information in AMCGCR_EL0 to get the number of architected counters (n) and AMEVTYPER0<n>_EL0 to find out the type. The same logic would apply to the auxiliary counters.Good, I think that's all we'll really need. I've not gone through the whole series (yet!) so I might've missed AMCGCR being used.
No, it's not used later in the patches either, specifically because this is version 1 and we should be able to rely on these first 4 architected counters for all future versions of the AMU implementation.
quoted
quoted
quoted
@@ -1150,6 +1152,59 @@ static bool has_hw_dbm(const struct arm64_cpu_capabilities *cap, #endif +#ifdef CONFIG_ARM64_AMU_EXTN + +/* + * This per cpu variable only signals that the CPU implementation supports + * the Activity Monitors Unit (AMU) but does not provide information + * regarding all the events that it supports. + * When this amu_feat per CPU variable is true, the user of this feature + * can only rely on the presence of the 4 fixed counters. But this does + * not guarantee that the counters are enabled or access to these counters + * is provided by code executed at higher exception levels. + * + * Also, to ensure the safe use of this per_cpu variable, the following + * accessor is defined to allow a read of amu_feat for the current cpu only + * from the current cpu. + * - cpu_has_amu_feat() + */ +static DEFINE_PER_CPU_READ_MOSTLY(u8, amu_feat); +Why not bool?I've changed it from bool after a sparse warning about expression using sizeof(bool) and found this is due to sizeof(bool) being compiler dependent. It does not change anything but I thought it might be a good idea to define it as 8-bit unsigned and rely on fixed size.I believe conveying the intent (a truth value) is more important than the underlying storage size in this case. It mostly matters when dealing with aggregates, but here it's just a free-standing variable. We already have a few per-CPU boolean variables in arm64/kernel/fpsimd.c and the commits aren't even a year old, so I'd go for ignoring sparse this time around.
Will do! Thanks, Ionela. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel