diff mbox series

[v5,20/22] target/arm: Create tagged ram when MTE is enabled

Message ID 20191011134744.2477-21-richard.henderson@linaro.org
State Superseded
Headers show
Series [v5,01/22] target/arm: Add MTE_ACTIVE to tb_flags | expand

Commit Message

Richard Henderson Oct. 11, 2019, 1:47 p.m. UTC
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.
---
 target/arm/cpu.h |  6 ++++++
 hw/arm/virt.c    | 54 ++++++++++++++++++++++++++++++++++++++++++++++++
 target/arm/cpu.c | 53 ++++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 110 insertions(+), 3 deletions(-)

-- 
2.17.1

Comments

Peter Maydell Dec. 5, 2019, 6:40 p.m. UTC | #1
On Fri, 11 Oct 2019 at 14:50, 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.

> ---

>  target/arm/cpu.h |  6 ++++++

>  hw/arm/virt.c    | 54 ++++++++++++++++++++++++++++++++++++++++++++++++

>  target/arm/cpu.c | 53 ++++++++++++++++++++++++++++++++++++++++++++---

>  3 files changed, 110 insertions(+), 3 deletions(-)

>

> diff --git a/target/arm/cpu.h b/target/arm/cpu.h

> index 93a362708b..faca43ea78 100644

> --- a/target/arm/cpu.h

> +++ b/target/arm/cpu.h

> @@ -765,6 +765,10 @@ struct ARMCPU {

>      /* MemoryRegion to use for secure physical accesses */

>      MemoryRegion *secure_memory;

>

> +    /* MemoryRegion to use for allocation tag accesses */

> +    MemoryRegion *tag_memory;

> +    MemoryRegion *secure_tag_memory;

> +

>      /* For v8M, pointer to the IDAU interface provided by board/SoC */

>      Object *idau;

>

> @@ -2956,6 +2960,8 @@ int cpu_mmu_index(CPUARMState *env, bool ifetch);

>  typedef enum ARMASIdx {

>      ARMASIdx_NS = 0,

>      ARMASIdx_S = 1,

> +    ARMASIdx_TagNS = 2,

> +    ARMASIdx_TagS = 3,

>  } ARMASIdx;

>

>  /* Return the Exception Level targeted by debug exceptions. */

> diff --git a/hw/arm/virt.c b/hw/arm/virt.c

> index d74538b021..573988ba4d 100644

> --- a/hw/arm/virt.c

> +++ b/hw/arm/virt.c

> @@ -1330,6 +1330,18 @@ static void create_secure_ram(VirtMachineState *vms,

>      g_free(nodename);

>  }

>

> +static void create_tag_ram(MemoryRegion *tag_sysmem,

> +                           hwaddr base, hwaddr size,

> +                           const char *name)

> +{

> +    MemoryRegion *tagram = g_new(MemoryRegion, 1);

> +

> +    memory_region_init_ram(tagram, NULL, name, size / 32, &error_fatal);

> +    memory_region_add_subregion(tag_sysmem, base / 32, tagram);

> +

> +    /* ??? Do we really need an fdt entry, or is MemTag enabled sufficient.  */


What's this '???' asking about? I would be surprised if the
kernel expected to have any kind of FDT for tag RAM
(with the exception that an implementation that puts tags
in a special part of normal-ram will want that not
to be described in the fdt as ram usable by the kernel), but
we should ask the kernel folks.

> +}

> +

>  static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)

>  {

>      const VirtMachineState *board = container_of(binfo, VirtMachineState,

> @@ -1485,6 +1497,8 @@ static void machvirt_init(MachineState *machine)

>      qemu_irq pic[NUM_IRQS];

>      MemoryRegion *sysmem = get_system_memory();

>      MemoryRegion *secure_sysmem = NULL;

> +    MemoryRegion *tag_sysmem = NULL;

> +    MemoryRegion *secure_tag_sysmem = NULL;

>      int n, virt_max_cpus;

>      MemoryRegion *ram = g_new(MemoryRegion, 1);

>      bool firmware_loaded;

> @@ -1652,6 +1666,35 @@ static void machvirt_init(MachineState *machine)

>                                       "secure-memory", &error_abort);

>          }

>

> +        /*

> +         * The cpu adds the property iff MemTag is supported.


We've had confusion before from non-native-speakers and
non-maths-geeks about 'iff' in comments; better to expand to
'if and only if'.

> +         * If it is, we must allocate the ram to back that up.

> +         */

> +        if (object_property_find(cpuobj, "tag-memory", NULL)) {

> +            if (!tag_sysmem) {

> +                tag_sysmem = g_new(MemoryRegion, 1);

> +                memory_region_init(tag_sysmem, OBJECT(machine),

> +                                   "tag-memory", UINT64_MAX / 32);

> +

> +                if (vms->secure) {

> +                    secure_tag_sysmem = g_new(MemoryRegion, 1);

> +                    memory_region_init(secure_tag_sysmem, OBJECT(machine),

> +                                       "secure-tag-memory", UINT64_MAX / 32);

> +

> +                    /* As with ram, secure-tag takes precedence over tag.  */

> +                    memory_region_add_subregion_overlap(secure_tag_sysmem, 0,

> +                                                        tag_sysmem, -1);

> +                }

> +            }


Are there really separate S and NS tag RAMs?

thanks
-- PMM
Richard Henderson Dec. 5, 2019, 7:24 p.m. UTC | #2
On 12/5/19 10:40 AM, Peter Maydell wrote:
>> +         * If it is, we must allocate the ram to back that up.

>> +         */

>> +        if (object_property_find(cpuobj, "tag-memory", NULL)) {

>> +            if (!tag_sysmem) {

>> +                tag_sysmem = g_new(MemoryRegion, 1);

>> +                memory_region_init(tag_sysmem, OBJECT(machine),

>> +                                   "tag-memory", UINT64_MAX / 32);

>> +

>> +                if (vms->secure) {

>> +                    secure_tag_sysmem = g_new(MemoryRegion, 1);

>> +                    memory_region_init(secure_tag_sysmem, OBJECT(machine),

>> +                                       "secure-tag-memory", UINT64_MAX / 32);

>> +

>> +                    /* As with ram, secure-tag takes precedence over tag.  */

>> +                    memory_region_add_subregion_overlap(secure_tag_sysmem, 0,

>> +                                                        tag_sysmem, -1);

>> +                }

>> +            }

> 

> Are there really separate S and NS tag RAMs?


Implementation defined, I believe.  As with everything about tag storage, it
would seem.  But since there are separate S and NS normal RAMS, I create
separate tag spaces to match.


r~
Peter Maydell Dec. 6, 2019, 9:51 a.m. UTC | #3
On Thu, 5 Dec 2019 at 19:24, Richard Henderson
<richard.henderson@linaro.org> wrote:
>

> On 12/5/19 10:40 AM, Peter Maydell wrote:

> > Are there really separate S and NS tag RAMs?

>

> Implementation defined, I believe.  As with everything about tag storage, it

> would seem.  But since there are separate S and NS normal RAMS, I create

> separate tag spaces to match.


Yeah, it seems like the obvious design. I just couldn't find anything
in the spec that said it was possible. I'm probably missing the
relevant paragraph.

-- PMM
diff mbox series

Patch

diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 93a362708b..faca43ea78 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -765,6 +765,10 @@  struct ARMCPU {
     /* MemoryRegion to use for secure physical accesses */
     MemoryRegion *secure_memory;
 
+    /* MemoryRegion to use for allocation tag accesses */
+    MemoryRegion *tag_memory;
+    MemoryRegion *secure_tag_memory;
+
     /* For v8M, pointer to the IDAU interface provided by board/SoC */
     Object *idau;
 
@@ -2956,6 +2960,8 @@  int cpu_mmu_index(CPUARMState *env, bool ifetch);
 typedef enum ARMASIdx {
     ARMASIdx_NS = 0,
     ARMASIdx_S = 1,
+    ARMASIdx_TagNS = 2,
+    ARMASIdx_TagS = 3,
 } ARMASIdx;
 
 /* Return the Exception Level targeted by debug exceptions. */
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index d74538b021..573988ba4d 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -1330,6 +1330,18 @@  static void create_secure_ram(VirtMachineState *vms,
     g_free(nodename);
 }
 
+static void create_tag_ram(MemoryRegion *tag_sysmem,
+                           hwaddr base, hwaddr size,
+                           const char *name)
+{
+    MemoryRegion *tagram = g_new(MemoryRegion, 1);
+
+    memory_region_init_ram(tagram, NULL, name, size / 32, &error_fatal);
+    memory_region_add_subregion(tag_sysmem, base / 32, tagram);
+
+    /* ??? Do we really need an fdt entry, or is MemTag enabled sufficient.  */
+}
+
 static void *machvirt_dtb(const struct arm_boot_info *binfo, int *fdt_size)
 {
     const VirtMachineState *board = container_of(binfo, VirtMachineState,
@@ -1485,6 +1497,8 @@  static void machvirt_init(MachineState *machine)
     qemu_irq pic[NUM_IRQS];
     MemoryRegion *sysmem = get_system_memory();
     MemoryRegion *secure_sysmem = NULL;
+    MemoryRegion *tag_sysmem = NULL;
+    MemoryRegion *secure_tag_sysmem = NULL;
     int n, virt_max_cpus;
     MemoryRegion *ram = g_new(MemoryRegion, 1);
     bool firmware_loaded;
@@ -1652,6 +1666,35 @@  static void machvirt_init(MachineState *machine)
                                      "secure-memory", &error_abort);
         }
 
+        /*
+         * The cpu adds the property iff MemTag is supported.
+         * If it is, we must allocate the ram to back that up.
+         */
+        if (object_property_find(cpuobj, "tag-memory", NULL)) {
+            if (!tag_sysmem) {
+                tag_sysmem = g_new(MemoryRegion, 1);
+                memory_region_init(tag_sysmem, OBJECT(machine),
+                                   "tag-memory", UINT64_MAX / 32);
+
+                if (vms->secure) {
+                    secure_tag_sysmem = g_new(MemoryRegion, 1);
+                    memory_region_init(secure_tag_sysmem, OBJECT(machine),
+                                       "secure-tag-memory", UINT64_MAX / 32);
+
+                    /* As with ram, secure-tag takes precedence over tag.  */
+                    memory_region_add_subregion_overlap(secure_tag_sysmem, 0,
+                                                        tag_sysmem, -1);
+                }
+            }
+
+            object_property_set_link(cpuobj, OBJECT(tag_sysmem),
+                                     "tag-memory", &error_abort);
+            if (vms->secure) {
+                object_property_set_link(cpuobj, OBJECT(secure_tag_sysmem),
+                                         "secure-tag-memory", &error_abort);
+            }
+        }
+
         object_property_set_bool(cpuobj, true, "realized", &error_fatal);
         object_unref(cpuobj);
     }
@@ -1695,6 +1738,17 @@  static void machvirt_init(MachineState *machine)
         create_uart(vms, pic, VIRT_SECURE_UART, secure_sysmem, serial_hd(1));
     }
 
+    if (tag_sysmem) {
+        create_tag_ram(tag_sysmem, vms->memmap[VIRT_MEM].base,
+                       machine->ram_size, "mach-virt.tag");
+        if (vms->secure) {
+            create_tag_ram(secure_tag_sysmem,
+                           vms->memmap[VIRT_SECURE_MEM].base,
+                           vms->memmap[VIRT_SECURE_MEM].size,
+                           "mach-virt.secure-tag");
+        }
+    }
+
     vms->highmem_ecam &= vms->highmem && (!firmware_loaded || aarch64);
 
     create_rtc(vms, pic);
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index 12fffa3ee4..a3a49cd5bf 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -1182,6 +1182,27 @@  void arm_cpu_post_init(Object *obj)
 
     qdev_property_add_static(DEVICE(obj), &arm_cpu_cfgend_property,
                              &error_abort);
+
+#ifndef CONFIG_USER_ONLY
+    if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64) &&
+        cpu_isar_feature(aa64_mte, cpu)) {
+        object_property_add_link(obj, "tag-memory",
+                                 TYPE_MEMORY_REGION,
+                                 (Object **)&cpu->tag_memory,
+                                 qdev_prop_allow_set_link_before_realize,
+                                 OBJ_PROP_LINK_STRONG,
+                                 &error_abort);
+
+        if (arm_feature(&cpu->env, ARM_FEATURE_EL3)) {
+            object_property_add_link(obj, "secure-tag-memory",
+                                     TYPE_MEMORY_REGION,
+                                     (Object **)&cpu->secure_tag_memory,
+                                     qdev_prop_allow_set_link_before_realize,
+                                     OBJ_PROP_LINK_STRONG,
+                                     &error_abort);
+        }
+    }
+#endif
 }
 
 static void arm_cpu_finalizefn(Object *obj)
@@ -1632,17 +1653,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;
+    }
 
+    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);
 
     /* No core_count specified, default to smp_cpus. */