Re: [PATCH v4 1/2] x86/cpufeatures: Add dedicated feature word for CPUID leaf 0x80000022[EAX]

[Sandipan Das]

On 3/19/2024 3:47 PM, Ingo Molnar wrote:
> 
> * Sandipan Das <sandipan.das@xxxxxxx> wrote:
> 
>> Move the existing scattered performance monitoring related feature bits
>> from CPUID leaf 0x80000022[EAX] into a dedicated word since additional
>> bits will be defined from the same leaf in the future. This includes
>> X86_FEATURE_PERFMON_V2 and X86_FEATURE_AMD_LBR_V2.
>>
>> Signed-off-by: Sandipan Das <sandipan.das@xxxxxxx>
>> ---
>>  arch/x86/include/asm/cpufeature.h        |  7 +++++--
>>  arch/x86/include/asm/cpufeatures.h       | 10 +++++++---
>>  arch/x86/include/asm/disabled-features.h |  3 ++-
>>  arch/x86/include/asm/required-features.h |  3 ++-
>>  arch/x86/kernel/cpu/common.c             |  3 +++
>>  arch/x86/kernel/cpu/scattered.c          |  2 --
>>  arch/x86/kvm/cpuid.c                     |  5 +----
>>  arch/x86/kvm/reverse_cpuid.h             |  1 -
>>  8 files changed, 20 insertions(+), 14 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
>> index a1273698fc43..68dd27d60748 100644
>> --- a/arch/x86/include/asm/cpufeature.h
>> +++ b/arch/x86/include/asm/cpufeature.h
>> @@ -33,6 +33,7 @@ enum cpuid_leafs
>>  	CPUID_7_EDX,
>>  	CPUID_8000_001F_EAX,
>>  	CPUID_8000_0021_EAX,
>> +	CPUID_8000_0022_EAX,
> 
>>  #define X86_FEATURE_SYSCALL32		( 3*32+14) /* "" syscall in IA32 userspace */
>>  #define X86_FEATURE_SYSENTER32		( 3*32+15) /* "" sysenter in IA32 userspace */
>>  #define X86_FEATURE_REP_GOOD		( 3*32+16) /* REP microcode works well */
>> -#define X86_FEATURE_AMD_LBR_V2		( 3*32+17) /* AMD Last Branch Record Extension Version 2 */
>> +/* FREE!				( 3*32+17) */
>>  #define X86_FEATURE_CLEAR_CPU_BUF	( 3*32+18) /* "" Clear CPU buffers using VERW */
>>  #define X86_FEATURE_ACC_POWER		( 3*32+19) /* AMD Accumulated Power Mechanism */
>>  #define X86_FEATURE_NOPL		( 3*32+20) /* The NOPL (0F 1F) instructions */
>> @@ -209,7 +209,7 @@
>>  #define X86_FEATURE_SSBD		( 7*32+17) /* Speculative Store Bypass Disable */
>>  #define X86_FEATURE_MBA			( 7*32+18) /* Memory Bandwidth Allocation */
>>  #define X86_FEATURE_RSB_CTXSW		( 7*32+19) /* "" Fill RSB on context switches */
>> -#define X86_FEATURE_PERFMON_V2		( 7*32+20) /* AMD Performance Monitoring Version 2 */
>> +/* FREE!				( 7*32+20) */
>>  #define X86_FEATURE_USE_IBPB		( 7*32+21) /* "" Indirect Branch Prediction Barrier enabled */
>>  #define X86_FEATURE_USE_IBRS_FW		( 7*32+22) /* "" Use IBRS during runtime firmware calls */
>>  #define X86_FEATURE_SPEC_STORE_BYPASS_DISABLE	( 7*32+23) /* "" Disable Speculative Store Bypass. */
>> @@ -459,6 +459,10 @@
>>  #define X86_FEATURE_IBPB_BRTYPE		(20*32+28) /* "" MSR_PRED_CMD[IBPB] flushes all branch type predictions */
>>  #define X86_FEATURE_SRSO_NO		(20*32+29) /* "" CPU is not affected by SRSO */
>>  
>> +/* AMD-defined performance monitoring features, CPUID level 0x80000022 (EAX), word 21 */
>> +#define X86_FEATURE_PERFMON_V2		(21*32+ 0) /* AMD Performance Monitoring Version 2 */
>> +#define X86_FEATURE_AMD_LBR_V2		(21*32+ 1) /* AMD Last Branch Record Extension Version 2 */
> 
> Thank you! I presume you tested both patches on the relevant system 
> with the X86_FEATURE_AMD_LBR_PMC_FREEZE bug?
> 

Yes, I tested them on systems which don't support freeze.

When kernel branches are captured on such systems, the records mostly
point to amd_pmu_lbr_read() and native_read_msr() which are called to
read the branch record MSRs. This is the expected result since LBR
does not stop recording branches after a PMC overflow.

E.g.

# To display the perf.data header info, please use --header/--header-only options.
#
#
# Total Lost Samples: 0
#
# Samples: 190K of event 'ex_ret_brn_tkn'
# Event count (approx.): 190144
#
# Overhead  Command  Source Shared Object  Source Symbol                Target Symbol                Basic Block Cycles
# ........  .......  ....................  ...........................  ...........................  ..................
#
    24.98%  branchy  [kernel.kallsyms]     [k] amd_pmu_lbr_read         [k] amd_pmu_lbr_read         -
    12.49%  branchy  [kernel.kallsyms]     [k] amd_pmu_lbr_read         [k] native_read_msr          -
    12.49%  branchy  [kernel.kallsyms]     [k] native_read_msr          [k] native_read_msr          -
    12.49%  branchy  [kernel.kallsyms]     [k] srso_alias_safe_ret      [k] amd_pmu_lbr_read         -
    12.49%  branchy  [kernel.kallsyms]     [k] srso_alias_safe_ret      [k] srso_alias_safe_ret      -
    12.49%  branchy  [kernel.kallsyms]     [k] srso_alias_return_thunk  [k] srso_alias_return_thunk  -
     6.25%  branchy  [kernel.kallsyms]     [k] native_read_msr          [k] srso_alias_return_thunk  -
     6.25%  branchy  [kernel.kallsyms]     [k] srso_alias_return_thunk  [k] srso_alias_safe_ret      -
     0.02%  perf-ex  [kernel.kallsyms]     [k] amd_pmu_lbr_read         [k] amd_pmu_lbr_read         -
     0.01%  perf-ex  [kernel.kallsyms]     [k] amd_pmu_lbr_read         [k] native_read_msr          -
     0.01%  perf-ex  [kernel.kallsyms]     [k] native_read_msr          [k] native_read_msr          -
     0.01%  perf-ex  [kernel.kallsyms]     [k] srso_alias_safe_ret      [k] amd_pmu_lbr_read         -
     0.01%  perf-ex  [kernel.kallsyms]     [k] srso_alias_safe_ret      [k] srso_alias_safe_ret      -
     0.01%  perf-ex  [kernel.kallsyms]     [k] srso_alias_return_thunk  [k] srso_alias_return_thunk  -
     0.00%  perf-ex  [kernel.kallsyms]     [k] native_read_msr          [k] srso_alias_return_thunk  -
     0.00%  perf-ex  [kernel.kallsyms]     [k] srso_alias_return_thunk  [k] srso_alias_safe_ret      -