Re: [RFC v1 32/38] target/arm: cpu: do not initialize TCG PMU for KVM

[Claudio Fontana]

On 2/21/21 10:53 AM, Philippe Mathieu-Daudé wrote:
> On 2/21/21 10:24 AM, Claudio Fontana wrote:
>> From: Claudio Fontana <cfontana@centriq4.arch.suse.de>
>>
>> KVM uses its own PMU initialization.
>>
>> Signed-off-by: Claudio Fontana <cfontana@suse.de>
>> ---
>>  target/arm/cpu.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/target/arm/cpu.c b/target/arm/cpu.c
>> index a8321fecf8..d334987cad 100644
>> --- a/target/arm/cpu.c
>> +++ b/target/arm/cpu.c
>> @@ -1648,6 +1648,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error 
>> **errp)
>>          unset_feature(env, ARM_FEATURE_PMU);
>>      }
>>      if (arm_feature(env, ARM_FEATURE_PMU)) {
>> +#ifdef CONFIG_TCG
> 
> Shouldn't this be #if !defined(CONFIG_KVM) ?

In this series pmu is built only for tcg in tcg/, and assumes the current pmu 
software implementation is TCG-only.

Should PMU instead be a separate feature, that is pulled in by TCG but not KVM?

Do we for example expect other hypervisors (hvf...) to use this PMU?


> 
>>          pmu_init(cpu);
>>  
>>          if (!kvm_enabled()) {
> 
> And remove this ^

Rather, maybe the check should be if (tcg_enabled()).

I think the code quoted (also in master) handles the --enable-tcg --enable-kvm 
build.

If you have both tcg and kvm "available" (currently controlled by what code is 
built, but in the future probably it will be modules),

it makes sese to only initialize the software PMU if kvm is not enabled (or, 
expressed better, if tcg is enabled).



> 
>> @@ -1659,6 +1660,7 @@ static void arm_cpu_realizefn(DeviceState *dev, Error 
>> **errp)
>>          cpu->pmu_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, arm_pmu_timer_cb,
>>                  cpu);
>>  #endif
>> +#endif /* CONFIG_TCG */
>>      } else {
>>          cpu->isar.id_aa64dfr0 =
>>              FIELD_DP64(cpu->isar.id_aa64dfr0, ID_AA64DFR0, PMUVER, 0);
>>
>