[RFC,v2] target-arm: provide skeleton for a64 insn decoding

provide a skeleton for a64 instruction decoding in translate-a64.c,
by dividing instructions into the classes defined by the
ARM Architecture Reference Manual(DDI0487A_a) C3

Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
Signed-off-by: Alex Bennée <alex@bennee.com>
Reviewed-by: Alex Bennée <alex@bennee.com>
---
 target-arm/translate-a64.c | 368 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 360 insertions(+), 8 deletions(-)

For the rationale, see v1 of the RFC at
http://lists.gnu.org/archive/html/qemu-devel/2013-11/msg01312.html

On 11/12/2013 11:29 PM, Claudio Fontana wrote:
> provide a skeleton for a64 instruction decoding in translate-a64.c,
> by dividing instructions into the classes defined by the
> ARM Architecture Reference Manual(DDI0487A_a) C3
> 
> Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
> Signed-off-by: Alex Bennée <alex@bennee.com>
> Reviewed-by: Alex Bennée <alex@bennee.com>
> ---
>  target-arm/translate-a64.c | 368 ++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 360 insertions(+), 8 deletions(-)
> 
> For the rationale, see v1 of the RFC at
> http://lists.gnu.org/archive/html/qemu-devel/2013-11/msg01312.html

Reviewed-by: Richard Henderson <rth@twiddle.net>


r~

On Tue, Nov 12, 2013 at 2:29 PM, Claudio Fontana
<claudio.fontana@linaro.org> wrote:
> provide a skeleton for a64 instruction decoding in translate-a64.c,
> by dividing instructions into the classes defined by the
> ARM Architecture Reference Manual(DDI0487A_a) C3
>
> Signed-off-by: Claudio Fontana <claudio.fontana@linaro.org>
> Signed-off-by: Alex Bennée <alex@bennee.com>
> Reviewed-by: Alex Bennée <alex@bennee.com>

Reviewed-by: Laurent Desnogues <laurent.desnogues@arm.com>

Except for a few minor comments below (which probably don't
require a new patch).

By the way it might be nice to add some bit patterns here and
there to know what bits have already been checked.  That will
help code review and debug.  Something like this:

/* 3322222222221111111111
 * 10987654321098765432109876543210
 * ___1101_________________________ */


Laurent

