Re: [PATCH v16 21/23] accel: introduce new accessor functions

[Claudio Fontana]

On 2/8/21 1:54 PM, Philippe Mathieu-Daudé wrote:
> On 2/8/21 1:50 PM, Claudio Fontana wrote:
>> On 2/5/21 9:14 PM, Richard Henderson wrote:
>>> On 2/4/21 6:39 AM, Claudio Fontana wrote:
>>>> @@ -6725,10 +6722,8 @@ static void x86_cpu_initfn(Object *obj)
>>>>          x86_cpu_load_model(cpu, xcc->model);
>>>>      }
>>>>  
>>>> -    /* if required, do the accelerator-specific cpu initialization */
>>>> -    if (cc->accel_cpu) {
>>>> -        cc->accel_cpu->cpu_instance_init(CPU(obj));
>>>> -    }
>>>> +    /* if required, do accelerator-specific cpu initializations */
>>>> +    accel_cpu_instance_init(CPU(obj));
>>>>  }
>>>
>>> Why is this only done for x86?
>>>
>>>
>>> r~
>>>
>>
>> It makes sense to include the other architectures.
>>
>> As the next step I would like to apply this to ARM, but to me it makes sense 
>> to first complete Philippe's series,
>> which reshuffles things so that TCG-only / KVM-only builds are both possible 
>> and error-free:
>>
>> https://www.mail-archive.com/qemu-devel@nongnu.org/msg777627.html
> 
> TBH this series is very unlikely to be merged before yours,
> so go ahead... (eventually you can cherry-pick what you need
> from it).
> 

Hi Philippe, Peter,

I am working on ARM right now,

splitting things between user/system, tcg/kvm.

One difficulty I found in particular is with the ARM PMU code.

Currently the PMU code is mixed in with the TCG helpers, in target/arm/helper.c.

Now, with KVM we should be using the KVM PMU, but still the KVM code currently 
calls pmu_init and registers the timer:

In target/arm/cpu.c initialization we see:

"
    if (arm_feature(env, ARM_FEATURE_PMU)) {
        pmu_init(cpu);

        if (!kvm_enabled()) {
            arm_register_pre_el_change_hook(cpu, &pmu_pre_el_change, 0);
            arm_register_el_change_hook(cpu, &pmu_post_el_change, 0);
        }

#ifndef CONFIG_USER_ONLY
        cpu->pmu_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, arm_pmu_timer_cb,
                cpu);
#endif
"

Now, even for KVM, pmu_init is called and the cpu->pmu_timer is registered,
but I don't see arm_pmu_timer_cb triggering.

Is the pmu_timer really necessary also for KVM builds, or should it actually be 
TCG-only?

Could you share your hints on which parts of the PMU code in helper.c should 
actually be shared if any?

Thanks a lot,

Claudio