Re: [PATCH] spapr: prevent hdec timer being set up under virtual hypervisor

[Nicholas Piggin]

Excerpts from Cédric Le Goater's message of February 15, 2022 4:05 am:
> On 2/14/22 14:32, Fabiano Rosas wrote:
>> Nicholas Piggin <npiggin@gmail.com> writes:
>> 
>>> The spapr virtual hypervisor does not require the hdecr timer.
>> 
>> Why is that? Is this not needed for an emulated powernv running KVM
>> guests?

Yeah, powernv does not use the virtual hypervisor, that's just for
pseries.

> It is now also running in the QEMU pseries machine (L0) when a
> nested is running. That's part of the implementation Nick did for
> the KVM-on-pseries-in-QEMU-TCG.
> 
> But isn't that redundant with the cpu_ppc_hdecr_init() and
> cpu_ppc_hdecr_exit() from the RFC ? and shouldn't that be
> created only once, when the first nested is started or when
> the machine is if cap-nested-hv=on ?

For now I create the timer for a vcpu while it is in H_ENTER_NESTED
and destroy it when the hcall returns.

Thanks,
Nick

> 
>>> Remove it.
>>>
>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>>> ---
>>>   hw/ppc/ppc.c            | 2 +-
>>>   hw/ppc/spapr_cpu_core.c | 6 +++---
>>>   2 files changed, 4 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
>>> index 462c87dba8..a7c262db93 100644
>>> --- a/hw/ppc/ppc.c
>>> +++ b/hw/ppc/ppc.c
>>> @@ -1072,7 +1072,7 @@ clk_setup_cb cpu_ppc_tb_init (CPUPPCState *env, 
>>> uint32_t freq)
>>>       }
>>>       /* Create new timer */
>>>       tb_env->decr_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, 
>>> &cpu_ppc_decr_cb, cpu);
>>> -    if (env->has_hv_mode) {
>>> +    if (env->has_hv_mode && !cpu->vhyp) {
> 
> hmm, Can we have (env->has_hv_mode && cpu->vhyp) ?  this is confusing :/
> 
> Thanks,
> 
> C.
> 
> 
>>>           tb_env->hdecr_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, 
>>> &cpu_ppc_hdecr_cb,
>>>                                                   cpu);
>>>       } else {
>>> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
>>> index a781e97f8d..ed84713960 100644
>>> --- a/hw/ppc/spapr_cpu_core.c
>>> +++ b/hw/ppc/spapr_cpu_core.c
>>> @@ -261,12 +261,12 @@ static bool spapr_realize_vcpu(PowerPCCPU *cpu, 
>>> SpaprMachineState *spapr,
>>>           return false;
>>>       }
>>>   
>>> -    /* Set time-base frequency to 512 MHz */
>>> -    cpu_ppc_tb_init(env, SPAPR_TIMEBASE_FREQ);
>>> -
>>>       cpu_ppc_set_vhyp(cpu, PPC_VIRTUAL_HYPERVISOR(spapr));
>>>       kvmppc_set_papr(cpu);
>>>   
>>> +    /* Set time-base frequency to 512 MHz. vhyp must be set first. */
>>> +    cpu_ppc_tb_init(env, SPAPR_TIMEBASE_FREQ);
>>> +
>>>       if (spapr_irq_cpu_intc_create(spapr, cpu, errp) < 0) {
>>>           qdev_unrealize(DEVICE(cpu));
>>>           return false;
>> 
> 
>