diff mbox series

[v2,02/10] target/i386: disable PERFCORE when "-pmu" is configured

Message ID 20250302220112.17653-3-dongli.zhang@oracle.com (mailing list archive)
State New
Headers show
Series target/i386/kvm/pmu: PMU Enhancement, Bugfix and Cleanup | expand

Commit Message

Dongli Zhang March 2, 2025, 10 p.m. UTC
Currently, AMD PMU support isn't determined based on CPUID, that is, the
"-pmu" option does not fully disable KVM AMD PMU virtualization.

To minimize AMD PMU features, remove PERFCORE when "-pmu" is configured.

To completely disable AMD PMU virtualization will be implemented via
KVM_CAP_PMU_CAPABILITY in upcoming patches.

As a reminder, neither CPUID_EXT3_PERFCORE nor
CPUID_8000_0022_EAX_PERFMON_V2 is removed from env->features[] when "-pmu"
is configured. Developers should query whether they are supported via
cpu_x86_cpuid() rather than relying on env->features[] in future patches.

Suggested-by: Zhao Liu <zhao1.liu@intel.com>
Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
---
 target/i386/cpu.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Xiaoyao Li March 3, 2025, 1:59 a.m. UTC | #1
On 3/3/2025 6:00 AM, Dongli Zhang wrote:
> Currently, AMD PMU support isn't determined based on CPUID, that is, the
> "-pmu" option does not fully disable KVM AMD PMU virtualization.
> 
> To minimize AMD PMU features, remove PERFCORE when "-pmu" is configured.
> 
> To completely disable AMD PMU virtualization will be implemented via
> KVM_CAP_PMU_CAPABILITY in upcoming patches.
> 
> As a reminder, neither CPUID_EXT3_PERFCORE nor
> CPUID_8000_0022_EAX_PERFMON_V2 is removed from env->features[] when "-pmu"
> is configured. Developers should query whether they are supported via
> cpu_x86_cpuid() rather than relying on env->features[] in future patches.

I don't think it is the correct direction to go.

env->features[] should be finalized before cpu_x86_cpuid() and 
env->features[] needs to be able to be exposed to guest directly. This 
ensures guest and QEMU have the same view of CPUIDs and it simplifies 
things.

We can adjust env->features[] by filtering all PMU related CPUIDs based 
on cpu->enable_pmu in x86_cpu_realizefn().

> Suggested-by: Zhao Liu <zhao1.liu@intel.com>
> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
> ---
>   target/i386/cpu.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index b6d6167910..61a671028a 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -7115,6 +7115,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>               !(env->hflags & HF_LMA_MASK)) {
>               *edx &= ~CPUID_EXT2_SYSCALL;
>           }
> +
> +        if (kvm_enabled() && IS_AMD_CPU(env) && !cpu->enable_pmu) {
> +            *ecx &= ~CPUID_EXT3_PERFCORE;
> +        }
>           break;
>       case 0x80000002:
>       case 0x80000003:
Dongli Zhang March 3, 2025, 6:45 p.m. UTC | #2
Hi Xiaoyao,

On 3/2/25 5:59 PM, Xiaoyao Li wrote:
> On 3/3/2025 6:00 AM, Dongli Zhang wrote:
>> Currently, AMD PMU support isn't determined based on CPUID, that is, the
>> "-pmu" option does not fully disable KVM AMD PMU virtualization.
>>
>> To minimize AMD PMU features, remove PERFCORE when "-pmu" is configured.
>>
>> To completely disable AMD PMU virtualization will be implemented via
>> KVM_CAP_PMU_CAPABILITY in upcoming patches.
>>
>> As a reminder, neither CPUID_EXT3_PERFCORE nor
>> CPUID_8000_0022_EAX_PERFMON_V2 is removed from env->features[] when "-pmu"
>> is configured. Developers should query whether they are supported via
>> cpu_x86_cpuid() rather than relying on env->features[] in future patches.
> 
> I don't think it is the correct direction to go.
> 
> env->features[] should be finalized before cpu_x86_cpuid() and env-
>>features[] needs to be able to be exposed to guest directly. This ensures
> guest and QEMU have the same view of CPUIDs and it simplifies things.
> 
> We can adjust env->features[] by filtering all PMU related CPUIDs based on
> cpu->enable_pmu in x86_cpu_realizefn().

Thank you very much for suggestion.

I see  code like below in x86_cpu_realizefn() to edit env->features[].

7982     /* On AMD CPUs, some CPUID[8000_0001].EDX bits must match the bits on
7983      * CPUID[1].EDX.
7984      */
7985     if (IS_AMD_CPU(env)) {
7986         env->features[FEAT_8000_0001_EDX] &= ~CPUID_EXT2_AMD_ALIASES;
7987         env->features[FEAT_8000_0001_EDX] |= (env->features[FEAT_1_EDX]
7988            & CPUID_EXT2_AMD_ALIASES);
7989     }

I may do something similar to them for CPUID_EXT3_PERFCORE and
CPUID_8000_0022_EAX_PERFMON_V2.

