Re: [Qemu-devel] [PATCH 1/4] target-ppc: implement load atomic instruction

[David Gibson]

On Mon, Feb 06, 2017 at 03:59:57PM +0530, Nikunj A Dadhania wrote:
> From: Balamuruhan S <address@hidden>
> 
> lwat: Load Word Atomic
> ldat: Load Doubleword Atomic
> 
> The instruction includes as function code (5 bits) which gives a detail
> on the operation to be performed. The patch implements five such
> functions.
> 
> Signed-off-by: Balamuruhan S <address@hidden>
> Signed-off-by: Harish S <address@hidden>
> Signed-off-by: Athira Rajeev <address@hidden>
> [ combine both lwat/ldat implementation using macro ]
> Signed-off-by: Nikunj A Dadhania <address@hidden>
> ---
>  target/ppc/internal.h  |  2 ++
>  target/ppc/translate.c | 50 
> ++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 52 insertions(+)
> 
> diff --git a/target/ppc/internal.h b/target/ppc/internal.h
> index 5b5b180..1f441c6 100644
> --- a/target/ppc/internal.h
> +++ b/target/ppc/internal.h
> @@ -133,6 +133,8 @@ EXTRACT_HELPER(UIMM4, 16, 4);
>  EXTRACT_HELPER(NB, 11, 5);
>  /* Shift count */
>  EXTRACT_HELPER(SH, 11, 5);
> +/* lwat/stwat/ldat/lwat */
> +EXTRACT_HELPER(FC, 11, 5);
>  /* Vector shift count */
>  EXTRACT_HELPER(VSH, 6, 4);
>  /* Mask start */
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index b48abae..f59184f 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -2976,6 +2976,54 @@ LARX(lbarx, DEF_MEMOP(MO_UB))
>  LARX(lharx, DEF_MEMOP(MO_UW))
>  LARX(lwarx, DEF_MEMOP(MO_UL))
>  
> +#define LD_ATOMIC(name, memop, tp, op, eop)                             \
> +static void gen_##name(DisasContext *ctx)                               \
> +{                                                                       \
> +    int len = MEMOP_GET_SIZE(memop);                                    \
> +    uint32_t gpr_FC = FC(ctx->opcode);                                  \
> +    TCGv EA = tcg_temp_local_new();                                     \
> +    TCGv_##tp t0, t1;                                                   \
> +                                                                        \
> +    gen_addr_register(ctx, EA);                                         \
> +    if (len > 1) {                                                      \
> +        gen_check_align(ctx, EA, len - 1);                              \
> +    }                                                                   \
> +    t0 = tcg_temp_new_##tp();                                           \
> +    t1 = tcg_temp_new_##tp();                                           \
> +    tcg_gen_##op(t0, cpu_gpr[rD(ctx->opcode) + 1]);                     \
> +                                                                        \
> +    switch (gpr_FC) {                                                   \
> +    case 0: /* Fetch and add */                                         \
> +        tcg_gen_atomic_fetch_add_##tp(t1, EA, t0, ctx->mem_idx, memop); \
> +        break;                                                          \
> +    case 1: /* Fetch and xor */                                         \
> +        tcg_gen_atomic_fetch_xor_##tp(t1, EA, t0, ctx->mem_idx, memop); \
> +        break;                                                          \
> +    case 2: /* Fetch and or */                                          \
> +        tcg_gen_atomic_fetch_or_##tp(t1, EA, t0, ctx->mem_idx, memop);  \
> +        break;                                                          \
> +    case 3: /* Fetch and 'and' */                                       \
> +        tcg_gen_atomic_fetch_and_##tp(t1, EA, t0, ctx->mem_idx, memop); \
> +        break;                                                          \
> +    case 8: /* Swap */                                                  \
> +        tcg_gen_atomic_xchg_##tp(t1, EA, t0, ctx->mem_idx, memop);      \
> +        break;                                                          \
> +    default:                                                            \
> +        /* invoke data storage error handler */                         \
> +        gen_inval_exception(ctx, POWERPC_EXCP_INVAL_INVAL);             \

Both your comment and the ISA say that an invalid FC will result in a
data storage (0x300) exception, but here you seem to be invoking an
invalid instruction exception (0x700).  Which is correct?

> +        break;                                                          \
> +    }                                                                   \
> +    tcg_gen_##eop(cpu_gpr[rD(ctx->opcode)], t1);                        \
> +    tcg_temp_free_##tp(t0);                                             \
> +    tcg_temp_free_##tp(t1);                                             \
> +    tcg_temp_free(EA);                                                  \
> +}
> +
> +LD_ATOMIC(lwat, DEF_MEMOP(MO_UL), i32, trunc_tl_i32, extu_i32_tl)
> +#if defined(TARGET_PPC64)
> +LD_ATOMIC(ldat, DEF_MEMOP(MO_Q), i64, mov_i64, mov_i64)
> +#endif
> +
>  #if defined(CONFIG_USER_ONLY)
>  static void gen_conditional_store(DisasContext *ctx, TCGv EA,
>                                    int reg, int memop)
> @@ -6230,10 +6278,12 @@ GEN_HANDLER(isync, 0x13, 0x16, 0x04, 0x03FFF801, 
> PPC_MEM),
>  GEN_HANDLER_E(lbarx, 0x1F, 0x14, 0x01, 0, PPC_NONE, PPC2_ATOMIC_ISA206),
>  GEN_HANDLER_E(lharx, 0x1F, 0x14, 0x03, 0, PPC_NONE, PPC2_ATOMIC_ISA206),
>  GEN_HANDLER(lwarx, 0x1F, 0x14, 0x00, 0x00000000, PPC_RES),
> +GEN_HANDLER_E(lwat, 0x1F, 0x06, 0x12, 0x00000001, PPC_NONE, PPC2_ISA300),
>  GEN_HANDLER_E(stbcx_, 0x1F, 0x16, 0x15, 0, PPC_NONE, PPC2_ATOMIC_ISA206),
>  GEN_HANDLER_E(sthcx_, 0x1F, 0x16, 0x16, 0, PPC_NONE, PPC2_ATOMIC_ISA206),
>  GEN_HANDLER2(stwcx_, "stwcx.", 0x1F, 0x16, 0x04, 0x00000000, PPC_RES),
>  #if defined(TARGET_PPC64)
> +GEN_HANDLER_E(ldat, 0x1F, 0x06, 0x13, 0x00000001, PPC_NONE, PPC2_ISA300),
>  GEN_HANDLER(ldarx, 0x1F, 0x14, 0x02, 0x00000000, PPC_64B),
>  GEN_HANDLER_E(lqarx, 0x1F, 0x14, 0x08, 0, PPC_NONE, PPC2_LSQ_ISA207),
>  GEN_HANDLER2(stdcx_, "stdcx.", 0x1F, 0x16, 0x06, 0x00000000, PPC_64B),

-- 
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

signature.asc
Description: PGP signature