All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 0/3] target/ppc: add support for hypervisor doorbells
@ 2018-01-16  7:41 Cédric Le Goater
  2018-01-16  7:41 ` [Qemu-devel] [PATCH 1/3] target/ppc: fix doorbell and hypervisor doorbell definitions Cédric Le Goater
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Cédric Le Goater @ 2018-01-16  7:41 UTC (permalink / raw)
  To: qemu-ppc, qemu-devel, David Gibson
  Cc: Benjamin Herrenschmidt, Cédric Le Goater

Hi,

The hypervisor doorbells are used by skiboot and Linux on POWER9
processors to wake up secondaries. This adds processor control support
to the Book3S architecture.

The full tree can be found here :

  https://github.com/legoater/qemu powernv-2.12

Thanks,

C.

Cédric Le Goater (3):
  target/ppc: fix doorbell and hypervisor doorbell definitions
  target/ppc: msgsnd and msgclr instructions need hypervisor privilege
  target/ppc: add support for hypervisor doorbells on book3s CPUs

 target/ppc/cpu.h            | 16 ++++++++++------
 target/ppc/excp_helper.c    | 39 ++++++++++++++++++++++++++++++++-------
 target/ppc/helper.h         |  2 +-
 target/ppc/translate.c      | 17 ++++++++++++++---
 target/ppc/translate_init.c |  2 +-
 5 files changed, 58 insertions(+), 18 deletions(-)

-- 
2.13.6

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 1/3] target/ppc: fix doorbell and hypervisor doorbell definitions
  2018-01-16  7:41 [Qemu-devel] [PATCH 0/3] target/ppc: add support for hypervisor doorbells Cédric Le Goater
@ 2018-01-16  7:41 ` Cédric Le Goater
  2018-01-16  7:41 ` [Qemu-devel] [PATCH 2/3] target/ppc: msgsnd and msgclr instructions need hypervisor privilege Cédric Le Goater
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Cédric Le Goater @ 2018-01-16  7:41 UTC (permalink / raw)
  To: qemu-ppc, qemu-devel, David Gibson
  Cc: Benjamin Herrenschmidt, Cédric Le Goater

commit f03a1af581b9 ("ppc: Fix POWER7 and POWER8 exception definitions")
introduced definitions for the server doorbell exceptions by reusing
the embedded definitions but this adds complexity in the powerpc_excp()
routine. Let's introduce specific definitions for the Server doorbells
exception.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 target/ppc/cpu.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 14aaa87fe825..b8f4dfc1084a 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -140,9 +140,6 @@ enum {
     POWERPC_EXCP_HYPPRIV  = 41, /* Embedded hypervisor priv instruction      */
     /* Vectors 42 to 63 are reserved                                         */
     /* Exceptions defined in the PowerPC server specification                */
-    /* Server doorbell variants */
-#define POWERPC_EXCP_SDOOR      POWERPC_EXCP_GDOORI
-#define POWERPC_EXCP_SDOOR_HV   POWERPC_EXCP_DOORI
     POWERPC_EXCP_RESET    = 64, /* System reset exception                    */
     POWERPC_EXCP_DSEG     = 65, /* Data segment exception                    */
     POWERPC_EXCP_ISEG     = 66, /* Instruction segment exception             */
@@ -189,8 +186,11 @@ enum {
     POWERPC_EXCP_HV_EMU   = 96, /* HV emulation assistance                   */
     POWERPC_EXCP_HV_MAINT = 97, /* HMI                                       */
     POWERPC_EXCP_HV_FU    = 98, /* Hypervisor Facility unavailable           */
+    /* Server doorbell variants */
+    POWERPC_EXCP_SDOOR    = 99,
+    POWERPC_EXCP_SDOOR_HV = 100,
     /* EOL                                                                   */
-    POWERPC_EXCP_NB       = 99,
+    POWERPC_EXCP_NB       = 101,
     /* QEMU exceptions: used internally during code translation              */
     POWERPC_EXCP_STOP         = 0x200, /* stop translation                   */
     POWERPC_EXCP_BRANCH       = 0x201, /* branch instruction                 */
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 2/3] target/ppc: msgsnd and msgclr instructions need hypervisor privilege
  2018-01-16  7:41 [Qemu-devel] [PATCH 0/3] target/ppc: add support for hypervisor doorbells Cédric Le Goater
  2018-01-16  7:41 ` [Qemu-devel] [PATCH 1/3] target/ppc: fix doorbell and hypervisor doorbell definitions Cédric Le Goater
