Re: [PATCH v2 01/10] target/i386: disable PerfMonV2 when PERFCORE unavailable

[dongli.zhang@oracle.com]

Hi Xiaoyao,

On 3/4/25 6:40 AM, Xiaoyao Li wrote:
> On 3/3/2025 6:00 AM, Dongli Zhang wrote:
>> When the PERFCORE is disabled with "-cpu host,-perfctr-core", it is
>> reflected in in guest dmesg.
>>
>> [    0.285136] Performance Events: AMD PMU driver.
> 
> I'm a little confused. wWhen no perfctr-core, AMD PMU driver can still be
> probed? (forgive me if I ask a silly question)

Intel use "cpuid -1 -l 0xa" to determine the support of PMU.

However, AMD doesn't use CPUID to determine PMU support (except AMD PMU
PerfMonV2).

I have derived everything from Linux kernel function amd_pmu_init().

As line 1521, the PMU isn't supported by old AMD CPUs.

1516 __init int amd_pmu_init(void)
1517 {
1518         int ret;
1519
1520         /* Performance-monitoring supported from K7 and later: */
1521         if (boot_cpu_data.x86 < 6)
1522                 return -ENODEV;
1523
1524         x86_pmu = amd_pmu;
1525
1526         ret = amd_core_pmu_init();


1. Therefore, at least 4 PMCs are available (without 'perfctr-core').

2. With 'perfctr-core', there are 6 PMCs. (line 1410)

1404 static int __init amd_core_pmu_init(void)
1405 {
1406         union cpuid_0x80000022_ebx ebx;
1407         u64 even_ctr_mask = 0ULL;
1408         int i;
1409
1410         if (!boot_cpu_has(X86_FEATURE_PERFCTR_CORE))
1411                 return 0;
1412
1413         /* Avoid calculating the value each time in the NMI handler */
1414         perf_nmi_window = msecs_to_jiffies(100);
1415
1416         /*
1417          * If core performance counter extensions exists, we must use
1418          * MSR_F15H_PERF_CTL/MSR_F15H_PERF_CTR msrs. See also
1419          * amd_pmu_addr_offset().
1420          */
1421         x86_pmu.eventsel        = MSR_F15H_PERF_CTL;
1422         x86_pmu.perfctr         = MSR_F15H_PERF_CTR;
1423         x86_pmu.cntr_mask64     = GENMASK_ULL(AMD64_NUM_COUNTERS_CORE
- 1, 0);


3. With PerfMonV2, extra global registers are available, as well as PMCs.
(line 1426)

1425         /* Check for Performance Monitoring v2 support */
1426         if (boot_cpu_has(X86_FEATURE_PERFMON_V2)) {
1427                 ebx.full = cpuid_ebx(EXT_PERFMON_DEBUG_FEATURES);
1428
1429                 /* Update PMU version for later usage */
1430                 x86_pmu.version = 2;
1431
1432                 /* Find the number of available Core PMCs */
1433                 x86_pmu.cntr_mask64 =
GENMASK_ULL(ebx.split.num_core_pmc - 1, 0);
1434
1435                 amd_pmu_global_cntr_mask = x86_pmu.cntr_mask64;
1436
1437                 /* Update PMC handling functions */
1438                 x86_pmu.enable_all = amd_pmu_v2_enable_all;
1439                 x86_pmu.disable_all = amd_pmu_v2_disable_all;
1440                 x86_pmu.enable = amd_pmu_v2_enable_event;
1441                 x86_pmu.handle_irq = amd_pmu_v2_handle_irq;
1442                 static_call_update(amd_pmu_test_overflow,
amd_pmu_test_overflow_status);
1443         }


That's why legacy 4-PMC PMU is probed after we disable perfctr-core.

- (boot_cpu_data.x86 < 6): No PMU.
- Without perfctr-core: 4 PMCs
- With perfctr-core: 6 PMCs
- PerfMonV2: PMCs (currently 6) + global PMU registers


May this resolve your concern in another thread that "This looks like a KVM
bug."? This isn't a KVM bug. It is because AMD's lack of the configuration
to disable PMU.

Thank you very much!

Dongli Zhang

> 
>> However, the guest CPUID indicates the PerfMonV2 is still available.
>>
>> CPU:
>>     Extended Performance Monitoring and Debugging (0x80000022):
>>        AMD performance monitoring V2         = true
>>        AMD LBR V2                            = false
>>        AMD LBR stack & PMC freezing          = false
>>        number of core perf ctrs              = 0x6 (6)
>>        number of LBR stack entries           = 0x0 (0)
>>        number of avail Northbridge perf ctrs = 0x0 (0)
>>        number of available UMC PMCs          = 0x0 (0)
>>        active UMCs bitmask                   = 0x0
>>
>> Disable PerfMonV2 in CPUID when PERFCORE is disabled.
>>
>> Suggested-by: Zhao Liu <zhao1.liu@intel.com>
> 
> Though I have above confusion of the description, the change itself looks
> good to me. So
> 
> Reviewed-by: Xiaoyao Li <xiaoyao.li@intel.com>
> 
>> Fixes: 209b0ac12074 ("target/i386: Add PerfMonV2 feature bit")
>> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
>> ---
>> Changed since v1:
>>    - Use feature_dependencies (suggested by Zhao Liu).
>>
>>   target/i386/cpu.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>> index 72ab147e85..b6d6167910 100644
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -1805,6 +1805,10 @@ static FeatureDep feature_dependencies[] = {
>>           .from = { FEAT_7_1_EDX,             CPUID_7_1_EDX_AVX10 },
>>           .to = { FEAT_24_0_EBX,              ~0ull },
>>       },
>> +    {
>> +        .from = { FEAT_8000_0001_ECX,       CPUID_EXT3_PERFCORE },
>> +        .to = { FEAT_8000_0022_EAX,        
>> CPUID_8000_0022_EAX_PERFMON_V2 },
>> +    },
>>   };
>>     typedef struct X86RegisterInfo32 {
>