qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 11/21] target-arm: Update generic cpreg code for


From: Peter Crosthwaite
Subject: Re: [Qemu-devel] [PATCH 11/21] target-arm: Update generic cpreg code for AArch64
Date: Thu, 19 Dec 2013 16:01:26 +1000

On Wed, Dec 18, 2013 at 1:12 AM, Peter Maydell <address@hidden> 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 <address@hidden>
> ---
>  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
>
>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]