@ 2018-01-16  7:41 ` Cédric Le Goater
  2018-01-16  7:41 ` [Qemu-devel] [PATCH 3/3] target/ppc: add support for hypervisor doorbells on book3s CPUs Cédric Le Goater
  2018-01-17  3:26 ` [Qemu-devel] [PATCH 0/3] target/ppc: add support for hypervisor doorbells David Gibson
  3 siblings, 0 replies; 8+ messages in thread
From: Cédric Le Goater @ 2018-01-16  7:41 UTC (permalink / raw)
  To: qemu-ppc, qemu-devel, David Gibson
  Cc: Benjamin Herrenschmidt, Cédric Le Goater

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 target/ppc/translate.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 6d16a0b22d44..bcd36d53537f 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -6169,7 +6169,7 @@ static void gen_msgclr(DisasContext *ctx)
 #if defined(CONFIG_USER_ONLY)
     GEN_PRIV;
 #else
-    CHK_SV;
+    CHK_HV;
     gen_helper_msgclr(cpu_env, cpu_gpr[rB(ctx->opcode)]);
 #endif /* defined(CONFIG_USER_ONLY) */
 }
@@ -6179,7 +6179,7 @@ static void gen_msgsnd(DisasContext *ctx)
 #if defined(CONFIG_USER_ONLY)
     GEN_PRIV;
 #else
-    CHK_SV;
+    CHK_HV;
     gen_helper_msgsnd(cpu_gpr[rB(ctx->opcode)]);
 #endif /* defined(CONFIG_USER_ONLY) */
 }
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [Qemu-devel] [PATCH 3/3] target/ppc: add support for hypervisor doorbells on book3s CPUs
  2018-01-16  7:41 [Qemu-devel] [PATCH 0/3] target/ppc: add support for hypervisor doorbells Cédric Le Goater
  2018-01-16  7:41 ` [Qemu-devel] [PATCH 1/3] target/ppc: fix doorbell and hypervisor doorbell definitions Cédric Le Goater
  2018-01-16  7:41 ` [Qemu-devel] [PATCH 2/3] target/ppc: msgsnd and msgclr instructions need hypervisor privilege Cédric Le Goater
@ 2018-01-16  7:41 ` Cédric Le Goater
  2018-01-16  7:45   ` Cédric Le Goater
  2018-01-18  4:15   ` David Gibson
  2018-01-17  3:26 ` [Qemu-devel] [PATCH 0/3] target/ppc: add support for hypervisor doorbells David Gibson
  3 siblings, 2 replies; 8+ messages in thread
From: Cédric Le Goater @ 2018-01-16  7:41 UTC (permalink / raw)
  To: qemu-ppc, qemu-devel, David Gibson
  Cc: Benjamin Herrenschmidt, Cédric Le Goater

The hypervisor doorbells are used by skiboot and Linux on POWER9
processors to wake up secondaries.

This adds processor control support to the Server architecture by
reusing the Embedded support. They are very similar, only the bits
definition of the CPU identifier differ.

Still to be done is message broadcast to all threads of the same
processor.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 target/ppc/cpu.h            |  8 ++++++--
 target/ppc/excp_helper.c    | 39 ++++++++++++++++++++++++++++++++-------
 target/ppc/helper.h         |  2 +-
 target/ppc/translate.c      | 13 ++++++++++++-
 target/ppc/translate_init.c |  2 +-
 5 files changed, 52 insertions(+), 12 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index b8f4dfc1084a..603a38cae83f 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -930,7 +930,7 @@ enum {
 #define BOOKE206_MAX_TLBN      4
 
 /*****************************************************************************/
-/* Embedded.Processor Control */
+/* Server and Embedded Processor Control */
 
 #define DBELL_TYPE_SHIFT               27
 #define DBELL_TYPE_MASK                (0x1f << DBELL_TYPE_SHIFT)
@@ -940,11 +940,15 @@ enum {
 #define DBELL_TYPE_G_DBELL_CRIT        (0x03 << DBELL_TYPE_SHIFT)
 #define DBELL_TYPE_G_DBELL_MC          (0x04 << DBELL_TYPE_SHIFT)
 
-#define DBELL_BRDCAST                  (1 << 26)
+#define DBELL_TYPE_DBELL_SERVER        (0x05 << DBELL_TYPE_SHIFT)
+
+#define DBELL_BRDCAST                  PPC_BIT(37)
 #define DBELL_LPIDTAG_SHIFT            14
 #define DBELL_LPIDTAG_MASK             (0xfff << DBELL_LPIDTAG_SHIFT)
 #define DBELL_PIRTAG_MASK              0x3fff
 
+#define DBELL_PROCIDTAG_MASK           PPC_BITMASK(44, 63)
+
 /*****************************************************************************/
 /* Segment page size information, used by recent hash MMUs
  * The format of this structure mirrors kvm_ppc_smmu_info
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 4e548a448747..0f32cab1ff57 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -417,6 +417,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
     case POWERPC_EXCP_HISI:      /* Hypervisor instruction storage exception */
     case POWERPC_EXCP_HDSEG:     /* Hypervisor data segment exception        */
     case POWERPC_EXCP_HISEG:     /* Hypervisor instruction segment exception */
+    case POWERPC_EXCP_SDOOR_HV:  /* Hypervisor Doorbell interrupt            */
     case POWERPC_EXCP_HV_EMU:
         srr0 = SPR_HSRR0;
         srr1 = SPR_HSRR1;
@@ -846,6 +847,11 @@ static void ppc_hw_interrupt(CPUPPCState *env)
             powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_DOORI);
             return;
         }
+        if (env->pending_interrupts & (1 << PPC_INTERRUPT_HDOORBELL)) {
+            env->pending_interrupts &= ~(1 << PPC_INTERRUPT_HDOORBELL);
+            powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_SDOOR_HV);
+            return;
+        }
         if (env->pending_interrupts & (1 << PPC_INTERRUPT_PERFM)) {
             env->pending_interrupts &= ~(1 << PPC_INTERRUPT_PERFM);
             powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_PERFM);
@@ -1088,8 +1094,8 @@ void helper_rfsvc(CPUPPCState *env)
     do_rfi(env, env->lr, env->ctr & 0x0000FFFF);
 }
 
-/* Embedded.Processor Control */
-static int dbell2irq(target_ulong rb)
+/* Server and Embedded Processor Control */
+static int dbell2irq(target_ulong rb, bool book3s)
 {
     int msg = rb & DBELL_TYPE_MASK;
     int irq = -1;
@@ -1109,12 +1115,21 @@ static int dbell2irq(target_ulong rb)
         break;
     }
 
+    /* A Directed Hypervisor Doorbell message is sent only if the
+     * message type is 5. All other types are reserved and the
+     * instruction is a no-op */
+    if (book3s && msg == DBELL_TYPE_DBELL_SERVER) {
+        irq = PPC_INTERRUPT_HDOORBELL;
+    }
+
     return irq;
 }
 
 void helper_msgclr(CPUPPCState *env, target_ulong rb)
 {
-    int irq = dbell2irq(rb);
+    /* 64-bit server processors compliant with arch 2.x */
+    bool book3s = (env->insns_flags & PPC_SEGMENT_64B);
+    int irq = dbell2irq(rb, book3s);
 
     if (irq < 0) {
         return;
@@ -1123,10 +1138,11 @@ void helper_msgclr(CPUPPCState *env, target_ulong rb)
     env->pending_interrupts &= ~(1 << irq);
 }
 
-void helper_msgsnd(target_ulong rb)
+void helper_msgsnd(CPUPPCState *env, target_ulong rb)
 {
-    int irq = dbell2irq(rb);
-    int pir = rb & DBELL_PIRTAG_MASK;
+    /* 64-bit server processors compliant with arch 2.x */
+    bool book3s = (env->insns_flags & PPC_SEGMENT_64B);
+    int irq = dbell2irq(rb, book3s);
     CPUState *cs;
 
     if (irq < 0) {
@@ -1137,8 +1153,17 @@ void helper_msgsnd(target_ulong rb)
     CPU_FOREACH(cs) {
         PowerPCCPU *cpu = POWERPC_CPU(cs);
         CPUPPCState *cenv = &cpu->env;
+        bool send;
 
-        if ((rb & DBELL_BRDCAST) || (cenv->spr[SPR_BOOKE_PIR] == pir)) {
+        /* TODO: broadcast message to all threads of the same  processor */
+        if (book3s) {
+            int pir = rb & DBELL_PROCIDTAG_MASK;
+            send = (cenv->spr_cb[SPR_PIR].default_value == pir);
+        } else {
+            int pir = rb & DBELL_PROCIDTAG_MASK;
+            send = (rb & DBELL_BRDCAST) || (cenv->spr[SPR_BOOKE_PIR] == pir);
+        }
+        if (send) {
             cenv->pending_interrupts |= 1 << irq;
             cpu_interrupt(cs, CPU_INTERRUPT_HARD);
         }
diff --git a/target/ppc/helper.h b/target/ppc/helper.h
index bb6a94a8b390..3e98bd9eecb8 100644
--- a/target/ppc/helper.h
+++ b/target/ppc/helper.h
@@ -677,7 +677,7 @@ DEF_HELPER_FLAGS_2(load_sr, TCG_CALL_NO_RWG, tl, env, tl)
 DEF_HELPER_FLAGS_3(store_sr, TCG_CALL_NO_RWG, void, env, tl, tl)
 
 DEF_HELPER_FLAGS_1(602_mfrom, TCG_CALL_NO_RWG_SE, tl, tl)
-DEF_HELPER_1(msgsnd, void, tl)
+DEF_HELPER_2(msgsnd, void, env, tl)
 DEF_HELPER_2(msgclr, void, env, tl)
 #endif
 
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index bcd36d53537f..1ad7e0fd4efd 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -6180,10 +6180,19 @@ static void gen_msgsnd(DisasContext *ctx)
     GEN_PRIV;
 #else
     CHK_HV;
-    gen_helper_msgsnd(cpu_gpr[rB(ctx->opcode)]);
+    gen_helper_msgsnd(cpu_env, cpu_gpr[rB(ctx->opcode)]);
 #endif /* defined(CONFIG_USER_ONLY) */
 }
 
+static void gen_msgsync(DisasContext *ctx)
+{
+#if defined(CONFIG_USER_ONLY)
+    GEN_PRIV;
+#else
+    CHK_HV;
+#endif /* defined(CONFIG_USER_ONLY) */
+    /* interpreted as no-op */
+}
 
 #if defined(TARGET_PPC64)
 static void gen_maddld(DisasContext *ctx)
@@ -6664,6 +6673,8 @@ GEN_HANDLER2_E(msgsnd, "msgsnd", 0x1F, 0x0E, 0x06, 0x03ff0001,
                PPC_NONE, PPC2_PRCNTL),
 GEN_HANDLER2_E(msgclr, "msgclr", 0x1F, 0x0E, 0x07, 0x03ff0001,
                PPC_NONE, PPC2_PRCNTL),
+GEN_HANDLER2_E(msgsync, "msgsync", 0x1F, 0x16, 0x1B, 0x00000000,
+               PPC_NONE, PPC2_PRCNTL),
 GEN_HANDLER(wrtee, 0x1F, 0x03, 0x04, 0x000FFC01, PPC_WRTEE),
 GEN_HANDLER(wrteei, 0x1F, 0x03, 0x05, 0x000E7C01, PPC_WRTEE),
 GEN_HANDLER(dlmzb, 0x1F, 0x0E, 0x02, 0x00000000, PPC_440_SPEC),
diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
index 70ff15a51a6b..55c99c97e377 100644
--- a/target/ppc/translate_init.c
+++ b/target/ppc/translate_init.c
@@ -8866,7 +8866,7 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
                         PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 |
                         PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
                         PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
-                        PPC2_TM | PPC2_PM_ISA206 | PPC2_ISA300;
+                        PPC2_TM | PPC2_PM_ISA206 | PPC2_ISA300 | PPC2_PRCNTL;
     pcc->msr_mask = (1ull << MSR_SF) |
                     (1ull << MSR_TM) |
                     (1ull << MSR_VR) |
-- 
2.13.6

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] target/ppc: add support for hypervisor doorbells on book3s CPUs
  2018-01-16  7:41 ` [Qemu-devel] [PATCH 3/3] target/ppc: add support for hypervisor doorbells on book3s CPUs Cédric Le Goater
