Re: [PATCH v5 00/22] target/arm: Implement ARMv8.5-MemTag, system mode

[Richard Henderson]

On 10/15/19 1:39 PM, Evgenii Stepanov wrote:
> Hi,
> 
> please find attached three random fixes for instruction translation
> and one for syscall emulation.

Thanks for the patches.

> @@ -2763,7 +2763,8 @@ static void disas_ldst_pair
>          return;
>      }
>  
> -    offset <<= size;
> +    // STGP offset is 16-scaled.
> +    offset <<= (size + set_tag);

Right.  I'll fix this with

    offset <<= (set_tag ? LOG2_TAG_GRANULE : size);

which I think is a bit clearer.

> @@ -3611,7 +3611,7 @@ static void disas_ldst_tag
>      int rt = extract32(insn, 0, 5);
>      int rn = extract32(insn, 5, 5);
>      uint64_t offset = sextract64(insn, 12, 9) << LOG2_TAG_GRANULE;
> -    int op2 = extract32(insn, 10, 3);
> +    int op2 = extract32(insn, 10, 2);

Yep.

> @@ -3679,7 +3679,7 @@ static void disas_ldst_tag(DisasContext *s, uint32_t 
> insn)
>      }
>  
>      dirty_addr = read_cpu_reg_sp(s, rn, true);
> -    if (index <= 0) {
> +    if (index >= 0) {
>          /* pre-index or signed offset */
>          tcg_gen_addi_i64(dirty_addr, dirty_addr, offset);
>      }
> @@ -3726,7 +3726,7 @@ static void disas_ldst_tag(DisasContext *s, uint32_t 
> insn)
>  
>      if (index != 0) {
>          /* pre-index or post-index */
> -        if (index > 0) {
> +        if (index < 0) {
>              /* post-index */
>              tcg_gen_addi_i64(dirty_addr, dirty_addr, offset);
>          }

Yep.

Ideally there'd be a kernel patch for MTE that works well enough to run RISU on
the fast model, and I'd be able to compare results.  I suppose in the meantime
more unit testing will have to do.

> +++ b/linux-user/qemu.h
> @@ -456,8 +456,16 @@ extern unsigned long guest_stack_size;
>  #define VERIFY_READ 0
>  #define VERIFY_WRITE 1 /* implies read access */
>  
> +static inline abi_ulong untagged_addr(abi_ulong addr) {
> +#if TARGET_ABI_BITS == 64
> +    addr &= (((abi_ulong)-1) >> 8);
> +#endif
> +    return addr;
> +}

At minimum this needs TARGET_AARCH64, because this kernel feature doesn't apply
to other targets.  But I'll see if I can do this such that it doesn't put
target-specific stuff in linux-user/qemu.h.


r~