All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: "Cédric Le Goater" <clg@kaod.org>
Cc: qemu-ppc@nongnu.org, qemu-devel@nongnu.org,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>
Subject: Re: [Qemu-devel] [PATCH 07/10] ppc: Add real mode CI load/store instructions for P7 and P8
Date: Wed, 15 Jun 2016 13:46:21 +1000	[thread overview]
Message-ID: <20160615034621.GG4882@voom.fritz.box> (raw)
In-Reply-To: <1465795496-15071-8-git-send-email-clg@kaod.org>

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

On Mon, Jun 13, 2016 at 07:24:53AM +0200, Cédric Le Goater wrote:
> From: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> 
> Those instructions are only available in hypervisor real mode and
> allow cache inhibited garded access to devices in that mode.
> 
> Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> [clg: fixed checkpatch.pl errors ]
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
> 
> This patch still has a couple of checkpatch issues which I did not
> know quite understand:
> 
> ERROR: Macros with complex values should be enclosed in parenthesis
> #176: FILE: target-ppc/translate.c:10238:
> +#define GEN_LDX_E(name, ldop, opc2, opc3, type, type2, chk)                   \
>  GEN_HANDLER_E(name##x, 0x1F, opc2, opc3, 0x00000001, type, type2),
> 
> ERROR: Macros with complex values should be enclosed in parenthesis
> #200: FILE: target-ppc/translate.c:10277:
> +#define GEN_STX_E(name, stop, opc2, opc3, type, type2, chk)                   \
>  GEN_HANDLER_E(name##x, 0x1F, opc2, opc3, 0x00000001, type, type2),

I believe that's because of the trailing comma in those expansions - I
think those are making checkpatch think the macro expands to an
expression, which should be parenthesised, but isn't.

That expansion is kinda weird, and quite possibly a bad idea, but it
wasn't actually changed by this patch - this just changes the macro
arguments.  So, I don't think it's within the scope of this patch to
fix it, i.e. I believe you can ignore the checkpatch warnings in this
case.

> total: 2 errors, 0 warnings, 196 lines checked
> 
> 
>  target-ppc/cpu.h            |  4 ++-
>  target-ppc/translate.c      | 59 ++++++++++++++++++++++++++++++++++++---------
>  target-ppc/translate_init.c |  6 +++--
>  3 files changed, 55 insertions(+), 14 deletions(-)
> 
> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index f005549c352e..61a24b19ffce 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -1912,6 +1912,8 @@ enum {
>      PPC_POPCNTB        = 0x0000000000001000ULL,
>      /*   string load / store                                                 */
>      PPC_STRING         = 0x0000000000002000ULL,
> +    /*   real mode cache inhibited load / store                              */
> +    PPC_CILDST         = 0x0000000000004000ULL,
>  
>      /* Floating-point unit extensions                                        */
>      /*   Optional floating point instructions                                */
> @@ -2026,7 +2028,7 @@ enum {
>                          | PPC_MFAPIDI | PPC_TLBIVA | PPC_TLBIVAX \
>                          | PPC_4xx_COMMON | PPC_40x_ICBT | PPC_RFMCI \
>                          | PPC_RFDI | PPC_DCR | PPC_DCRX | PPC_DCRUX \
> -                        | PPC_POPCNTWD)
> +                        | PPC_POPCNTWD | PPC_CILDST)
>  
>      /* extended type values */
>  
> diff --git a/target-ppc/translate.c b/target-ppc/translate.c
> index 2ec858063ecc..b594b18399f7 100644
> --- a/target-ppc/translate.c
> +++ b/target-ppc/translate.c
> @@ -192,7 +192,7 @@ struct DisasContext {
>      uint32_t opcode;
>      uint32_t exception;
>      /* Routine used to access memory */
> -    bool pr, hv;
> +    bool pr, hv, dr;
>      bool lazy_tlb_flush;
>      int mem_idx;
>      int access_type;
> @@ -387,6 +387,7 @@ typedef struct opcode_t {
>  #if defined(CONFIG_USER_ONLY)
>  #define CHK_HV GEN_PRIV
>  #define CHK_SV GEN_PRIV
> +#define CHK_HVRM GEN_PRIV
>  #else
>  #define CHK_HV                                                          \
>      do {                                                                \
> @@ -400,6 +401,12 @@ typedef struct opcode_t {
>              GEN_PRIV;            \
>          }                        \
>      } while (0)
> +#define CHK_HVRM                                            \
> +    do {                                                    \
> +        if (unlikely(ctx->pr || !ctx->hv || ctx->dr)) {     \
> +            GEN_PRIV;                                       \
> +        }                                                   \
> +    } while (0)
>  #endif
>  
>  #define CHK_NONE
> @@ -2895,18 +2902,23 @@ static void glue(gen_, name##ux)(DisasContext *ctx)
>      tcg_temp_free(EA);                                                        \
>  }
>  
> -#define GEN_LDX_E(name, ldop, opc2, opc3, type, type2)                        \
> +#define GEN_LDX_E(name, ldop, opc2, opc3, type, type2, chk)                   \
>  static void glue(gen_, name##x)(DisasContext *ctx)                            \
>  {                                                                             \
>      TCGv EA;                                                                  \
> +    chk;                                                                      \
>      gen_set_access_type(ctx, ACCESS_INT);                                     \
>      EA = tcg_temp_new();                                                      \
>      gen_addr_reg_index(ctx, EA);                                              \
>      gen_qemu_##ldop(ctx, cpu_gpr[rD(ctx->opcode)], EA);                       \
>      tcg_temp_free(EA);                                                        \
>  }
> +
>  #define GEN_LDX(name, ldop, opc2, opc3, type)                                 \
> -    GEN_LDX_E(name, ldop, opc2, opc3, type, PPC_NONE)
> +    GEN_LDX_E(name, ldop, opc2, opc3, type, PPC_NONE, CHK_NONE)
> +
> +#define GEN_LDX_HVRM(name, ldop, opc2, opc3, type)                            \
> +    GEN_LDX_E(name, ldop, opc2, opc3, type, PPC_NONE, CHK_HVRM)
>  
>  #define GEN_LDS(name, ldop, op, type)                                         \
>  GEN_LD(name, ldop, op | 0x20, type);                                          \
> @@ -2932,6 +2944,12 @@ GEN_LDUX(ld, ld64, 0x15, 0x01, PPC_64B);
>  /* ldx */
>  GEN_LDX(ld, ld64, 0x15, 0x00, PPC_64B);
>  
> +/* CI load/store variants */
> +GEN_LDX_HVRM(ldcix, ld64, 0x15, 0x1b, PPC_CILDST)
> +GEN_LDX_HVRM(lwzcix, ld32u, 0x15, 0x15, PPC_CILDST)
> +GEN_LDX_HVRM(lhzcix, ld16u, 0x15, 0x19, PPC_CILDST)
> +GEN_LDX_HVRM(lbzcix, ld8u, 0x15, 0x1a, PPC_CILDST)
> +
>  static void gen_ld(DisasContext *ctx)
>  {
>      TCGv EA;
> @@ -3050,10 +3068,11 @@ static void glue(gen_, name##ux)(DisasContext *ctx)
>      tcg_temp_free(EA);                                                        \
>  }
>  
> -#define GEN_STX_E(name, stop, opc2, opc3, type, type2)                        \
> +#define GEN_STX_E(name, stop, opc2, opc3, type, type2, chk)                   \
>  static void glue(gen_, name##x)(DisasContext *ctx)                            \
>  {                                                                             \
>      TCGv EA;                                                                  \
> +    chk;                                                                      \
>      gen_set_access_type(ctx, ACCESS_INT);                                     \
>      EA = tcg_temp_new();                                                      \
>      gen_addr_reg_index(ctx, EA);                                              \
> @@ -3061,7 +3080,10 @@ static void glue(gen_, name##x)(DisasContext *ctx)                            \
>      tcg_temp_free(EA);                                                        \
>  }
>  #define GEN_STX(name, stop, opc2, opc3, type)                                 \
> -    GEN_STX_E(name, stop, opc2, opc3, type, PPC_NONE)
> +    GEN_STX_E(name, stop, opc2, opc3, type, PPC_NONE, CHK_NONE)
> +
> +#define GEN_STX_HVRM(name, stop, opc2, opc3, type)                            \
> +    GEN_STX_E(name, stop, opc2, opc3, type, PPC_NONE, CHK_HVRM)
>  
>  #define GEN_STS(name, stop, op, type)                                         \
>  GEN_ST(name, stop, op | 0x20, type);                                          \
> @@ -3078,6 +3100,10 @@ GEN_STS(stw, st32, 0x04, PPC_INTEGER);
>  #if defined(TARGET_PPC64)
>  GEN_STUX(std, st64, 0x15, 0x05, PPC_64B);
>  GEN_STX(std, st64, 0x15, 0x04, PPC_64B);
> +GEN_STX_HVRM(stdcix, st64, 0x15, 0x1f, PPC_CILDST)
> +GEN_STX_HVRM(stwcix, st32, 0x15, 0x1c, PPC_CILDST)
> +GEN_STX_HVRM(sthcix, st16, 0x15, 0x1d, PPC_CILDST)
> +GEN_STX_HVRM(stbcix, st8, 0x15, 0x1e, PPC_CILDST)
>  
>  static void gen_std(DisasContext *ctx)
>  {
> @@ -3166,7 +3192,7 @@ static inline void gen_qemu_ld64ur(DisasContext *ctx, TCGv arg1, TCGv arg2)
>      TCGMemOp op = MO_Q | (ctx->default_tcg_memop_mask ^ MO_BSWAP);
>      tcg_gen_qemu_ld_i64(arg1, arg2, ctx->mem_idx, op);
>  }
> -GEN_LDX_E(ldbr, ld64ur, 0x14, 0x10, PPC_NONE, PPC2_DBRX);
> +GEN_LDX_E(ldbr, ld64ur, 0x14, 0x10, PPC_NONE, PPC2_DBRX, CHK_NONE);
>  #endif  /* TARGET_PPC64 */
>  
>  /* sthbrx */
> @@ -3192,7 +3218,7 @@ static inline void gen_qemu_st64r(DisasContext *ctx, TCGv arg1, TCGv arg2)
>      TCGMemOp op = MO_Q | (ctx->default_tcg_memop_mask ^ MO_BSWAP);
>      tcg_gen_qemu_st_i64(arg1, arg2, ctx->mem_idx, op);
>  }
> -GEN_STX_E(stdbr, st64r, 0x14, 0x14, PPC_NONE, PPC2_DBRX);
> +GEN_STX_E(stdbr, st64r, 0x14, 0x14, PPC_NONE, PPC2_DBRX, CHK_NONE);
>  #endif  /* TARGET_PPC64 */
>  
>  /***                    Integer load and store multiple                    ***/
> @@ -10209,7 +10235,7 @@ GEN_HANDLER(name, opc, 0xFF, 0xFF, 0x00000000, type),
>  GEN_HANDLER(name##u, opc, 0xFF, 0xFF, 0x00000000, type),
>  #define GEN_LDUX(name, ldop, opc2, opc3, type)                                \
>  GEN_HANDLER(name##ux, 0x1F, opc2, opc3, 0x00000001, type),
> -#define GEN_LDX_E(name, ldop, opc2, opc3, type, type2)                        \
> +#define GEN_LDX_E(name, ldop, opc2, opc3, type, type2, chk)                   \
>  GEN_HANDLER_E(name##x, 0x1F, opc2, opc3, 0x00000001, type, type2),
>  #define GEN_LDS(name, ldop, op, type)                                         \
>  GEN_LD(name, ldop, op | 0x20, type)                                           \
> @@ -10226,7 +10252,13 @@ GEN_LDUX(lwa, ld32s, 0x15, 0x0B, PPC_64B)
>  GEN_LDX(lwa, ld32s, 0x15, 0x0A, PPC_64B)
>  GEN_LDUX(ld, ld64, 0x15, 0x01, PPC_64B)
>  GEN_LDX(ld, ld64, 0x15, 0x00, PPC_64B)
> -GEN_LDX_E(ldbr, ld64ur, 0x14, 0x10, PPC_NONE, PPC2_DBRX)
> +GEN_LDX_E(ldbr, ld64ur, 0x14, 0x10, PPC_NONE, PPC2_DBRX, CHK_NONE)
> +
> +/* HV/P7 and later only */
> +GEN_LDX_HVRM(ldcix, ld64, 0x15, 0x1b, PPC_CILDST)
> +GEN_LDX_HVRM(lwzcix, ld32u, 0x15, 0x18, PPC_CILDST)
> +GEN_LDX_HVRM(lhzcix, ld16u, 0x15, 0x19, PPC_CILDST)
> +GEN_LDX_HVRM(lbzcix, ld8u, 0x15, 0x1a, PPC_CILDST)
>  #endif
>  GEN_LDX(lhbr, ld16ur, 0x16, 0x18, PPC_INTEGER)
>  GEN_LDX(lwbr, ld32ur, 0x16, 0x10, PPC_INTEGER)
> @@ -10242,7 +10274,7 @@ GEN_HANDLER(name, opc, 0xFF, 0xFF, 0x00000000, type),
>  GEN_HANDLER(stop##u, opc, 0xFF, 0xFF, 0x00000000, type),
>  #define GEN_STUX(name, stop, opc2, opc3, type)                                \
>  GEN_HANDLER(name##ux, 0x1F, opc2, opc3, 0x00000001, type),
> -#define GEN_STX_E(name, stop, opc2, opc3, type, type2)                        \
> +#define GEN_STX_E(name, stop, opc2, opc3, type, type2, chk)                   \
>  GEN_HANDLER_E(name##x, 0x1F, opc2, opc3, 0x00000001, type, type2),
>  #define GEN_STS(name, stop, op, type)                                         \
>  GEN_ST(name, stop, op | 0x20, type)                                           \
> @@ -10256,7 +10288,11 @@ GEN_STS(stw, st32, 0x04, PPC_INTEGER)
>  #if defined(TARGET_PPC64)
>  GEN_STUX(std, st64, 0x15, 0x05, PPC_64B)
>  GEN_STX(std, st64, 0x15, 0x04, PPC_64B)
> -GEN_STX_E(stdbr, st64r, 0x14, 0x14, PPC_NONE, PPC2_DBRX)
> +GEN_STX_E(stdbr, st64r, 0x14, 0x14, PPC_NONE, PPC2_DBRX, CHK_NONE)
> +GEN_STX_HVRM(stdcix, st64, 0x15, 0x1f, PPC_CILDST)
> +GEN_STX_HVRM(stwcix, st32, 0x15, 0x1c, PPC_CILDST)
> +GEN_STX_HVRM(sthcix, st16, 0x15, 0x1d, PPC_CILDST)
> +GEN_STX_HVRM(stbcix, st8, 0x15, 0x1e, PPC_CILDST)
>  #endif
>  GEN_STX(sthbr, st16r, 0x16, 0x1C, PPC_INTEGER)
>  GEN_STX(stwbr, st32r, 0x16, 0x14, PPC_INTEGER)
> @@ -11424,6 +11460,7 @@ void gen_intermediate_code(CPUPPCState *env, struct TranslationBlock *tb)
>      ctx.spr_cb = env->spr_cb;
>      ctx.pr = msr_pr;
>      ctx.mem_idx = env->dmmu_idx;
> +    ctx.dr = msr_dr;
>  #if !defined(CONFIG_USER_ONLY)
>      ctx.hv = msr_hv || !env->has_hv_mode;
>  #endif
> diff --git a/target-ppc/translate_init.c b/target-ppc/translate_init.c
> index 3b4234b325d1..6f2f760728bb 100644
> --- a/target-ppc/translate_init.c
> +++ b/target-ppc/translate_init.c
> @@ -8395,7 +8395,8 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
>                         PPC_MEM_TLBIE | PPC_MEM_TLBSYNC |
>                         PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC |
>                         PPC_SEGMENT_64B | PPC_SLBI |
> -                       PPC_POPCNTB | PPC_POPCNTWD;
> +                       PPC_POPCNTB | PPC_POPCNTWD |
> +                       PPC_CILDST;
>      pcc->insns_flags2 = PPC2_VSX | PPC2_DFP | PPC2_DBRX | PPC2_ISA205 |
>                          PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 |
>                          PPC2_ATOMIC_ISA206 | PPC2_FP_CVT_ISA206 |
> @@ -8476,7 +8477,8 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
>                         PPC_MEM_TLBIE | PPC_MEM_TLBSYNC |
>                         PPC_64B | PPC_64H | PPC_64BX | PPC_ALTIVEC |
>                         PPC_SEGMENT_64B | PPC_SLBI |
> -                       PPC_POPCNTB | PPC_POPCNTWD;
> +                       PPC_POPCNTB | PPC_POPCNTWD |
> +                       PPC_CILDST;
>      pcc->insns_flags2 = PPC2_VSX | PPC2_VSX207 | PPC2_DFP | PPC2_DBRX |
>                          PPC2_PERM_ISA206 | PPC2_DIVE_ISA206 |
>                          PPC2_ATOMIC_ISA206 | PPC2_FP_CVT_ISA206 |
> -- 
> 2.1.4
> 

-- 
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: 819 bytes --]

  reply	other threads:[~2016-06-15  3:47 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-13  5:24 [Qemu-devel] [PATCH 00/10] rework exception model to support the HV mode Cédric Le Goater
2016-06-13  5:24 ` [Qemu-devel] [PATCH 01/10] ppc: Fix rfi/rfid/hrfi/... emulation Cédric Le Goater
2016-06-16  1:07   ` David Gibson
2016-06-17  2:27     ` [Qemu-devel] [Qemu-ppc] " David Gibson
2016-06-17  5:54       ` Cédric Le Goater
2016-06-17  6:03         ` Cédric Le Goater
2016-06-17  6:28           ` David Gibson
2016-06-17  6:39             ` Cédric Le Goater
2016-06-17  7:10           ` Thomas Huth
2016-06-17  7:17             ` Cédric Le Goater
2016-06-17 10:41             ` Cédric Le Goater
2016-06-17 11:02               ` Thomas Huth
2016-06-17 11:11                 ` Alexander Graf
2016-06-17 14:32                 ` Cédric Le Goater
2016-06-18 23:35                   ` Benjamin Herrenschmidt
2016-06-19 12:49                     ` Cédric Le Goater
2016-06-19 13:00                       ` Alexander Graf
2016-06-19 17:21                         ` Cédric Le Goater
2016-06-19 22:15                           ` Benjamin Herrenschmidt
2016-06-19 22:35                             ` Benjamin Herrenschmidt
2016-06-20  7:08                               ` Benjamin Herrenschmidt
2016-06-20  7:11                                 ` Alexander Graf
2016-06-20  8:02                                 ` Benjamin Herrenschmidt
2016-06-20  9:32                                   ` Benjamin Herrenschmidt
2016-06-20 13:55                                     ` Alexander Graf
2016-06-21  8:21                                     ` Mark Cave-Ayland
2016-06-21  9:33                                       ` Benjamin Herrenschmidt
2016-06-21  9:37                                         ` Benjamin Herrenschmidt
2016-06-19 14:08                       ` Benjamin Herrenschmidt
2016-06-19 17:23                         ` Cédric Le Goater
2016-06-19 21:12                           ` Benjamin Herrenschmidt
2016-06-20  2:19                             ` David Gibson
2016-06-20  6:17                               ` Cédric Le Goater
2016-06-20  7:47                                 ` Thomas Huth
2016-06-20  8:21                                   ` Benjamin Herrenschmidt
2016-06-20  8:46                                   ` Cédric Le Goater
2016-06-20  8:18                                 ` Benjamin Herrenschmidt
2016-06-20  6:10                             ` Cédric Le Goater
2016-06-20  8:18                               ` Benjamin Herrenschmidt
2016-06-18 23:30                 ` Benjamin Herrenschmidt
2016-06-18 23:29               ` Benjamin Herrenschmidt
2016-06-17  6:19     ` [Qemu-devel] " Cédric Le Goater
2016-06-13  5:24 ` [Qemu-devel] [PATCH 02/10] ppc: Create cpu_ppc_set_papr() helper (for LPCR) Cédric Le Goater
2016-06-14  6:15   ` David Gibson
2016-06-14  6:52     ` Cédric Le Goater
2016-06-15  1:01       ` David Gibson
2016-06-13  5:24 ` [Qemu-devel] [PATCH 03/10] ppc: Rework POWER7 & POWER8 exception model (part 2) Cédric Le Goater
2016-06-14  6:25   ` David Gibson
2016-06-14 21:19     ` Benjamin Herrenschmidt
2016-06-15  1:00       ` David Gibson
2016-06-13  5:24 ` [Qemu-devel] [PATCH 04/10] ppc: Fix POWER7 and POWER8 exception definitions Cédric Le Goater
2016-06-13  5:24 ` [Qemu-devel] [PATCH 05/10] ppc: Fix generation if ISI/DSI vs. HV mode Cédric Le Goater
2016-06-14  6:34   ` David Gibson
2016-06-14  6:42     ` Cédric Le Goater
2016-06-15  1:09       ` David Gibson
2016-06-13  5:24 ` [Qemu-devel] [PATCH 06/10] ppc: Rework generation of priv and inval interrupts Cédric Le Goater
2016-06-15  1:19   ` David Gibson
2016-06-15  4:31     ` Benjamin Herrenschmidt
2016-06-15  5:06       ` David Gibson
2016-06-13  5:24 ` [Qemu-devel] [PATCH 07/10] ppc: Add real mode CI load/store instructions for P7 and P8 Cédric Le Goater
2016-06-15  3:46   ` David Gibson [this message]
2016-06-13  5:24 ` [Qemu-devel] [PATCH 08/10] ppc: Turn a bunch of booleans from int to bool Cédric Le Goater
2016-06-13  5:24 ` [Qemu-devel] [PATCH 09/10] ppc: Move exception generation code out of line Cédric Le Goater
2016-06-13  7:44   ` Thomas Huth
2016-06-13  8:36     ` Cédric Le Goater
2016-06-15  1:57       ` David Gibson
2016-06-13  5:24 ` [Qemu-devel] [PATCH 10/10] ppc: Add P7/P8 Power Management instructions Cédric Le Goater
2016-06-15  1:56   ` David Gibson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160615034621.GG4882@voom.fritz.box \
    --to=david@gibson.dropbear.id.au \
    --cc=benh@kernel.crashing.org \
    --cc=clg@kaod.org \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.