Received-SPF: pass (google.com: domain of linux-kernel+bounces-198607-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <ad3e457a-cc3d-4422-9be8-656b4f925711@arm.com>
Date: Mon, 3 Jun 2024 10:42:53 +0530
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH V17 1/9] arm64/sysreg: Add BRBE registers and fields
To: Mark Rutland <mark.rutland@arm.com>
Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org,
 will@kernel.org, catalin.marinas@arm.com, Mark Brown <broonie@kernel.org>,
 James Clark <james.clark@arm.com>, Rob Herring <robh@kernel.org>,
 Marc Zyngier <maz@kernel.org>, Suzuki Poulose <suzuki.poulose@arm.com>,
 Peter Zijlstra <peterz@infradead.org>, Ingo Molnar <mingo@redhat.com>,
 Arnaldo Carvalho de Melo <acme@kernel.org>, linux-perf-users@vger.kernel.org
References: <20240405024639.1179064-1-anshuman.khandual@arm.com>
 <20240405024639.1179064-2-anshuman.khandual@arm.com>
 <Zkygkt2a9HBHfmeu@J2N7QTR9R3>
Content-Language: en-US
From: Anshuman Khandual <anshuman.khandual@arm.com>
In-Reply-To: <Zkygkt2a9HBHfmeu@J2N7QTR9R3>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit


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@arm.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.