On 5/26/24 17:56, Nicholas Piggin wrote:
The way SMT thread siblings are matched is clunky, using hard-coded
logic that checks the PIR SPR.
Change that to use a new core_index variable in the CPUPPCState,
where all siblings have the same core_index. CPU realize routines have
flexibility in setting core/sibling topology.
Signed-off-by: Nicholas Piggin <npig...@gmail.com>
---
target/ppc/cpu.h | 5 ++++-
hw/ppc/pnv_core.c | 2 ++
hw/ppc/spapr_cpu_core.c | 3 +++
3 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index dac13d4dac..9a89083932 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1247,6 +1247,9 @@ struct CPUArchState {
/* when a memory exception occurs, the access type is stored here */
int access_type;
+ /* For SMT processors */
+ int core_index;
+
#if !defined(CONFIG_USER_ONLY)
/* MMU context, only relevant for full system emulation */
#if defined(TARGET_PPC64)
@@ -1404,7 +1407,7 @@ struct CPUArchState {
};
#define _CORE_ID(cs) \
- (POWERPC_CPU(cs)->env.spr_cb[SPR_PIR].default_value & ~(cs->nr_threads -
1))
+ (POWERPC_CPU(cs)->env.core_index)
Dont we want to keep the logical & with ~(cs->nr_threads - 1) ?
How's it taken care ?
#define THREAD_SIBLING_FOREACH(cs, cs_sibling) \
CPU_FOREACH(cs_sibling) \
diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
index 9b5edd9e48..0f61aabb77 100644
--- a/hw/ppc/pnv_core.c
+++ b/hw/ppc/pnv_core.c
@@ -252,6 +252,8 @@ static void pnv_core_cpu_realize(PnvCore *pc, PowerPCCPU
*cpu, Error **errp,
pir_spr->default_value = pir;
tir_spr->default_value = tir;
+ env->core_index = core_hwid;
+
/* Set time-base frequency to 512 MHz */
cpu_ppc_tb_init(env, PNV_TIMEBASE_FREQ);
}
diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index e7c9edd033..059d372c8a 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -300,16 +300,19 @@ static PowerPCCPU *spapr_create_vcpu(SpaprCpuCore *sc,
int i, Error **errp)
g_autofree char *id = NULL;
CPUState *cs;
PowerPCCPU *cpu;
+ CPUPPCState *env;
obj = object_new(scc->cpu_type);
cs = CPU(obj);
cpu = POWERPC_CPU(obj);
+ env = &cpu->env;
/*
* All CPUs start halted. CPU0 is unhalted from the machine level reset
code
* 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;
We could just do cpu->env.core_index and avoid creating local var env.
regards,
Harsh
cs->cpu_index = cc->core_id + i;
if (!spapr_set_vcpu_id(cpu, cs->cpu_index, errp)) {
return NULL;
