Re: [Qemu-devel] [PATCH] target-arm: always use user mode registers as operands for load/store multiple

[Peter Maydell]

On 19 February 2015 at 12:55, Ildar Isaev <address@hidden> wrote:
> Pseudocode fragment for STM instruction in ARMv8 spec:
>
> if registers<i> == '1' then // Store User mode register
>     MemA[address,4] = Rmode[i, M32_User];

This pseudocode is specifically for the "user mode" variant
of STM. The code you're changing implements all forms of
STM/LDM and your patch breaks the normal forms of the
instructions...

> Signed-off-by: Ildar Isaev <address@hidden>
> ---
>  target-arm/translate.c | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/target-arm/translate.c b/target-arm/translate.c
> index 622aa03..ca0ce3f 100644
> --- a/target-arm/translate.c
> +++ b/target-arm/translate.c
> @@ -8907,16 +8907,14 @@ static void disas_arm_insn(DisasContext *s, unsigned 
> int insn)
>                              /* load */
>                              tmp = tcg_temp_new_i32();
>                              gen_aa32_ld32u(tmp, addr, get_mem_index(s));
> -                            if (user) {
> +                            if (!user && (i == rn)) {
> +                                loaded_var = tmp;
> +                                loaded_base = 1;
> +                            } else {
>                                  tmp2 = tcg_const_i32(i);
>                                  gen_helper_set_user_reg(cpu_env, tmp2, tmp);
>                                  tcg_temp_free_i32(tmp2);
>                                  tcg_temp_free_i32(tmp);
> -                            } else if (i == rn) {
> -                                loaded_var = tmp;
> -                                loaded_base = 1;
> -                            } else {
> -                                store_reg_from_load(s, i, tmp);

This change is clearly wrong. It changes the behaviour for the
case of !user, i != rn from "call store_reg_from_load()" to
"call gen_helper_set_user_reg()", which means that:
 (1) using a normal LDM from privileged mode will write to
     the wrong registers for r13 and r14 (and r8..r12 if we
     are in FIQ mode)
 (2) using a normal LDM from user mode will have no effect
     if it tries to load r13 and r14

Again, what incorrect behaviour are you actually seeing?

thanks
-- PMM