From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:58526) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WlCDs-0006GC-2y for qemu-devel@nongnu.org; Fri, 16 May 2014 03:06:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WlCDi-0001EZ-To for qemu-devel@nongnu.org; Fri, 16 May 2014 03:06:27 -0400 Received: from mail-lb0-x232.google.com ([2a00:1450:4010:c04::232]:45762) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WlCDi-0001DE-GS for qemu-devel@nongnu.org; Fri, 16 May 2014 03:06:18 -0400 Received: by mail-lb0-f178.google.com with SMTP id w7so1582277lbi.37 for ; Fri, 16 May 2014 00:06:17 -0700 (PDT) Message-ID: <5375B8E8.4040506@gmail.com> Date: Fri, 16 May 2014 11:06:16 +0400 From: Sergey Fedorov MIME-Version: 1.0 References: <1399997768-32014-1-git-send-email-aggelerf@ethz.ch> <1399997768-32014-15-git-send-email-aggelerf@ethz.ch> <53750A82.1080404@gmail.com> <6456F5DA-5262-4B95-9656-11E1DCAC3EEB@ethz.ch> In-Reply-To: <6456F5DA-5262-4B95-9656-11E1DCAC3EEB@ethz.ch> Content-Type: text/plain; charset=windows-1251 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH v2 14/23] target-arm: add banked coprocessor register type and macros List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Aggeler Fabian Cc: "Edgar E. Iglesias" , QEMU Developers , Peter Maydell On 15.05.2014 23:10, Aggeler Fabian wrote: > On 15 May 2014, at 20:42, Sergey Fedorov wrote: > >> 13.05.2014 20:15, Fabian Aggeler wrote: >>> @@ -771,6 +862,14 @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid) >>> * IO indicates that this register does I/O and therefore its accesses >>> * need to be surrounded by gen_io_start()/gen_io_end(). In particular, >>> * registers which implement clocks or timers require this. >>> + * In an implementation with Security Extensions supporting Aarch32 cp regs can >>> + * be banked or common. If a register is common it references the same variable >>> + * from both worlds (non-secure and secure). For cp regs which neither set >>> + * ARM_CP_SECURE nor ARM_CP_NONSECURE (default) we assume it's common and it >>> + * will be inserted twice into the hashtable. If a register has >>> + * ARM_CP_BANKED/ARM_CP_BANKED_64BIT set, it will be inserted twice but with >>> + * different offset respectively. This way Aarch32 registers which can be >>> + * mapped to Aarch64 PL3 registers can be inserted individually. >>> */ >> I think it could be done with just adding a single flag indicating that >> NS tag of reginfo should be considered, otherwise insert reginfo into >> the hashtable twice. >> In order to distinguish 64 bit register, there is already ARM_CP_64BIT >> macro defined. > The distinction whether the underlying field is 64bit or 32bit was necessary > because some cp regs are not of type ARM_CP_64BIT but get mapped > to (the lower or upper 32 bits of) a 64bit field. So to be able to add the correct offset > in the case of a banked register I came up with these two types. > > Since there are only a few registers left which don’t get mapped to EL3 registers > we could define them explicitly and therefore get rid of ARM_CP_BANKED and > ARM_CP_BANKED_64BIT as well as increasing the offset before adding them to > the hashtable. > > But like this we would still need 2 bits, one to specify whether it is ns or not, and > one whether it is common or banked. Or am I missing something here? NS tag can be added into ARMCPRegInfo structure, just like that was with AArch64 (ARMCPRegInfo::state). Regards, Sergey. > >> Regards, >> Sergey >> >>> #define ARM_CP_SPECIAL 1 >>> #define ARM_CP_CONST 2 >>> @@ -779,16 +878,20 @@ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid) >>> #define ARM_CP_OVERRIDE 16 >>> #define ARM_CP_NO_MIGRATE 32 >>> #define ARM_CP_IO 64 >>> -#define ARM_CP_NOP (ARM_CP_SPECIAL | (1 << 8)) >>> -#define ARM_CP_WFI (ARM_CP_SPECIAL | (2 << 8)) >>> -#define ARM_CP_NZCV (ARM_CP_SPECIAL | (3 << 8)) >>> -#define ARM_CP_CURRENTEL (ARM_CP_SPECIAL | (4 << 8)) >>> -#define ARM_CP_DC_ZVA (ARM_CP_SPECIAL | (5 << 8)) >>> +#define ARM_CP_SECURE (1 << 7) >>> +#define ARM_CP_NONSECURE (1 << 8) >>> +#define ARM_CP_BANKED (ARM_CP_NONSECURE | ARM_CP_SECURE) >>> +#define ARM_CP_BANKED_64BIT ((1 << 9) | ARM_CP_BANKED) >>> +#define ARM_CP_NOP (ARM_CP_SPECIAL | (1 << 10)) >>> +#define ARM_CP_WFI (ARM_CP_SPECIAL | (2 << 10)) >>> +#define ARM_CP_NZCV (ARM_CP_SPECIAL | (3 << 10)) >>> +#define ARM_CP_CURRENTEL (ARM_CP_SPECIAL | (4 << 10)) >>> +#define ARM_CP_DC_ZVA (ARM_CP_SPECIAL | (5 << 10)) >>> #define ARM_LAST_SPECIAL ARM_CP_DC_ZVA >>> /* Used only as a terminator for ARMCPRegInfo lists */ >>> #define ARM_CP_SENTINEL 0xffff >>> /* Mask of only the flag bits in a type field */ >>> -#define ARM_CP_FLAG_MASK 0x7f >>> +#define ARM_CP_FLAG_MASK 0x3ff >>> >>> /* Valid values for ARMCPRegInfo state field, indicating which of >>> * the AArch32 and AArch64 execution states this register is visible in.