Re: [Qemu-devel] [PATCH v4 05/21] target-arm: A32: Use get_mem_index for load/stores

[Peter Maydell <peter.maydell@linaro.org>]

On 23 May 2014 01:42, Edgar E. Iglesias <edgar.iglesias@gmail.com> wrote:
> From: "Edgar E. Iglesias" <edgar.iglesias@xilinx.com>
>
> Avoid using IS_USER directly as the MMU-idx to simplify future
> changes to the MMU layout.
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Edgar E. Iglesias <edgar.iglesias@xilinx.com>
> @@ -9946,6 +9950,8 @@ static int disas_thumb2_insn(CPUARMState *env, DisasContext *s, uint16_t insn_hw
>          }
>          if (insn & (1 << 20)) {
>              /* Load.  */
> +            user = user ? 0 : get_mem_index(s);
> +
>              tmp = tcg_temp_new_i32();
>              switch (op) {
>              case 0:

This conversion from 'user is a flag which is true if in
user mode' to 'user is the memory index we want to use'
is being done only in the 'Load' part of this if, which means
that all the stores in the 'else' clause are going to have
the wrong memory index.

I think we should fix this by replacing this 'user' variable
with a simple 'mmuidx'. Then we can say
   mmuidx = get_mem_index(s);
at the point which currently does user = IS_USER();
and
   mmuidx = MEM_USER_IDX;
in the bit of decode that currently does user = 1;

This is probably going to be clearest done as a
separate patch before this one.

thanks
-- PMM