Re: [PATCH v7 2/9] target/i386: disable PERFCORE when "-pmu" is configured

[Dongli Zhang <dongli.zhang@oracle.com>]

Hi Zhao,

On 11/19/25 3:06 AM, Zhao Liu wrote:
> Hi Dongli,
> 
>> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
>> index 3653f8953e..4fcade89bc 100644
>> --- a/target/i386/cpu.c
>> +++ b/target/i386/cpu.c
>> @@ -8360,6 +8360,10 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
>>              !(env->hflags & HF_LMA_MASK)) {
>>              *edx &= ~CPUID_EXT2_SYSCALL;
>>          }
>> +
>> +        if (!cpu->enable_pmu) {
>> +            *ecx &= ~CPUID_EXT3_PERFCORE;
>> +        }
> 
> Learned the lessons from PDCM [*]:
> 
> [*] https://urldefense.com/v3/__https://lore.kernel.org/qemu-devel/20250923104136.133875-3-pbonzini@redhat.com/__;!!ACWV5N9M2RV99hQ!IGoeOCDjD7razBgwrJ-A9SjI1XIwq5kxmkPA5_NLOucYzMbsbCOQFe3jc7YyN0Ks1bNpP70F77klp0_--kV1$ 
> 
> Directly masking CPUID bit off is not a good idea... modifying the
> CPUID, even when fixing or adding dependencies, can easily break
> migration.
> 
> So a safe way is to add a compat option. And I think it would be better
> if patch 1 also has a compat option. A single compat option could cover
> patch 1 & 2.
> 
> I have these thoughts:
> 
> * For "-pmu" dependency, it can be checked as [*] did.
> * For normal feature bit dependency (patch 1), it seems possible to add
>   compat_prop condition in feature_dependencies[].
> 
> I attached a draft for discussion (which is on the top of the whole
> series).
> 
> Note, we are currently in the RC phase for v10.2, and the
> pc_compat_10_2[] array is not yet added, which will be added before the
> release of v10.2. Therefore, we have enough time for discussion I think.

Thank you very much for feedback!

It is very important to maintain live migration compatibility.

> 
> If you think it's fine, I'll post compat_prop support separately as an

I think it's fine. I don’t have a better idea. Please send the patch for further
discussion.

> RFC. The specific compat option can be left to this series.

Please let me know whether you would like to include Patch 2 on
"amd_perfmon_always_on" as part of the "compat_prop" patch, or if you'd prefer
that I re-create Patch 2 with your Suggested-by.

Either option works for me.


It seems the Patches 3 - 9 are not impacted by this Live Migration issue.
Perhaps they may be accepted (or as well as Patch 2 "amd_perfmon_always_on")
without "compat_prop" patch? They are independent with each other.

Another concern is Patch 3. Something unexpected may occur when live migrating
from a KVM host without KVM_PMU_CAP_DISABLE to one that has it enabled. The
migration will succeed, but the perceived perf/vPMU support could change.
Can we assume it is the user's responsibility to ensure compatibility between
KVM hosts when "-pmu" is specified?

> 
> Although wait for a while, in a way, it is lucky :) - there's an
> opportunity to avoid making mistakes for us.
> 

Thank you very much!

Dongli Zhang