Re: [Qemu-devel] [PULL 12/52] target-arm: Update generic cpreg code for AArch64

[Peter Maydell]

On 6 January 2014 11:30, Peter Maydell <address@hidden> wrote:
> @@ -1946,7 +1947,34 @@ static void add_cpreg_to_hashtable(ARMCPU *cpu, const 
> ARMCPRegInfo *r,
>      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->state == ARM_CP_STATE_BOTH && state == ARM_CP_STATE_AA32) {
> +        /* The AArch32 view of a shared register sees the lower 32 bits
> +         * of a 64 bit backing field. It is not migratable as the AArch64
> +         * view handles that. AArch64 also handles reset.
> +         * We assume it is a cp15 register.
> +         */
> +        r2->cp = 15;
> +        r2->type |= ARM_CP_NO_MIGRATE;
> +        r2->resetfn = arm_cp_reset_ignore;
> +#ifdef HOST_WORDS_BIGENDIAN
> +        if (r2->fieldoffset) {
> +            r2->fieldoffset += sizeof(uint32_t);
> +        }
> +#endif
> +    }
> +    if (state == ARM_CP_STATE_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);
> +    }

There's a bug here which somehow slipped through my testing.
The following fixup corrects it:

diff --git a/target-arm/helper.c b/target-arm/helper.c
index e7d88ea..f91e3fd 100644
--- a/target-arm/helper.c
+++ b/target-arm/helper.c
@@ -2039,10 +2039,10 @@ static void add_cpreg_to_hashtable(ARMCPU
*cpu, const ARMCPRegInfo *r,
         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);
+        *key = ENCODE_AA64_CP_REG(r2->cp, r2->crn, crm,
+                                  r2->opc0, opc1, opc2);
     } else {
-        *key = ENCODE_CP_REG(r->cp, is64, r->crn, crm, opc1, opc2);
+        *key = ENCODE_CP_REG(r2->cp, is64, r2->crn, crm, opc1, opc2);
     }
     if (opaque) {
         r2->opaque = opaque;

Otherwise AArch32 CPUs don't get a TPIDR_EL1 (when the
later patch which marks that ARM_CP_STATE_BOTH is applied),
which means Linux crashes trying to boot. Oops.

(Only the change to use r2->cp in the ENCODE_CP_REG line
is strictly necessary for this fix, but for consistency
it's better to use r2 always.)

thanks
-- PMM