[PATCH v2 12/12] target/ppc: add SMT support to msgsnd broadcast

Nicholas Piggin posted 12 patches 6 months, 1 week ago
[PATCH v2 12/12] target/ppc: add SMT support to msgsnd broadcast
Posted by Nicholas Piggin 6 months, 1 week ago
msgsnd has a broadcast mode that sends hypervisor doorbells to all
threads belonging to the same core as the target. A "subcore" mode
sends to all or one thread depending on 1LPAR mode.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/cpu.h                              |  6 +-
 target/ppc/helper.h                           |  2 +-
 target/ppc/excp_helper.c                      | 57 +++++++++++++------
 .../ppc/translate/processor-ctrl-impl.c.inc   |  2 +-
 4 files changed, 46 insertions(+), 21 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index e4c342b17d..e201b7f6c2 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1163,7 +1163,11 @@ FIELD(FPSCR, FI, FPSCR_FI, 1)
 
 #define DBELL_TYPE_DBELL_SERVER        (0x05 << DBELL_TYPE_SHIFT)
 
-#define DBELL_BRDCAST                  PPC_BIT(37)
+#define DBELL_BRDCAST_MASK             PPC_BITMASK(37, 38)
+#define DBELL_BRDCAST_SHIFT            25
+#define DBELL_BRDCAST_SUBPROC          (0x1 << DBELL_BRDCAST_SHIFT)
+#define DBELL_BRDCAST_CORE             (0x2 << DBELL_BRDCAST_SHIFT)
+
 #define DBELL_LPIDTAG_SHIFT            14
 #define DBELL_LPIDTAG_MASK             (0xfff << DBELL_LPIDTAG_SHIFT)
 #define DBELL_PIRTAG_MASK              0x3fff
diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index 57bf8354e7..dd92c6a937 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -695,7 +695,7 @@ DEF_HELPER_FLAGS_3(store_sr, TCG_CALL_NO_RWG, void, env, tl, tl)
 
 DEF_HELPER_1(msgsnd, void, tl)
 DEF_HELPER_2(msgclr, void, env, tl)
-DEF_HELPER_1(book3s_msgsnd, void, tl)
+DEF_HELPER_2(book3s_msgsnd, void, env, tl)
 DEF_HELPER_2(book3s_msgclr, void, env, tl)
 #endif
 
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index e786a9044b..0a9e8539a4 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -2978,7 +2978,7 @@ void helper_msgsnd(target_ulong rb)
         PowerPCCPU *cpu = POWERPC_CPU(cs);
         CPUPPCState *cenv = &cpu->env;
 
-        if ((rb & DBELL_BRDCAST) || (cenv->spr[SPR_BOOKE_PIR] == pir)) {
+        if ((rb & DBELL_BRDCAST_MASK) || (cenv->spr[SPR_BOOKE_PIR] == pir)) {
             ppc_set_irq(cpu, irq, 1);
         }
     }
@@ -2997,6 +2997,16 @@ static bool dbell_type_server(target_ulong rb)
     return (rb & DBELL_TYPE_MASK) == DBELL_TYPE_DBELL_SERVER;
 }
 
+static inline bool dbell_bcast_core(target_ulong rb)
+{
+    return (rb & DBELL_BRDCAST_MASK) == DBELL_BRDCAST_CORE;
+}
+
+static inline bool dbell_bcast_subproc(target_ulong rb)
+{
+    return (rb & DBELL_BRDCAST_MASK) == DBELL_BRDCAST_SUBPROC;
+}
+
 void helper_book3s_msgclr(CPUPPCState *env, target_ulong rb)
 {
     if (!dbell_type_server(rb)) {
@@ -3006,32 +3016,43 @@ void helper_book3s_msgclr(CPUPPCState *env, target_ulong rb)
     ppc_set_irq(env_archcpu(env), PPC_INTERRUPT_HDOORBELL, 0);
 }
 
-static void book3s_msgsnd_common(int pir, int irq)
+void helper_book3s_msgsnd(CPUPPCState *env, target_ulong rb)
 {
-    CPUState *cs;
+    int pir = rb & DBELL_PROCIDTAG_MASK;
+    bool brdcast = false;
+    CPUState *cs, *ccs;
+    PowerPCCPU *cpu;
 
-    bql_lock();
-    CPU_FOREACH(cs) {
-        PowerPCCPU *cpu = POWERPC_CPU(cs);
-        CPUPPCState *cenv = &cpu->env;
+    if (!dbell_type_server(rb)) {
+        return;
+    }
 
-        /* TODO: broadcast message to all threads of the same  processor */
-        if (cenv->spr_cb[SPR_PIR].default_value == pir) {
-            ppc_set_irq(cpu, irq, 1);
-        }
+    cpu = ppc_get_vcpu_by_pir(pir);
+    if (!cpu) {
+        return;
     }
-    bql_unlock();
-}
+    cs = CPU(cpu);
 
-void helper_book3s_msgsnd(target_ulong rb)
-{
-    int pir = rb & DBELL_PROCIDTAG_MASK;
+    if (dbell_bcast_core(rb) || (dbell_bcast_subproc(rb) &&
+                                 (env->flags & POWERPC_FLAG_SMT_1LPAR))) {
+        brdcast = true;
+    }
 
-    if (!dbell_type_server(rb)) {
+    if (cs->nr_threads == 1 || !brdcast) {
+        ppc_set_irq(cpu, PPC_INTERRUPT_HDOORBELL, 1);
         return;
     }
 
-    book3s_msgsnd_common(pir, PPC_INTERRUPT_HDOORBELL);
+    /*
+     * Why is bql needed for walking CPU list? Answer seems to be because ppc
+     * irq handling needs it, but ppc_set_irq takes the lock itself if needed,
+     * so could this be removed?
+     */
+    bql_lock();
+    THREAD_SIBLING_FOREACH(cs, ccs) {
+        ppc_set_irq(POWERPC_CPU(ccs), PPC_INTERRUPT_HDOORBELL, 1);
+    }
+    bql_unlock();
 }
 
 #ifdef TARGET_PPC64
diff --git a/target/ppc/translate/processor-ctrl-impl.c.inc b/target/ppc/translate/processor-ctrl-impl.c.inc
index 0142801985..8abbb89630 100644
--- a/target/ppc/translate/processor-ctrl-impl.c.inc
+++ b/target/ppc/translate/processor-ctrl-impl.c.inc
@@ -59,7 +59,7 @@ static bool trans_MSGSND(DisasContext *ctx, arg_X_rb *a)
 
 #if !defined(CONFIG_USER_ONLY)
     if (is_book3s_arch2x(ctx)) {
-        gen_helper_book3s_msgsnd(cpu_gpr[a->rb]);
+        gen_helper_book3s_msgsnd(tcg_env, cpu_gpr[a->rb]);
     } else {
         gen_helper_msgsnd(cpu_gpr[a->rb]);
     }
-- 
2.43.0
Re: [PATCH v2 12/12] target/ppc: add SMT support to msgsnd broadcast
Posted by Miles Glenn 6 months, 1 week ago
Reviewed-by: Glenn Miles <milesg@linux.ibm.com>

Thanks,

Glenn

On Tue, 2024-05-21 at 11:30 +1000, Nicholas Piggin wrote:
> msgsnd has a broadcast mode that sends hypervisor doorbells to all
> threads belonging to the same core as the target. A "subcore" mode
> sends to all or one thread depending on 1LPAR mode.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  target/ppc/cpu.h                              |  6 +-
>  target/ppc/helper.h                           |  2 +-
>  target/ppc/excp_helper.c                      | 57 +++++++++++++--
> ----
>  .../ppc/translate/processor-ctrl-impl.c.inc   |  2 +-
>  4 files changed, 46 insertions(+), 21 deletions(-)
> 
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index e4c342b17d..e201b7f6c2 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -1163,7 +1163,11 @@ FIELD(FPSCR, FI, FPSCR_FI, 1)
>  
>  #define DBELL_TYPE_DBELL_SERVER        (0x05 << DBELL_TYPE_SHIFT)
>  
> -#define DBELL_BRDCAST                  PPC_BIT(37)
> +#define DBELL_BRDCAST_MASK             PPC_BITMASK(37, 38)
> +#define DBELL_BRDCAST_SHIFT            25
> +#define DBELL_BRDCAST_SUBPROC          (0x1 << DBELL_BRDCAST_SHIFT)
> +#define DBELL_BRDCAST_CORE             (0x2 << DBELL_BRDCAST_SHIFT)
> +
>  #define DBELL_LPIDTAG_SHIFT            14
>  #define DBELL_LPIDTAG_MASK             (0xfff <<
> DBELL_LPIDTAG_SHIFT)
>  #define DBELL_PIRTAG_MASK              0x3fff
> diff --git a/target/ppc/helper.h b/target/ppc/helper.h
> index 57bf8354e7..dd92c6a937 100644
> --- a/target/ppc/helper.h
> +++ b/target/ppc/helper.h
> @@ -695,7 +695,7 @@ DEF_HELPER_FLAGS_3(store_sr, TCG_CALL_NO_RWG,
> void, env, tl, tl)
>  
>  DEF_HELPER_1(msgsnd, void, tl)
>  DEF_HELPER_2(msgclr, void, env, tl)
> -DEF_HELPER_1(book3s_msgsnd, void, tl)
> +DEF_HELPER_2(book3s_msgsnd, void, env, tl)
>  DEF_HELPER_2(book3s_msgclr, void, env, tl)
>  #endif
>  
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index e786a9044b..0a9e8539a4 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -2978,7 +2978,7 @@ void helper_msgsnd(target_ulong rb)
>          PowerPCCPU *cpu = POWERPC_CPU(cs);
>          CPUPPCState *cenv = &cpu->env;
>  
> -        if ((rb & DBELL_BRDCAST) || (cenv->spr[SPR_BOOKE_PIR] ==
> pir)) {
> +        if ((rb & DBELL_BRDCAST_MASK) || (cenv->spr[SPR_BOOKE_PIR]
> == pir)) {
>              ppc_set_irq(cpu, irq, 1);
>          }
>      }
> @@ -2997,6 +2997,16 @@ static bool dbell_type_server(target_ulong rb)
>      return (rb & DBELL_TYPE_MASK) == DBELL_TYPE_DBELL_SERVER;
>  }
>  
> +static inline bool dbell_bcast_core(target_ulong rb)
> +{
> +    return (rb & DBELL_BRDCAST_MASK) == DBELL_BRDCAST_CORE;
> +}
> +
> +static inline bool dbell_bcast_subproc(target_ulong rb)
> +{
> +    return (rb & DBELL_BRDCAST_MASK) == DBELL_BRDCAST_SUBPROC;
> +}
> +
>  void helper_book3s_msgclr(CPUPPCState *env, target_ulong rb)
>  {
>      if (!dbell_type_server(rb)) {
> @@ -3006,32 +3016,43 @@ void helper_book3s_msgclr(CPUPPCState *env,
> target_ulong rb)
>      ppc_set_irq(env_archcpu(env), PPC_INTERRUPT_HDOORBELL, 0);
>  }
>  
> -static void book3s_msgsnd_common(int pir, int irq)
> +void helper_book3s_msgsnd(CPUPPCState *env, target_ulong rb)
>  {
> -    CPUState *cs;
> +    int pir = rb & DBELL_PROCIDTAG_MASK;
> +    bool brdcast = false;
> +    CPUState *cs, *ccs;
> +    PowerPCCPU *cpu;
>  
> -    bql_lock();
> -    CPU_FOREACH(cs) {
> -        PowerPCCPU *cpu = POWERPC_CPU(cs);
> -        CPUPPCState *cenv = &cpu->env;
> +    if (!dbell_type_server(rb)) {
> +        return;
> +    }
>  
> -        /* TODO: broadcast message to all threads of the
> same  processor */
> -        if (cenv->spr_cb[SPR_PIR].default_value == pir) {
> -            ppc_set_irq(cpu, irq, 1);
> -        }
> +    cpu = ppc_get_vcpu_by_pir(pir);
> +    if (!cpu) {
> +        return;
>      }
> -    bql_unlock();
> -}
> +    cs = CPU(cpu);
>  
> -void helper_book3s_msgsnd(target_ulong rb)
> -{
> -    int pir = rb & DBELL_PROCIDTAG_MASK;
> +    if (dbell_bcast_core(rb) || (dbell_bcast_subproc(rb) &&
> +                                 (env->flags &
> POWERPC_FLAG_SMT_1LPAR))) {
> +        brdcast = true;
> +    }
>  
> -    if (!dbell_type_server(rb)) {
> +    if (cs->nr_threads == 1 || !brdcast) {
> +        ppc_set_irq(cpu, PPC_INTERRUPT_HDOORBELL, 1);
>          return;
>      }
>  
> -    book3s_msgsnd_common(pir, PPC_INTERRUPT_HDOORBELL);
> +    /*
> +     * Why is bql needed for walking CPU list? Answer seems to be
> because ppc
> +     * irq handling needs it, but ppc_set_irq takes the lock itself
> if needed,
> +     * so could this be removed?
> +     */
> +    bql_lock();
> +    THREAD_SIBLING_FOREACH(cs, ccs) {
> +        ppc_set_irq(POWERPC_CPU(ccs), PPC_INTERRUPT_HDOORBELL, 1);
> +    }
> +    bql_unlock();
>  }
>  
>  #ifdef TARGET_PPC64
> diff --git a/target/ppc/translate/processor-ctrl-impl.c.inc
> b/target/ppc/translate/processor-ctrl-impl.c.inc
> index 0142801985..8abbb89630 100644
> --- a/target/ppc/translate/processor-ctrl-impl.c.inc
> +++ b/target/ppc/translate/processor-ctrl-impl.c.inc
> @@ -59,7 +59,7 @@ static bool trans_MSGSND(DisasContext *ctx,
> arg_X_rb *a)
>  
>  #if !defined(CONFIG_USER_ONLY)
>      if (is_book3s_arch2x(ctx)) {
> -        gen_helper_book3s_msgsnd(cpu_gpr[a->rb]);
> +        gen_helper_book3s_msgsnd(tcg_env, cpu_gpr[a->rb]);
>      } else {
>          gen_helper_msgsnd(cpu_gpr[a->rb]);
>      }