Re: [PATCH 04/13] x86/cpufeatures: Add LbrExtV2 feature bit

[Sandipan Das]

Hi Peter,

On 8/22/2022 6:56 PM, Peter Zijlstra wrote:
> On Mon, Aug 22, 2022 at 06:22:25PM +0530, Sandipan Das wrote:
>> AMD LbrExtV2 is similar to Intel LBR. Unlike BRS, LbrExtV2 does not rely on
> 
> LbrExtV2 must be the most terrible name ever, please stop using it. Heck
> your own code calls it lbr_v2 wherever it can.
> 
> So can we please just kill that name entirely?
> 
> $ quilt diff --combine - | grep -i lbrext_v2
> +       if (x86_pmu.version < 2 || !boot_cpu_has(X86_FEATURE_LBREXT_V2))
> +#define X86_FEATURE_LBREXT_V2          ( 3*32+17) /* AMD Last Branch Record Extension Version 2 */
> +       { X86_FEATURE_LBREXT_V2,        CPUID_EAX,  1, 0x80000022, 0 },
> 
> Is the complete usage of this silly name.
> 

I don't have any reservations about the name :) Its the name of the feature
bit from CPUID 0x80000022 EAX as mentioned in the processor manuals.

Unlike BRS, LBRv2 (if I may call it so) is an architected feature starting
with Zen4. BRS is model-specific and available only on Zen3. LBRv2 supersedes
it and from an architectural perspective, future platforms are slated to have
LBRv2 and not BRS. As Stephane alluded to earlier on this thread, AMD's legacy
LBR is 1-entry deep. We want to be able to differentiate from that as well as
BRS. For these reasons, and also to keep it disjoint from Intel's LBR, if you
so prefer, I can rename this feature to AMD_LBR_V2.

>> interrupt holding. The branch records are "frozen" at the time of counter
>> overflow.
> 
> Yes, I get all that. It is also significantly different from Intel LBR
> in all details and shares not a single line of code, so also calling it
> LBR is confusing at best.
> 
> The MSRs are called AMD_SAMPL_BR, so why not call the thing BRS_V2 ?

Sorry for the confusion with the register names. Since the SAMP_BR_FROM
and SAMP_BR_TO registers used by BRS have the same addresses as that of
the LBR_TO_V2 and LBR_FROM_V2 registers, I chose to reuse the definitions.

It it helps and considering LBRv2 is an architected feature, I can rename
AMD_SAMP_BR_* to AMD_LBR_V2_* or have these MSR definitions duplicated with
different names.

- Sandipan