Dongli Zhang



> 
>> Suggested-by: Zhao Liu <zhao1.liu@intel.com>
>> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
>> ---
>>   target/i386/cpu.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>> index b6d6167910..61a671028a 100644
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -7115,6 +7115,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t
>> index, uint32_t count,
>>               !(env->hflags & HF_LMA_MASK)) {
>>               *edx &= ~CPUID_EXT2_SYSCALL;
>>           }
>> +
>> +        if (kvm_enabled() && IS_AMD_CPU(env) && !cpu->enable_pmu) {
>> +            *ecx &= ~CPUID_EXT3_PERFCORE;
>> +        }
>>           break;
>>       case 0x80000002:
>>       case 0x80000003:
>
Xiaoyao Li March 4, 2025, 6:11 a.m. UTC | #3
On 3/4/2025 2:45 AM, dongli.zhang@oracle.com wrote:
> Hi Xiaoyao,
> 
> On 3/2/25 5:59 PM, Xiaoyao Li wrote:
>> On 3/3/2025 6:00 AM, Dongli Zhang wrote:
>>> Currently, AMD PMU support isn't determined based on CPUID, that is, the
>>> "-pmu" option does not fully disable KVM AMD PMU virtualization.
>>>
>>> To minimize AMD PMU features, remove PERFCORE when "-pmu" is configured.
>>>
>>> To completely disable AMD PMU virtualization will be implemented via
>>> KVM_CAP_PMU_CAPABILITY in upcoming patches.
>>>
>>> As a reminder, neither CPUID_EXT3_PERFCORE nor
>>> CPUID_8000_0022_EAX_PERFMON_V2 is removed from env->features[] when "-pmu"
>>> is configured. Developers should query whether they are supported via
>>> cpu_x86_cpuid() rather than relying on env->features[] in future patches.
>>
>> I don't think it is the correct direction to go.
>>
>> env->features[] should be finalized before cpu_x86_cpuid() and env-
>>> features[] needs to be able to be exposed to guest directly. This ensures
>> guest and QEMU have the same view of CPUIDs and it simplifies things.
>>
>> We can adjust env->features[] by filtering all PMU related CPUIDs based on
>> cpu->enable_pmu in x86_cpu_realizefn().
> 
> Thank you very much for suggestion.
> 
> I see  code like below in x86_cpu_realizefn() to edit env->features[].
> 
> 7982     /* On AMD CPUs, some CPUID[8000_0001].EDX bits must match the bits on
> 7983      * CPUID[1].EDX.
> 7984      */
> 7985     if (IS_AMD_CPU(env)) {
> 7986         env->features[FEAT_8000_0001_EDX] &= ~CPUID_EXT2_AMD_ALIASES;
> 7987         env->features[FEAT_8000_0001_EDX] |= (env->features[FEAT_1_EDX]
> 7988            & CPUID_EXT2_AMD_ALIASES);
> 7989     }
> 
> I may do something similar to them for CPUID_EXT3_PERFCORE and
> CPUID_8000_0022_EAX_PERFMON_V2.

I just sent a series for CPUID_EXT_PDCM[1]. I think you can put 
CPUID_EXT3_PERFCORE and CPUID_8000_0022_EAX_PERFMON_V2 at the same place.

[1] 
https://lore.kernel.org/qemu-devel/20250304052450.465445-1-xiaoyao.li@intel.com/T/#m31c6777131b6361d7c3af22b09532bdc785dbc06

> Dongli Zhang
> 
> 
> 
>>
>>> Suggested-by: Zhao Liu <zhao1.liu@intel.com>
>>> Signed-off-by: Dongli Zhang <dongli.zhang@oracle.com>
>>> ---
>>>    target/i386/cpu.c | 4 ++++
>>>    1 file changed, 4 insertions(+)
>>>
>>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>>> index b6d6167910..61a671028a 100644
>>> --- a/target/i386/cpu.c
>>> +++ b/target/i386/cpu.c
>>> @@ -7115,6 +7115,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t
>>> index, uint32_t count,
>>>                !(env->hflags & HF_LMA_MASK)) {
>>>                *edx &= ~CPUID_EXT2_SYSCALL;
>>>            }
>>> +
>>> +        if (kvm_enabled() && IS_AMD_CPU(env) && !cpu->enable_pmu) {
>>> +            *ecx &= ~CPUID_EXT3_PERFCORE;
>>> +        }
>>>            break;
>>>        case 0x80000002:
>>>        case 0x80000003:
>>
>
diff mbox series

Patch

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index b6d6167910..61a671028a 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -7115,6 +7115,10 @@  void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
             !(env->hflags & HF_LMA_MASK)) {
             *edx &= ~CPUID_EXT2_SYSCALL;
         }
+
+        if (kvm_enabled() && IS_AMD_CPU(env) && !cpu->enable_pmu) {
+            *ecx &= ~CPUID_EXT3_PERFCORE;
+        }
         break;
     case 0x80000002:
     case 0x80000003: