diff mbox series

[RFC,07/10] target/ppc: Add helpers to check for SMT sibling threads

Message ID 20240526122612.473476-8-npiggin@gmail.com
State New
Headers show
Series ppc/pnv: Better big-core model, lpar-per-core, PC unit | expand

Commit Message

Nicholas Piggin May 26, 2024, 12:26 p.m. UTC 
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(-)

Comments

Harsh Prateek Bora May 28, 2024, 9:16 a.m. UTC | #1 
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;
Nicholas Piggin May 29, 2024, 12:31 a.m. UTC | #2 
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;
Cédric Le Goater May 29, 2024, 6:34 a.m. UTC | #3 
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;
Nicholas Piggin May 30, 2024, 6:38 a.m. UTC | #4 
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 mbox series

Patch

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;