All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 1/1] ppc: spapr: Move VCPU ID calculation into sPAPR
@ 2017-08-31  6:38 Sam Bobroff
  2017-09-04 10:18 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
       [not found] ` <20170904101945.A6B6EAC041@b01ledav006.gho.pok.ibm.com>
  0 siblings, 2 replies; 5+ messages in thread
From: Sam Bobroff @ 2017-08-31  6:38 UTC (permalink / raw)
  To: qemu-ppc, qemu-devel; +Cc: david

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 <sam.bobroff@au1.ibm.com>
---
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;
+        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;
-- 
2.14.1.4.g334a7be4f

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/1] ppc: spapr: Move VCPU ID calculation into sPAPR
  2017-08-31  6:38 [Qemu-devel] [PATCH 1/1] ppc: spapr: Move VCPU ID calculation into sPAPR Sam Bobroff
@ 2017-09-04 10:18 ` Greg Kurz
  2017-09-04 11:12   ` David Gibson
       [not found] ` <20170904101945.A6B6EAC041@b01ledav006.gho.pok.ibm.com>
  1 sibling, 1 reply; 5+ messages in thread
From: Greg Kurz @ 2017-09-04 10:18 UTC (permalink / raw)
  To: Sam Bobroff; +Cc: qemu-ppc, qemu-devel, david

[-- Attachment #1: Type: text/plain, Size: 4417 bytes --]

On Thu, 31 Aug 2017 16:38:46 +1000
Sam Bobroff <sam.bobroff@au1.ibm.com> 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 <sam.bobroff@au1.ibm.com>
> ---
> 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 <groug@kaod.org>

> +        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;


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/1] ppc: spapr: Move VCPU ID calculation into sPAPR
  2017-09-04 10:18 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
@ 2017-09-04 11:12   ` David Gibson
  0 siblings, 0 replies; 5+ messages in thread
From: David Gibson @ 2017-09-04 11:12 UTC (permalink / raw)
  To: Greg Kurz; +Cc: Sam Bobroff, qemu-ppc, qemu-devel

[-- Attachment #1: Type: text/plain, Size: 3956 bytes --]

On Mon, Sep 04, 2017 at 12:18:57PM +0200, Greg Kurz wrote:
> On Thu, 31 Aug 2017 16:38:46 +1000
> Sam Bobroff <sam.bobroff@au1.ibm.com> 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 <sam.bobroff@au1.ibm.com>
> > ---
> > 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.
> > 
> 
> 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. :)

Right, I suggested this simplication on just that basis.

Applied to ppc-for-2.11.

> 
> \o/
> 
> Reviewed-by: Greg Kurz <groug@kaod.org>
> 
> > +        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;
> 



-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/1] ppc: spapr: Move VCPU ID calculation into sPAPR
       [not found] ` <20170904101945.A6B6EAC041@b01ledav006.gho.pok.ibm.com>
@ 2017-09-04 23:31   ` Sam Bobroff
  2017-09-05 14:58     ` Greg Kurz
  0 siblings, 1 reply; 5+ messages in thread
From: Sam Bobroff @ 2017-09-04 23:31 UTC (permalink / raw)
  To: Greg Kurz; +Cc: qemu-ppc, qemu-devel, david

On Mon, Sep 04, 2017 at 12:18:57PM +0200, Greg Kurz wrote:
> On Thu, 31 Aug 2017 16:38:46 +1000
> Sam Bobroff <sam.bobroff@au1.ibm.com> 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 <sam.bobroff@au1.ibm.com>
> > ---
> > 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 <groug@kaod.org>

Thanks for the review, and the nice simplification was David's idea :-)

> > +        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;
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [Qemu-devel] [Qemu-ppc] [PATCH 1/1] ppc: spapr: Move VCPU ID calculation into sPAPR
  2017-09-04 23:31   ` Sam Bobroff
@ 2017-09-05 14:58     ` Greg Kurz
  0 siblings, 0 replies; 5+ messages in thread
From: Greg Kurz @ 2017-09-05 14:58 UTC (permalink / raw)
  To: Sam Bobroff; +Cc: qemu-ppc, qemu-devel, david

[-- Attachment #1: Type: text/plain, Size: 5215 bytes --]

On Tue, 5 Sep 2017 09:31:47 +1000
Sam Bobroff <sam.bobroff@au1.ibm.com> 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 <sam.bobroff@au1.ibm.com> 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 <sam.bobroff@au1.ibm.com>
> > > ---
> > > 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 <groug@kaod.org>  
> 
> 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;  
> >   
> 
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-09-05 14:59 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-31  6:38 [Qemu-devel] [PATCH 1/1] ppc: spapr: Move VCPU ID calculation into sPAPR Sam Bobroff
2017-09-04 10:18 ` [Qemu-devel] [Qemu-ppc] " Greg Kurz
2017-09-04 11:12   ` David Gibson
     [not found] ` <20170904101945.A6B6EAC041@b01ledav006.gho.pok.ibm.com>
2017-09-04 23:31   ` Sam Bobroff
2017-09-05 14:58     ` Greg Kurz

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.