| Message ID | 20220214123116.1546406-1-npiggin@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | spapr: prevent hdec timer being set up under virtual hypervisor | expand |
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;
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;
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; >
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; >> > >
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;
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(-)