On 15 May 2014, at 20:42, Sergey Fedorov <serge.f...@gmail.com> 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? > > 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. >
