1. Patchew
  2. QEMU
  3. View series

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

Nicholas Piggin posted 1 patch 3 years, 11 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220214123116.1546406-1-npiggin@gmail.com
Maintainers: David Gibson <david@gibson.dropbear.id.au>, Greg Kurz <groug@kaod.org>, Daniel Henrique Barboza <danielhb413@gmail.com>, "Cédric Le Goater" <clg@kaod.org>
hw/ppc/ppc.c            | 2 +-
hw/ppc/spapr_cpu_core.c | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
[PATCH] spapr: prevent hdec timer being set up under virtual hypervisor
Posted by Nicholas Piggin 3 years, 11 months ago 
The spapr virtual hypervisor does not require the hdecr timer.
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) {
         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;
-- 
2.23.0
Re: [PATCH] spapr: prevent hdec timer being set up under virtual hypervisor
Posted by Daniel Henrique Barboza 3 years, 11 months ago 

On 2/14/22 09:31, Nicholas Piggin wrote:
> The spapr virtual hypervisor does not require the hdecr timer.
> Remove it.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@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) {
>           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;
Re: [PATCH] spapr: prevent hdec timer being set up under virtual hypervisor
Posted by Fabiano Rosas 3 years, 11 months ago 
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?

> 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) {
>          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;
Re: [PATCH] spapr: prevent hdec timer being set up under virtual hypervisor
Posted by Cédric Le Goater 3 years, 11 months ago 
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?

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 ?

>> 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;
>
Re: [PATCH] spapr: prevent hdec timer being set up under virtual hypervisor
Posted by Nicholas Piggin 3 years, 11 months ago 
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;
>> 
> 
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.