> ---
>  target-arm/translate-a64.c | 368 ++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 360 insertions(+), 8 deletions(-)
>
> For the rationale, see v1 of the RFC at
> http://lists.gnu.org/archive/html/qemu-devel/2013-11/msg01312.html
>
> diff --git a/target-arm/translate-a64.c b/target-arm/translate-a64.c
> index f120088..c4b2bb5 100644
> --- a/target-arm/translate-a64.c
> +++ b/target-arm/translate-a64.c
> @@ -107,17 +107,346 @@ static void gen_exception_insn(DisasContext *s, int offset, int excp)
>      s->is_jmp = DISAS_JUMP;
>  }
>
> -static void real_unallocated_encoding(DisasContext *s)
> +static void unallocated_encoding(DisasContext *s)
>  {
> -    fprintf(stderr, "Unknown instruction: %#x\n", s->insn);
>      gen_exception_insn(s, 4, EXCP_UDEF);
>  }
>
> -#define unallocated_encoding(s) do { \
> -    fprintf(stderr, "unallocated encoding at line: %d\n", __LINE__); \
> -    real_unallocated_encoding(s); \
> -    } while (0)
> +#define unsupported_encoding(s, insn)                        \
> +    do {                                                     \
> +        qemu_log_mask(LOG_UNIMP,                                        \
> +                      "%s:%d: unsupported instruction encoding 0x%08x", \
> +                      __FILE__, __LINE__, insn);                        \
> +        unallocated_encoding(s);                                        \
> +    } while (0);
>
> +/*
> + * the instruction disassembly implemented here matches
> + * the instruction encoding classifications in chapter 3 (C3)
> + * of the ARM Architecture Reference Manual (DDI0487A_a)
> + */
> +
> +/* Unconditional branch (immediate) */
> +static void disas_uncond_b_imm(DisasContext *s, uint32_t insn)
> +{
> +    unsupported_encoding(s, insn);
> +}
> +
> +/* Compare & branch (immediate) */
> +static void disas_comp_b_imm(DisasContext *s, uint32_t insn)
> +{
> +    unsupported_encoding(s, insn);
> +}
> +
> +/* Test & branch (immediate) */
> +static void disas_test_b_imm(DisasContext *s, uint32_t insn)
> +{
> +    unsupported_encoding(s, insn);
> +}
> +
> +/* Conditional branch (immediate) */
> +static void disas_cond_b_imm(DisasContext *s, uint32_t insn)
> +{
> +    unsupported_encoding(s, insn);
> +}
> +
> +/* System */
> +static void disas_sys(DisasContext *s, uint32_t insn)
> +{
> +    unsupported_encoding(s, insn);
> +}
> +
> +/* Exception generation */
> +static void disas_exc(DisasContext *s, uint32_t insn)
> +{
> +    unsupported_encoding(s, insn);
> +}
> +
> +/* Unconditional branch (register) */
> +static void disas_uncond_b_reg(DisasContext *s, uint32_t insn)
> +{
> +    unsupported_encoding(s, insn);
> +}
> +
> +/* C3.2 Branches, exception generating and system instructions */
> +static void disas_b_exc_sys(DisasContext *s, uint32_t insn)
> +{
> +    switch (extract32(insn, 25, 7)) {
> +    case 0x0a: case 0x0b:
> +    case 0x4a: case 0x4b: /* Unconditional branch (immediate) */
> +        disas_uncond_b_imm(s, insn);
> +        break;
> +    case 0x1a: case 0x5a: /* Compare & branch (immediate) */
> +        disas_comp_b_imm(s, insn);
> +        break;
> +    case 0x1b: case 0x5b: /* Test & branch (immediate) */
> +        disas_test_b_imm(s, insn);
> +        break;
> +    case 0x2a: /* Conditional branch (immediate) */
> +        disas_cond_b_imm(s, insn);
> +        break;
> +    case 0x6a: /* Exception generation / System */
> +        if (insn & (1 << 24)) {
> +            disas_sys(s, insn);

Table C3-2 explicitly states that bits 22 and 23 are 0.  It would
perhaps be better to check that here or add some comment
in disas_sys so that you don't forget to check these bits.

> +        } else {
> +            disas_exc(s, insn);
> +        }
> +        break;
> +    case 0x6b: /* Unconditional branch (register) */
> +        disas_uncond_b_reg(s, insn);
> +        break;
> +    default:
> +        unallocated_encoding(s);
> +        break;
> +    }
> +}
> +
> +/* Load/store exclusive */
> +static void disas_ldst_excl(DisasContext *s, uint32_t insn)
> +{
> +    unsupported_encoding(s, insn);
> +}
> +
> +/* Load register (literal) */
> +static void disas_ld_lit(DisasContext *s, uint32_t insn)
> +{
> +    unsupported_encoding(s, insn);
> +}
> +
> +/* Load/store pair (all forms) */
> +static void disas_ldst_pair(DisasContext *s, uint32_t insn)
> +{
> +    unsupported_encoding(s, insn);
> +}
> +
> +/* Load/store register (all forms) */
> +static void disas_ldst_reg(DisasContext *s, uint32_t insn)
> +{
> +    unsupported_encoding(s, insn);
> +}
> +
> +/* AdvSIMD load/store multiple structures */
> +static void disas_ldst_multiple_struct(DisasContext *s, uint32_t insn)
> +{
> +    unsupported_encoding(s, insn);
> +}
> +
> +/* AdvSIMD load/store single structure */
> +static void disas_ldst_single_struct(DisasContext *s, uint32_t insn)
> +{
> +    unsupported_encoding(s, insn);
> +}
> +
> +/* C3.3 Loads and stores */
> +static void disas_ldst(DisasContext *s, uint32_t insn)
> +{
> +    switch (extract32(insn, 24, 6)) {
> +    case 0x08: /* Load/store exclusive */
> +        disas_ldst_excl(s, insn);
> +        break;
> +    case 0x18: case 0x1c: /* Load register (literal) */
> +        disas_ld_lit(s, insn);
> +        break;
> +    case 0x28: case 0x29:
> +    case 0x2c: case 0x2d: /* Load/store pair (all forms) */
> +        disas_ldst_pair(s, insn);
> +        break;
> +    case 0x38: case 0x39:
> +    case 0x3c: case 0x3d: /* Load/store register (all forms) */
> +        disas_ldst_reg(s, insn);
> +        break;
> +    case 0x0c: /* AdvSIMD load/store multiple structures */
> +        disas_ldst_multiple_struct(s, insn);
> +        break;
> +    case 0x0d: /* AdvSIMD load/store single structure */
> +        disas_ldst_single_struct(s, insn);
> +        break;

Similar comment as above for cases 38, 38, 3c, 3d, 0c and 0d.

> +    default:
> +        unallocated_encoding(s);
> +        break;
> +    }
> +}
> +
> +/* PC-rel. addressing */
> +static void disas_pc_rel_adr(DisasContext *s, uint32_t insn)
> +{
> +    unsupported_encoding(s, insn);
> +}
> +
> +/* Add/subtract (immediate) */
> +static void disas_add_sub_imm(DisasContext *s, uint32_t insn)
> +{
> +    unsupported_encoding(s, insn);
> +}
> +
> +/* Logical (immediate) */
> +static void disas_logic_imm(DisasContext *s, uint32_t insn)
> +{
> +    unsupported_encoding(s, insn);
> +}
> +
> +/* Move wide (immediate) */
> +static void disas_movw_imm(DisasContext *s, uint32_t insn)
> +{
> +    unsupported_encoding(s, insn);
> +}
> +
> +/* Bitfield */
> +static void disas_bitfield(DisasContext *s, uint32_t insn)
> +{
> +    unsupported_encoding(s, insn);
> +}
> +
> +/* Extract */
> +static void disas_extract(DisasContext *s, uint32_t insn)
> +{
> +    unsupported_encoding(s, insn);
> +}
> +
> +/* C3.4 Data processing - immediate */
> +static void disas_data_proc_imm(DisasContext *s, uint32_t insn)
> +{
> +    switch (extract32(insn, 23, 6)) {
> +    case 0x20: case 0x21: /* PC-rel. addressing */
> +        disas_pc_rel_adr(s, insn);
> +        break;
> +    case 0x22: case 0x23: /* Add/subtract (immediate) */
> +        disas_add_sub_imm(s, insn);
> +        break;
> +    case 0x24: /* Logical (immediate) */
> +        disas_logic_imm(s, insn);
> +        break;
> +    case 0x25: /* Move wide (immediate) */
> +        disas_movw_imm(s, insn);
> +        break;
> +    case 0x26: /* Bitfield */
> +        disas_bitfield(s, insn);
> +        break;
> +    case 0x27: /* Extract */
> +        disas_extract(s, insn);
> +        break;

You don't have to extract bits 26-28 since you know they are 100.

> +    default:
> +        unallocated_encoding(s);
> +        break;
> +    }
> +}
> +
> +/* Logical (shifted register) */
> +static void disas_logic_reg(DisasContext *s, uint32_t insn)
> +{
> +    unsupported_encoding(s, insn);
> +}
> +
> +/* Add/subtract (extended register) */
> +static void disas_add_sub_ext_reg(DisasContext *s, uint32_t insn)
> +{
> +    unsupported_encoding(s, insn);
> +}
> +
> +/* Add/subtract (shifted register) */
> +static void disas_add_sub_reg(DisasContext *s, uint32_t insn)
> +{
> +    unsupported_encoding(s, insn);
> +}
> +
> +/* Data-processing (3 source) */
> +static void disas_data_proc_3src(DisasContext *s, uint32_t insn)
> +{
> +    unsupported_encoding(s, insn);
> +}
> +
> +/* Add/subtract (with carry) */
> +static void disas_adc_sbc(DisasContext *s, uint32_t insn)
> +{
> +    unsupported_encoding(s, insn);
> +}
> +
> +/* Conditional compare (immediate) */
> +static void disas_cc_imm(DisasContext *s, uint32_t insn)
> +{
> +    unsupported_encoding(s, insn);
> +}
> +
> +/* Conditional compare (register) */
> +static void disas_cc_reg(DisasContext *s, uint32_t insn)
> +{
> +    unsupported_encoding(s, insn);
> +}
> +
> +/* Conditional select */
> +static void disas_csel(DisasContext *s, uint32_t insn)
> +{
> +    unsupported_encoding(s, insn);
> +}
> +
> +/* Data-processing (1 source) */
> +static void disas_data_proc_1src(DisasContext *s, uint32_t insn)
> +{
> +    unsupported_encoding(s, insn);
> +}
> +
> +/* Data-processing (2 source) */
> +static void disas_data_proc_2src(DisasContext *s, uint32_t insn)
> +{
> +    unsupported_encoding(s, insn);
> +}
> +
> +/* C3.5 Data processing - register */
> +static void disas_data_proc_reg(DisasContext *s, uint32_t insn)
> +{
> +    switch (extract32(insn, 24, 5)) {
> +    case 0x0a: /* Logical (shifted register) */
> +        disas_logic_reg(s, insn);
> +        break;
> +    case 0x0b: /* Add/subtract */
> +        if (insn & (1 << 21)) { /* (extended register) */
> +            disas_add_sub_ext_reg(s, insn);
> +        } else {
> +            disas_add_sub_reg(s, insn);
> +        }
> +        break;
> +    case 0x1b: /* Data-processing (3 source) */
> +        disas_data_proc_3src(s, insn);
> +        break;
> +    case 0x1a:
> +        switch (extract32(insn, 21, 3)) {
> +        case 0x0: /* Add/subtract (with carry) */
> +            disas_adc_sbc(s, insn);
> +            break;
> +        case 0x2: /* Conditional compare */
> +            if (insn & (1 << 11)) { /* (immediate) */
> +                disas_cc_imm(s, insn);
> +            } else {            /* (register) */
> +                disas_cc_reg(s, insn);
> +            }
> +            break;
> +        case 0x4: /* Conditional select */
> +            disas_csel(s, insn);
> +            break;
> +        case 0x6: /* Data-processing */
> +            if (insn & (1 << 30)) { /* (1 source) */
> +                disas_data_proc_1src(s, insn);
> +            } else {            /* (2 source) */
> +                disas_data_proc_2src(s, insn);
> +            }
> +            break;
> +        default:
> +            unallocated_encoding(s);
> +            break;
> +        }

Do we really want to mimic the split of the ARM ARM?
For instance disas_cc_imm and disas_cc_reg will be very
similar.

> +    default:
> +        unallocated_encoding(s);
> +        break;
> +    }
> +}
> +
> +/* C3.6 Data processing - SIMD and floating point */
> +static void disas_data_proc_simd_fp(DisasContext *s, uint32_t insn)
> +{
> +    unsupported_encoding(s, insn);
> +}
> +
> +/* C3.1 A64 instruction index by encoding */
>  void disas_a64_insn(CPUARMState *env, DisasContext *s)
>  {
>      uint32_t insn;
> @@ -126,10 +455,33 @@ void disas_a64_insn(CPUARMState *env, DisasContext *s)
>      s->insn = insn;
>      s->pc += 4;
>
> -    switch ((insn >> 24) & 0x1f) {
> -    default:
> +    switch (extract32(insn, 25, 4)) {
> +    case 0x0: case 0x1: case 0x2: case 0x3: /* UNALLOCATED */
>          unallocated_encoding(s);
>          break;
> +    case 0x8: case 0x9: /* Data processing - immediate */
> +        disas_data_proc_imm(s, insn);
> +        break;
> +    case 0xa: case 0xb: /* Branch, exception generation and system insns */
> +        disas_b_exc_sys(s, insn);
> +        break;
> +    case 0x4:
> +    case 0x6:
> +    case 0xc:
> +    case 0xe:      /* Loads and stores */
> +        disas_ldst(s, insn);
> +        break;
> +    case 0x5:
> +    case 0xd:      /* Data processing - register */
> +        disas_data_proc_reg(s, insn);
> +        break;
> +    case 0x7:
> +    case 0xf:      /* Data processing - SIMD and floating point */
> +        disas_data_proc_simd_fp(s, insn);
> +        break;
> +    default:
> +        assert(FALSE); /* all 15 cases should be handled above */
> +        break;
>      }
>
>      if (unlikely(s->singlestep_enabled) && (s->is_jmp == DISAS_TB_JUMP)) {
> --
> 1.8.1
>
>