Re: [PATCH V17 1/9] arm64/sysreg: Add BRBE registers and fields

[Anshuman Khandual]

On 5/21/24 18:54, Mark Rutland wrote:
> On Fri, Apr 05, 2024 at 08:16:31AM +0530, Anshuman Khandual wrote:
>> This adds BRBE related register definitions and various other related field
>> macros there in. These will be used subsequently in a BRBE driver, which is
>> being added later on. While here, this drops redundant register definitions
>> from the header i.e (arch/arm64/include/asm/sysreg.h).
>>
>> BRBINFx_EL1_TYPE_IMPDEF_TRAP_EL3 register field value has been derived from
>> latest ARM DDI 0601 ID121123, AKA 2023-12 instead of latest ARM ARM i.e ARM
>> DDI 0487J.a. Please find the definition here.
>>
>> https://developer.arm.com/documentation/ddi0601/2023-12/
> 
> The K.a release of the ARM ARM has now been published, and that includes
> the "0b110000 IMPLEMENTATION DEFINED exception to EL3" value, so we can
> point to that now.

Sure.

> 
> With that in mind, for the commit message we can say:
> 
> | This patch adds definitions related to the Branch Record Buffer
> | Extension (BRBE) as per ARM DDI 0487K.a. These will be used by KVM and
> | a BRBE driver in subsequent patches.
> |
> | Some existing BRBE definitions in asm/sysreg.h are replaced with
> | equivalent generated definitions.

Replaced the commit message wordings as suggested.

> 
> Aside from a couple of minor issues below, this looks good to me. With
> those fixed up (and the commit message above):
> 
> Reviewed-by: Mark Rutland <mark.rutland@xxxxxxx>

Thanks Mark.

> 
> [...]
> 
>> +Sysreg	BRBIDR0_EL1	2	1	9	2	0
>> +Res0	63:16
>> +Enum	15:12	CC
>> +	0b101	20_BIT
>> +EndEnum
> 
> Please pad this to the width of the field, i.e.
> 
> 	0b0101	20_bit

Done.

> 
>> +Enum	11:8	FORMAT
>> +	0b0	0
>> +EndEnum
> 
> Likewise, this should be padded to the width of the field.

Done.

> 
> As with the BRBFCR_EL1.BANK definitions, I reckon it's worth givin the
> enum value a prefix, so please make this:
> 
> 	0b0000	FORMAT_0

Sure, will change as suggested.