@ 2018-01-16  7:45   ` Cédric Le Goater
  2018-01-18  4:15   ` David Gibson
  1 sibling, 0 replies; 8+ messages in thread
From: Cédric Le Goater @ 2018-01-16  7:45 UTC (permalink / raw)
  To: qemu-ppc, qemu-devel, David Gibson

On 01/16/2018 08:41 AM, Cédric Le Goater wrote:
> The hypervisor doorbells are used by skiboot and Linux on POWER9
> processors to wake up secondaries.
> 
> This adds processor control support to the Server architecture by
> reusing the Embedded support. They are very similar, only the bits
> definition of the CPU identifier differ.
> 
> Still to be done is message broadcast to all threads of the same
> processor.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  target/ppc/cpu.h            |  8 ++++++--
>  target/ppc/excp_helper.c    | 39 ++++++++++++++++++++++++++++++++-------
>  target/ppc/helper.h         |  2 +-
>  target/ppc/translate.c      | 13 ++++++++++++-
>  target/ppc/translate_init.c |  2 +-
>  5 files changed, 52 insertions(+), 12 deletions(-)
> 
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index b8f4dfc1084a..603a38cae83f 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -930,7 +930,7 @@ enum {
>  #define BOOKE206_MAX_TLBN      4
>  
>  /*****************************************************************************/
> -/* Embedded.Processor Control */
> +/* Server and Embedded Processor Control */
>  
>  #define DBELL_TYPE_SHIFT               27
>  #define DBELL_TYPE_MASK                (0x1f << DBELL_TYPE_SHIFT)
> @@ -940,11 +940,15 @@ enum {
>  #define DBELL_TYPE_G_DBELL_CRIT        (0x03 << DBELL_TYPE_SHIFT)
>  #define DBELL_TYPE_G_DBELL_MC          (0x04 << DBELL_TYPE_SHIFT)
>  
> -#define DBELL_BRDCAST                  (1 << 26)
> +#define DBELL_TYPE_DBELL_SERVER        (0x05 << DBELL_TYPE_SHIFT)
> +
> +#define DBELL_BRDCAST                  PPC_BIT(37)
>  #define DBELL_LPIDTAG_SHIFT            14
>  #define DBELL_LPIDTAG_MASK             (0xfff << DBELL_LPIDTAG_SHIFT)
>  #define DBELL_PIRTAG_MASK              0x3fff
>  
> +#define DBELL_PROCIDTAG_MASK           PPC_BITMASK(44, 63)
> +
>  /*****************************************************************************/
>  /* Segment page size information, used by recent hash MMUs
>   * The format of this structure mirrors kvm_ppc_smmu_info
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 4e548a448747..0f32cab1ff57 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -417,6 +417,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
>      case POWERPC_EXCP_HISI:      /* Hypervisor instruction storage exception */
>      case POWERPC_EXCP_HDSEG:     /* Hypervisor data segment exception        */
>      case POWERPC_EXCP_HISEG:     /* Hypervisor instruction segment exception */
> +    case POWERPC_EXCP_SDOOR_HV:  /* Hypervisor Doorbell interrupt            */
>      case POWERPC_EXCP_HV_EMU:
>          srr0 = SPR_HSRR0;
>          srr1 = SPR_HSRR1;
> @@ -846,6 +847,11 @@ static void ppc_hw_interrupt(CPUPPCState *env)
>              powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_DOORI);
>              return;
>          }
> +        if (env->pending_interrupts & (1 << PPC_INTERRUPT_HDOORBELL)) {
> +            env->pending_interrupts &= ~(1 << PPC_INTERRUPT_HDOORBELL);
> +            powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_SDOOR_HV);
> +            return;
> +        }
>          if (env->pending_interrupts & (1 << PPC_INTERRUPT_PERFM)) {
>              env->pending_interrupts &= ~(1 << PPC_INTERRUPT_PERFM);
>              powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_PERFM);
> @@ -1088,8 +1094,8 @@ void helper_rfsvc(CPUPPCState *env)
>      do_rfi(env, env->lr, env->ctr & 0x0000FFFF);
>  }
>  
> -/* Embedded.Processor Control */
> -static int dbell2irq(target_ulong rb)
> +/* Server and Embedded Processor Control */
> +static int dbell2irq(target_ulong rb, bool book3s)
>  {
>      int msg = rb & DBELL_TYPE_MASK;
>      int irq = -1;
> @@ -1109,12 +1115,21 @@ static int dbell2irq(target_ulong rb)
>          break;
>      }
>  
> +    /* A Directed Hypervisor Doorbell message is sent only if the
> +     * message type is 5. All other types are reserved and the
> +     * instruction is a no-op */
> +    if (book3s && msg == DBELL_TYPE_DBELL_SERVER) {
> +        irq = PPC_INTERRUPT_HDOORBELL;
> +    }
> +
>      return irq;
>  }
>  
>  void helper_msgclr(CPUPPCState *env, target_ulong rb)
>  {
> -    int irq = dbell2irq(rb);
> +    /* 64-bit server processors compliant with arch 2.x */
> +    bool book3s = (env->insns_flags & PPC_SEGMENT_64B);
> +    int irq = dbell2irq(rb, book3s);
>  
>      if (irq < 0) {
>          return;
> @@ -1123,10 +1138,11 @@ void helper_msgclr(CPUPPCState *env, target_ulong rb)
>      env->pending_interrupts &= ~(1 << irq);
>  }
>  
> -void helper_msgsnd(target_ulong rb)
> +void helper_msgsnd(CPUPPCState *env, target_ulong rb)
>  {
> -    int irq = dbell2irq(rb);
> -    int pir = rb & DBELL_PIRTAG_MASK;
> +    /* 64-bit server processors compliant with arch 2.x */
> +    bool book3s = (env->insns_flags & PPC_SEGMENT_64B);
> +    int irq = dbell2irq(rb, book3s);
>      CPUState *cs;
>  
>      if (irq < 0) {
> @@ -1137,8 +1153,17 @@ void helper_msgsnd(target_ulong rb)
>      CPU_FOREACH(cs) {
>          PowerPCCPU *cpu = POWERPC_CPU(cs);
>          CPUPPCState *cenv = &cpu->env;
> +        bool send;
>  
> -        if ((rb & DBELL_BRDCAST) || (cenv->spr[SPR_BOOKE_PIR] == pir)) {
> +        /* TODO: broadcast message to all threads of the same  processor */
> +        if (book3s) {
> +            int pir = rb & DBELL_PROCIDTAG_MASK;
> +            send = (cenv->spr_cb[SPR_PIR].default_value == pir);
> +        } else {
> +            int pir = rb & DBELL_PROCIDTAG_MASK;

sigh. I missed that one. It should be 

		int pir = rb & DBELL_PIRTAG_MASK

Tell me if it needs a resend.

Thanks,

C. 

> +            send = (rb & DBELL_BRDCAST) || (cenv->spr[SPR_BOOKE_PIR] == pir);
> +        }
> +        if (send) {
>              cenv->pending_interrupts |= 1 << irq;
>              cpu_interrupt(cs, CPU_INTERRUPT_HARD);
>          }
> diff --git a/target/ppc/helper.h b/target/ppc/helper.h
> index bb6a94a8b390..3e98bd9eecb8 100644
> --- a/target/ppc/helper.h
> +++ b/target/ppc/helper.h
> @@ -677,7 +677,7 @@ DEF_HELPER_FLAGS_2(load_sr, TCG_CALL_NO_RWG, tl, env, tl)
>  DEF_HELPER_FLAGS_3(store_sr, TCG_CALL_NO_RWG, void, env, tl, tl)
>  
>  DEF_HELPER_FLAGS_1(602_mfrom, TCG_CALL_NO_RWG_SE, tl, tl)
> -DEF_HELPER_1(msgsnd, void, tl)
> +DEF_HELPER_2(msgsnd, void, env, tl)
>  DEF_HELPER_2(msgclr, void, env, tl)
>  #endif
>  
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index bcd36d53537f..1ad7e0fd4efd 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -6180,10 +6180,19 @@ static void gen_msgsnd(DisasContext *ctx)
>      GEN_PRIV;
>  #else
>      CHK_HV;
> -    gen_helper_msgsnd(cpu_gpr[rB(ctx->opcode)]);
> +    gen_helper_msgsnd(cpu_env, cpu_gpr[rB(ctx->opcode)]);
>  #endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
> +static void gen_msgsync(DisasContext *ctx)
> +{
> +#if defined(CONFIG_USER_ONLY)
> +    GEN_PRIV;
> +#else
> +    CHK_HV;
> +#endif /* defined(CONFIG_USER_ONLY) */
> +    /* interpreted as no-op */
> +}
>  
>  #if defined(TARGET_PPC64)
>  static void gen_maddld(DisasContext *ctx)
> @@ -6664,6 +6673,8 @@ GEN_HANDLER2_E(msgsnd, "msgsnd", 0x1F, 0x0E, 0x06, 0x03ff0001,
>                 PPC_NONE, PPC2_PRCNTL),
>  GEN_HANDLER2_E(msgclr, "msgclr", 0x1F, 0x0E, 0x07, 0x03ff0001,
>                 PPC_NONE, PPC2_PRCNTL),
> +GEN_HANDLER2_E(msgsync, "msgsync", 0x1F, 0x16, 0x1B, 0x00000000,
> +               PPC_NONE, PPC2_PRCNTL),
>  GEN_HANDLER(wrtee, 0x1F, 0x03, 0x04, 0x000FFC01, PPC_WRTEE),
>  GEN_HANDLER(wrteei, 0x1F, 0x03, 0x05, 0x000E7C01, PPC_WRTEE),
>  GEN_HANDLER(dlmzb, 0x1F, 0x0E, 0x02, 0x00000000, PPC_440_SPEC),
> diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
> index 70ff15a51a6b..55c99c97e377 100644
> --- a/target/ppc/translate_init.c
> +++ b/target/ppc/translate_init.c
> @@ -8866,7 +8866,7 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
>                          PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 |
>                          PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
>                          PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
> -                        PPC2_TM | PPC2_PM_ISA206 | PPC2_ISA300;
> +                        PPC2_TM | PPC2_PM_ISA206 | PPC2_ISA300 | PPC2_PRCNTL;
>      pcc->msr_mask = (1ull << MSR_SF) |
>                      (1ull << MSR_TM) |
>                      (1ull << MSR_VR) |
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 0/3] target/ppc: add support for hypervisor doorbells
  2018-01-16  7:41 [Qemu-devel] [PATCH 0/3] target/ppc: add support for hypervisor doorbells Cédric Le Goater
                   ` (2 preceding siblings ...)
  2018-01-16  7:41 ` [Qemu-devel] [PATCH 3/3] target/ppc: add support for hypervisor doorbells on book3s CPUs Cédric Le Goater
@ 2018-01-17  3:26 ` David Gibson
  3 siblings, 0 replies; 8+ messages in thread
From: David Gibson @ 2018-01-17  3:26 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel, Benjamin Herrenschmidt

[-- Attachment #1: Type: text/plain, Size: 1245 bytes --]

On Tue, Jan 16, 2018 at 08:41:54AM +0100, Cédric Le Goater wrote:
> Hi,
> 
> The hypervisor doorbells are used by skiboot and Linux on POWER9
> processors to wake up secondaries. This adds processor control support
> to the Book3S architecture.
> 
> The full tree can be found here :
> 
>   https://github.com/legoater/qemu powernv-2.12

1 & 2 look like correct fixes regardless of anything else, so I've
applied them.  Still looking at 3/3.

> 
> Thanks,
> 
> C.
> 
> Cédric Le Goater (3):
>   target/ppc: fix doorbell and hypervisor doorbell definitions
>   target/ppc: msgsnd and msgclr instructions need hypervisor privilege
>   target/ppc: add support for hypervisor doorbells on book3s CPUs
> 
>  target/ppc/cpu.h            | 16 ++++++++++------
>  target/ppc/excp_helper.c    | 39 ++++++++++++++++++++++++++++++++-------
>  target/ppc/helper.h         |  2 +-
>  target/ppc/translate.c      | 17 ++++++++++++++---
>  target/ppc/translate_init.c |  2 +-
>  5 files changed, 58 insertions(+), 18 deletions(-)
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] target/ppc: add support for hypervisor doorbells on book3s CPUs
  2018-01-16  7:41 ` [Qemu-devel] [PATCH 3/3] target/ppc: add support for hypervisor doorbells on book3s CPUs Cédric Le Goater
  2018-01-16  7:45   ` Cédric Le Goater
@ 2018-01-18  4:15   ` David Gibson
  2018-01-18  8:13     ` Cédric Le Goater
  1 sibling, 1 reply; 8+ messages in thread
From: David Gibson @ 2018-01-18  4:15 UTC (permalink / raw)
  To: Cédric Le Goater; +Cc: qemu-ppc, qemu-devel, Benjamin Herrenschmidt

[-- Attachment #1: Type: text/plain, Size: 9225 bytes --]

On Tue, Jan 16, 2018 at 08:41:57AM +0100, Cédric Le Goater wrote:
> The hypervisor doorbells are used by skiboot and Linux on POWER9
> processors to wake up secondaries.
> 
> This adds processor control support to the Server architecture by
> reusing the Embedded support. They are very similar, only the bits
> definition of the CPU identifier differ.
> 
> Still to be done is message broadcast to all threads of the same
> processor.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
>  target/ppc/cpu.h            |  8 ++++++--
>  target/ppc/excp_helper.c    | 39 ++++++++++++++++++++++++++++++++-------
>  target/ppc/helper.h         |  2 +-
>  target/ppc/translate.c      | 13 ++++++++++++-
>  target/ppc/translate_init.c |  2 +-
>  5 files changed, 52 insertions(+), 12 deletions(-)
> 
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index b8f4dfc1084a..603a38cae83f 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -930,7 +930,7 @@ enum {
>  #define BOOKE206_MAX_TLBN      4
>  
>  /*****************************************************************************/
> -/* Embedded.Processor Control */
> +/* Server and Embedded Processor Control */
>  
>  #define DBELL_TYPE_SHIFT               27
>  #define DBELL_TYPE_MASK                (0x1f << DBELL_TYPE_SHIFT)
> @@ -940,11 +940,15 @@ enum {
>  #define DBELL_TYPE_G_DBELL_CRIT        (0x03 << DBELL_TYPE_SHIFT)
>  #define DBELL_TYPE_G_DBELL_MC          (0x04 << DBELL_TYPE_SHIFT)
>  
> -#define DBELL_BRDCAST                  (1 << 26)
> +#define DBELL_TYPE_DBELL_SERVER        (0x05 << DBELL_TYPE_SHIFT)
> +
> +#define DBELL_BRDCAST                  PPC_BIT(37)
>  #define DBELL_LPIDTAG_SHIFT            14
>  #define DBELL_LPIDTAG_MASK             (0xfff << DBELL_LPIDTAG_SHIFT)
>  #define DBELL_PIRTAG_MASK              0x3fff
>  
> +#define DBELL_PROCIDTAG_MASK           PPC_BITMASK(44, 63)
> +
>  /*****************************************************************************/
>  /* Segment page size information, used by recent hash MMUs
>   * The format of this structure mirrors kvm_ppc_smmu_info
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 4e548a448747..0f32cab1ff57 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -417,6 +417,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
>      case POWERPC_EXCP_HISI:      /* Hypervisor instruction storage exception */
>      case POWERPC_EXCP_HDSEG:     /* Hypervisor data segment exception        */
>      case POWERPC_EXCP_HISEG:     /* Hypervisor instruction segment exception */
> +    case POWERPC_EXCP_SDOOR_HV:  /* Hypervisor Doorbell interrupt            */
>      case POWERPC_EXCP_HV_EMU:
>          srr0 = SPR_HSRR0;
>          srr1 = SPR_HSRR1;
> @@ -846,6 +847,11 @@ static void ppc_hw_interrupt(CPUPPCState *env)
>              powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_DOORI);
>              return;
>          }
> +        if (env->pending_interrupts & (1 << PPC_INTERRUPT_HDOORBELL)) {
> +            env->pending_interrupts &= ~(1 << PPC_INTERRUPT_HDOORBELL);
> +            powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_SDOOR_HV);
> +            return;
> +        }
>          if (env->pending_interrupts & (1 << PPC_INTERRUPT_PERFM)) {
>              env->pending_interrupts &= ~(1 << PPC_INTERRUPT_PERFM);
>              powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_PERFM);
> @@ -1088,8 +1094,8 @@ void helper_rfsvc(CPUPPCState *env)
>      do_rfi(env, env->lr, env->ctr & 0x0000FFFF);
>  }
>  
> -/* Embedded.Processor Control */
> -static int dbell2irq(target_ulong rb)
> +/* Server and Embedded Processor Control */
> +static int dbell2irq(target_ulong rb, bool book3s)
>  {
>      int msg = rb & DBELL_TYPE_MASK;
>      int irq = -1;
> @@ -1109,12 +1115,21 @@ static int dbell2irq(target_ulong rb)
>          break;
>      }
>  
> +    /* A Directed Hypervisor Doorbell message is sent only if the
> +     * message type is 5. All other types are reserved and the
> +     * instruction is a no-op */

I don't see that the logic here accomplishes that.  Other types will
return the same value as for embedded - that doesn't seem like it will
result in a no-op.

> +    if (book3s && msg == DBELL_TYPE_DBELL_SERVER) {
> +        irq = PPC_INTERRUPT_HDOORBELL;
> +    }
> +
>      return irq;
>  }
>  
>  void helper_msgclr(CPUPPCState *env, target_ulong rb)
>  {
> -    int irq = dbell2irq(rb);
> +    /* 64-bit server processors compliant with arch 2.x */
> +    bool book3s = (env->insns_flags & PPC_SEGMENT_64B);

Keying off an otherwise unrelated instruction bit seems bogus to me.

> +    int irq = dbell2irq(rb, book3s);
>  
>      if (irq < 0) {
>          return;
> @@ -1123,10 +1138,11 @@ void helper_msgclr(CPUPPCState *env, target_ulong rb)
>      env->pending_interrupts &= ~(1 << irq);
>  }
>  
> -void helper_msgsnd(target_ulong rb)
> +void helper_msgsnd(CPUPPCState *env, target_ulong rb)
>  {
> -    int irq = dbell2irq(rb);
> -    int pir = rb & DBELL_PIRTAG_MASK;
> +    /* 64-bit server processors compliant with arch 2.x */
> +    bool book3s = (env->insns_flags & PPC_SEGMENT_64B);
> +    int irq = dbell2irq(rb, book3s);
>      CPUState *cs;
>  
>      if (irq < 0) {
> @@ -1137,8 +1153,17 @@ void helper_msgsnd(target_ulong rb)
>      CPU_FOREACH(cs) {
>          PowerPCCPU *cpu = POWERPC_CPU(cs);
>          CPUPPCState *cenv = &cpu->env;
> +        bool send;
>  
> -        if ((rb & DBELL_BRDCAST) || (cenv->spr[SPR_BOOKE_PIR] == pir)) {
> +        /* TODO: broadcast message to all threads of the same  processor */
> +        if (book3s) {
> +            int pir = rb & DBELL_PROCIDTAG_MASK;
> +            send = (cenv->spr_cb[SPR_PIR].default_value == pir);
> +        } else {
> +            int pir = rb & DBELL_PROCIDTAG_MASK;
> +            send = (rb & DBELL_BRDCAST) || (cenv->spr[SPR_BOOKE_PIR] == pir);
> +        }
> +        if (send) {
>              cenv->pending_interrupts |= 1 << irq;
>              cpu_interrupt(cs, CPU_INTERRUPT_HARD);
>          }

TBH, these functions are small enough and the booke vs. books
differences large enough that I think you'd be better off just having
separate implementations for the booke and books variants.  Those in
turn would have separate instruction bits.

> diff --git a/target/ppc/helper.h b/target/ppc/helper.h
> index bb6a94a8b390..3e98bd9eecb8 100644
> --- a/target/ppc/helper.h
> +++ b/target/ppc/helper.h
> @@ -677,7 +677,7 @@ DEF_HELPER_FLAGS_2(load_sr, TCG_CALL_NO_RWG, tl, env, tl)
>  DEF_HELPER_FLAGS_3(store_sr, TCG_CALL_NO_RWG, void, env, tl, tl)
>  
>  DEF_HELPER_FLAGS_1(602_mfrom, TCG_CALL_NO_RWG_SE, tl, tl)
> -DEF_HELPER_1(msgsnd, void, tl)
> +DEF_HELPER_2(msgsnd, void, env, tl)
>  DEF_HELPER_2(msgclr, void, env, tl)
>  #endif
>  
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index bcd36d53537f..1ad7e0fd4efd 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -6180,10 +6180,19 @@ static void gen_msgsnd(DisasContext *ctx)
>      GEN_PRIV;
>  #else
>      CHK_HV;
> -    gen_helper_msgsnd(cpu_gpr[rB(ctx->opcode)]);
> +    gen_helper_msgsnd(cpu_env, cpu_gpr[rB(ctx->opcode)]);
>  #endif /* defined(CONFIG_USER_ONLY) */
>  }
>  
> +static void gen_msgsync(DisasContext *ctx)
> +{
> +#if defined(CONFIG_USER_ONLY)
> +    GEN_PRIV;
> +#else
> +    CHK_HV;
> +#endif /* defined(CONFIG_USER_ONLY) */
> +    /* interpreted as no-op */
> +}
>  
>  #if defined(TARGET_PPC64)
>  static void gen_maddld(DisasContext *ctx)
> @@ -6664,6 +6673,8 @@ GEN_HANDLER2_E(msgsnd, "msgsnd", 0x1F, 0x0E, 0x06, 0x03ff0001,
>                 PPC_NONE, PPC2_PRCNTL),
>  GEN_HANDLER2_E(msgclr, "msgclr", 0x1F, 0x0E, 0x07, 0x03ff0001,
>                 PPC_NONE, PPC2_PRCNTL),
> +GEN_HANDLER2_E(msgsync, "msgsync", 0x1F, 0x16, 0x1B, 0x00000000,
> +               PPC_NONE, PPC2_PRCNTL),
>  GEN_HANDLER(wrtee, 0x1F, 0x03, 0x04, 0x000FFC01, PPC_WRTEE),
>  GEN_HANDLER(wrteei, 0x1F, 0x03, 0x05, 0x000E7C01, PPC_WRTEE),
>  GEN_HANDLER(dlmzb, 0x1F, 0x0E, 0x02, 0x00000000, PPC_440_SPEC),
> diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
> index 70ff15a51a6b..55c99c97e377 100644
> --- a/target/ppc/translate_init.c
> +++ b/target/ppc/translate_init.c
> @@ -8866,7 +8866,7 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
>                          PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 |
>                          PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
>                          PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
> -                        PPC2_TM | PPC2_PM_ISA206 | PPC2_ISA300;
> +                        PPC2_TM | PPC2_PM_ISA206 | PPC2_ISA300 | PPC2_PRCNTL;
>      pcc->msr_mask = (1ull << MSR_SF) |
>                      (1ull << MSR_TM) |
>                      (1ull << MSR_VR) |

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Qemu-devel] [PATCH 3/3] target/ppc: add support for hypervisor doorbells on book3s CPUs
  2018-01-18  4:15   ` David Gibson
@ 2018-01-18  8:13     ` Cédric Le Goater
  0 siblings, 0 replies; 8+ messages in thread
From: Cédric Le Goater @ 2018-01-18  8:13 UTC (permalink / raw)
  To: David Gibson; +Cc: qemu-ppc, qemu-devel, Benjamin Herrenschmidt

On 01/18/2018 05:15 AM, David Gibson wrote:
> On Tue, Jan 16, 2018 at 08:41:57AM +0100, Cédric Le Goater wrote:
>> The hypervisor doorbells are used by skiboot and Linux on POWER9
>> processors to wake up secondaries.
>>
>> This adds processor control support to the Server architecture by
>> reusing the Embedded support. They are very similar, only the bits
>> definition of the CPU identifier differ.
>>
>> Still to be done is message broadcast to all threads of the same
>> processor.
>>
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
>> ---
>>  target/ppc/cpu.h            |  8 ++++++--
>>  target/ppc/excp_helper.c    | 39 ++++++++++++++++++++++++++++++++-------
>>  target/ppc/helper.h         |  2 +-
>>  target/ppc/translate.c      | 13 ++++++++++++-
>>  target/ppc/translate_init.c |  2 +-
>>  5 files changed, 52 insertions(+), 12 deletions(-)
>>
>> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
>> index b8f4dfc1084a..603a38cae83f 100644
>> --- a/target/ppc/cpu.h
>> +++ b/target/ppc/cpu.h
>> @@ -930,7 +930,7 @@ enum {
>>  #define BOOKE206_MAX_TLBN      4
>>  
>>  /*****************************************************************************/
>> -/* Embedded.Processor Control */
>> +/* Server and Embedded Processor Control */
>>  
>>  #define DBELL_TYPE_SHIFT               27
>>  #define DBELL_TYPE_MASK                (0x1f << DBELL_TYPE_SHIFT)
>> @@ -940,11 +940,15 @@ enum {
>>  #define DBELL_TYPE_G_DBELL_CRIT        (0x03 << DBELL_TYPE_SHIFT)
>>  #define DBELL_TYPE_G_DBELL_MC          (0x04 << DBELL_TYPE_SHIFT)
>>  
>> -#define DBELL_BRDCAST                  (1 << 26)
>> +#define DBELL_TYPE_DBELL_SERVER        (0x05 << DBELL_TYPE_SHIFT)
>> +
>> +#define DBELL_BRDCAST                  PPC_BIT(37)
>>  #define DBELL_LPIDTAG_SHIFT            14
>>  #define DBELL_LPIDTAG_MASK             (0xfff << DBELL_LPIDTAG_SHIFT)
>>  #define DBELL_PIRTAG_MASK              0x3fff
>>  
>> +#define DBELL_PROCIDTAG_MASK           PPC_BITMASK(44, 63)
>> +
>>  /*****************************************************************************/
>>  /* Segment page size information, used by recent hash MMUs
>>   * The format of this structure mirrors kvm_ppc_smmu_info
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>> index 4e548a448747..0f32cab1ff57 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -417,6 +417,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
>>      case POWERPC_EXCP_HISI:      /* Hypervisor instruction storage exception */
>>      case POWERPC_EXCP_HDSEG:     /* Hypervisor data segment exception        */
>>      case POWERPC_EXCP_HISEG:     /* Hypervisor instruction segment exception */
>> +    case POWERPC_EXCP_SDOOR_HV:  /* Hypervisor Doorbell interrupt            */
>>      case POWERPC_EXCP_HV_EMU:
>>          srr0 = SPR_HSRR0;
>>          srr1 = SPR_HSRR1;
>> @@ -846,6 +847,11 @@ static void ppc_hw_interrupt(CPUPPCState *env)
>>              powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_DOORI);
>>              return;
>>          }
>> +        if (env->pending_interrupts & (1 << PPC_INTERRUPT_HDOORBELL)) {
>> +            env->pending_interrupts &= ~(1 << PPC_INTERRUPT_HDOORBELL);
>> +            powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_SDOOR_HV);
>> +            return;
>> +        }
>>          if (env->pending_interrupts & (1 << PPC_INTERRUPT_PERFM)) {
>>              env->pending_interrupts &= ~(1 << PPC_INTERRUPT_PERFM);
>>              powerpc_excp(cpu, env->excp_model, POWERPC_EXCP_PERFM);
>> @@ -1088,8 +1094,8 @@ void helper_rfsvc(CPUPPCState *env)
>>      do_rfi(env, env->lr, env->ctr & 0x0000FFFF);
>>  }
>>  
>> -/* Embedded.Processor Control */
>> -static int dbell2irq(target_ulong rb)
>> +/* Server and Embedded Processor Control */
>> +static int dbell2irq(target_ulong rb, bool book3s)
>>  {
>>      int msg = rb & DBELL_TYPE_MASK;
>>      int irq = -1;
>> @@ -1109,12 +1115,21 @@ static int dbell2irq(target_ulong rb)
>>          break;
>>      }
>>  
>> +    /* A Directed Hypervisor Doorbell message is sent only if the
>> +     * message type is 5. All other types are reserved and the
>> +     * instruction is a no-op */
> 
> I don't see that the logic here accomplishes that.  Other types will
> return the same value as for embedded - that doesn't seem like it will
> result in a no-op.

yes ... I tried to reuse existing code. I will add a specific routine
for book3s.

>> +    if (book3s && msg == DBELL_TYPE_DBELL_SERVER) {
>> +        irq = PPC_INTERRUPT_HDOORBELL;
>> +    }
>> +
>>      return irq;
>>  }
>>  
>>  void helper_msgclr(CPUPPCState *env, target_ulong rb)
>>  {
>> -    int irq = dbell2irq(rb);
>> +    /* 64-bit server processors compliant with arch 2.x */
>> +    bool book3s = (env->insns_flags & PPC_SEGMENT_64B);
> 
> Keying off an otherwise unrelated instruction bit seems bogus to me.

That is the option we took for rfi in commit 6ca038c292d9  "ppc: restrict 
the use of the rfi instruction"

May be we could introduce an helper for it ?

> 
>> +    int irq = dbell2irq(rb, book3s);
>>  
>>      if (irq < 0) {
>>          return;
>> @@ -1123,10 +1138,11 @@ void helper_msgclr(CPUPPCState *env, target_ulong rb)
>>      env->pending_interrupts &= ~(1 << irq);
>>  }
>>  
>> -void helper_msgsnd(target_ulong rb)
>> +void helper_msgsnd(CPUPPCState *env, target_ulong rb)
>>  {
>> -    int irq = dbell2irq(rb);
>> -    int pir = rb & DBELL_PIRTAG_MASK;
>> +    /* 64-bit server processors compliant with arch 2.x */
>> +    bool book3s = (env->insns_flags & PPC_SEGMENT_64B);
>> +    int irq = dbell2irq(rb, book3s);
>>      CPUState *cs;
>>  
>>      if (irq < 0) {
>> @@ -1137,8 +1153,17 @@ void helper_msgsnd(target_ulong rb)
>>      CPU_FOREACH(cs) {
>>          PowerPCCPU *cpu = POWERPC_CPU(cs);
>>          CPUPPCState *cenv = &cpu->env;
>> +        bool send;
>>  
>> -        if ((rb & DBELL_BRDCAST) || (cenv->spr[SPR_BOOKE_PIR] == pir)) {
>> +        /* TODO: broadcast message to all threads of the same  processor */
>> +        if (book3s) {
>> +            int pir = rb & DBELL_PROCIDTAG_MASK;
>> +            send = (cenv->spr_cb[SPR_PIR].default_value == pir);
>> +        } else {
>> +            int pir = rb & DBELL_PROCIDTAG_MASK;
>> +            send = (rb & DBELL_BRDCAST) || (cenv->spr[SPR_BOOKE_PIR] == pir);
>> +        }
>> +        if (send) {
>>              cenv->pending_interrupts |= 1 << irq;
>>              cpu_interrupt(cs, CPU_INTERRUPT_HARD);
>>          }
> 
> TBH, these functions are small enough and the booke vs. books
> differences large enough that I think you'd be better off just having
> separate implementations for the booke and books variants.  Those in
> turn would have separate instruction bits.

ok. I will and in that case we could introduce a specific dbell2irq()
for each architecture possibly ?

Thanks,

C. 

> 
>> diff --git a/target/ppc/helper.h b/target/ppc/helper.h
>> index bb6a94a8b390..3e98bd9eecb8 100644
>> --- a/target/ppc/helper.h
>> +++ b/target/ppc/helper.h
>> @@ -677,7 +677,7 @@ DEF_HELPER_FLAGS_2(load_sr, TCG_CALL_NO_RWG, tl, env, tl)
>>  DEF_HELPER_FLAGS_3(store_sr, TCG_CALL_NO_RWG, void, env, tl, tl)
>>  
>>  DEF_HELPER_FLAGS_1(602_mfrom, TCG_CALL_NO_RWG_SE, tl, tl)
>> -DEF_HELPER_1(msgsnd, void, tl)
>> +DEF_HELPER_2(msgsnd, void, env, tl)
>>  DEF_HELPER_2(msgclr, void, env, tl)
>>  #endif
>>  
>> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
>> index bcd36d53537f..1ad7e0fd4efd 100644
>> --- a/target/ppc/translate.c
>> +++ b/target/ppc/translate.c
>> @@ -6180,10 +6180,19 @@ static void gen_msgsnd(DisasContext *ctx)
>>      GEN_PRIV;
>>  #else
>>      CHK_HV;
>> -    gen_helper_msgsnd(cpu_gpr[rB(ctx->opcode)]);
>> +    gen_helper_msgsnd(cpu_env, cpu_gpr[rB(ctx->opcode)]);
>>  #endif /* defined(CONFIG_USER_ONLY) */
>>  }
>>  
>> +static void gen_msgsync(DisasContext *ctx)
>> +{
>> +#if defined(CONFIG_USER_ONLY)
>> +    GEN_PRIV;
>> +#else
>> +    CHK_HV;
>> +#endif /* defined(CONFIG_USER_ONLY) */
>> +    /* interpreted as no-op */
>> +}
>>  
>>  #if defined(TARGET_PPC64)
>>  static void gen_maddld(DisasContext *ctx)
>> @@ -6664,6 +6673,8 @@ GEN_HANDLER2_E(msgsnd, "msgsnd", 0x1F, 0x0E, 0x06, 0x03ff0001,
>>                 PPC_NONE, PPC2_PRCNTL),
>>  GEN_HANDLER2_E(msgclr, "msgclr", 0x1F, 0x0E, 0x07, 0x03ff0001,
>>                 PPC_NONE, PPC2_PRCNTL),
>> +GEN_HANDLER2_E(msgsync, "msgsync", 0x1F, 0x16, 0x1B, 0x00000000,
>> +               PPC_NONE, PPC2_PRCNTL),
>>  GEN_HANDLER(wrtee, 0x1F, 0x03, 0x04, 0x000FFC01, PPC_WRTEE),
>>  GEN_HANDLER(wrteei, 0x1F, 0x03, 0x05, 0x000E7C01, PPC_WRTEE),
>>  GEN_HANDLER(dlmzb, 0x1F, 0x0E, 0x02, 0x00000000, PPC_440_SPEC),
>> diff --git a/target/ppc/translate_init.c b/target/ppc/translate_init.c
>> index 70ff15a51a6b..55c99c97e377 100644
>> --- a/target/ppc/translate_init.c
>> +++ b/target/ppc/translate_init.c
>> @@ -8866,7 +8866,7 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
>>                          PPC2_FP_TST_ISA206 | PPC2_BCTAR_ISA207 |
>>                          PPC2_LSQ_ISA207 | PPC2_ALTIVEC_207 |
>>                          PPC2_ISA205 | PPC2_ISA207S | PPC2_FP_CVT_S64 |
>> -                        PPC2_TM | PPC2_PM_ISA206 | PPC2_ISA300;
>> +                        PPC2_TM | PPC2_PM_ISA206 | PPC2_ISA300 | PPC2_PRCNTL;
>>      pcc->msr_mask = (1ull << MSR_SF) |
>>                      (1ull << MSR_TM) |
>>                      (1ull << MSR_VR) |
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2018-01-18  8:13 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-16  7:41 [Qemu-devel] [PATCH 0/3] target/ppc: add support for hypervisor doorbells Cédric Le Goater
2018-01-16  7:41 ` [Qemu-devel] [PATCH 1/3] target/ppc: fix doorbell and hypervisor doorbell definitions Cédric Le Goater
2018-01-16  7:41 ` [Qemu-devel] [PATCH 2/3] target/ppc: msgsnd and msgclr instructions need hypervisor privilege Cédric Le Goater
2018-01-16  7:41 ` [Qemu-devel] [PATCH 3/3] target/ppc: add support for hypervisor doorbells on book3s CPUs Cédric Le Goater
2018-01-16  7:45   ` Cédric Le Goater
2018-01-18  4:15   ` David Gibson
2018-01-18  8:13     ` Cédric Le Goater
2018-01-17  3:26 ` [Qemu-devel] [PATCH 0/3] target/ppc: add support for hypervisor doorbells David Gibson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.