[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH 1/1] ppc: spapr: Move VCPU ID calculation into sPA
From: |
Greg Kurz |
Subject: |
Re: [Qemu-ppc] [PATCH 1/1] ppc: spapr: Move VCPU ID calculation into sPAPR |
Date: |
Tue, 5 Sep 2017 16:58:59 +0200 |
On Tue, 5 Sep 2017 09:31:47 +1000
Sam Bobroff <address@hidden> wrote:
> On Mon, Sep 04, 2017 at 12:18:57PM +0200, Greg Kurz wrote:
> > On Thu, 31 Aug 2017 16:38:46 +1000
> > Sam Bobroff <address@hidden> wrote:
> >
> > > Move the calculation of a CPU's VCPU ID out of the generic PPC code
> > > (ppc_cpu_realizefn()) and into sPAPR specific code
> > > (spapr_cpu_core_realize()) where it belongs.
> > >
> > > Unfortunately, due to the way things are ordered, we still need to
> > > default the VCPU ID in ppc_cpu_realizfn() but at least doing that
> > > doesn't require any interaction with sPAPR.
> > >
> > > Signed-off-by: Sam Bobroff <address@hidden>
> > > ---
> > > This is follow up work arising from my work to clean up the way CPU VCPU
> > > IDs are
> > > handled on PowerPC. It had looked like it would be difficult to move the
> > > actual
> > > VCPU ID calculation out of generic code but it turned out to be OK.
> > >
> > > It's based on dgibson/ppc-for-2.11.
> > >
> > > Cheers,
> > > Sam.
> > >
> > > hw/ppc/spapr_cpu_core.c | 11 +++++++++++
> > > target/ppc/translate_init.c | 18 +++---------------
> > > 2 files changed, 14 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> > > index 5e319d9bbb..84dcc6e264 100644
> > > --- a/hw/ppc/spapr_cpu_core.c
> > > +++ b/hw/ppc/spapr_cpu_core.c
> > > @@ -211,6 +211,7 @@ error:
> > >
> > > static void spapr_cpu_core_realize(DeviceState *dev, Error **errp)
> > > {
> > > + sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> > > sPAPRCPUCore *sc = SPAPR_CPU_CORE(OBJECT(dev));
> > > sPAPRCPUCoreClass *scc = SPAPR_CPU_CORE_GET_CLASS(OBJECT(dev));
> > > CPUCore *cc = CPU_CORE(OBJECT(dev));
> > > @@ -237,6 +238,16 @@ static void spapr_cpu_core_realize(DeviceState *dev,
> > > Error **errp)
> > > cs = CPU(obj);
> > > cpu = POWERPC_CPU(cs);
> > > cs->cpu_index = cc->core_id + i;
> > > + cpu->vcpu_id = (cc->core_id * spapr->vsmt / smp_threads) + i;
> >
> > According to what we currently have in the generic code, this should be:
> >
> > ((cc->core_id + i) / smp_threads) * spapr->vsmt + ((cc->core_id + i) %
> > smp_threads)
> >
> > but since cc->core_id is a multiple of smp_threads and i < cc->nr_threads
> > <= smp_threads,
> > then we can indeed simplify the computation as you did. :)
> >
> > \o/
> >
> > Reviewed-by: Greg Kurz <address@hidden>
>
> Thanks for the review, and the nice simplification was David's idea :-)
>
Yeah, David said so earlier... I probably missed the mail with the
suggestion while I was on vacation. :)
> > > + if (kvm_enabled() && !kvm_vcpu_id_is_valid(cpu->vcpu_id)) {
> > > + error_setg(&local_err, "Can't create CPU with id %d in KVM",
> > > + cpu->vcpu_id);
> > > + error_append_hint(&local_err, "Adjust the number of cpus to
> > > %d "
> > > + "or try to raise the number of threads per
> > > core\n",
> > > + cpu->vcpu_id * smp_threads / spapr->vsmt);
> > > + goto err;
> > > + }
> > > +
> > >
> > > /* Set NUMA node for the threads belonged to core */
> > > cpu->node_id = sc->node_id;
> > > diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
> > > index 7f6a349e43..1f7286c893 100644
> > > --- a/target/ppc/translate_init.c
> > > +++ b/target/ppc/translate_init.c
> > > @@ -9903,28 +9903,15 @@ static void ppc_cpu_realizefn(DeviceState *dev,
> > > Error **errp)
> > > PowerPCCPU *cpu = POWERPC_CPU(dev);
> > > PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
> > > Error *local_err = NULL;
> > > -#if !defined(CONFIG_USER_ONLY)
> > > - int max_smt = kvmppc_smt_threads();
> > > -#endif
> > >
> > > cpu_exec_realizefn(cs, &local_err);
> > > if (local_err != NULL) {
> > > error_propagate(errp, local_err);
> > > return;
> > > }
> > > -
> > > -#if !defined(CONFIG_USER_ONLY)
> > > - cpu->vcpu_id = (cs->cpu_index / smp_threads) * max_smt
> > > - + (cs->cpu_index % smp_threads);
> > > -
> > > - if (kvm_enabled() && !kvm_vcpu_id_is_valid(cpu->vcpu_id)) {
> > > - error_setg(errp, "Can't create CPU with id %d in KVM",
> > > cpu->vcpu_id);
> > > - error_append_hint(errp, "Adjust the number of cpus to %d "
> > > - "or try to raise the number of threads per
> > > core\n",
> > > - cpu->vcpu_id * smp_threads / max_smt);
> > > - goto unrealize;
> > > + if (cpu->vcpu_id == UNASSIGNED_CPU_INDEX) {
> > > + cpu->vcpu_id = cs->cpu_index;
> > > }
> > > -#endif
> > >
> > > if (tcg_enabled()) {
> > > if (ppc_fixup_cpu(cpu) != 0) {
> > > @@ -10625,6 +10612,7 @@ static void ppc_cpu_initfn(Object *obj)
> > > CPUPPCState *env = &cpu->env;
> > >
> > > cs->env_ptr = env;
> > > + cpu->vcpu_id = UNASSIGNED_CPU_INDEX;
> > >
> > > env->msr_mask = pcc->msr_mask;
> > > env->mmu_model = pcc->mmu_model;
> >
>
>
pgpkGjZICqglY.pgp
Description: OpenPGP digital signature