From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47623) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1gLBK0-0005V1-VV for qemu-devel@nongnu.org; Fri, 09 Nov 2018 13:15:57 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1gLBJz-0002MP-RC for qemu-devel@nongnu.org; Fri, 09 Nov 2018 13:15:56 -0500 Received: from mail-ot1-x341.google.com ([2607:f8b0:4864:20::341]:41879) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1gLBJz-0002EE-BJ for qemu-devel@nongnu.org; Fri, 09 Nov 2018 13:15:55 -0500 Received: by mail-ot1-x341.google.com with SMTP id u16so2122593otk.8 for ; Fri, 09 Nov 2018 10:15:41 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <20181109173553.22341-2-peter.maydell@linaro.org> References: <20181109173553.22341-1-peter.maydell@linaro.org> <20181109173553.22341-2-peter.maydell@linaro.org> From: Peter Maydell Date: Fri, 9 Nov 2018 18:15:20 +0000 Message-ID: Content-Type: text/plain; charset="UTF-8" Subject: Re: [Qemu-devel] [Qemu-arm] [PATCH 1/2] target/arm: Hyp mode R14 is shared with User and System List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-arm , QEMU Developers Cc: Adam Lackorzynski , "patches@linaro.org" On 9 November 2018 at 17:35, Peter Maydell wrote: > Hyp mode is an exception to the general rule that each AArch32 > mode has its own r13, r14 and SPSR -- it has a banked r13 and > SPSR but shares its r14 with User and System mode. We were > incorrectly implementing it as banked, which meant that on > entry to Hyp mode r14 was 0 rather than the USR/SYS r14. > > We provide a new function r14_bank_number() which is like > the existing bank_number() but provides the index into > env->banked_r14[]; bank_number() provides the index to use > for env->banked_r13[] and env->banked_cpsr[]. > > All the points in the code that were using bank_number() > to index into env->banked_r14[] are updated for consintency: > * switch_mode() -- this is the only place where we fix > an actual bug > * aarch64_sync_32_to_64() and aarch64_sync_64_to_32(): > no behavioural change as we already special-cased Hyp R14 > * kvm32.c: no behavioural change since the guest can't ever > be in Hyp mode, but conceptually the right thing to do > * msr_banked()/mrs_banked(): we can never get to the case > that accesses banked_r14[] with tgtmode == ARM_CPU_MODE_HYP, > so no behavioural change > > Signed-off-by: Peter Maydell > --- > target/arm/internals.h | 16 ++++++++++++++++ > target/arm/helper.c | 29 +++++++++++++++-------------- > target/arm/kvm32.c | 4 ++-- > target/arm/op_helper.c | 2 +- > 4 files changed, 34 insertions(+), 17 deletions(-) Rats, this bit accidentally didn't make it into this patch: diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c index 2b62c53f5b5..eb6fb82fb81 100644 --- a/target/arm/op_helper.c +++ b/target/arm/op_helper.c @@ -725,7 +725,7 @@ uint32_t HELPER(mrs_banked)(CPUARMState *env, uint32_t tgtmode, uint32_t regno) case 13: return env->banked_r13[bank_number(tgtmode)]; case 14: - return env->banked_r14[bank_number(tgtmode)]; + return env->banked_r14[r14_bank_number(tgtmode)]; case 8 ... 12: switch (tgtmode) { case ARM_CPU_MODE_USR: (it's one of the "no behavioural change" bits). thanks -- PMM