Re: [PATCH 3/7] target/i386/kvm: init PMU information only once

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Zhao,

On 11/10/24 7:29 AM, Zhao Liu wrote:
> Hi Dongli,
> 
>>  int kvm_arch_init_vcpu(CPUState *cs)
>>  {
>>      struct {
>> @@ -2237,6 +2247,13 @@ int kvm_arch_init_vcpu(CPUState *cs)
>>      cpuid_i = kvm_x86_build_cpuid(env, cpuid_data.entries, cpuid_i);
>>      cpuid_data.cpuid.nent = cpuid_i;
>>  
>> +    /*
>> +     * Initialize PMU information only once for the first vCPU.
>> +     */
>> +    if (cs == first_cpu) {
>> +        kvm_init_pmu_info(env);
>> +    }
>> +
> 
> Thank you for the optimization. However, I think it’s not necessary
> because:
> 
> * This is not a hot path and not a performance bottleneck.
> * Many CPUID leaves are consistent across CPUs, and 0xA is just one of them.
> * And encoding them all in kvm_x86_build_cpuid() is a common pattern.
>   Separating out 0xa disrupts code readability and fragments the CPUID encoding.
> 
> Therefore, code maintainability and correctness might be more important here,
> than performance concern.

I am going to remove this patch in v2.

Just a reminder, we may have more code in this function by other patches,
including the initialization of both Intel and AMD PMU infortmation (PerfMonV2).

Thank you very much!

Dongli Zhang

> 
>>      if (((env->cpuid_version >> 8)&0xF) >= 6
>>          && (env->features[FEAT_1_EDX] & (CPUID_MCE | CPUID_MCA)) ==
>>             (CPUID_MCE | CPUID_MCA)) {
>> -- 
>> 2.39.3
>>





[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux