Re: [PATCH v7 41/42] target/arm: Create tagged ram when MTE is enabled

[Peter Maydell]

On Wed, 3 Jun 2020 at 02:14, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> v5: Assign cs->num_ases to the final value first.
>     Downgrade to ID_AA64PFR1.MTE=1 if tag memory is not available.
> v6: Add secure tag memory for EL3.
> ---


>  static void arm_cpu_finalizefn(Object *obj)
> @@ -1735,17 +1754,43 @@ static void arm_cpu_realizefn(DeviceState *dev, Error 
> **errp)
>      MachineState *ms = MACHINE(qdev_get_machine());
>      unsigned int smp_cpus = ms->smp.cpus;
>
> -    if (cpu->has_el3 || arm_feature(env, ARM_FEATURE_M_SECURITY)) {
> +    /*
> +     * We must set cs->num_ases to the final value before
> +     * the first call to cpu_address_space_init.
> +     */
> +    if (cpu->tag_memory != NULL) {
> +        cs->num_ases = 4;
> +    } else if (cpu->has_el3 || arm_feature(env, ARM_FEATURE_M_SECURITY)) {
>          cs->num_ases = 2;
> +    } else {
> +        cs->num_ases = 1;
> +    }

1: neither MTE nor TrustZone
2: TrustZone but not MTE
...but why is MTE always 4 even if no TrustZone? I would have
expected MTE-no-TZ to have 2 ASes...

> +    if (cpu->has_el3 || arm_feature(env, ARM_FEATURE_M_SECURITY)) {
>          if (!cpu->secure_memory) {
>              cpu->secure_memory = cs->memory;
>          }
>          cpu_address_space_init(cs, ARMASIdx_S, "cpu-secure-memory",
>                                 cpu->secure_memory);
> -    } else {
> -        cs->num_ases = 1;
>      }
> +
> +    if (cpu->tag_memory != NULL) {
> +        cpu_address_space_init(cs, ARMASIdx_TagNS, "cpu-tag-memory",
> +                               cpu->tag_memory);
> +        if (cpu->has_el3) {
> +            cpu_address_space_init(cs, ARMASIdx_TagS, "cpu-tag-memory",
> +                                   cpu->secure_tag_memory);
> +        }
> +    } else if (cpu_isar_feature(aa64_mte, cpu)) {
> +        /*
> +         * Since there is no tag memory, we can't meaningfully support MTE
> +         * to its fullest.  To avoid problems later, when we would come to
> +         * use the tag memory, downgrade support to insns only.
> +         */
> +        cpu->isar.id_aa64pfr1 =
> +            FIELD_DP64(cpu->isar.id_aa64pfr1, ID_AA64PFR1, MTE, 1);
> +    }
> +
>      cpu_address_space_init(cs, ARMASIdx_NS, "cpu-memory", cs->memory);

...and indeed the code here only inits the ARMASIdx_NS and
ARMASIdx_TagNS spaces in that case. I guess we need to leave
ARMASIdx_S in the array but unused since we're effectively
indexing them by constant integer, but even so shouldn't
num_ases be 3 (0 and 2 used, 1 present but empty, 3 not needed) ?

thanks
-- PMM