From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48156) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XbB6h-0004gX-1V for qemu-devel@nongnu.org; Mon, 06 Oct 2014 12:26:03 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XbB6W-0002Fh-5Q for qemu-devel@nongnu.org; Mon, 06 Oct 2014 12:25:54 -0400 Received: from mail-la0-f46.google.com ([209.85.215.46]:46207) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XbB6V-0002FP-Pd for qemu-devel@nongnu.org; Mon, 06 Oct 2014 12:25:44 -0400 Received: by mail-la0-f46.google.com with SMTP id gi9so4705195lab.19 for ; Mon, 06 Oct 2014 09:25:42 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1412113785-21525-13-git-send-email-greg.bellows@linaro.org> References: <1412113785-21525-1-git-send-email-greg.bellows@linaro.org> <1412113785-21525-13-git-send-email-greg.bellows@linaro.org> From: Peter Maydell Date: Mon, 6 Oct 2014 17:25:22 +0100 Message-ID: Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [PATCH v5 12/33] target-arm: insert Aarch32 cpregs twice into hashtable List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Greg Bellows Cc: Sergey Fedorov , QEMU Developers , Fabian Aggeler , "Edgar E. Iglesias" On 30 September 2014 22:49, Greg Bellows wrote: > From: Fabian Aggeler > > Prepare for cp register banking by inserting every cp register twice, > once for secure world and once for non-secure world. > > Signed-off-by: Fabian Aggeler > Signed-off-by: Greg Bellows > > ---------- > v4 -> v5 > - Added use of ARM CP secure/non-secure bank flags during register processing > in define_one_arm_cp_reg_with_opaque(). We now only register the specified > bank if only one flag is specified, otherwise we register both a secure and > non-secure instance. > --- > target-arm/cpu.h | 14 +++++++++++--- > target-arm/helper.c | 30 ++++++++++++++++++++++++++---- > target-arm/translate.c | 14 +++++++++----- > 3 files changed, 46 insertions(+), 12 deletions(-) > > diff --git a/target-arm/cpu.h b/target-arm/cpu.h > index 9681d45..220571c 100644 > --- a/target-arm/cpu.h > +++ b/target-arm/cpu.h > @@ -864,6 +864,7 @@ void armv7m_nvic_complete_irq(void *opaque, int irq); > * Crn, Crm, opc1, opc2 fields > * 32 or 64 bit register (ie is it accessed via MRC/MCR > * or via MRRC/MCRR?) > + * non-secure/secure bank (Aarch32 only) ...so if this is an AArch64 register is the bit in the hash table key set or clear? > * We allow 4 bits for opc1 because MRRC/MCRR have a 4 bit field. > * (In this case crn and opc2 should be zero.) > * For AArch64, there is no 32/64 bit size distinction; > @@ -881,9 +882,16 @@ void armv7m_nvic_complete_irq(void *opaque, int irq); > #define CP_REG_AA64_SHIFT 28 > #define CP_REG_AA64_MASK (1 << CP_REG_AA64_SHIFT) > > -#define ENCODE_CP_REG(cp, is64, crn, crm, opc1, opc2) \ > - (((cp) << 16) | ((is64) << 15) | ((crn) << 11) | \ > - ((crm) << 7) | ((opc1) << 3) | (opc2)) > +/* To enable banking of coprocessor registers depending on ns-bit we > + * add a bit to distinguish between secure and non-secure cpregs in the > + * hashtable. > + */ > +#define CP_REG_NS_SHIFT 27 > +#define CP_REG_NS_MASK(nsbit) (nsbit << CP_REG_NS_SHIFT) Bit 27 is already used, as part of the COPROC field. There's a reason the AA64 bit is 28... > + > +#define ENCODE_CP_REG(cp, is64, crn, crm, opc1, opc2, ns) \ > + (CP_REG_NS_MASK(ns) | ((cp) << 16) | ((is64) << 15) | \ > + ((crn) << 11) | ((crm) << 7) | ((opc1) << 3) | (opc2)) Doesn't this break KVM's accessing of the hashtable? You probably need to make kvm_to_cpreg_id OR in the NS bit as appropriate, and cpreg_to_kvm_id mask it out, since if we're using KVM then we're always non-secure. thanks -- PMM