Re: [Qemu-devel] [PATCH v3 35/69] target/arm: Convert Clear-Exclusive, Barriers

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 8/28/19 9:04 PM, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> v3: Check m-profile for all; fix missing isa check for ISB;
> fix thumb isa check for CLREX and DSB.
> ---
>  target/arm/translate.c       | 127 ++++++++++++++++-------------------
>  target/arm/a32-uncond.decode |  10 +++
>  target/arm/t32.decode        |  10 +++
>  3 files changed, 78 insertions(+), 69 deletions(-)
> 
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index 46e3f946d5..003b8ac414 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -10122,6 +10122,63 @@ static bool trans_SRS(DisasContext *s, arg_SRS *a)
>      return true;
>  }
>  
> +/*
> + * Clear-Exclusive, Barriers
> + */
> +
> +static bool trans_CLREX(DisasContext *s, arg_CLREX *a)
> +{
> +    if (s->thumb
> +        ? !ENABLE_ARCH_7 && !arm_dc_feature(s, ARM_FEATURE_M)
> +        : !ENABLE_ARCH_6K) {
> +        return false;
> +    }
> +    gen_clrex(s);
> +    return true;
> +}
> +
> +static bool trans_DSB(DisasContext *s, arg_DSB *a)
> +{
> +    if (!ENABLE_ARCH_7 && !arm_dc_feature(s, ARM_FEATURE_M)) {
> +        return false;
> +    }
> +    tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC);
> +    return true;
> +}
> +
> +static bool trans_DMB(DisasContext *s, arg_DMB *a)
> +{
> +    return trans_DSB(s, NULL);
> +}
> +
> +static bool trans_ISB(DisasContext *s, arg_ISB *a)
> +{
> +    if (!ENABLE_ARCH_7 && !arm_dc_feature(s, ARM_FEATURE_M)) {
> +        return false;
> +    }
> +    /*
> +     * We need to break the TB after this insn to execute
> +     * self-modifying code correctly and also to take
> +     * any pending interrupts immediately.
> +     */
> +    gen_goto_tb(s, 0, s->base.pc_next);
> +    return true;
> +}
> +
> +static bool trans_SB(DisasContext *s, arg_SB *a)
> +{
> +    if (!dc_isar_feature(aa32_sb, s)) {
> +        return false;
> +    }
> +    /*
> +     * TODO: There is no speculation barrier opcode
> +     * for TCG; MB and end the TB instead.
> +     */
> +    tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC);
> +    gen_goto_tb(s, 0, s->base.pc_next);
> +    return true;
> +}
> +
>  /*
>   * Legacy decoder.
>   */
> @@ -10215,38 +10272,6 @@ static void disas_arm_insn(DisasContext *s, unsigned int insn)
>                  s->base.is_jmp = DISAS_UPDATE;
>              }
>              return;
> -        } else if ((insn & 0x0fffff00) == 0x057ff000) {
> -            switch ((insn >> 4) & 0xf) {
> -            case 1: /* clrex */
> -                ARCH(6K);
> -                gen_clrex(s);
> -                return;
> -            case 4: /* dsb */
> -            case 5: /* dmb */
> -                ARCH(7);
> -                tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC);
> -                return;
> -            case 6: /* isb */
> -                /* We need to break the TB after this insn to execute
> -                 * self-modifying code correctly and also to take
> -                 * any pending interrupts immediately.
> -                 */
> -                gen_goto_tb(s, 0, s->base.pc_next);
> -                return;
> -            case 7: /* sb */
> -                if ((insn & 0xf) || !dc_isar_feature(aa32_sb, s)) {
> -                    goto illegal_op;
> -                }
> -                /*
> -                 * TODO: There is no speculation barrier opcode
> -                 * for TCG; MB and end the TB instead.
> -                 */
> -                tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC);
> -                gen_goto_tb(s, 0, s->base.pc_next);
> -                return;
> -            default:
> -                goto illegal_op;
> -            }
>          } else if ((insn & 0x0e000f00) == 0x0c000100) {
>              if (arm_dc_feature(s, ARM_FEATURE_IWMMXT)) {
>                  /* iWMMXt register transfer.  */
> @@ -10707,43 +10732,7 @@ static void disas_thumb2_insn(DisasContext *s, uint32_t insn)
>                              gen_set_psr_im(s, offset, 0, imm);
>                          }
>                          break;
> -                    case 3: /* Special control operations.  */
> -                        if (!arm_dc_feature(s, ARM_FEATURE_V7) &&
> -                            !arm_dc_feature(s, ARM_FEATURE_M)) {
> -                            goto illegal_op;
> -                        }
> -                        op = (insn >> 4) & 0xf;
> -                        switch (op) {
> -                        case 2: /* clrex */
> -                            gen_clrex(s);
> -                            break;
> -                        case 4: /* dsb */
> -                        case 5: /* dmb */
> -                            tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC);
> -                            break;
> -                        case 6: /* isb */
> -                            /* We need to break the TB after this insn
> -                             * to execute self-modifying code correctly
> -                             * and also to take any pending interrupts
> -                             * immediately.
> -                             */
> -                            gen_goto_tb(s, 0, s->base.pc_next);
> -                            break;
> -                        case 7: /* sb */
> -                            if ((insn & 0xf) || !dc_isar_feature(aa32_sb, s)) {
> -                                goto illegal_op;
> -                            }
> -                            /*
> -                             * TODO: There is no speculation barrier opcode
> -                             * for TCG; MB and end the TB instead.
> -                             */
> -                            tcg_gen_mb(TCG_MO_ALL | TCG_BAR_SC);
> -                            gen_goto_tb(s, 0, s->base.pc_next);
> -                            break;
> -                        default:
> -                            goto illegal_op;
> -                        }
> -                        break;
> +                    case 3: /* Special control operations, in decodetree */
>                      case 4: /* bxj, in decodetree */
>                          goto illegal_op;
>                      case 5: /* Exception return.  */
> diff --git a/target/arm/a32-uncond.decode b/target/arm/a32-uncond.decode
> index 64548a93e2..c7e9df8030 100644
> --- a/target/arm/a32-uncond.decode
> +++ b/target/arm/a32-uncond.decode
> @@ -22,6 +22,7 @@
>  # All of those that have a COND field in insn[31:28] are in a32.decode
>  #
>  
> +&empty           !extern
>  &i               !extern imm
>  
>  # Branch with Link and Exchange
> @@ -37,3 +38,12 @@ BLX_i            1111 101 . ........................          &i imm=%imm24h
>  
>  RFE              1111 100 pu:2 0 w:1 1 rn:4 0000 1010 0000 0000   &rfe
>  SRS              1111 100 pu:2 1 w:1 0 1101 0000 0101 000 mode:5  &srs
> +
> +# Clear-Exclusive, Barriers
> +
> +# QEMU does not require the option field for the barriers.
> +CLREX            1111 0101 0111 1111 1111 0000 0001 1111
> +DSB              1111 0101 0111 1111 1111 0000 0100 ----
> +DMB              1111 0101 0111 1111 1111 0000 0101 ----
> +ISB              1111 0101 0111 1111 1111 0000 0110 ----
> +SB               1111 0101 0111 1111 1111 0000 0111 0000
> diff --git a/target/arm/t32.decode b/target/arm/t32.decode
> index c8a8aeceee..18c268e712 100644
> --- a/target/arm/t32.decode
> +++ b/target/arm/t32.decode
> @@ -305,6 +305,16 @@ CLZ              1111 1010 1011 ---- 1111 .... 1000 ....      @rdm
>        # of the space is "reserved hint, behaves as nop".
>        NOP        1111 0011 1010 1111 1000 0000 ---- ----
>      }
> +
> +    # Miscelaneous control

"Miscellaneous"

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> +    {
> +      CLREX      1111 0011 1011 1111 1000 1111 0010 1111
> +      DSB        1111 0011 1011 1111 1000 1111 0100 ----
> +      DMB        1111 0011 1011 1111 1000 1111 0101 ----
> +      ISB        1111 0011 1011 1111 1000 1111 0110 ----
> +      SB         1111 0011 1011 1111 1000 1111 0111 0000
> +    }
> +
>      # Note that the v7m insn overlaps both the normal and banked insn.
>      {
>        MRS_bank   1111 0011 111 r:1 .... 1000 rd:4   001. 0000  \
>