Re: [PATCH 3/8] perf: pmuv3: Add common defines for the PMU version

[Marc Zyngier <maz@kernel.org>]

On Thu, 26 Jan 2023 20:44:39 +0000,
Zaid Al-Bassam <zalbassam@google.com> wrote:
> 
> The current PMU version defines are available for arm64 only,
> As we want to add PMUv3 support to arm (32-bit), this patch makes
> these defines available for both arm/arm64 by defining them in
> the common arm_pmuv3.h header.
> 
> Signed-off-by: Zaid Al-Bassam <zalbassam@google.com>
> ---
>  drivers/perf/arm_pmuv3.c       | 8 ++++----
>  include/linux/perf/arm_pmuv3.h | 6 ++++++
>  2 files changed, 10 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/perf/arm_pmuv3.c b/drivers/perf/arm_pmuv3.c
> index 94e4098b662d..505f0758260c 100644
> --- a/drivers/perf/arm_pmuv3.c
> +++ b/drivers/perf/arm_pmuv3.c
> @@ -392,7 +392,7 @@ static const struct attribute_group armv8_pmuv3_caps_attr_group = {
>   */
>  static bool armv8pmu_has_long_event(struct arm_pmu *cpu_pmu)
>  {
> -	return (cpu_pmu->pmuver >= ID_AA64DFR0_EL1_PMUVer_V3P5);
> +	return (cpu_pmu->pmuver >= ARMV8_PMU_DFR_VER_V3P5);

This doesn't really makes any sense. As per the architecture spec,
PMUv3p5 on AArch32 cannot expose the top 32 bits (DDI0487I.a, G8.4.10
"PMEVCNTR<n>, Performance Monitors Event Count Registers, n = 0 -
30"):

<quote>
There is no means to access bits [63:32] directly from AArch32 state.
</quote>

So on AArch32, this should always return false, no ifs, no buts.

Also, turning the architectural symbols (ID_AA64DFR0_EL1_*) into
custom stuff is a total non-starter. We generate these names and want
to use them everywhere.

Either you abstract them in the architecture specific headers, or you
define the AArch64 name in the AArch32 code.

Thanks,

	M.

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