First, it's not a good practice that values in env->features[] cannot be
directly used for guest CPUID in void cpu_x86_cpuid(), but require further
adjustment there. env->features[] are supposed to be finalized at cpu
realization, so that after it env->features[] is reliable.

Second, there is one dependency entry relates to CPUID_EXT_PDCM in
feature_dependencies[]. QEMU needs to get correct value of
CPUID_EXT_PDCM in env->features[] to ensure applying the dependencies
correctly.

Patch 1 resolves above two points.

Patch 2 is a enhancement to give users a warning when they request pdcm
explicitly while PMU disabled.

Xiaoyao Li (2):
  i386/cpu: Move adjustment of CPUID_EXT_PDCM before
    feature_dependencies[] check
  i386/cpu: Warn about why CPUID_EXT_PDCM is not available

 target/i386/cpu.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

-- 
2.34.1

Queued, thanks; sorry about the delay.

Paolo

Hi Xiaoyao,

> First, it's not a good practice that values in env->features[] cannot be
> directly used for guest CPUID in void cpu_x86_cpuid(), but require further
> adjustment there. env->features[] are supposed to be finalized at cpu
> realization, so that after it env->features[] is reliable.
> 
> Second, there is one dependency entry relates to CPUID_EXT_PDCM in
> feature_dependencies[]. QEMU needs to get correct value of
> CPUID_EXT_PDCM in env->features[] to ensure applying the dependencies
> correctly.

I agree that this is a very good idea, especially since PDCM has a
dependency entry.

"pmu" is totally a property rather than a feature bit, which makes the
dependency relationships in the code complex. Therefore, I think it's
worth having a series to clarify the dependencies of pmu as much as
possible.

I remember Dapeng/Zide also have fixes for pmu dependencies, and if
possible, I could help you combine this series with others' cleanups.

Additionally, I think patch 1 and patch 2 can be merged together. Do you
agree?

Thanks,
Zhao

On 3/7/2025 12:22 AM, Zhao Liu wrote:
> Hi Xiaoyao,
> 
>> First, it's not a good practice that values in env->features[] cannot be
>> directly used for guest CPUID in void cpu_x86_cpuid(), but require further
>> adjustment there. env->features[] are supposed to be finalized at cpu
>> realization, so that after it env->features[] is reliable.
>>
>> Second, there is one dependency entry relates to CPUID_EXT_PDCM in
>> feature_dependencies[]. QEMU needs to get correct value of
>> CPUID_EXT_PDCM in env->features[] to ensure applying the dependencies
>> correctly.
> 
> I agree that this is a very good idea, especially since PDCM has a
> dependency entry.
> 
> "pmu" is totally a property rather than a feature bit, which makes the
> dependency relationships in the code complex. Therefore, I think it's
> worth having a series to clarify the dependencies of pmu as much as
> possible.
> 
> I remember Dapeng/Zide also have fixes for pmu dependencies, and if
> possible, I could help you combine this series with others' cleanups.

The reason I sent out this small series quickly is mainly for Dongli to 
as a reference.

In fact, there are mess on LBR enabling that it checks enable_pmu 
everytime with CPUID_7_0_EDX_ARCH_LBR as well as 
CPUID_8000_0022_EAX_PERFMON_V2. That's on my WIP list to clean it up.

I think I need to check if they are duplicated with Dapeng/Zide's series.

> Additionally, I think patch 1 and patch 2 can be merged together. Do you
> agree?

IMHO, they stand as their own. I'll leave it to Paolo to make the decision.

> Thanks,
> Zhao
>