Re: [Qemu-arm] [PATCH 05/17] target/arm: Suppress tag check for sp+offset

[Peter Maydell]

On Mon, 14 Jan 2019 at 01:11, Richard Henderson
<address@hidden> wrote:
>
> R0078 specifies that base register, or base register plus immediate
> offset, is unchecked when the base register is SP.
>
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  target/arm/translate-a64.c | 37 ++++++++++++++++++-------------------
>  1 file changed, 18 insertions(+), 19 deletions(-)
>
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index 5c2577a9ac..ee95ba7165 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -336,12 +336,11 @@ static void gen_a64_set_pc(DisasContext *s, TCGv_i64 
> src)
>   * This is always a fresh temporary, as we need to be able to
>   * increment this independently of a dirty write-back address.
>   */
> -static TCGv_i64 clean_data_tbi(DisasContext *s, TCGv_i64 addr)
> +static TCGv_i64 clean_data_tbi(DisasContext *s, TCGv_i64 addr, bool sp_off)

I think sp_off is not sufficiently general here. For instance
if you look at the pseudocode for LDR (immediate)
 
https://developer.arm.com/docs/ddi0596/b/base-instructions-alphabetic-order/ldr-immediate-load-register-immediate
we do the tag check if wback || n != 31.

That is, when the spec says "base register only, or base register
plus immediate offset addressing form", it is referencing the
list of addressing modes in the v8A Arm ARM DDA0487D.a C1.3.3,
and "pre-indexed" and "post-indexed" are separate from "base + immediate".
It looks like your patch is treating pre-indexed and
post-indexed the same as base+imm.

thanks
-- PMM