[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <YzWBokiO1KSZNtcl@sirena.org.uk>
Date:   Thu, 29 Sep 2022 12:29:38 +0100
From:   Mark Brown <broonie@...nel.org>
To:     Anshuman Khandual <anshuman.khandual@....com>
Cc:     linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, peterz@...radead.org,
        acme@...nel.org, mark.rutland@....com, will@...nel.org,
        catalin.marinas@....com, Marc Zyngier <maz@...nel.org>
Subject: Re: [PATCH V3 1/7] arm64/perf: Add BRBE registers and fields
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).
Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)
Powered by blists - more mailing lists