Re: [PATCH V3 1/7] arm64/perf: Add BRBE registers and fields

[Mark Brown]

On Thu, Sep 29, 2022 at 01:28:51PM +0530, Anshuman Khandual wrote:

Thanks for doing this work - I did spot a few small issues though.

>  /* id_aa64dfr0 */
> +#define ID_AA64DFR0_BRBE_SHIFT		52
>  #define ID_AA64DFR0_MTPMU_SHIFT		48
>  #define ID_AA64DFR0_TRBE_SHIFT		44
>  #define ID_AA64DFR0_TRACE_FILT_SHIFT	40
> @@ -848,6 +952,9 @@
>  #define ID_AA64DFR0_PMSVER_8_2		0x1
>  #define ID_AA64DFR0_PMSVER_8_3		0x2
>  
> +#define ID_AA64DFR0_BRBE		0x1
> +#define ID_AA64DFR0_BRBE_V1P1		0x2
> +
>  #define ID_DFR0_PERFMON_SHIFT		24
>  
>  #define ID_DFR0_PERFMON_8_0		0x3

This is already done in -next as a result of ID_AA64DFR0_EL1 being
converted, the enumberation define comes out as
ID_AA64DFR0_EL1_BRBE_BRBE_V1P1.

> +# This is just a dummy register declaration to get all common field masks and
> +# shifts for accessing given BRBINF contents.
> +Sysreg	BRBINF_EL1	2	1	8	0	0
> +Res0	63:47
> +Field	46	CCU
> +Field	45:32	CC
> +Res0	31:18
> +Field	17	LASTFAILED
> +Field	16	TX

According to DDI0487I.a bit 16 is called T not TX.

> +Res0	15:14
> +Enum	13:8		TYPE

It's probably worth noting in the comment the issue with Enums here
that's meaning you're not using a SysregFields - I'm not sure what
people will think of this but providing a definition using the ID for
the 0th register does seem expedient.

> +Enum	7:6	EL
> +	0b00	EL0
> +	0b01	EL1
> +	0b10	EL2
> +EndEnum

According to DDI0487I.a 0b11 has the value EL3 (when FEAT_BRBEv1p1).

> +Sysreg	BRBCR_EL1	2	1	9	0	0
> +Res0	63:24
> +Field	23 	EXCEPTION
> +Field	22 	ERTN
> +Res0	21:9
> +Field	8 	FZP
> +Field	7	ZERO

According to DDI0487I.a bit 7 is Res0.

> +Field	2	ZERO1

According to DDI0487I.a bit 2 is Res0.

> +Sysreg	BRBFCR_EL1	2	1	9	0	1

> +Field	16	ENL

Accoding to DDI0487I.a this is EnI (ie, an L not an I).

> +Sysreg	BRBINFINJ_EL1	2	1	9	1	0

> +Field	16	TX

According to DDI0487I.a this is T not TX.

> +Enum	7:6	EL
> +	0b00	EL0
> +	0b01	EL1
> +	0b10	EL2
> +EndEnum

According to DDI0487I.a 0b11 has the value EL3 (when FEAT_BRBEv1p1).

Attachment: signature.asc
Description: PGP signature