| Message ID | 20250304052450.465445-1-xiaoyao.li@intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | i386: Adjust CPUID_EXT_PDCM based on enable_pmu at realization | expand |
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 >
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(-)