| Message ID | 1387293144-11554-12-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | New |
| Headers | show |
On Wed, Dec 18, 2013 at 1:12 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > Update the generic cpreg support code to also handle AArch64: > AArch64-visible registers coexist in the same hash table with > AArch32-visible ones, with a bit in the hash key distinguishing > them. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > target-arm/cpu.h | 59 ++++++++++++++++++++++++++++++++++++++----- > target-arm/helper.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++- > target-arm/kvm-consts.h | 37 +++++++++++++++++++++++++++ > 3 files changed, 155 insertions(+), 7 deletions(-) > > diff --git a/target-arm/cpu.h b/target-arm/cpu.h > index 56ed591..901f882 100644 > --- a/target-arm/cpu.h > +++ b/target-arm/cpu.h > @@ -572,18 +572,43 @@ void armv7m_nvic_complete_irq(void *opaque, int irq); > * or via MRRC/MCRR?) > * 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; > + * instead all registers have a 2 bit op0, 3 bit op1 and op2, > + * and 4 bit CRn and CRm. The encoding patterns are chosen > + * to be easy to convert to and from the KVM encodings, and also > + * so that the hashtable can contain both AArch32 and AArch64 > + * registers (to allow for interprocessing where we might run > + * 32 bit code on a 64 bit core). > */ > +/* This bit is private to our hashtable cpreg; in KVM register > + * IDs the AArch64/32 distinction is the KVM_REG_ARM/ARM64 > + * in the upper bits of the 64 bit ID. > + */ > +#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)) > > +#define ENCODE_AA64_CP_REG(cp, crn, crm, op0, op1, op2) \ > + (CP_REG_AA64_MASK | \ > + ((cp) << CP_REG_ARM_COPROC_SHIFT) | \ > + ((op0) << CP_REG_ARM64_SYSREG_OP0_SHIFT) | \ > + ((op1) << CP_REG_ARM64_SYSREG_OP1_SHIFT) | \ > + ((crn) << CP_REG_ARM64_SYSREG_CRN_SHIFT) | \ > + ((crm) << CP_REG_ARM64_SYSREG_CRM_SHIFT) | \ > + ((op2) << CP_REG_ARM64_SYSREG_OP2_SHIFT)) > + > /* Convert a full 64 bit KVM register ID to the truncated 32 bit > * version used as a key for the coprocessor register hashtable > */ > static inline uint32_t kvm_to_cpreg_id(uint64_t kvmid) > { > uint32_t cpregid = kvmid; > - if ((kvmid & CP_REG_SIZE_MASK) == CP_REG_SIZE_U64) { > + if ((kvmid & CP_REG_ARCH_MASK) == CP_REG_ARM64) { > + cpregid |= CP_REG_AA64_MASK; > + } else if ((kvmid & CP_REG_SIZE_MASK) == CP_REG_SIZE_U64) { > cpregid |= (1 << 15); > } > return cpregid; > @@ -594,11 +619,18 @@ static inline uint32_t kvm_to_cpreg_id(uint64_t kvmid) > */ > static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid) > { > - uint64_t kvmid = cpregid & ~(1 << 15); > - if (cpregid & (1 << 15)) { > - kvmid |= CP_REG_SIZE_U64 | CP_REG_ARM; > + uint64_t kvmid; > + > + if (cpregid & CP_REG_AA64_MASK) { > + kvmid = cpregid & ~CP_REG_AA64_MASK; > + kvmid |= CP_REG_SIZE_U64 | CP_REG_ARM64; > } else { > - kvmid |= CP_REG_SIZE_U32 | CP_REG_ARM; > + kvmid = cpregid & ~(1 << 15); > + if (cpregid & (1 << 15)) { > + kvmid |= CP_REG_SIZE_U64 | CP_REG_ARM; > + } else { > + kvmid |= CP_REG_SIZE_U32 | CP_REG_ARM; > + } > } > return kvmid; > } > @@ -626,13 +658,14 @@ 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_AA64 128 > #define ARM_CP_NOP (ARM_CP_SPECIAL | (1 << 8)) > #define ARM_CP_WFI (ARM_CP_SPECIAL | (2 << 8)) > #define ARM_LAST_SPECIAL ARM_CP_WFI > /* 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 0xff > > /* Return true if cptype is a valid type field. This is used to try to > * catch errors where the sentinel has been accidentally left off the end > @@ -655,6 +688,8 @@ static inline bool cptype_valid(int cptype) > * (ie anything visible in PL2 is visible in S-PL1, some things are only > * visible in S-PL1) but "Secure PL1" is a bit of a mouthful, we bend the > * terminology a little and call this PL3. > + * In AArch64 things are somewhat simpler as the PLx bits line up exactly > + * with the ELx exception levels. > * > * If access permissions for a register are more complex than can be > * described with these bits, then use a laxer set of restrictions, and > @@ -676,6 +711,10 @@ static inline bool cptype_valid(int cptype) > > static inline int arm_current_pl(CPUARMState *env) > { > + if (env->aarch64) { > + return extract32(env->pstate, 2, 2); > + } > + > if ((env->uncached_cpsr & 0x1f) == ARM_CPU_MODE_USR) { > return 0; > } > @@ -713,10 +752,18 @@ struct ARMCPRegInfo { > * then behave differently on read/write if necessary. > * For 64 bit registers, only crm and opc1 are relevant; crn and opc2 > * must both be zero. > + * For AArch64-visible registers, opc0 is also used. > + * Since there are no "coprocessors" in AArch64, cp is purely used as a > + * way to distinguish (for KVM's benefit) guest-visible system registers > + * from demuxed ones provided to preserve the "no side effects on > + * KVM register read/write from QEMU" semantics. cp==0x13 is guest > + * visible (to match KVM's encoding); cp==0 will be converted to > + * cp==0x13 when the ARMCPRegInfo is registered, for convenience. > */ > uint8_t cp; > uint8_t crn; > uint8_t crm; > + uint8_t opc0; > uint8_t opc1; > uint8_t opc2; > /* Register type: ARM_CP_* bits/values */ > diff --git a/target-arm/helper.c b/target-arm/helper.c > index 6ebd7dc..975a762 100644 > --- a/target-arm/helper.c > +++ b/target-arm/helper.c > @@ -1951,6 +1951,11 @@ void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu, > * At least one of the original and the second definition should > * include ARM_CP_OVERRIDE in its type bits -- this is just a guard > * against accidental use. > + * > + * ARM_CP_AA64 is set in the type field to define a register to > + * be visible when in AArch64 state. In this case r->opc0 may also > + * be set, and the ARM_CP_64BIT flag must not be set. opc0 can't > + * be wildcarded. > */ > int crm, opc1, opc2; > int crmmin = (r->crm == CP_ANY) ? 0 : r->crm; > @@ -1961,6 +1966,53 @@ void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu, > int opc2max = (r->opc2 == CP_ANY) ? 7 : r->opc2; > /* 64 bit registers have only CRm and Opc1 fields */ > assert(!((r->type & ARM_CP_64BIT) && (r->opc2 || r->crn))); > + /* AArch64 regs are all 64 bit so the ARM_CP_64BIT flag is meaningless */ > + assert((r->type & (ARM_CP_64BIT|ARM_CP_AA64)) > + != (ARM_CP_64BIT|ARM_CP_AA64)); > + /* op0 only exists in the AArch64 encodings */ > + assert((r->type & ARM_CP_AA64) || (r->opc0 == 0)); > + /* The AArch64 pseudocode CheckSystemAccess() specifies that op1 > + * encodes a minimum access level for the register. We roll this > + * runtime check into our general permission check code, so check > + * here that the reginfo's specified permissions are strict enough > + * to encompass the generic architectural permission check. > + */ > + if (r->type & ARM_CP_AA64) { > + int mask = 0; > + switch (r->opc1) { > + case 0: case 1: case 2: > + /* min_EL EL1 */ > + mask = PL1_RW; > + break; > + case 3: > + /* min_EL EL0 */ > + mask = PL0_RW; > + break; > + case 4: > + /* min_EL EL2 */ > + mask = PL2_RW; > + break; > + case 5: > + /* unallocated encoding, so not possible */ > + assert(false); > + break; > + case 6: > + /* min_EL EL3 */ > + mask = PL3_RW; > + break; > + case 7: > + /* min_EL EL1, secure mode only (we don't check the latter) */ > + mask = PL1_RW; > + break; > + default: > + /* broken reginfo with out of range opc1 */ A nit: "out-of-range" > + assert(false); > + break; > + } > + /* assert our permissions are not too lax (stricter is fine) */ > + assert((r->access & ~mask) == 0); > + } > + > /* Check that the register definition has enough info to handle > * reads and writes if they are permitted. > */ > @@ -1980,7 +2032,19 @@ void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu, > uint32_t *key = g_new(uint32_t, 1); > ARMCPRegInfo *r2 = g_memdup(r, sizeof(ARMCPRegInfo)); > int is64 = (r->type & ARM_CP_64BIT) ? 1 : 0; > - *key = ENCODE_CP_REG(r->cp, is64, r->crn, crm, opc1, opc2); > + if (r->type & ARM_CP_AA64) { > + /* To allow abbreviation of ARMCPRegInfo > + * definitions, we treat cp == 0 as equivalent to > + * the value for "standard guest-visible sysreg". > + */ > + if (r->cp == 0) { > + r2->cp = CP_REG_ARM64_SYSREG_CP; > + } > + *key = ENCODE_AA64_CP_REG(r2->cp, r->crn, crm, > + r->opc0, opc1, opc2); You have mixed terminology here with "opc" and "op". Should they be unionised in ARMCPRegInfo? union { uint8_t op1; uint8_t opc1; }; > + } else { > + *key = ENCODE_CP_REG(r->cp, is64, r->crn, crm, opc1, opc2); > + } Why the mutual exclusivity between 32 and 64 bit defs? Shouldn't it be possible to define with one ARMCPRegInfo a register that exists in both 32 and 64 and this code can just double add it to the hash table? May need two flags to describe existence in either or both schemes accordingly. Regards, Peter > if (opaque) { > r2->opaque = opaque; > } > diff --git a/target-arm/kvm-consts.h b/target-arm/kvm-consts.h > index 2bba0bd..d006146 100644 > --- a/target-arm/kvm-consts.h > +++ b/target-arm/kvm-consts.h > @@ -29,12 +29,14 @@ > #define CP_REG_SIZE_U32 0x0020000000000000ULL > #define CP_REG_SIZE_U64 0x0030000000000000ULL > #define CP_REG_ARM 0x4000000000000000ULL > +#define CP_REG_ARCH_MASK 0xff00000000000000ULL > > MISMATCH_CHECK(CP_REG_SIZE_SHIFT, KVM_REG_SIZE_SHIFT) > MISMATCH_CHECK(CP_REG_SIZE_MASK, KVM_REG_SIZE_MASK) > MISMATCH_CHECK(CP_REG_SIZE_U32, KVM_REG_SIZE_U32) > MISMATCH_CHECK(CP_REG_SIZE_U64, KVM_REG_SIZE_U64) > MISMATCH_CHECK(CP_REG_ARM, KVM_REG_ARM) > +MISMATCH_CHECK(CP_REG_ARCH_MASK, KVM_REG_ARCH_MASK) > > #define PSCI_FN_BASE 0x95c1ba5e > #define PSCI_FN(n) (PSCI_FN_BASE + (n)) > @@ -59,6 +61,41 @@ MISMATCH_CHECK(PSCI_FN_MIGRATE, KVM_PSCI_FN_MIGRATE) > MISMATCH_CHECK(QEMU_KVM_ARM_TARGET_CORTEX_A15, KVM_ARM_TARGET_CORTEX_A15) > #endif > > +#define CP_REG_ARM64 0x6000000000000000ULL > +#define CP_REG_ARM_COPROC_MASK 0x000000000FFF0000 > +#define CP_REG_ARM_COPROC_SHIFT 16 > +#define CP_REG_ARM64_SYSREG (0x0013 << CP_REG_ARM_COPROC_SHIFT) > +#define CP_REG_ARM64_SYSREG_OP0_MASK 0x000000000000c000 > +#define CP_REG_ARM64_SYSREG_OP0_SHIFT 14 > +#define CP_REG_ARM64_SYSREG_OP1_MASK 0x0000000000003800 > +#define CP_REG_ARM64_SYSREG_OP1_SHIFT 11 > +#define CP_REG_ARM64_SYSREG_CRN_MASK 0x0000000000000780 > +#define CP_REG_ARM64_SYSREG_CRN_SHIFT 7 > +#define CP_REG_ARM64_SYSREG_CRM_MASK 0x0000000000000078 > +#define CP_REG_ARM64_SYSREG_CRM_SHIFT 3 > +#define CP_REG_ARM64_SYSREG_OP2_MASK 0x0000000000000007 > +#define CP_REG_ARM64_SYSREG_OP2_SHIFT 0 > + > +/* No kernel define but it's useful to QEMU */ > +#define CP_REG_ARM64_SYSREG_CP (CP_REG_ARM64_SYSREG >> CP_REG_ARM_COPROC_SHIFT) > + > +#ifdef TARGET_AARCH64 > +MISMATCH_CHECK(CP_REG_ARM64, KVM_REG_ARM64) > +MISMATCH_CHECK(CP_REG_ARM_COPROC_MASK, KVM_REG_ARM_COPROC_MASK) > +MISMATCH_CHECK(CP_REG_ARM_COPROC_SHIFT, KVM_REG_ARM_COPROC_SHIFT) > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG, KVM_REG_ARM64_SYSREG) > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP0_MASK, KVM_REG_ARM64_SYSREG_OP0_MASK) > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP0_SHIFT, KVM_REG_ARM64_SYREG_OP0_SHIFT) > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP1_MASK, KVM_REG_ARM64_SYSREG_OP1_MASK) > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP1_SHIFT, KVM_REG_ARM64_SYREG_OP1_SHIFT) > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_CRN_MASK, KVM_REG_ARM64_SYSREG_CRN_MASK) > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_CRN_SHIFT, KVM_REG_ARM64_SYREG_CRN_SHIFT) > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_CRM_MASK, KVM_REG_ARM64_SYSREG_CRM_MASK) > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_CRM_SHIFT, KVM_REG_ARM64_SYREG_CRM_SHIFT) > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP2_MASK, KVM_REG_ARM64_SYSREG_OP2_MASK) > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP2_SHIFT, KVM_REG_ARM64_SYREG_OP2_SHIFT) > +#endif > + > #undef MISMATCH_CHECK > > #endif > -- > 1.8.5 > >
On 19 December 2013 06:01, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote: > On Wed, Dec 18, 2013 at 1:12 AM, Peter Maydell <peter.maydell@linaro.org> wrote: >> + *key = ENCODE_AA64_CP_REG(r2->cp, r->crn, crm, >> + r->opc0, opc1, opc2); > > You have mixed terminology here with "opc" and "op". Should they be > unionised in ARMCPRegInfo? > > union { > uint8_t op1; > uint8_t opc1; > }; That seems pretty ugly to me. The terminology mixing is kind of inevitable since AArch32 uses opc and AArch64 uses op. >> + } else { >> + *key = ENCODE_CP_REG(r->cp, is64, r->crn, crm, opc1, opc2); >> + } > > Why the mutual exclusivity between 32 and 64 bit defs? Shouldn't it be > possible to define with one ARMCPRegInfo a register that exists in > both 32 and 64 and this code can just double add it to the hash table? > May need two flags to describe existence in either or both schemes > accordingly. Almost all the shared registers appear as 32 bit on the AArch32 side and 64 bits on the AArch64 side. This means the required fieldoffset value is different [or potentially so for bigendian hosts]. So you'd only be able to share registers which were genuinely 64 bit on both sides, which are very rare. So it didn't seem worth trying to accommodate it. -- PMM
On Thu, Dec 19, 2013 at 7:11 PM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 19 December 2013 06:01, Peter Crosthwaite > <peter.crosthwaite@xilinx.com> wrote: >> On Wed, Dec 18, 2013 at 1:12 AM, Peter Maydell <peter.maydell@linaro.org> wrote: >>> + *key = ENCODE_AA64_CP_REG(r2->cp, r->crn, crm, >>> + r->opc0, opc1, opc2); >> >> You have mixed terminology here with "opc" and "op". Should they be >> unionised in ARMCPRegInfo? >> >> union { >> uint8_t op1; >> uint8_t opc1; >> }; > > That seems pretty ugly to me. The terminology mixing is kind of > inevitable since AArch32 uses opc and AArch64 uses op. > The union does self-document that fact however. >>> + } else { >>> + *key = ENCODE_CP_REG(r->cp, is64, r->crn, crm, opc1, opc2); >>> + } >> >> Why the mutual exclusivity between 32 and 64 bit defs? Shouldn't it be >> possible to define with one ARMCPRegInfo a register that exists in >> both 32 and 64 and this code can just double add it to the hash table? >> May need two flags to describe existence in either or both schemes >> accordingly. > > Almost all the shared registers appear as 32 bit on the AArch32 > side and 64 bits on the AArch64 side. Really? Reading v8, there are many 32 bit regs with these op0 AArch64 encodings, for example the documentation of MIDR_EL1: Configurations MIDR_EL1 is architecturally mapped to AArch32 register MIDR. MIDR_EL1 is architecturally mapped to external register MIDR_EL1. Attributes MIDR_EL1 is a 32-bit register CTR, REVIDR, ID_PFRx, CSSELR, CLIDR, DACR32 to name a few more that are similarly documented and by no means an exhaustive list (I just randomly picked those from the list). Regards, Peter > This means the required > fieldoffset value is different [or potentially so for bigendian hosts]. > So you'd only be able to share registers which were genuinely > 64 bit on both sides, which are very rare. So it didn't seem worth > trying to accommodate it. > The support 32 vs 64 encoding scheme seems quite othogonal to the backing sotre bit width. > -- PMM >
On Wed, Dec 18, 2013 at 1:12 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > Update the generic cpreg support code to also handle AArch64: > AArch64-visible registers coexist in the same hash table with > AArch32-visible ones, with a bit in the hash key distinguishing > them. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > target-arm/cpu.h | 59 ++++++++++++++++++++++++++++++++++++++----- > target-arm/helper.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++- > target-arm/kvm-consts.h | 37 +++++++++++++++++++++++++++ > 3 files changed, 155 insertions(+), 7 deletions(-) > > diff --git a/target-arm/cpu.h b/target-arm/cpu.h > index 56ed591..901f882 100644 > --- a/target-arm/cpu.h > +++ b/target-arm/cpu.h > @@ -572,18 +572,43 @@ void armv7m_nvic_complete_irq(void *opaque, int irq); > * or via MRRC/MCRR?) > * 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; > + * instead all registers have a 2 bit op0, 3 bit op1 and op2, > + * and 4 bit CRn and CRm. The encoding patterns are chosen > + * to be easy to convert to and from the KVM encodings, and also > + * so that the hashtable can contain both AArch32 and AArch64 > + * registers (to allow for interprocessing where we might run > + * 32 bit code on a 64 bit core). > */ > +/* This bit is private to our hashtable cpreg; in KVM register > + * IDs the AArch64/32 distinction is the KVM_REG_ARM/ARM64 > + * in the upper bits of the 64 bit ID. > + */ > +#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)) > > +#define ENCODE_AA64_CP_REG(cp, crn, crm, op0, op1, op2) \ > + (CP_REG_AA64_MASK | \ > + ((cp) << CP_REG_ARM_COPROC_SHIFT) | \ > + ((op0) << CP_REG_ARM64_SYSREG_OP0_SHIFT) | \ > + ((op1) << CP_REG_ARM64_SYSREG_OP1_SHIFT) | \ > + ((crn) << CP_REG_ARM64_SYSREG_CRN_SHIFT) | \ > + ((crm) << CP_REG_ARM64_SYSREG_CRM_SHIFT) | \ > + ((op2) << CP_REG_ARM64_SYSREG_OP2_SHIFT)) > + > /* Convert a full 64 bit KVM register ID to the truncated 32 bit > * version used as a key for the coprocessor register hashtable > */ > static inline uint32_t kvm_to_cpreg_id(uint64_t kvmid) > { > uint32_t cpregid = kvmid; > - if ((kvmid & CP_REG_SIZE_MASK) == CP_REG_SIZE_U64) { > + if ((kvmid & CP_REG_ARCH_MASK) == CP_REG_ARM64) { > + cpregid |= CP_REG_AA64_MASK; > + } else if ((kvmid & CP_REG_SIZE_MASK) == CP_REG_SIZE_U64) { > cpregid |= (1 << 15); > } > return cpregid; > @@ -594,11 +619,18 @@ static inline uint32_t kvm_to_cpreg_id(uint64_t kvmid) > */ > static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid) > { > - uint64_t kvmid = cpregid & ~(1 << 15); > - if (cpregid & (1 << 15)) { > - kvmid |= CP_REG_SIZE_U64 | CP_REG_ARM; > + uint64_t kvmid; > + > + if (cpregid & CP_REG_AA64_MASK) { > + kvmid = cpregid & ~CP_REG_AA64_MASK; > + kvmid |= CP_REG_SIZE_U64 | CP_REG_ARM64; > } else { > - kvmid |= CP_REG_SIZE_U32 | CP_REG_ARM; > + kvmid = cpregid & ~(1 << 15); > + if (cpregid & (1 << 15)) { > + kvmid |= CP_REG_SIZE_U64 | CP_REG_ARM; > + } else { > + kvmid |= CP_REG_SIZE_U32 | CP_REG_ARM; > + } > } > return kvmid; > } > @@ -626,13 +658,14 @@ 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_AA64 128 > #define ARM_CP_NOP (ARM_CP_SPECIAL | (1 << 8)) > #define ARM_CP_WFI (ARM_CP_SPECIAL | (2 << 8)) > #define ARM_LAST_SPECIAL ARM_CP_WFI > /* 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 0xff > > /* Return true if cptype is a valid type field. This is used to try to > * catch errors where the sentinel has been accidentally left off the end > @@ -655,6 +688,8 @@ static inline bool cptype_valid(int cptype) > * (ie anything visible in PL2 is visible in S-PL1, some things are only > * visible in S-PL1) but "Secure PL1" is a bit of a mouthful, we bend the > * terminology a little and call this PL3. > + * In AArch64 things are somewhat simpler as the PLx bits line up exactly > + * with the ELx exception levels. > * > * If access permissions for a register are more complex than can be > * described with these bits, then use a laxer set of restrictions, and > @@ -676,6 +711,10 @@ static inline bool cptype_valid(int cptype) > > static inline int arm_current_pl(CPUARMState *env) > { > + if (env->aarch64) { > + return extract32(env->pstate, 2, 2); > + } > + > if ((env->uncached_cpsr & 0x1f) == ARM_CPU_MODE_USR) { > return 0; > } > @@ -713,10 +752,18 @@ struct ARMCPRegInfo { > * then behave differently on read/write if necessary. > * For 64 bit registers, only crm and opc1 are relevant; crn and opc2 > * must both be zero. > + * For AArch64-visible registers, opc0 is also used. > + * Since there are no "coprocessors" in AArch64, cp is purely used as a > + * way to distinguish (for KVM's benefit) guest-visible system registers > + * from demuxed ones provided to preserve the "no side effects on > + * KVM register read/write from QEMU" semantics. cp==0x13 is guest > + * visible (to match KVM's encoding); cp==0 will be converted to > + * cp==0x13 when the ARMCPRegInfo is registered, for convenience. > */ > uint8_t cp; > uint8_t crn; > uint8_t crm; > + uint8_t opc0; > uint8_t opc1; > uint8_t opc2; > /* Register type: ARM_CP_* bits/values */ > diff --git a/target-arm/helper.c b/target-arm/helper.c > index 6ebd7dc..975a762 100644 > --- a/target-arm/helper.c > +++ b/target-arm/helper.c > @@ -1951,6 +1951,11 @@ void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu, > * At least one of the original and the second definition should > * include ARM_CP_OVERRIDE in its type bits -- this is just a guard > * against accidental use. > + * > + * ARM_CP_AA64 is set in the type field to define a register to > + * be visible when in AArch64 state. In this case r->opc0 may also > + * be set, and the ARM_CP_64BIT flag must not be set. opc0 can't > + * be wildcarded. > */ > int crm, opc1, opc2; > int crmmin = (r->crm == CP_ANY) ? 0 : r->crm; > @@ -1961,6 +1966,53 @@ void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu, > int opc2max = (r->opc2 == CP_ANY) ? 7 : r->opc2; > /* 64 bit registers have only CRm and Opc1 fields */ > assert(!((r->type & ARM_CP_64BIT) && (r->opc2 || r->crn))); > + /* AArch64 regs are all 64 bit so the ARM_CP_64BIT flag is meaningless */ > + assert((r->type & (ARM_CP_64BIT|ARM_CP_AA64)) > + != (ARM_CP_64BIT|ARM_CP_AA64)); > + /* op0 only exists in the AArch64 encodings */ > + assert((r->type & ARM_CP_AA64) || (r->opc0 == 0)); > + /* The AArch64 pseudocode CheckSystemAccess() specifies that op1 > + * encodes a minimum access level for the register. We roll this > + * runtime check into our general permission check code, so check > + * here that the reginfo's specified permissions are strict enough > + * to encompass the generic architectural permission check. > + */ > + if (r->type & ARM_CP_AA64) { > + int mask = 0; > + switch (r->opc1) { > + case 0: case 1: case 2: > + /* min_EL EL1 */ > + mask = PL1_RW; > + break; > + case 3: > + /* min_EL EL0 */ > + mask = PL0_RW; > + break; > + case 4: > + /* min_EL EL2 */ > + mask = PL2_RW; > + break; > + case 5: > + /* unallocated encoding, so not possible */ > + assert(false); > + break; > + case 6: > + /* min_EL EL3 */ > + mask = PL3_RW; > + break; > + case 7: > + /* min_EL EL1, secure mode only (we don't check the latter) */ > + mask = PL1_RW; > + break; > + default: > + /* broken reginfo with out of range opc1 */ "op1". Regards, Peter
On 20 December 2013 04:24, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote: > On Thu, Dec 19, 2013 at 7:11 PM, Peter Maydell <peter.maydell@linaro.org> wrote: >> Almost all the shared registers appear as 32 bit on the AArch32 >> side and 64 bits on the AArch64 side. > > Really? Reading v8, there are many 32 bit regs with these op0 AArch64 > encodings, for example the documentation of MIDR_EL1: > > Configurations > MIDR_EL1 is architecturally mapped to AArch32 register MIDR. > MIDR_EL1 is architecturally mapped to external register MIDR_EL1. > Attributes > MIDR_EL1 is a 32-bit register "32 bit register" in the AArch64 documentation is a shorthand for "64 bit register with the top 32 bits (currently) RES0". There are no 32 bit system registers in AArch64 by definition, because there's no way to load only 32 bits in or out of them: MSR/MRS are always 64 bit transfers. > CTR, REVIDR, ID_PFRx, CSSELR, CLIDR, DACR32 to name a few more that > are similarly documented and by no means an exhaustive list (I just > randomly picked those from the list). These too are all 64 bits for AArch64. >> This means the required >> fieldoffset value is different [or potentially so for bigendian hosts]. >> So you'd only be able to share registers which were genuinely >> 64 bit on both sides, which are very rare. So it didn't seem worth >> trying to accommodate it. >> > > The support 32 vs 64 encoding scheme seems quite othogonal to the > backing sotre bit width. Well, we could add 4 to the fieldoffset for the copy of the reginfo that we're using for the 32 bit side if TARGET_WORDS_BIGENDIAN, I guess. I'll have a think about this, because there are some wrinkles relating to reset that might be usefully solved this way. -- PMM
On 17 December 2013 15:12, Peter Maydell <peter.maydell@linaro.org> wrote: > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP0_MASK, KVM_REG_ARM64_SYSREG_OP0_MASK) > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP0_SHIFT, KVM_REG_ARM64_SYREG_OP0_SHIFT) > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP1_MASK, KVM_REG_ARM64_SYSREG_OP1_MASK) > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP1_SHIFT, KVM_REG_ARM64_SYREG_OP1_SHIFT) > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_CRN_MASK, KVM_REG_ARM64_SYSREG_CRN_MASK) > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_CRN_SHIFT, KVM_REG_ARM64_SYREG_CRN_SHIFT) > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_CRM_MASK, KVM_REG_ARM64_SYSREG_CRM_MASK) > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_CRM_SHIFT, KVM_REG_ARM64_SYREG_CRM_SHIFT) > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP2_MASK, KVM_REG_ARM64_SYSREG_OP2_MASK) > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP2_SHIFT, KVM_REG_ARM64_SYREG_OP2_SHIFT) Spot the typo which breaks compilation on aarch64 hosts :-) -- PMM
On 20 December 2013 04:24, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote: > On Thu, Dec 19, 2013 at 7:11 PM, Peter Maydell <peter.maydell@linaro.org> wrote: >> On 19 December 2013 06:01, Peter Crosthwaite >> <peter.crosthwaite@xilinx.com> wrote: >>> On Wed, Dec 18, 2013 at 1:12 AM, Peter Maydell <peter.maydell@linaro.org> wrote: >>>> + *key = ENCODE_AA64_CP_REG(r2->cp, r->crn, crm, >>>> + r->opc0, opc1, opc2); >>> >>> You have mixed terminology here with "opc" and "op". Should they be >>> unionised in ARMCPRegInfo? >>> >>> union { >>> uint8_t op1; >>> uint8_t opc1; >>> }; >> >> That seems pretty ugly to me. The terminology mixing is kind of >> inevitable since AArch32 uses opc and AArch64 uses op. >> > > The union does self-document that fact however. I think that would be unnecessarily confusing. I'd rather just have our code stick with the old style "opc". It's not like it's a major difference. > The support 32 vs 64 encoding scheme seems quite othogonal to the > backing sotre bit width. At the moment it isn't, though -- 64 bit registers have 64 bit backing fields (and for AArch32 are accessed via MRRC/MCRR), 32 bit registers have 32 bit backing fields. Bear in mind also that we have to interact with KVM, which also treats all AArch64 sysregs as 64 bit (and AArch32 ones as 32 bit for MRC/MCR and 64 bit for MRRC/MCRR). thanks -- PMM
On 20 December 2013 10:00, Peter Maydell <peter.maydell@linaro.org> wrote: > I'll have a think about this, because there are some wrinkles relating > to reset that might be usefully solved this way. So my conclusion here is that really this boils down to how we want to deal with sysregs for AArch64 capable CPUs in general. There are two models I can think of: Option 1: * we provide new reginfo definitions that describe the AArch64 views of the system registers * for the AArch32 views, we just fall through and use the existing definitions we've written for AArch32-only CPUs Option 2: * we provide new reginfo definitions for the AArch64 views of the registers, which include annotations to say whether there is an AArch32 view of this register * for the few registers which aren't neatly arranged so the crn/crm/opc1/opc2 line up, we just split up into a separate reginfo for AArch64 and AArch32 * register_cp_regs_for_features() has separate AArch32 and AArch64 versions Option 1 is what my initial patch was assuming. Option 2 is what the idea of being able to mark a reginfo as "this provides both AArch32 and AArch64 register views" implies, I think. Having thought about it a bit I actually prefer Option 2 of these -- it means that for a 64 bit CPU the reginfo dealing with a particular CPUState field will be in one place rather than two, and we also get a chance to audit which cp regs go into v8 AArch64 emulated CPUs, so we don't leak old backwards-compatible dummy definitions by accident. thanks -- PMM
On Fri, Dec 20, 2013 at 04:43:23PM +0000, Peter Maydell wrote: > On 17 December 2013 15:12, Peter Maydell <peter.maydell@linaro.org> wrote: > > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP0_MASK, KVM_REG_ARM64_SYSREG_OP0_MASK) > > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP0_SHIFT, KVM_REG_ARM64_SYREG_OP0_SHIFT) > > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP1_MASK, KVM_REG_ARM64_SYSREG_OP1_MASK) > > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP1_SHIFT, KVM_REG_ARM64_SYREG_OP1_SHIFT) > > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_CRN_MASK, KVM_REG_ARM64_SYSREG_CRN_MASK) > > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_CRN_SHIFT, KVM_REG_ARM64_SYREG_CRN_SHIFT) > > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_CRM_MASK, KVM_REG_ARM64_SYSREG_CRM_MASK) > > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_CRM_SHIFT, KVM_REG_ARM64_SYREG_CRM_SHIFT) > > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP2_MASK, KVM_REG_ARM64_SYSREG_OP2_MASK) > > +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP2_SHIFT, KVM_REG_ARM64_SYREG_OP2_SHIFT) > > Spot the typo which breaks compilation on aarch64 hosts :-) > SYREG, nice one :-) -Christoffer
On Sat, Dec 21, 2013 at 4:16 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 20 December 2013 10:00, Peter Maydell <peter.maydell@linaro.org> wrote: >> I'll have a think about this, because there are some wrinkles relating >> to reset that might be usefully solved this way. > > So my conclusion here is that really this boils down to how we > want to deal with sysregs for AArch64 capable CPUs in general. > There are two models I can think of: > > Option 1: > * we provide new reginfo definitions that describe the AArch64 > views of the system registers > * for the AArch32 views, we just fall through and use the existing > definitions we've written for AArch32-only CPUs > > Option 2: > * we provide new reginfo definitions for the AArch64 views of > the registers, which include annotations to say whether there > is an AArch32 view of this register So to clarify, would MIDR and friends be in this bucket? And does t obsolete the old MIDR def such there is only one CPRegInfo globally? > * for the few registers which aren't neatly arranged so the > crn/crm/opc1/opc2 line up, we just split up into a separate > reginfo for AArch64 and AArch32 ACK, that sounds awkward but there nothing we can do abt it. how many are there? The few I checked always line up. > * register_cp_regs_for_features() has separate AArch32 > and AArch64 versions > Do we need this? With this scheme its still possible to enforce exclusivity in the CPRegInfo themselves. > Option 1 is what my initial patch was assuming. Option 2 > is what the idea of being able to mark a reginfo as "this > provides both AArch32 and AArch64 register views" > implies, I think. > > Having thought about it a bit I actually prefer Option 2 > of these -- it means that for a 64 bit CPU the reginfo > dealing with a particular CPUState field will be in one > place rather than two, and we also get a chance to > audit which cp regs go into v8 AArch64 emulated CPUs, > so we don't leak old backwards-compatible dummy > definitions by accident. Agreed. Regards, Peter > > thanks > -- PMM >
On 20 December 2013 21:41, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote: > On Sat, Dec 21, 2013 at 4:16 AM, Peter Maydell <peter.maydell@linaro.org> wrote: >> On 20 December 2013 10:00, Peter Maydell <peter.maydell@linaro.org> wrote: >>> I'll have a think about this, because there are some wrinkles relating >>> to reset that might be usefully solved this way. >> >> So my conclusion here is that really this boils down to how we >> want to deal with sysregs for AArch64 capable CPUs in general. >> There are two models I can think of: >> >> Option 1: >> * we provide new reginfo definitions that describe the AArch64 >> views of the system registers >> * for the AArch32 views, we just fall through and use the existing >> definitions we've written for AArch32-only CPUs >> >> Option 2: >> * we provide new reginfo definitions for the AArch64 views of >> the registers, which include annotations to say whether there >> is an AArch32 view of this register > > So to clarify, would MIDR and friends be in this bucket? And does t > obsolete the old MIDR def such there is only one CPRegInfo globally? I think we end up with a MIDR def for old CPUs and a MIDR for v8-and-up CPUs which handles both AArch64 and AArch32 views: { .name = "MIDR_EL1", .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 0, .access = PL1_R, .type = ARM_CP_CONST|ARM_CP_AA64|ARM_CP_AA32VIEW, .resetvalue = cpu->midr }, because (a) ARMv8 gets rid of the "unimplemented c15 0 0 0 x registers read as MIDR" and (b) we can ignore the annoying special case handling for the TI925. (I wonder if it would be nicer to have a .state field (values CP_STATE_AA32 == 0, CP_STATE_AA64, CP_STATE_BOTH) rather than shoehorning it into the flags field...) For cases where we don't have annoying v7-and-earlier legacy to deal with we can just have one reginfo struct I think. I think it's the weird stuff that's going to get irritating because we're going to end up with "not for v8" logic to suppress half of them. >> * for the few registers which aren't neatly arranged so the >> crn/crm/opc1/opc2 line up, we just split up into a separate >> reginfo for AArch64 and AArch32 > > ACK, that sounds awkward but there nothing we can do abt it. how many > are there? The few I checked always line up. Well, everything in cp14 doesn't have architectural equivalents, for a start. In AArch32 TCMTR is crn=0 crm=0 opc1=0 opc2=2 but that is OSDTRRX_EL1 in AArch64. The TLB and cache maintenance ops don't always match up because they've been tidied up, I think. The docs don't make it terribly easy to compile a list of mismatches though. >> * register_cp_regs_for_features() has separate AArch32 >> and AArch64 versions >> > > Do we need this? With this scheme its still possible to enforce > exclusivity in the CPRegInfo themselves. Mmm, I guess we don't really, though it does mean we're a bit more likely to have dubious stuff lurking around in the AArch32-visible sysregs. (OTOH that's pretty irrelevant until we have support for either Hyp or TrustZone, since otherwise EL1 is always AArch64...) -- PMM
On 20 December 2013 22:07, Peter Maydell <peter.maydell@linaro.org> wrote: > On 20 December 2013 21:41, Peter Crosthwaite > <peter.crosthwaite@xilinx.com> wrote: >> On Sat, Dec 21, 2013 at 4:16 AM, Peter Maydell <peter.maydell@linaro.org> wrote: >>> * for the few registers which aren't neatly arranged so the >>> crn/crm/opc1/opc2 line up, we just split up into a separate >>> reginfo for AArch64 and AArch32 >> >> ACK, that sounds awkward but there nothing we can do abt it. how many >> are there? The few I checked always line up. > > Well, everything in cp14 doesn't have architectural equivalents, > for a start. In AArch32 TCMTR is crn=0 crm=0 opc1=0 opc2=2 > but that is OSDTRRX_EL1 in AArch64. The TLB and cache > maintenance ops don't always match up because they've been > tidied up, I think. The docs don't make it terribly easy to compile > a list of mismatches though. More generally I think the way that AArch64 uses op1 to group the registers by exception-level-access-rights is going to make it a bit tricky to do the mapping; we either need to (1) have .opc1 be the AA32 opc1 and infer AA64 op1 from the permission flags (2) have .opc1 be the AA64 op1 and insist that the AA32 opc1 is always zero (or always same as AA64 op1?), and require split reginfo structs if this isn't so (3) have both op1 and opc1 fields in the reginfo struct I don't much like any of these but I guess (2) is least worst. -- PMM
On Sat, Dec 21, 2013 at 8:07 AM, Peter Maydell <peter.maydell@linaro.org> wrote: > On 20 December 2013 21:41, Peter Crosthwaite > <peter.crosthwaite@xilinx.com> wrote: >> On Sat, Dec 21, 2013 at 4:16 AM, Peter Maydell <peter.maydell@linaro.org> wrote: >>> On 20 December 2013 10:00, Peter Maydell <peter.maydell@linaro.org> wrote: >>>> I'll have a think about this, because there are some wrinkles relating >>>> to reset that might be usefully solved this way. >>> >>> So my conclusion here is that really this boils down to how we >>> want to deal with sysregs for AArch64 capable CPUs in general. >>> There are two models I can think of: >>> >>> Option 1: >>> * we provide new reginfo definitions that describe the AArch64 >>> views of the system registers >>> * for the AArch32 views, we just fall through and use the existing >>> definitions we've written for AArch32-only CPUs >>> >>> Option 2: >>> * we provide new reginfo definitions for the AArch64 views of >>> the registers, which include annotations to say whether there >>> is an AArch32 view of this register >> >> So to clarify, would MIDR and friends be in this bucket? And does t >> obsolete the old MIDR def such there is only one CPRegInfo globally? > > I think we end up with a MIDR def for old CPUs and a MIDR > for v8-and-up CPUs which handles both AArch64 and AArch32 views: > { .name = "MIDR_EL1", > .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 0, > .access = PL1_R, > .type = ARM_CP_CONST|ARM_CP_AA64|ARM_CP_AA32VIEW, > .resetvalue = cpu->midr }, > > because (a) ARMv8 gets rid of the "unimplemented c15 0 0 0 x > registers read as MIDR" Ok so MIDR is an annoying special case (Some of the "friends" I listed early are probably better examples). > and (b) we can ignore the annoying > special case handling for the TI925. > > (I wonder if it would be nicer to have a .state field (values Ideally, I think its about which encoding schemes have visibility of this reg. From what we discussed so far, it seems that there are three separate encoding schemes: 32/MRC - the current 99% case, but needs to now support access to widened-to-64 registers (upper RES0 policy) 32/MRRC - 64bit regs accessible from 32 64/MRS - What you are adding here One hot encode accessibility via each of these in a ".encodings" field, then the implementations or MRC, MRRC and MRS just check independently whether they are allowed or not. > CP_STATE_AA32 == 0, CP_STATE_AA64, CP_STATE_BOTH) > rather than shoehorning it into the flags field...) > Agreed - new field. > For cases where we don't have annoying v7-and-earlier legacy > to deal with we can just have one reginfo struct I think. > > I think it's the weird stuff that's going to get irritating because > we're going to end up with "not for v8" logic to suppress half of > them. > >>> * for the few registers which aren't neatly arranged so the >>> crn/crm/opc1/opc2 line up, we just split up into a separate >>> reginfo for AArch64 and AArch32 >> >> ACK, that sounds awkward but there nothing we can do abt it. how many >> are there? The few I checked always line up. > > Well, everything in cp14 doesn't have architectural equivalents, > for a start. In AArch32 TCMTR is crn=0 crm=0 opc1=0 opc2=2 > but that is OSDTRRX_EL1 in AArch64. The TLB and cache > maintenance ops don't always match up because they've been > tidied up, I think. The docs don't make it terribly easy to compile > a list of mismatches though. > >>> * register_cp_regs_for_features() has separate AArch32 >>> and AArch64 versions >>> >> >> Do we need this? With this scheme its still possible to enforce >> exclusivity in the CPRegInfo themselves. > > Mmm, I guess we don't really, though it does mean we're > a bit more likely to have dubious stuff lurking around in > the AArch32-visible sysregs. (OTOH that's pretty irrelevant > until we have support for either Hyp or TrustZone, since > otherwise EL1 is always AArch64...) Is there more to it that just enforcing MRS/MRC/MRRC perms? An MRS only defined CP reg should never have AArch32 visibility unless I am missing something. Regards, Peter > > -- PMM >
On 20 December 2013 22:29, Peter Crosthwaite <peter.crosthwaite@xilinx.com> wrote: > On Sat, Dec 21, 2013 at 8:07 AM, Peter Maydell <peter.maydell@linaro.org> wrote: >> On 20 December 2013 21:41, Peter Crosthwaite >> <peter.crosthwaite@xilinx.com> wrote: >>> So to clarify, would MIDR and friends be in this bucket? And does t >>> obsolete the old MIDR def such there is only one CPRegInfo globally? >> >> I think we end up with a MIDR def for old CPUs and a MIDR >> for v8-and-up CPUs which handles both AArch64 and AArch32 views: >> { .name = "MIDR_EL1", >> .opc0 = 3, .crn = 0, .crm = 0, .opc1 = 0, .opc2 = 0, >> .access = PL1_R, >> .type = ARM_CP_CONST|ARM_CP_AA64|ARM_CP_AA32VIEW, >> .resetvalue = cpu->midr }, >> >> because (a) ARMv8 gets rid of the "unimplemented c15 0 0 0 x >> registers read as MIDR" > > Ok so MIDR is an annoying special case (Some of the "friends" I listed > early are probably better examples). I think the friends would probably look much like the MIDR example I give above. >> and (b) we can ignore the annoying >> special case handling for the TI925. >> >> (I wonder if it would be nicer to have a .state field (values > > Ideally, I think its about which encoding schemes have visibility of > this reg. From what we discussed so far, it seems that there are three > separate encoding schemes: > > 32/MRC - the current 99% case, but needs to now support access to > widened-to-64 registers (upper RES0 policy) > 32/MRRC - 64bit regs accessible from 32 > 64/MRS - What you are adding here > > One hot encode accessibility via each of these in a ".encodings" > field, then the implementations or MRC, MRRC and MRS just check > independently whether they are allowed or not. Whether we split into .encodings or use flag fields or whatever doesn't matter for that -- for "is there a register here via this encoding scheme?" what we do is just do a hashtable lookup and we expect to get a match only if the entire "this access method plus all its op*/cr* fields" matches. This is what ENCODE_CP_REG() and the new AArch64 equivalent macro are doing. How we lay things out in the reginfo struct is purely down to what makes writing reginfo structs easiest. The code which populates the hashtable just throws all the relevant information into the correct ENCODE_CP_REG macro. >> CP_STATE_AA32 == 0, CP_STATE_AA64, CP_STATE_BOTH) >> rather than shoehorning it into the flags field...) >> > > Agreed - new field. >>>> * register_cp_regs_for_features() has separate AArch32 >>>> and AArch64 versions >>>> >>> >>> Do we need this? With this scheme its still possible to enforce >>> exclusivity in the CPRegInfo themselves. >> >> Mmm, I guess we don't really, though it does mean we're >> a bit more likely to have dubious stuff lurking around in >> the AArch32-visible sysregs. (OTOH that's pretty irrelevant >> until we have support for either Hyp or TrustZone, since >> otherwise EL1 is always AArch64...) > > Is there more to it that just enforcing MRS/MRC/MRRC perms? An MRS > only defined CP reg should never have AArch32 visibility unless I am > missing something. That's correct. The issue is just that if we allow all the existing "if v6 register these registers, if v5 register these, etc" code to run then v8 CPUs potentially end up with (AArch32) sysregs they don't really have. That is however pretty much masked by the fact you can't actually get at the AArch32 sysregs mostly unless you have an Arch32 EL1, which in turn you're not going to see in an AArch64 CPU unless EL2 or EL3 are present and AArch64. -- PMM
On 20 December 2013 22:16, Peter Maydell <peter.maydell@linaro.org> wrote: > More generally I think the way that AArch64 uses op1 to group > the registers by exception-level-access-rights is going to make it > a bit tricky to do the mapping; we either need to > (1) have .opc1 be the AA32 opc1 and infer AA64 op1 from > the permission flags > (2) have .opc1 be the AA64 op1 and insist that the AA32 opc1 > is always zero (or always same as AA64 op1?), and require > split reginfo structs if this isn't so > (3) have both op1 and opc1 fields in the reginfo struct Having waded through the docs a bit more I think the correct answer here is "opc1 is AA64 op1 and AA32 opc1". The definitions line up in almost all cases. The few exceptions are where the AA64 definition has been brought into line with the "opc1 indicates exception level access rights" pattern but the old AA32 definition had a zero opc1 despite not being an EL1 register (typically because it was accessible by EL0; the EL2 registers do match up with the AArch32 encodings). There aren't very many of these, but since the only registers this patchset really cares about are the EL0-accessible registers almost all the ones we're going to add here fall into the "can't share reginfo" category. TPIDR_EL1 (AArch32 TPIDRPRW) is shareable but none of the others are. So I'm going for the "all of opc1/opc2/crn/crm have to match on AA32 and AA64 for a shared reginfo", but it's going to look a little pointless until we get the system emulation done next year :-) thanks -- PMM
diff --git a/target-arm/cpu.h b/target-arm/cpu.h index 56ed591..901f882 100644 --- a/target-arm/cpu.h +++ b/target-arm/cpu.h @@ -572,18 +572,43 @@ void armv7m_nvic_complete_irq(void *opaque, int irq); * or via MRRC/MCRR?) * 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; + * instead all registers have a 2 bit op0, 3 bit op1 and op2, + * and 4 bit CRn and CRm. The encoding patterns are chosen + * to be easy to convert to and from the KVM encodings, and also + * so that the hashtable can contain both AArch32 and AArch64 + * registers (to allow for interprocessing where we might run + * 32 bit code on a 64 bit core). */ +/* This bit is private to our hashtable cpreg; in KVM register + * IDs the AArch64/32 distinction is the KVM_REG_ARM/ARM64 + * in the upper bits of the 64 bit ID. + */ +#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)) +#define ENCODE_AA64_CP_REG(cp, crn, crm, op0, op1, op2) \ + (CP_REG_AA64_MASK | \ + ((cp) << CP_REG_ARM_COPROC_SHIFT) | \ + ((op0) << CP_REG_ARM64_SYSREG_OP0_SHIFT) | \ + ((op1) << CP_REG_ARM64_SYSREG_OP1_SHIFT) | \ + ((crn) << CP_REG_ARM64_SYSREG_CRN_SHIFT) | \ + ((crm) << CP_REG_ARM64_SYSREG_CRM_SHIFT) | \ + ((op2) << CP_REG_ARM64_SYSREG_OP2_SHIFT)) + /* Convert a full 64 bit KVM register ID to the truncated 32 bit * version used as a key for the coprocessor register hashtable */ static inline uint32_t kvm_to_cpreg_id(uint64_t kvmid) { uint32_t cpregid = kvmid; - if ((kvmid & CP_REG_SIZE_MASK) == CP_REG_SIZE_U64) { + if ((kvmid & CP_REG_ARCH_MASK) == CP_REG_ARM64) { + cpregid |= CP_REG_AA64_MASK; + } else if ((kvmid & CP_REG_SIZE_MASK) == CP_REG_SIZE_U64) { cpregid |= (1 << 15); } return cpregid; @@ -594,11 +619,18 @@ static inline uint32_t kvm_to_cpreg_id(uint64_t kvmid) */ static inline uint64_t cpreg_to_kvm_id(uint32_t cpregid) { - uint64_t kvmid = cpregid & ~(1 << 15); - if (cpregid & (1 << 15)) { - kvmid |= CP_REG_SIZE_U64 | CP_REG_ARM; + uint64_t kvmid; + + if (cpregid & CP_REG_AA64_MASK) { + kvmid = cpregid & ~CP_REG_AA64_MASK; + kvmid |= CP_REG_SIZE_U64 | CP_REG_ARM64; } else { - kvmid |= CP_REG_SIZE_U32 | CP_REG_ARM; + kvmid = cpregid & ~(1 << 15); + if (cpregid & (1 << 15)) { + kvmid |= CP_REG_SIZE_U64 | CP_REG_ARM; + } else { + kvmid |= CP_REG_SIZE_U32 | CP_REG_ARM; + } } return kvmid; } @@ -626,13 +658,14 @@ 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_AA64 128 #define ARM_CP_NOP (ARM_CP_SPECIAL | (1 << 8)) #define ARM_CP_WFI (ARM_CP_SPECIAL | (2 << 8)) #define ARM_LAST_SPECIAL ARM_CP_WFI /* 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 0xff /* Return true if cptype is a valid type field. This is used to try to * catch errors where the sentinel has been accidentally left off the end @@ -655,6 +688,8 @@ static inline bool cptype_valid(int cptype) * (ie anything visible in PL2 is visible in S-PL1, some things are only * visible in S-PL1) but "Secure PL1" is a bit of a mouthful, we bend the * terminology a little and call this PL3. + * In AArch64 things are somewhat simpler as the PLx bits line up exactly + * with the ELx exception levels. * * If access permissions for a register are more complex than can be * described with these bits, then use a laxer set of restrictions, and @@ -676,6 +711,10 @@ static inline bool cptype_valid(int cptype) static inline int arm_current_pl(CPUARMState *env) { + if (env->aarch64) { + return extract32(env->pstate, 2, 2); + } + if ((env->uncached_cpsr & 0x1f) == ARM_CPU_MODE_USR) { return 0; } @@ -713,10 +752,18 @@ struct ARMCPRegInfo { * then behave differently on read/write if necessary. * For 64 bit registers, only crm and opc1 are relevant; crn and opc2 * must both be zero. + * For AArch64-visible registers, opc0 is also used. + * Since there are no "coprocessors" in AArch64, cp is purely used as a + * way to distinguish (for KVM's benefit) guest-visible system registers + * from demuxed ones provided to preserve the "no side effects on + * KVM register read/write from QEMU" semantics. cp==0x13 is guest + * visible (to match KVM's encoding); cp==0 will be converted to + * cp==0x13 when the ARMCPRegInfo is registered, for convenience. */ uint8_t cp; uint8_t crn; uint8_t crm; + uint8_t opc0; uint8_t opc1; uint8_t opc2; /* Register type: ARM_CP_* bits/values */ diff --git a/target-arm/helper.c b/target-arm/helper.c index 6ebd7dc..975a762 100644 --- a/target-arm/helper.c +++ b/target-arm/helper.c @@ -1951,6 +1951,11 @@ void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu, * At least one of the original and the second definition should * include ARM_CP_OVERRIDE in its type bits -- this is just a guard * against accidental use. + * + * ARM_CP_AA64 is set in the type field to define a register to + * be visible when in AArch64 state. In this case r->opc0 may also + * be set, and the ARM_CP_64BIT flag must not be set. opc0 can't + * be wildcarded. */ int crm, opc1, opc2; int crmmin = (r->crm == CP_ANY) ? 0 : r->crm; @@ -1961,6 +1966,53 @@ void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu, int opc2max = (r->opc2 == CP_ANY) ? 7 : r->opc2; /* 64 bit registers have only CRm and Opc1 fields */ assert(!((r->type & ARM_CP_64BIT) && (r->opc2 || r->crn))); + /* AArch64 regs are all 64 bit so the ARM_CP_64BIT flag is meaningless */ + assert((r->type & (ARM_CP_64BIT|ARM_CP_AA64)) + != (ARM_CP_64BIT|ARM_CP_AA64)); + /* op0 only exists in the AArch64 encodings */ + assert((r->type & ARM_CP_AA64) || (r->opc0 == 0)); + /* The AArch64 pseudocode CheckSystemAccess() specifies that op1 + * encodes a minimum access level for the register. We roll this + * runtime check into our general permission check code, so check + * here that the reginfo's specified permissions are strict enough + * to encompass the generic architectural permission check. + */ + if (r->type & ARM_CP_AA64) { + int mask = 0; + switch (r->opc1) { + case 0: case 1: case 2: + /* min_EL EL1 */ + mask = PL1_RW; + break; + case 3: + /* min_EL EL0 */ + mask = PL0_RW; + break; + case 4: + /* min_EL EL2 */ + mask = PL2_RW; + break; + case 5: + /* unallocated encoding, so not possible */ + assert(false); + break; + case 6: + /* min_EL EL3 */ + mask = PL3_RW; + break; + case 7: + /* min_EL EL1, secure mode only (we don't check the latter) */ + mask = PL1_RW; + break; + default: + /* broken reginfo with out of range opc1 */ + assert(false); + break; + } + /* assert our permissions are not too lax (stricter is fine) */ + assert((r->access & ~mask) == 0); + } + /* Check that the register definition has enough info to handle * reads and writes if they are permitted. */ @@ -1980,7 +2032,19 @@ void define_one_arm_cp_reg_with_opaque(ARMCPU *cpu, uint32_t *key = g_new(uint32_t, 1); ARMCPRegInfo *r2 = g_memdup(r, sizeof(ARMCPRegInfo)); int is64 = (r->type & ARM_CP_64BIT) ? 1 : 0; - *key = ENCODE_CP_REG(r->cp, is64, r->crn, crm, opc1, opc2); + if (r->type & ARM_CP_AA64) { + /* To allow abbreviation of ARMCPRegInfo + * definitions, we treat cp == 0 as equivalent to + * the value for "standard guest-visible sysreg". + */ + if (r->cp == 0) { + r2->cp = CP_REG_ARM64_SYSREG_CP; + } + *key = ENCODE_AA64_CP_REG(r2->cp, r->crn, crm, + r->opc0, opc1, opc2); + } else { + *key = ENCODE_CP_REG(r->cp, is64, r->crn, crm, opc1, opc2); + } if (opaque) { r2->opaque = opaque; } diff --git a/target-arm/kvm-consts.h b/target-arm/kvm-consts.h index 2bba0bd..d006146 100644 --- a/target-arm/kvm-consts.h +++ b/target-arm/kvm-consts.h @@ -29,12 +29,14 @@ #define CP_REG_SIZE_U32 0x0020000000000000ULL #define CP_REG_SIZE_U64 0x0030000000000000ULL #define CP_REG_ARM 0x4000000000000000ULL +#define CP_REG_ARCH_MASK 0xff00000000000000ULL MISMATCH_CHECK(CP_REG_SIZE_SHIFT, KVM_REG_SIZE_SHIFT) MISMATCH_CHECK(CP_REG_SIZE_MASK, KVM_REG_SIZE_MASK) MISMATCH_CHECK(CP_REG_SIZE_U32, KVM_REG_SIZE_U32) MISMATCH_CHECK(CP_REG_SIZE_U64, KVM_REG_SIZE_U64) MISMATCH_CHECK(CP_REG_ARM, KVM_REG_ARM) +MISMATCH_CHECK(CP_REG_ARCH_MASK, KVM_REG_ARCH_MASK) #define PSCI_FN_BASE 0x95c1ba5e #define PSCI_FN(n) (PSCI_FN_BASE + (n)) @@ -59,6 +61,41 @@ MISMATCH_CHECK(PSCI_FN_MIGRATE, KVM_PSCI_FN_MIGRATE) MISMATCH_CHECK(QEMU_KVM_ARM_TARGET_CORTEX_A15, KVM_ARM_TARGET_CORTEX_A15) #endif +#define CP_REG_ARM64 0x6000000000000000ULL +#define CP_REG_ARM_COPROC_MASK 0x000000000FFF0000 +#define CP_REG_ARM_COPROC_SHIFT 16 +#define CP_REG_ARM64_SYSREG (0x0013 << CP_REG_ARM_COPROC_SHIFT) +#define CP_REG_ARM64_SYSREG_OP0_MASK 0x000000000000c000 +#define CP_REG_ARM64_SYSREG_OP0_SHIFT 14 +#define CP_REG_ARM64_SYSREG_OP1_MASK 0x0000000000003800 +#define CP_REG_ARM64_SYSREG_OP1_SHIFT 11 +#define CP_REG_ARM64_SYSREG_CRN_MASK 0x0000000000000780 +#define CP_REG_ARM64_SYSREG_CRN_SHIFT 7 +#define CP_REG_ARM64_SYSREG_CRM_MASK 0x0000000000000078 +#define CP_REG_ARM64_SYSREG_CRM_SHIFT 3 +#define CP_REG_ARM64_SYSREG_OP2_MASK 0x0000000000000007 +#define CP_REG_ARM64_SYSREG_OP2_SHIFT 0 + +/* No kernel define but it's useful to QEMU */ +#define CP_REG_ARM64_SYSREG_CP (CP_REG_ARM64_SYSREG >> CP_REG_ARM_COPROC_SHIFT) + +#ifdef TARGET_AARCH64 +MISMATCH_CHECK(CP_REG_ARM64, KVM_REG_ARM64) +MISMATCH_CHECK(CP_REG_ARM_COPROC_MASK, KVM_REG_ARM_COPROC_MASK) +MISMATCH_CHECK(CP_REG_ARM_COPROC_SHIFT, KVM_REG_ARM_COPROC_SHIFT) +MISMATCH_CHECK(CP_REG_ARM64_SYSREG, KVM_REG_ARM64_SYSREG) +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP0_MASK, KVM_REG_ARM64_SYSREG_OP0_MASK) +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP0_SHIFT, KVM_REG_ARM64_SYREG_OP0_SHIFT) +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP1_MASK, KVM_REG_ARM64_SYSREG_OP1_MASK) +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP1_SHIFT, KVM_REG_ARM64_SYREG_OP1_SHIFT) +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_CRN_MASK, KVM_REG_ARM64_SYSREG_CRN_MASK) +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_CRN_SHIFT, KVM_REG_ARM64_SYREG_CRN_SHIFT) +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_CRM_MASK, KVM_REG_ARM64_SYSREG_CRM_MASK) +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_CRM_SHIFT, KVM_REG_ARM64_SYREG_CRM_SHIFT) +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP2_MASK, KVM_REG_ARM64_SYSREG_OP2_MASK) +MISMATCH_CHECK(CP_REG_ARM64_SYSREG_OP2_SHIFT, KVM_REG_ARM64_SYREG_OP2_SHIFT) +#endif + #undef MISMATCH_CHECK #endif
Update the generic cpreg support code to also handle AArch64: AArch64-visible registers coexist in the same hash table with AArch32-visible ones, with a bit in the hash key distinguishing them. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- target-arm/cpu.h | 59 ++++++++++++++++++++++++++++++++++++++----- target-arm/helper.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++- target-arm/kvm-consts.h | 37 +++++++++++++++++++++++++++ 3 files changed, 155 insertions(+), 7 deletions(-)