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

[Xiaoyao Li <xiaoyao.li@intel.com>]

On 3/5/2025 6:53 AM, dongli.zhang@oracle.com wrote:
> 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.

It helps a lot! Yes, it doesn't a KVM bug.

Thanks for your elaborated explanation!

> 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 {
>>
>