[PATCH 10/18] ppc: Add has_smt_siblings property to CPUPPCState

Nicholas Piggin posted 18 patches 4 months, 2 weeks ago
Maintainers: "Cédric Le Goater" <clg@kaod.org>, Nicholas Piggin <npiggin@gmail.com>, "Frédéric Barrat" <fbarrat@linux.ibm.com>, Daniel Henrique Barboza <danielhb413@gmail.com>, David Gibson <david@gibson.dropbear.id.au>, Harsh Prateek Bora <harshpb@linux.ibm.com>, Eduardo Habkost <eduardo@habkost.net>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Yanan Wang <wangyanan55@huawei.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>
There is a newer version of this series
[PATCH 10/18] ppc: Add has_smt_siblings property to CPUPPCState
Posted by Nicholas Piggin 4 months, 2 weeks ago
The decision to branch out to a slower SMT path in instruction
emulation will become a bit more complicated with the way that
"big-core" topology that will be implemented in subsequent changes.
Hide these details from the wider CPU emulation code with a bool
has_smt_siblings flag that can be set by machine initialisation.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/cpu.h        |  3 ++-
 hw/ppc/pnv_core.c       |  3 +++
 hw/ppc/spapr_cpu_core.c | 15 +++++++++++----
 3 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index a74b753c99..35a1cb65cb 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1248,6 +1248,7 @@ struct CPUArchState {
     int access_type;
 
     /* For SMT processors */
+    bool has_smt_siblings;
     int core_index;
 
 #if !defined(CONFIG_USER_ONLY)
@@ -1516,7 +1517,7 @@ struct PowerPCCPUClass {
 
 static inline bool ppc_cpu_core_single_threaded(CPUState *cs)
 {
-    return cs->nr_threads == 1;
+    return !POWERPC_CPU(cs)->env.has_smt_siblings;
 }
 
 static inline bool ppc_cpu_lpar_single_threaded(CPUState *cs)
diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index 7bda29b9c7..8cfa94fbfa 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -288,6 +288,9 @@ static void pnv_core_realize(DeviceState *dev, Error **errp)
         cpu = POWERPC_CPU(obj);
 
         pc->threads[i] = POWERPC_CPU(obj);
+        if (cc->nr_threads > 1) {
+            cpu->env.has_smt_siblings = true;
+        }
 
         snprintf(name, sizeof(name), "thread[%d]", i);
         object_property_add_child(OBJECT(pc), name, obj);
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index d9116c8409..2c6eeb41a4 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -314,12 +314,13 @@ static PowerPCCPU *spapr_create_vcpu(SpaprCpuCore *sc, int i, Error **errp)
      * and the rest are explicitly started up by the guest using an RTAS call.
      */
     qdev_prop_set_bit(DEVICE(obj), "start-powered-off", true);
-    env->core_index = cc->core_id;
     cs->cpu_index = cc->core_id + i;
     if (!spapr_set_vcpu_id(cpu, cs->cpu_index, errp)) {
         return NULL;
     }
 
+    env->core_index = cc->core_id;
+
     cpu->node_id = sc->node_id;
 
     id = g_strdup_printf("thread[%d]", i);
@@ -350,9 +351,15 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp)
     qemu_register_reset(spapr_cpu_core_reset_handler, sc);
     sc->threads = g_new0(PowerPCCPU *, cc->nr_threads);
     for (i = 0; i < cc->nr_threads; i++) {
-        sc->threads[i] = spapr_create_vcpu(sc, i, errp);
-        if (!sc->threads[i] ||
-            !spapr_realize_vcpu(sc->threads[i], spapr, sc, i, errp)) {
+        PowerPCCPU *cpu;
+
+        cpu = spapr_create_vcpu(sc, i, errp);
+        sc->threads[i] = cpu;
+        if (cpu && cc->nr_threads > 1) {
+            cpu->env.has_smt_siblings = true;
+        }
+
+        if (!cpu || !spapr_realize_vcpu(cpu, spapr, sc, i, errp)) {
             spapr_cpu_core_unrealize(dev);
             return;
         }
-- 
2.45.1
Re: [PATCH 10/18] ppc: Add has_smt_siblings property to CPUPPCState
Posted by Cédric Le Goater 4 months, 2 weeks ago
On 7/11/24 16:18, Nicholas Piggin wrote:
> The decision to branch out to a slower SMT path in instruction
> emulation will become a bit more complicated with the way that
> "big-core" topology that will be implemented in subsequent changes.
> Hide these details from the wider CPU emulation code with a bool
> has_smt_siblings flag that can be set by machine initialisation.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>


The distinction between what's done in pnv_core_realize() and
pnv_core_cpu_realize() is not clear. Not this patch fault though.

Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
>   target/ppc/cpu.h        |  3 ++-
>   hw/ppc/pnv_core.c       |  3 +++
>   hw/ppc/spapr_cpu_core.c | 15 +++++++++++----
>   3 files changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index a74b753c99..35a1cb65cb 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1248,6 +1248,7 @@ struct CPUArchState {
>       int access_type;
>   
>       /* For SMT processors */
> +    bool has_smt_siblings;
>       int core_index;
>   
>   #if !defined(CONFIG_USER_ONLY)
> @@ -1516,7 +1517,7 @@ struct PowerPCCPUClass {
>   
>   static inline bool ppc_cpu_core_single_threaded(CPUState *cs)
>   {
> -    return cs->nr_threads == 1;
> +    return !POWERPC_CPU(cs)->env.has_smt_siblings;
>   }
>   
>   static inline bool ppc_cpu_lpar_single_threaded(CPUState *cs)
> diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
> index 7bda29b9c7..8cfa94fbfa 100644
> --- a/hw/ppc/pnv_core.c
> +++ b/hw/ppc/pnv_core.c
> @@ -288,6 +288,9 @@ static void pnv_core_realize(DeviceState *dev, Error **errp)
>           cpu = POWERPC_CPU(obj);
>   
>           pc->threads[i] = POWERPC_CPU(obj);
> +        if (cc->nr_threads > 1) {
> +            cpu->env.has_smt_siblings = true;
> +        }
>   
>           snprintf(name, sizeof(name), "thread[%d]", i);
>           object_property_add_child(OBJECT(pc), name, obj);
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index d9116c8409..2c6eeb41a4 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -314,12 +314,13 @@ static PowerPCCPU *spapr_create_vcpu(SpaprCpuCore *sc, int i, Error **errp)
>        * and the rest are explicitly started up by the guest using an RTAS call.
>        */
>       qdev_prop_set_bit(DEVICE(obj), "start-powered-off", true);
> -    env->core_index = cc->core_id;
>       cs->cpu_index = cc->core_id + i;
>       if (!spapr_set_vcpu_id(cpu, cs->cpu_index, errp)) {
>           return NULL;
>       }
>   
> +    env->core_index = cc->core_id;
> +
>       cpu->node_id = sc->node_id;
>   
>       id = g_strdup_printf("thread[%d]", i);
> @@ -350,9 +351,15 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp)
>       qemu_register_reset(spapr_cpu_core_reset_handler, sc);
>       sc->threads = g_new0(PowerPCCPU *, cc->nr_threads);
>       for (i = 0; i < cc->nr_threads; i++) {
> -        sc->threads[i] = spapr_create_vcpu(sc, i, errp);
> -        if (!sc->threads[i] ||
> -            !spapr_realize_vcpu(sc->threads[i], spapr, sc, i, errp)) {
> +        PowerPCCPU *cpu;
> +
> +        cpu = spapr_create_vcpu(sc, i, errp);
> +        sc->threads[i] = cpu;
> +        if (cpu && cc->nr_threads > 1) {
> +            cpu->env.has_smt_siblings = true;
> +        }
> +
> +        if (!cpu || !spapr_realize_vcpu(cpu, spapr, sc, i, errp)) {
>               spapr_cpu_core_unrealize(dev);
>               return;
>           }