[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ad3e457a-cc3d-4422-9be8-656b4f925711@arm.com>
Date: Mon, 3 Jun 2024 10:42:53 +0530
From: Anshuman Khandual <anshuman.khandual@....com>
To: Mark Rutland <mark.rutland@....com>
Cc: linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
will@...nel.org, catalin.marinas@....com, Mark Brown <broonie@...nel.org>,
James Clark <james.clark@....com>, Rob Herring <robh@...nel.org>,
Marc Zyngier <maz@...nel.org>, Suzuki Poulose <suzuki.poulose@....com>,
Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>, linux-perf-users@...r.kernel.org
Subject: Re: [PATCH V17 1/9] arm64/sysreg: Add BRBE registers and fields
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@....com>
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.
Powered by blists - more mailing lists