Subject: Re: [V2 1/4] powerpc/perf: Refactor the code definition of perf reg extended mask

From: Michael Ellerman <[email-masked]>
Date: Tue, 05 Oct 2021 16:52:52 +1100

Athira Rajeev <[email-masked]> writes:
> PERF_REG_PMU_MASK_300 and PERF_REG_PMU_MASK_31 defines the mask
> value for extended registers. Current definition of these mask values
> uses hex constant and does not use registers by name, making it less
> readable. Patch refactor the macro values by or'ing together the actual
> register value constants. Also include PERF_REG_EXTENDED_MAX as
> part of enum definition.
>
> Suggested-by: Michael Ellerman <[email-masked]>
> Signed-off-by: Athira Rajeev <[email-masked]>
> ---
>  arch/powerpc/include/uapi/asm/perf_regs.h | 21 +++++++++++++--------
>  1 file changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/include/uapi/asm/perf_regs.h b/arch/powerpc/include/uapi/asm/perf_regs.h
> index 578b3ee86105..fb1d8a9b4393 100644
> --- a/arch/powerpc/include/uapi/asm/perf_regs.h
> +++ b/arch/powerpc/include/uapi/asm/perf_regs.h
> @@ -61,27 +61,32 @@ enum perf_event_powerpc_regs {
>  	PERF_REG_POWERPC_PMC4,
>  	PERF_REG_POWERPC_PMC5,
>  	PERF_REG_POWERPC_PMC6,
> -	/* Max regs without the extended regs */
> +	/* Max mask value for interrupt regs w/o extended regs */
>  	PERF_REG_POWERPC_MAX = PERF_REG_POWERPC_MMCRA + 1,
> +	/* Max mask value for interrupt regs including extended regs */
> +	PERF_REG_EXTENDED_MAX = PERF_REG_POWERPC_PMC6 + 1,
>  };
>  
>  #define PERF_REG_PMU_MASK	((1ULL << PERF_REG_POWERPC_MAX) - 1)
>  
> -/* Exclude MMCR3, SIER2, SIER3 for CPU_FTR_ARCH_300 */
> -#define	PERF_EXCLUDE_REG_EXT_300	(7ULL << PERF_REG_POWERPC_MMCR3)
> -
>  /*
>   * PERF_REG_EXTENDED_MASK value for CPU_FTR_ARCH_300
>   * includes 9 SPRS from MMCR0 to PMC6 excluding the
> - * unsupported SPRS in PERF_EXCLUDE_REG_EXT_300.
> + * unsupported SPRS MMCR3, SIER2 and SIER3.
>   */
> -#define PERF_REG_PMU_MASK_300   ((0xfffULL << PERF_REG_POWERPC_MMCR0) - PERF_EXCLUDE_REG_EXT_300)
> +#define PERF_REG_PMU_MASK_300	\
> +	((1ul << PERF_REG_POWERPC_MMCR0) | (1ul << PERF_REG_POWERPC_MMCR1) | \
> +	(1ul << PERF_REG_POWERPC_MMCR2) | (1ul << PERF_REG_POWERPC_PMC1) | \
> +	(1ul << PERF_REG_POWERPC_PMC2) | (1ul << PERF_REG_POWERPC_PMC3) | \
> +	(1ul << PERF_REG_POWERPC_PMC4) | (1ul << PERF_REG_POWERPC_PMC5) | \
> +	(1ul << PERF_REG_POWERPC_PMC6))

These all need to be unsigned long long. Otherwise when building on big
endian (which defaults to 32-bit), we see errors such as:

  In file included from /home/michael/linux/tools/perf/arch/powerpc/include/perf_regs.h:7:0,
                   from arch/powerpc/util/../../../util/perf_regs.h:30,
                   from arch/powerpc/util/perf_regs.c:7:
  arch/powerpc/util/perf_regs.c: In function ‘arch__intr_reg_mask’:
  /home/michael/linux/tools/arch/powerpc/include/uapi/asm/perf_regs.h:78:8: error: left shift count >= width of type [-Werror=shift-count-overflow]
    ((1ul << PERF_REG_POWERPC_MMCR0) | (1ul << PERF_REG_POWERPC_MMCR1) | \
          ^
  arch/powerpc/util/perf_regs.c:206:19: note: in expansion of macro ‘PERF_REG_PMU_MASK_300’
     extended_mask = PERF_REG_PMU_MASK_300;
                     ^

cheers