| Message ID | 20240526122612.473476-8-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ppc/pnv: Better big-core model, lpar-per-core, PC unit | expand |
On 5/26/24 17:56, Nicholas Piggin wrote: > Add helpers for TCG code to determine if there are SMT siblings > sharing per-core and per-lpar registers. This simplifies the > callers and makes SMT register topology simpler to modify with > later changes. > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- > target/ppc/cpu.h | 7 +++++++ > target/ppc/cpu_init.c | 2 +- > target/ppc/excp_helper.c | 16 +++++++--------- > target/ppc/misc_helper.c | 27 ++++++--------------------- > target/ppc/timebase_helper.c | 20 +++++++------------- > 5 files changed, 28 insertions(+), 44 deletions(-) > > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h > index 9a89083932..8fd6ade471 100644 > --- a/target/ppc/cpu.h > +++ b/target/ppc/cpu.h > @@ -1406,6 +1406,13 @@ struct CPUArchState { > uint64_t pmu_base_time; > }; > > +#define PPC_CPU_HAS_CORE_SIBLINGS(cs) \ > + (cs->nr_threads > 1) > + > +#define PPC_CPU_HAS_LPAR_SIBLINGS(cs) \ > + ((POWERPC_CPU(cs)->env.flags & POWERPC_FLAG_SMT_1LPAR) && \ > + PPC_CPU_HAS_CORE_SIBLINGS(cs)) > + > #define _CORE_ID(cs) \ > (POWERPC_CPU(cs)->env.core_index) > > diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c > index ae483e20c4..e71ee008ed 100644 > --- a/target/ppc/cpu_init.c > +++ b/target/ppc/cpu_init.c > @@ -6975,7 +6975,7 @@ static void ppc_cpu_realize(DeviceState *dev, Error **errp) > > pcc->parent_realize(dev, errp); > > - if (env_cpu(env)->nr_threads > 1) { > + if (PPC_CPU_HAS_CORE_SIBLINGS(cs)) { > env->flags |= POWERPC_FLAG_SMT; > } > > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c > index 0cd542675f..fd45da0f2b 100644 > --- a/target/ppc/excp_helper.c > +++ b/target/ppc/excp_helper.c > @@ -3029,7 +3029,7 @@ void helper_book3s_msgsnd(CPUPPCState *env, target_ulong rb) > brdcast = true; > } > > - if (cs->nr_threads == 1 || !brdcast) { > + if (!PPC_CPU_HAS_CORE_SIBLINGS(cs) || !brdcast) { Since there are multiple usage of above macro in negation below as well, we may probably want to introduce another macro PPC_CPU_HAS_SINGLE_CORE which checks only for nr_threads == 1. Anyways, Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com> > ppc_set_irq(cpu, PPC_INTERRUPT_HDOORBELL, 1); > return; > } > @@ -3067,21 +3067,19 @@ void helper_book3s_msgsndp(CPUPPCState *env, target_ulong rb) > CPUState *cs = env_cpu(env); > PowerPCCPU *cpu = env_archcpu(env); > CPUState *ccs; > - uint32_t nr_threads = cs->nr_threads; > int ttir = rb & PPC_BITMASK(57, 63); > > helper_hfscr_facility_check(env, HFSCR_MSGP, "msgsndp", HFSCR_IC_MSGP); > > - if (!(env->flags & POWERPC_FLAG_SMT_1LPAR)) { > - nr_threads = 1; /* msgsndp behaves as 1-thread in LPAR-per-thread mode*/ > - } > - > - if (!dbell_type_server(rb) || ttir >= nr_threads) { > + if (!dbell_type_server(rb)) { > return; > } > > - if (nr_threads == 1) { > - ppc_set_irq(cpu, PPC_INTERRUPT_DOORBELL, 1); > + /* msgsndp behaves as 1-thread in LPAR-per-thread mode*/ > + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { > + if (ttir == 0) { > + ppc_set_irq(cpu, PPC_INTERRUPT_DOORBELL, 1); > + } > return; > } > > diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c > index 46ba3a5584..598c956cdd 100644 > --- a/target/ppc/misc_helper.c > +++ b/target/ppc/misc_helper.c > @@ -49,9 +49,8 @@ void helper_spr_core_write_generic(CPUPPCState *env, uint32_t sprn, > { > CPUState *cs = env_cpu(env); > CPUState *ccs; > - uint32_t nr_threads = cs->nr_threads; > > - if (nr_threads == 1) { > + if (!PPC_CPU_HAS_CORE_SIBLINGS(cs)) { > env->spr[sprn] = val; > return; > } > @@ -196,7 +195,7 @@ void helper_store_ptcr(CPUPPCState *env, target_ulong val) > return; > } > > - if (cs->nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) { > + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { > env->spr[SPR_PTCR] = val; > tlb_flush(cs); > } else { > @@ -243,16 +242,12 @@ target_ulong helper_load_dpdes(CPUPPCState *env) > { > CPUState *cs = env_cpu(env); > CPUState *ccs; > - uint32_t nr_threads = cs->nr_threads; > target_ulong dpdes = 0; > > helper_hfscr_facility_check(env, HFSCR_MSGP, "load DPDES", HFSCR_IC_MSGP); > > - if (!(env->flags & POWERPC_FLAG_SMT_1LPAR)) { > - nr_threads = 1; /* DPDES behaves as 1-thread in LPAR-per-thread mode */ > - } > - > - if (nr_threads == 1) { > + /* DPDES behaves as 1-thread in LPAR-per-thread mode */ > + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { > if (env->pending_interrupts & PPC_INTERRUPT_DOORBELL) { > dpdes = 1; > } > @@ -279,21 +274,11 @@ void helper_store_dpdes(CPUPPCState *env, target_ulong val) > PowerPCCPU *cpu = env_archcpu(env); > CPUState *cs = env_cpu(env); > CPUState *ccs; > - uint32_t nr_threads = cs->nr_threads; > > helper_hfscr_facility_check(env, HFSCR_MSGP, "store DPDES", HFSCR_IC_MSGP); > > - if (!(env->flags & POWERPC_FLAG_SMT_1LPAR)) { > - nr_threads = 1; /* DPDES behaves as 1-thread in LPAR-per-thread mode */ > - } > - > - if (val & ~(nr_threads - 1)) { > - qemu_log_mask(LOG_GUEST_ERROR, "Invalid DPDES register value " > - TARGET_FMT_lx"\n", val); > - val &= (nr_threads - 1); /* Ignore the invalid bits */ > - } > - > - if (nr_threads == 1) { > + /* DPDES behaves as 1-thread in LPAR-per-thread mode */ > + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { > ppc_set_irq(cpu, PPC_INTERRUPT_DOORBELL, val & 0x1); > return; > } > diff --git a/target/ppc/timebase_helper.c b/target/ppc/timebase_helper.c > index 788c498d63..abe7b95696 100644 > --- a/target/ppc/timebase_helper.c > +++ b/target/ppc/timebase_helper.c > @@ -63,9 +63,8 @@ void helper_store_purr(CPUPPCState *env, target_ulong val) > { > CPUState *cs = env_cpu(env); > CPUState *ccs; > - uint32_t nr_threads = cs->nr_threads; > > - if (nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) { > + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { > cpu_ppc_store_purr(env, val); > return; > } > @@ -82,9 +81,8 @@ void helper_store_tbl(CPUPPCState *env, target_ulong val) > { > CPUState *cs = env_cpu(env); > CPUState *ccs; > - uint32_t nr_threads = cs->nr_threads; > > - if (nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) { > + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { > cpu_ppc_store_tbl(env, val); > return; > } > @@ -99,9 +97,8 @@ void helper_store_tbu(CPUPPCState *env, target_ulong val) > { > CPUState *cs = env_cpu(env); > CPUState *ccs; > - uint32_t nr_threads = cs->nr_threads; > > - if (nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) { > + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { > cpu_ppc_store_tbu(env, val); > return; > } > @@ -141,9 +138,8 @@ void helper_store_hdecr(CPUPPCState *env, target_ulong val) > { > CPUState *cs = env_cpu(env); > CPUState *ccs; > - uint32_t nr_threads = cs->nr_threads; > > - if (nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) { > + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { > cpu_ppc_store_hdecr(env, val); > return; > } > @@ -158,9 +154,8 @@ void helper_store_vtb(CPUPPCState *env, target_ulong val) > { > CPUState *cs = env_cpu(env); > CPUState *ccs; > - uint32_t nr_threads = cs->nr_threads; > > - if (nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) { > + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { > cpu_ppc_store_vtb(env, val); > return; > } > @@ -175,9 +170,8 @@ void helper_store_tbu40(CPUPPCState *env, target_ulong val) > { > CPUState *cs = env_cpu(env); > CPUState *ccs; > - uint32_t nr_threads = cs->nr_threads; > > - if (nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) { > + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { > cpu_ppc_store_tbu40(env, val); > return; > } > @@ -288,7 +282,7 @@ static void write_tfmr(CPUPPCState *env, target_ulong val) > { > CPUState *cs = env_cpu(env); > > - if (cs->nr_threads == 1) { > + if (!PPC_CPU_HAS_CORE_SIBLINGS(cs)) { > env->spr[SPR_TFMR] = val; > } else { > CPUState *ccs;
On Tue May 28, 2024 at 7:16 PM AEST, Harsh Prateek Bora wrote: > > > On 5/26/24 17:56, Nicholas Piggin wrote: > > Add helpers for TCG code to determine if there are SMT siblings > > sharing per-core and per-lpar registers. This simplifies the > > callers and makes SMT register topology simpler to modify with > > later changes. > > > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > > --- > > target/ppc/cpu.h | 7 +++++++ > > target/ppc/cpu_init.c | 2 +- > > target/ppc/excp_helper.c | 16 +++++++--------- > > target/ppc/misc_helper.c | 27 ++++++--------------------- > > target/ppc/timebase_helper.c | 20 +++++++------------- > > 5 files changed, 28 insertions(+), 44 deletions(-) > > > > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h > > index 9a89083932..8fd6ade471 100644 > > --- a/target/ppc/cpu.h > > +++ b/target/ppc/cpu.h > > @@ -1406,6 +1406,13 @@ struct CPUArchState { > > uint64_t pmu_base_time; > > }; > > > > +#define PPC_CPU_HAS_CORE_SIBLINGS(cs) \ > > + (cs->nr_threads > 1) > > + > > +#define PPC_CPU_HAS_LPAR_SIBLINGS(cs) \ > > + ((POWERPC_CPU(cs)->env.flags & POWERPC_FLAG_SMT_1LPAR) && \ > > + PPC_CPU_HAS_CORE_SIBLINGS(cs)) > > + > > #define _CORE_ID(cs) \ > > (POWERPC_CPU(cs)->env.core_index) > > > > diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c > > index ae483e20c4..e71ee008ed 100644 > > --- a/target/ppc/cpu_init.c > > +++ b/target/ppc/cpu_init.c > > @@ -6975,7 +6975,7 @@ static void ppc_cpu_realize(DeviceState *dev, Error **errp) > > > > pcc->parent_realize(dev, errp); > > > > - if (env_cpu(env)->nr_threads > 1) { > > + if (PPC_CPU_HAS_CORE_SIBLINGS(cs)) { > > env->flags |= POWERPC_FLAG_SMT; > > } > > > > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c > > index 0cd542675f..fd45da0f2b 100644 > > --- a/target/ppc/excp_helper.c > > +++ b/target/ppc/excp_helper.c > > @@ -3029,7 +3029,7 @@ void helper_book3s_msgsnd(CPUPPCState *env, target_ulong rb) > > brdcast = true; > > } > > > > - if (cs->nr_threads == 1 || !brdcast) { > > + if (!PPC_CPU_HAS_CORE_SIBLINGS(cs) || !brdcast) { > > Since there are multiple usage of above macro in negation below as well, > we may probably want to introduce another macro PPC_CPU_HAS_SINGLE_CORE Ah, you mean SINGLE_THREAD. Yes it would read a bit better. Thanks, Nick > which checks only for nr_threads == 1. Anyways, > > Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com> > > > > ppc_set_irq(cpu, PPC_INTERRUPT_HDOORBELL, 1); > > return; > > } > > @@ -3067,21 +3067,19 @@ void helper_book3s_msgsndp(CPUPPCState *env, target_ulong rb) > > CPUState *cs = env_cpu(env); > > PowerPCCPU *cpu = env_archcpu(env); > > CPUState *ccs; > > - uint32_t nr_threads = cs->nr_threads; > > int ttir = rb & PPC_BITMASK(57, 63); > > > > helper_hfscr_facility_check(env, HFSCR_MSGP, "msgsndp", HFSCR_IC_MSGP); > > > > - if (!(env->flags & POWERPC_FLAG_SMT_1LPAR)) { > > - nr_threads = 1; /* msgsndp behaves as 1-thread in LPAR-per-thread mode*/ > > - } > > - > > - if (!dbell_type_server(rb) || ttir >= nr_threads) { > > + if (!dbell_type_server(rb)) { > > return; > > } > > > > - if (nr_threads == 1) { > > - ppc_set_irq(cpu, PPC_INTERRUPT_DOORBELL, 1); > > + /* msgsndp behaves as 1-thread in LPAR-per-thread mode*/ > > + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { > > + if (ttir == 0) { > > + ppc_set_irq(cpu, PPC_INTERRUPT_DOORBELL, 1); > > + } > > return; > > } > > > > diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c > > index 46ba3a5584..598c956cdd 100644 > > --- a/target/ppc/misc_helper.c > > +++ b/target/ppc/misc_helper.c > > @@ -49,9 +49,8 @@ void helper_spr_core_write_generic(CPUPPCState *env, uint32_t sprn, > > { > > CPUState *cs = env_cpu(env); > > CPUState *ccs; > > - uint32_t nr_threads = cs->nr_threads; > > > > - if (nr_threads == 1) { > > + if (!PPC_CPU_HAS_CORE_SIBLINGS(cs)) { > > env->spr[sprn] = val; > > return; > > } > > @@ -196,7 +195,7 @@ void helper_store_ptcr(CPUPPCState *env, target_ulong val) > > return; > > } > > > > - if (cs->nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) { > > + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { > > env->spr[SPR_PTCR] = val; > > tlb_flush(cs); > > } else { > > @@ -243,16 +242,12 @@ target_ulong helper_load_dpdes(CPUPPCState *env) > > { > > CPUState *cs = env_cpu(env); > > CPUState *ccs; > > - uint32_t nr_threads = cs->nr_threads; > > target_ulong dpdes = 0; > > > > helper_hfscr_facility_check(env, HFSCR_MSGP, "load DPDES", HFSCR_IC_MSGP); > > > > - if (!(env->flags & POWERPC_FLAG_SMT_1LPAR)) { > > - nr_threads = 1; /* DPDES behaves as 1-thread in LPAR-per-thread mode */ > > - } > > - > > - if (nr_threads == 1) { > > + /* DPDES behaves as 1-thread in LPAR-per-thread mode */ > > + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { > > if (env->pending_interrupts & PPC_INTERRUPT_DOORBELL) { > > dpdes = 1; > > } > > @@ -279,21 +274,11 @@ void helper_store_dpdes(CPUPPCState *env, target_ulong val) > > PowerPCCPU *cpu = env_archcpu(env); > > CPUState *cs = env_cpu(env); > > CPUState *ccs; > > - uint32_t nr_threads = cs->nr_threads; > > > > helper_hfscr_facility_check(env, HFSCR_MSGP, "store DPDES", HFSCR_IC_MSGP); > > > > - if (!(env->flags & POWERPC_FLAG_SMT_1LPAR)) { > > - nr_threads = 1; /* DPDES behaves as 1-thread in LPAR-per-thread mode */ > > - } > > - > > - if (val & ~(nr_threads - 1)) { > > - qemu_log_mask(LOG_GUEST_ERROR, "Invalid DPDES register value " > > - TARGET_FMT_lx"\n", val); > > - val &= (nr_threads - 1); /* Ignore the invalid bits */ > > - } > > - > > - if (nr_threads == 1) { > > + /* DPDES behaves as 1-thread in LPAR-per-thread mode */ > > + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { > > ppc_set_irq(cpu, PPC_INTERRUPT_DOORBELL, val & 0x1); > > return; > > } > > diff --git a/target/ppc/timebase_helper.c b/target/ppc/timebase_helper.c > > index 788c498d63..abe7b95696 100644 > > --- a/target/ppc/timebase_helper.c > > +++ b/target/ppc/timebase_helper.c > > @@ -63,9 +63,8 @@ void helper_store_purr(CPUPPCState *env, target_ulong val) > > { > > CPUState *cs = env_cpu(env); > > CPUState *ccs; > > - uint32_t nr_threads = cs->nr_threads; > > > > - if (nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) { > > + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { > > cpu_ppc_store_purr(env, val); > > return; > > } > > @@ -82,9 +81,8 @@ void helper_store_tbl(CPUPPCState *env, target_ulong val) > > { > > CPUState *cs = env_cpu(env); > > CPUState *ccs; > > - uint32_t nr_threads = cs->nr_threads; > > > > - if (nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) { > > + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { > > cpu_ppc_store_tbl(env, val); > > return; > > } > > @@ -99,9 +97,8 @@ void helper_store_tbu(CPUPPCState *env, target_ulong val) > > { > > CPUState *cs = env_cpu(env); > > CPUState *ccs; > > - uint32_t nr_threads = cs->nr_threads; > > > > - if (nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) { > > + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { > > cpu_ppc_store_tbu(env, val); > > return; > > } > > @@ -141,9 +138,8 @@ void helper_store_hdecr(CPUPPCState *env, target_ulong val) > > { > > CPUState *cs = env_cpu(env); > > CPUState *ccs; > > - uint32_t nr_threads = cs->nr_threads; > > > > - if (nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) { > > + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { > > cpu_ppc_store_hdecr(env, val); > > return; > > } > > @@ -158,9 +154,8 @@ void helper_store_vtb(CPUPPCState *env, target_ulong val) > > { > > CPUState *cs = env_cpu(env); > > CPUState *ccs; > > - uint32_t nr_threads = cs->nr_threads; > > > > - if (nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) { > > + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { > > cpu_ppc_store_vtb(env, val); > > return; > > } > > @@ -175,9 +170,8 @@ void helper_store_tbu40(CPUPPCState *env, target_ulong val) > > { > > CPUState *cs = env_cpu(env); > > CPUState *ccs; > > - uint32_t nr_threads = cs->nr_threads; > > > > - if (nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) { > > + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { > > cpu_ppc_store_tbu40(env, val); > > return; > > } > > @@ -288,7 +282,7 @@ static void write_tfmr(CPUPPCState *env, target_ulong val) > > { > > CPUState *cs = env_cpu(env); > > > > - if (cs->nr_threads == 1) { > > + if (!PPC_CPU_HAS_CORE_SIBLINGS(cs)) { > > env->spr[SPR_TFMR] = val; > > } else { > > CPUState *ccs;
On 5/26/24 14:26, Nicholas Piggin wrote: > Add helpers for TCG code to determine if there are SMT siblings > sharing per-core and per-lpar registers. This simplifies the > callers and makes SMT register topology simpler to modify with > later changes. > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- > target/ppc/cpu.h | 7 +++++++ > target/ppc/cpu_init.c | 2 +- > target/ppc/excp_helper.c | 16 +++++++--------- > target/ppc/misc_helper.c | 27 ++++++--------------------- > target/ppc/timebase_helper.c | 20 +++++++------------- > 5 files changed, 28 insertions(+), 44 deletions(-) > > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h > index 9a89083932..8fd6ade471 100644 > --- a/target/ppc/cpu.h > +++ b/target/ppc/cpu.h > @@ -1406,6 +1406,13 @@ struct CPUArchState { > uint64_t pmu_base_time; > }; > > +#define PPC_CPU_HAS_CORE_SIBLINGS(cs) \ > + (cs->nr_threads > 1) (cs)->nr_threads > + > +#define PPC_CPU_HAS_LPAR_SIBLINGS(cs) \ > + ((POWERPC_CPU(cs)->env.flags & POWERPC_FLAG_SMT_1LPAR) && \ > + PPC_CPU_HAS_CORE_SIBLINGS(cs)) > + I tend to prefer static inline when things get complex. The rest looks good. Thanks, C. > #define _CORE_ID(cs) \ > (POWERPC_CPU(cs)->env.core_index) > > diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c > index ae483e20c4..e71ee008ed 100644 > --- a/target/ppc/cpu_init.c > +++ b/target/ppc/cpu_init.c > @@ -6975,7 +6975,7 @@ static void ppc_cpu_realize(DeviceState *dev, Error **errp) > > pcc->parent_realize(dev, errp); > > - if (env_cpu(env)->nr_threads > 1) { > + if (PPC_CPU_HAS_CORE_SIBLINGS(cs)) { > env->flags |= POWERPC_FLAG_SMT; > } > > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c > index 0cd542675f..fd45da0f2b 100644 > --- a/target/ppc/excp_helper.c > +++ b/target/ppc/excp_helper.c > @@ -3029,7 +3029,7 @@ void helper_book3s_msgsnd(CPUPPCState *env, target_ulong rb) > brdcast = true; > } > > - if (cs->nr_threads == 1 || !brdcast) { > + if (!PPC_CPU_HAS_CORE_SIBLINGS(cs) || !brdcast) { > ppc_set_irq(cpu, PPC_INTERRUPT_HDOORBELL, 1); > return; > } > @@ -3067,21 +3067,19 @@ void helper_book3s_msgsndp(CPUPPCState *env, target_ulong rb) > CPUState *cs = env_cpu(env); > PowerPCCPU *cpu = env_archcpu(env); > CPUState *ccs; > - uint32_t nr_threads = cs->nr_threads; > int ttir = rb & PPC_BITMASK(57, 63); > > helper_hfscr_facility_check(env, HFSCR_MSGP, "msgsndp", HFSCR_IC_MSGP); > > - if (!(env->flags & POWERPC_FLAG_SMT_1LPAR)) { > - nr_threads = 1; /* msgsndp behaves as 1-thread in LPAR-per-thread mode*/ > - } > - > - if (!dbell_type_server(rb) || ttir >= nr_threads) { > + if (!dbell_type_server(rb)) { > return; > } > > - if (nr_threads == 1) { > - ppc_set_irq(cpu, PPC_INTERRUPT_DOORBELL, 1); > + /* msgsndp behaves as 1-thread in LPAR-per-thread mode*/ > + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { > + if (ttir == 0) { > + ppc_set_irq(cpu, PPC_INTERRUPT_DOORBELL, 1); > + } > return; > } > > diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c > index 46ba3a5584..598c956cdd 100644 > --- a/target/ppc/misc_helper.c > +++ b/target/ppc/misc_helper.c > @@ -49,9 +49,8 @@ void helper_spr_core_write_generic(CPUPPCState *env, uint32_t sprn, > { > CPUState *cs = env_cpu(env); > CPUState *ccs; > - uint32_t nr_threads = cs->nr_threads; > > - if (nr_threads == 1) { > + if (!PPC_CPU_HAS_CORE_SIBLINGS(cs)) { > env->spr[sprn] = val; > return; > } > @@ -196,7 +195,7 @@ void helper_store_ptcr(CPUPPCState *env, target_ulong val) > return; > } > > - if (cs->nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) { > + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { > env->spr[SPR_PTCR] = val; > tlb_flush(cs); > } else { > @@ -243,16 +242,12 @@ target_ulong helper_load_dpdes(CPUPPCState *env) > { > CPUState *cs = env_cpu(env); > CPUState *ccs; > - uint32_t nr_threads = cs->nr_threads; > target_ulong dpdes = 0; > > helper_hfscr_facility_check(env, HFSCR_MSGP, "load DPDES", HFSCR_IC_MSGP); > > - if (!(env->flags & POWERPC_FLAG_SMT_1LPAR)) { > - nr_threads = 1; /* DPDES behaves as 1-thread in LPAR-per-thread mode */ > - } > - > - if (nr_threads == 1) { > + /* DPDES behaves as 1-thread in LPAR-per-thread mode */ > + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { > if (env->pending_interrupts & PPC_INTERRUPT_DOORBELL) { > dpdes = 1; > } > @@ -279,21 +274,11 @@ void helper_store_dpdes(CPUPPCState *env, target_ulong val) > PowerPCCPU *cpu = env_archcpu(env); > CPUState *cs = env_cpu(env); > CPUState *ccs; > - uint32_t nr_threads = cs->nr_threads; > > helper_hfscr_facility_check(env, HFSCR_MSGP, "store DPDES", HFSCR_IC_MSGP); > > - if (!(env->flags & POWERPC_FLAG_SMT_1LPAR)) { > - nr_threads = 1; /* DPDES behaves as 1-thread in LPAR-per-thread mode */ > - } > - > - if (val & ~(nr_threads - 1)) { > - qemu_log_mask(LOG_GUEST_ERROR, "Invalid DPDES register value " > - TARGET_FMT_lx"\n", val); > - val &= (nr_threads - 1); /* Ignore the invalid bits */ > - } > - > - if (nr_threads == 1) { > + /* DPDES behaves as 1-thread in LPAR-per-thread mode */ > + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { > ppc_set_irq(cpu, PPC_INTERRUPT_DOORBELL, val & 0x1); > return; > } > diff --git a/target/ppc/timebase_helper.c b/target/ppc/timebase_helper.c > index 788c498d63..abe7b95696 100644 > --- a/target/ppc/timebase_helper.c > +++ b/target/ppc/timebase_helper.c > @@ -63,9 +63,8 @@ void helper_store_purr(CPUPPCState *env, target_ulong val) > { > CPUState *cs = env_cpu(env); > CPUState *ccs; > - uint32_t nr_threads = cs->nr_threads; > > - if (nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) { > + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { > cpu_ppc_store_purr(env, val); > return; > } > @@ -82,9 +81,8 @@ void helper_store_tbl(CPUPPCState *env, target_ulong val) > { > CPUState *cs = env_cpu(env); > CPUState *ccs; > - uint32_t nr_threads = cs->nr_threads; > > - if (nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) { > + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { > cpu_ppc_store_tbl(env, val); > return; > } > @@ -99,9 +97,8 @@ void helper_store_tbu(CPUPPCState *env, target_ulong val) > { > CPUState *cs = env_cpu(env); > CPUState *ccs; > - uint32_t nr_threads = cs->nr_threads; > > - if (nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) { > + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { > cpu_ppc_store_tbu(env, val); > return; > } > @@ -141,9 +138,8 @@ void helper_store_hdecr(CPUPPCState *env, target_ulong val) > { > CPUState *cs = env_cpu(env); > CPUState *ccs; > - uint32_t nr_threads = cs->nr_threads; > > - if (nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) { > + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { > cpu_ppc_store_hdecr(env, val); > return; > } > @@ -158,9 +154,8 @@ void helper_store_vtb(CPUPPCState *env, target_ulong val) > { > CPUState *cs = env_cpu(env); > CPUState *ccs; > - uint32_t nr_threads = cs->nr_threads; > > - if (nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) { > + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { > cpu_ppc_store_vtb(env, val); > return; > } > @@ -175,9 +170,8 @@ void helper_store_tbu40(CPUPPCState *env, target_ulong val) > { > CPUState *cs = env_cpu(env); > CPUState *ccs; > - uint32_t nr_threads = cs->nr_threads; > > - if (nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) { > + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { > cpu_ppc_store_tbu40(env, val); > return; > } > @@ -288,7 +282,7 @@ static void write_tfmr(CPUPPCState *env, target_ulong val) > { > CPUState *cs = env_cpu(env); > > - if (cs->nr_threads == 1) { > + if (!PPC_CPU_HAS_CORE_SIBLINGS(cs)) { > env->spr[SPR_TFMR] = val; > } else { > CPUState *ccs;
On Wed May 29, 2024 at 4:34 PM AEST, Cédric Le Goater wrote: > On 5/26/24 14:26, Nicholas Piggin wrote: > > Add helpers for TCG code to determine if there are SMT siblings > > sharing per-core and per-lpar registers. This simplifies the > > callers and makes SMT register topology simpler to modify with > > later changes. > > > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > > --- > > target/ppc/cpu.h | 7 +++++++ > > target/ppc/cpu_init.c | 2 +- > > target/ppc/excp_helper.c | 16 +++++++--------- > > target/ppc/misc_helper.c | 27 ++++++--------------------- > > target/ppc/timebase_helper.c | 20 +++++++------------- > > 5 files changed, 28 insertions(+), 44 deletions(-) > > > > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h > > index 9a89083932..8fd6ade471 100644 > > --- a/target/ppc/cpu.h > > +++ b/target/ppc/cpu.h > > @@ -1406,6 +1406,13 @@ struct CPUArchState { > > uint64_t pmu_base_time; > > }; > > > > +#define PPC_CPU_HAS_CORE_SIBLINGS(cs) \ > > + (cs->nr_threads > 1) > > (cs)->nr_threads > > > + > > +#define PPC_CPU_HAS_LPAR_SIBLINGS(cs) \ > > + ((POWERPC_CPU(cs)->env.flags & POWERPC_FLAG_SMT_1LPAR) && \ > > + PPC_CPU_HAS_CORE_SIBLINGS(cs)) > > + > > I tend to prefer static inline when things get complex. Yeah agree it has become too much. Thanks, Nick
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h index 9a89083932..8fd6ade471 100644 --- a/target/ppc/cpu.h +++ b/target/ppc/cpu.h @@ -1406,6 +1406,13 @@ struct CPUArchState { uint64_t pmu_base_time; }; +#define PPC_CPU_HAS_CORE_SIBLINGS(cs) \ + (cs->nr_threads > 1) + +#define PPC_CPU_HAS_LPAR_SIBLINGS(cs) \ + ((POWERPC_CPU(cs)->env.flags & POWERPC_FLAG_SMT_1LPAR) && \ + PPC_CPU_HAS_CORE_SIBLINGS(cs)) + #define _CORE_ID(cs) \ (POWERPC_CPU(cs)->env.core_index) diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c index ae483e20c4..e71ee008ed 100644 --- a/target/ppc/cpu_init.c +++ b/target/ppc/cpu_init.c @@ -6975,7 +6975,7 @@ static void ppc_cpu_realize(DeviceState *dev, Error **errp) pcc->parent_realize(dev, errp); - if (env_cpu(env)->nr_threads > 1) { + if (PPC_CPU_HAS_CORE_SIBLINGS(cs)) { env->flags |= POWERPC_FLAG_SMT; } diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c index 0cd542675f..fd45da0f2b 100644 --- a/target/ppc/excp_helper.c +++ b/target/ppc/excp_helper.c @@ -3029,7 +3029,7 @@ void helper_book3s_msgsnd(CPUPPCState *env, target_ulong rb) brdcast = true; } - if (cs->nr_threads == 1 || !brdcast) { + if (!PPC_CPU_HAS_CORE_SIBLINGS(cs) || !brdcast) { ppc_set_irq(cpu, PPC_INTERRUPT_HDOORBELL, 1); return; } @@ -3067,21 +3067,19 @@ void helper_book3s_msgsndp(CPUPPCState *env, target_ulong rb) CPUState *cs = env_cpu(env); PowerPCCPU *cpu = env_archcpu(env); CPUState *ccs; - uint32_t nr_threads = cs->nr_threads; int ttir = rb & PPC_BITMASK(57, 63); helper_hfscr_facility_check(env, HFSCR_MSGP, "msgsndp", HFSCR_IC_MSGP); - if (!(env->flags & POWERPC_FLAG_SMT_1LPAR)) { - nr_threads = 1; /* msgsndp behaves as 1-thread in LPAR-per-thread mode*/ - } - - if (!dbell_type_server(rb) || ttir >= nr_threads) { + if (!dbell_type_server(rb)) { return; } - if (nr_threads == 1) { - ppc_set_irq(cpu, PPC_INTERRUPT_DOORBELL, 1); + /* msgsndp behaves as 1-thread in LPAR-per-thread mode*/ + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { + if (ttir == 0) { + ppc_set_irq(cpu, PPC_INTERRUPT_DOORBELL, 1); + } return; } diff --git a/target/ppc/misc_helper.c b/target/ppc/misc_helper.c index 46ba3a5584..598c956cdd 100644 --- a/target/ppc/misc_helper.c +++ b/target/ppc/misc_helper.c @@ -49,9 +49,8 @@ void helper_spr_core_write_generic(CPUPPCState *env, uint32_t sprn, { CPUState *cs = env_cpu(env); CPUState *ccs; - uint32_t nr_threads = cs->nr_threads; - if (nr_threads == 1) { + if (!PPC_CPU_HAS_CORE_SIBLINGS(cs)) { env->spr[sprn] = val; return; } @@ -196,7 +195,7 @@ void helper_store_ptcr(CPUPPCState *env, target_ulong val) return; } - if (cs->nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) { + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { env->spr[SPR_PTCR] = val; tlb_flush(cs); } else { @@ -243,16 +242,12 @@ target_ulong helper_load_dpdes(CPUPPCState *env) { CPUState *cs = env_cpu(env); CPUState *ccs; - uint32_t nr_threads = cs->nr_threads; target_ulong dpdes = 0; helper_hfscr_facility_check(env, HFSCR_MSGP, "load DPDES", HFSCR_IC_MSGP); - if (!(env->flags & POWERPC_FLAG_SMT_1LPAR)) { - nr_threads = 1; /* DPDES behaves as 1-thread in LPAR-per-thread mode */ - } - - if (nr_threads == 1) { + /* DPDES behaves as 1-thread in LPAR-per-thread mode */ + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { if (env->pending_interrupts & PPC_INTERRUPT_DOORBELL) { dpdes = 1; } @@ -279,21 +274,11 @@ void helper_store_dpdes(CPUPPCState *env, target_ulong val) PowerPCCPU *cpu = env_archcpu(env); CPUState *cs = env_cpu(env); CPUState *ccs; - uint32_t nr_threads = cs->nr_threads; helper_hfscr_facility_check(env, HFSCR_MSGP, "store DPDES", HFSCR_IC_MSGP); - if (!(env->flags & POWERPC_FLAG_SMT_1LPAR)) { - nr_threads = 1; /* DPDES behaves as 1-thread in LPAR-per-thread mode */ - } - - if (val & ~(nr_threads - 1)) { - qemu_log_mask(LOG_GUEST_ERROR, "Invalid DPDES register value " - TARGET_FMT_lx"\n", val); - val &= (nr_threads - 1); /* Ignore the invalid bits */ - } - - if (nr_threads == 1) { + /* DPDES behaves as 1-thread in LPAR-per-thread mode */ + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { ppc_set_irq(cpu, PPC_INTERRUPT_DOORBELL, val & 0x1); return; } diff --git a/target/ppc/timebase_helper.c b/target/ppc/timebase_helper.c index 788c498d63..abe7b95696 100644 --- a/target/ppc/timebase_helper.c +++ b/target/ppc/timebase_helper.c @@ -63,9 +63,8 @@ void helper_store_purr(CPUPPCState *env, target_ulong val) { CPUState *cs = env_cpu(env); CPUState *ccs; - uint32_t nr_threads = cs->nr_threads; - if (nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) { + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { cpu_ppc_store_purr(env, val); return; } @@ -82,9 +81,8 @@ void helper_store_tbl(CPUPPCState *env, target_ulong val) { CPUState *cs = env_cpu(env); CPUState *ccs; - uint32_t nr_threads = cs->nr_threads; - if (nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) { + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { cpu_ppc_store_tbl(env, val); return; } @@ -99,9 +97,8 @@ void helper_store_tbu(CPUPPCState *env, target_ulong val) { CPUState *cs = env_cpu(env); CPUState *ccs; - uint32_t nr_threads = cs->nr_threads; - if (nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) { + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { cpu_ppc_store_tbu(env, val); return; } @@ -141,9 +138,8 @@ void helper_store_hdecr(CPUPPCState *env, target_ulong val) { CPUState *cs = env_cpu(env); CPUState *ccs; - uint32_t nr_threads = cs->nr_threads; - if (nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) { + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { cpu_ppc_store_hdecr(env, val); return; } @@ -158,9 +154,8 @@ void helper_store_vtb(CPUPPCState *env, target_ulong val) { CPUState *cs = env_cpu(env); CPUState *ccs; - uint32_t nr_threads = cs->nr_threads; - if (nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) { + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { cpu_ppc_store_vtb(env, val); return; } @@ -175,9 +170,8 @@ void helper_store_tbu40(CPUPPCState *env, target_ulong val) { CPUState *cs = env_cpu(env); CPUState *ccs; - uint32_t nr_threads = cs->nr_threads; - if (nr_threads == 1 || !(env->flags & POWERPC_FLAG_SMT_1LPAR)) { + if (!PPC_CPU_HAS_LPAR_SIBLINGS(cs)) { cpu_ppc_store_tbu40(env, val); return; } @@ -288,7 +282,7 @@ static void write_tfmr(CPUPPCState *env, target_ulong val) { CPUState *cs = env_cpu(env); - if (cs->nr_threads == 1) { + if (!PPC_CPU_HAS_CORE_SIBLINGS(cs)) { env->spr[SPR_TFMR] = val; } else { CPUState *ccs;
Add helpers for TCG code to determine if there are SMT siblings sharing per-core and per-lpar registers. This simplifies the callers and makes SMT register topology simpler to modify with later changes. Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- target/ppc/cpu.h | 7 +++++++ target/ppc/cpu_init.c | 2 +- target/ppc/excp_helper.c | 16 +++++++--------- target/ppc/misc_helper.c | 27 ++++++--------------------- target/ppc/timebase_helper.c | 20 +++++++------------- 5 files changed, 28 insertions(+), 44 deletions(-)