From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:42447)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <peter.maydell@linaro.org>) id 1Wnqj0-0004jL-2x
	for qemu-devel@nongnu.org; Fri, 23 May 2014 10:45:39 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <peter.maydell@linaro.org>) id 1Wnqiu-0000qV-8v
	for qemu-devel@nongnu.org; Fri, 23 May 2014 10:45:33 -0400
Received: from mail-la0-f54.google.com ([209.85.215.54]:57679)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <peter.maydell@linaro.org>) id 1Wnqiu-0000q4-1j
	for qemu-devel@nongnu.org; Fri, 23 May 2014 10:45:28 -0400
Received: by mail-la0-f54.google.com with SMTP id pv20so4386189lab.13
	for <qemu-devel@nongnu.org>; Fri, 23 May 2014 07:45:26 -0700 (PDT)
MIME-Version: 1.0
In-Reply-To: <1400805738-11889-6-git-send-email-edgar.iglesias@gmail.com>
References: <1400805738-11889-1-git-send-email-edgar.iglesias@gmail.com>
	<1400805738-11889-6-git-send-email-edgar.iglesias@gmail.com>
From: Peter Maydell <peter.maydell@linaro.org>
Date: Fri, 23 May 2014 15:45:06 +0100
Message-ID: <CAFEAcA8KDQfpOG-2YDsvxL65_affBnsyU-jTW49b218p4XBE0g@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [Qemu-devel] [PATCH v4 05/21] target-arm: A32: Use
	get_mem_index for load/stores
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: "Edgar E. Iglesias" <edgar.iglesias@gmail.com>
Cc: Rob Herring <rob.herring@linaro.org>, Peter Crosthwaite <peter.crosthwaite@xilinx.com>, Fabian Aggeler <aggelerf@ethz.ch>, QEMU Developers <qemu-devel@nongnu.org>, Alexander Graf <agraf@suse.de>, John Williams <john.williams@xilinx.com>, =?UTF-8?B?QWxleCBCZW5uw6ll?= <alex.bennee@linaro.org>, Christoffer Dall <christoffer.dall@linaro.org>, Richard Henderson <rth@twiddle.net>

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