Hi Zhao,
On 3/6/25 8:50 AM, Zhao Liu wrote:
> Hi Dongli,
>
>> 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) {
>
> No need to check "kvm_enabled() && IS_AMD_CPU(env)" because:
>
> * "pmu" is a general CPU property option which should cover all PMU
> related features, and not kvm-specific/vendor-specific.
> * this bit is reserved on Intel. So the following operation doesn't
> affect Intel.
>
> I think Xiaoyao's idea about checking in x86_cpu_expand_features() is
> good. And I believe it's worth having another cleanup series to revisit
> pmu dependencies. I can help you later to consolidate and move this
> check to x86_cpu_expand_features(), so this patch can focus on correctly
> defining the current dependency relationship.
That means I don't need to change anything except:
1. Remove "kvm_enabled() && IS_AMD_CPU(env)" since the bit is reserved by
Intel.
2. Add your Reviewed-by.
Thank you very much!
Dongli Zhang
>
> With the above nit fixed,
>
> Reviewed-by: Zhao Liu <zhao1.liu@xxxxxxxxx>
>
>
>
