Re: [Qemu-devel] [PATCH 09/13] target/arm: Add v8M stack checks for Thumb2 LDM/STM

[Philippe Mathieu-Daudé]

On 02/10/2018 18:35, Peter Maydell wrote:
> Add the v8M stack checks for:
>  * LDM (T2 encoding)
>  * STM (T2 encoding)
> 
> This includes the 32-bit encodings of the instructions listed
> in v8M ARM ARM rule R_YVWT as
>  * LDM, LDMIA, LDMFD
>  * LDMDB, LDMEA
>  * POP (multiple registers)
>  * PUSH (muliple registers)
>  * STM, STMIA, STMEA
>  * STMDB, STMFD
> 
> We perform the stack limit before doing any other part
> of the load or store.
> 
> Signed-off-by: Peter Maydell <address@hidden>

Reviewed-by: Philippe Mathieu-Daudé <address@hidden>

> ---
>  target/arm/translate.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/target/arm/translate.c b/target/arm/translate.c
> index c16d6075d94..3fb378a492d 100644
> --- a/target/arm/translate.c
> +++ b/target/arm/translate.c
> @@ -10524,6 +10524,7 @@ static void disas_thumb2_insn(DisasContext *s, 
> uint32_t insn)
>              } else {
>                  int i, loaded_base = 0;
>                  TCGv_i32 loaded_var;
> +                bool wback = extract32(insn, 21, 1);
>                  /* Load/store multiple.  */
>                  addr = load_reg(s, rn);
>                  offset = 0;
> @@ -10531,10 +10532,26 @@ static void disas_thumb2_insn(DisasContext *s, 
> uint32_t insn)
>                      if (insn & (1 << i))
>                          offset += 4;
>                  }
> +
>                  if (insn & (1 << 24)) {
>                      tcg_gen_addi_i32(addr, addr, -offset);
>                  }
>  
> +                if (s->v8m_stackcheck && rn == 13 && wback) {
> +                    /*
> +                     * If the writeback is incrementing SP rather than
> +                     * decrementing it, and the initial SP is below the
> +                     * stack limit but the final written-back SP would
> +                     * be above, then then we must not perform any memory
> +                     * accesses, but it is IMPDEF whether we generate
> +                     * an exception. We choose to do so in this case.
> +                     * At this point 'addr' is the lowest address, so
> +                     * either the original SP (if incrementing) or our
> +                     * final SP (if decrementing), so that's what we check.
> +                     */
> +                    gen_helper_v8m_stackcheck(cpu_env, addr);
> +                }
> +
>                  loaded_var = NULL;
>                  for (i = 0; i < 16; i++) {
>                      if ((insn & (1 << i)) == 0)
> @@ -10562,7 +10579,7 @@ static void disas_thumb2_insn(DisasContext *s, 
> uint32_t insn)
>                  if (loaded_base) {
>                      store_reg(s, rn, loaded_var);
>                  }
> -                if (insn & (1 << 21)) {
> +                if (wback) {
>                      /* Base register writeback.  */
>                      if (insn & (1 << 24)) {
>                          tcg_gen_addi_i32(addr, addr, -offset);
>