[PATCH v2 21/47] hw/arm/xlnx-versal: add the mp_affinity property to the CPU mapping

Luc Michel posted 47 patches 2 months, 3 weeks ago
Maintainers: Alistair Francis <alistair@alistair23.me>, "Edgar E. Iglesias" <edgar.iglesias@gmail.com>, Peter Maydell <peter.maydell@linaro.org>
There is a newer version of this series
[PATCH v2 21/47] hw/arm/xlnx-versal: add the mp_affinity property to the CPU mapping
Posted by Luc Michel 2 months, 3 weeks ago
Add a way to configure the MP affinity value of the CPUs given their
core and cluster IDs. For the Versal APU CPUs, the MP affinity value is
directly given by the core ID.

Signed-off-by: Luc Michel <luc.michel@amd.com>
Reviewed-by: Francisco Iglesias <francisco.iglesias@amd.com>
---
 hw/arm/xlnx-versal.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c
index 5a08ad07b28..35c32de0159 100644
--- a/hw/arm/xlnx-versal.c
+++ b/hw/arm/xlnx-versal.c
@@ -88,10 +88,18 @@ typedef struct VersalCpuClusterMap {
     size_t num_core;
     size_t num_cluster;
     uint32_t qemu_cluster_id;
     bool dtb_expose;
 
+    struct {
+        uint64_t base;
+        uint64_t core_mask;
+        uint64_t core_shift;
+        uint64_t cluster_mask;
+        uint64_t cluster_shift;
+    } mp_affinity;
+
     enum StartPoweredOffMode start_powered_off;
 } VersalCpuClusterMap;
 
 typedef struct VersalMap {
     VersalCpuClusterMap apu;
@@ -196,10 +204,15 @@ static const VersalMap VERSAL_MAP = {
         .name = "apu",
         .cpu_model = ARM_CPU_TYPE_NAME("cortex-a72"),
         .num_cluster = 1,
         .num_core = 2,
         .qemu_cluster_id = 0,
+        .mp_affinity = {
+            .base = 0x0,
+            .core_mask = 0xff,
+            .core_shift = 0,
+        },
         .start_powered_off = SPO_SECONDARIES,
         .dtb_expose = true,
         .gic = {
             .version = 3,
             .dist = 0xf9000000,
@@ -565,23 +578,31 @@ static DeviceState *versal_create_cpu(Versal *s,
                                       size_t core_idx)
 {
     DeviceState *cpu = qdev_new(map->cpu_model);
     ARMCPU *arm_cpu = ARM_CPU(cpu);
     Object *obj = OBJECT(cpu);
+    uint64_t affinity;
     bool start_off;
     size_t idx = cluster_idx * map->num_core + core_idx;
     g_autofree char *name;
     g_autofree char *node = NULL;
 
+    affinity = map->mp_affinity.base;
+    affinity |= (cluster_idx & map->mp_affinity.cluster_mask)
+        << map->mp_affinity.cluster_shift;
+    affinity |= (core_idx & map->mp_affinity.core_mask)
+        << map->mp_affinity.core_shift;
+
     start_off = map->start_powered_off == SPO_ALL
         || ((map->start_powered_off == SPO_SECONDARIES)
             && (cluster_idx || core_idx));
 
     name = g_strdup_printf("%s[*]", map->name);
     object_property_add_child(OBJECT(qemu_cluster), name, obj);
     object_property_set_bool(obj, "start-powered-off", start_off,
                              &error_abort);
+    qdev_prop_set_uint64(cpu, "mp-affinity", affinity);
     qdev_prop_set_int32(cpu, "core-count",  map->num_core);
     object_property_set_link(obj, "memory", OBJECT(cpu_mr), &error_abort);
     qdev_realize_and_unref(cpu, NULL, &error_fatal);
 
     if (!map->dtb_expose) {
-- 
2.50.1
Re: [PATCH v2 21/47] hw/arm/xlnx-versal: add the mp_affinity property to the CPU mapping
Posted by Philippe Mathieu-Daudé 2 months, 3 weeks ago
Hi Luc,

On 20/8/25 10:25, Luc Michel wrote:
> Add a way to configure the MP affinity value of the CPUs given their
> core and cluster IDs. For the Versal APU CPUs, the MP affinity value is
> directly given by the core ID.
> 
> Signed-off-by: Luc Michel <luc.michel@amd.com>
> Reviewed-by: Francisco Iglesias <francisco.iglesias@amd.com>
> ---
>   hw/arm/xlnx-versal.c | 21 +++++++++++++++++++++
>   1 file changed, 21 insertions(+)
> 
> diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c
> index 5a08ad07b28..35c32de0159 100644
> --- a/hw/arm/xlnx-versal.c
> +++ b/hw/arm/xlnx-versal.c
> @@ -88,10 +88,18 @@ typedef struct VersalCpuClusterMap {
>       size_t num_core;
>       size_t num_cluster;
>       uint32_t qemu_cluster_id;
>       bool dtb_expose;
>   
> +    struct {
> +        uint64_t base;
> +        uint64_t core_mask;
> +        uint64_t core_shift;
> +        uint64_t cluster_mask;
> +        uint64_t cluster_shift;
> +    } mp_affinity;


> @@ -565,23 +578,31 @@ static DeviceState *versal_create_cpu(Versal *s,
>                                         size_t core_idx)
>   {
>       DeviceState *cpu = qdev_new(map->cpu_model);
>       ARMCPU *arm_cpu = ARM_CPU(cpu);
>       Object *obj = OBJECT(cpu);
> +    uint64_t affinity;
>       bool start_off;
>       size_t idx = cluster_idx * map->num_core + core_idx;
>       g_autofree char *name;
>       g_autofree char *node = NULL;
>   
> +    affinity = map->mp_affinity.base;
> +    affinity |= (cluster_idx & map->mp_affinity.cluster_mask)
> +        << map->mp_affinity.cluster_shift;
> +    affinity |= (core_idx & map->mp_affinity.core_mask)
> +        << map->mp_affinity.core_shift;

Could we expand/re-use arm_build_mp_affinity() here?
Re: [PATCH v2 21/47] hw/arm/xlnx-versal: add the mp_affinity property to the CPU mapping
Posted by Luc Michel 2 months, 3 weeks ago
On 14:18 Wed 20 Aug     , Philippe Mathieu-Daudé wrote:
> Hi Luc,
> 
> On 20/8/25 10:25, Luc Michel wrote:
> > Add a way to configure the MP affinity value of the CPUs given their
> > core and cluster IDs. For the Versal APU CPUs, the MP affinity value is
> > directly given by the core ID.
> > 
> > Signed-off-by: Luc Michel <luc.michel@amd.com>
> > Reviewed-by: Francisco Iglesias <francisco.iglesias@amd.com>
> > ---
> >   hw/arm/xlnx-versal.c | 21 +++++++++++++++++++++
> >   1 file changed, 21 insertions(+)
> > 
> > diff --git a/hw/arm/xlnx-versal.c b/hw/arm/xlnx-versal.c
> > index 5a08ad07b28..35c32de0159 100644
> > --- a/hw/arm/xlnx-versal.c
> > +++ b/hw/arm/xlnx-versal.c
> > @@ -88,10 +88,18 @@ typedef struct VersalCpuClusterMap {
> >       size_t num_core;
> >       size_t num_cluster;
> >       uint32_t qemu_cluster_id;
> >       bool dtb_expose;
> > 
> > +    struct {
> > +        uint64_t base;
> > +        uint64_t core_mask;
> > +        uint64_t core_shift;
> > +        uint64_t cluster_mask;
> > +        uint64_t cluster_shift;
> > +    } mp_affinity;
> 
> 
> > @@ -565,23 +578,31 @@ static DeviceState *versal_create_cpu(Versal *s,
> >                                         size_t core_idx)
> >   {
> >       DeviceState *cpu = qdev_new(map->cpu_model);
> >       ARMCPU *arm_cpu = ARM_CPU(cpu);
> >       Object *obj = OBJECT(cpu);
> > +    uint64_t affinity;
> >       bool start_off;
> >       size_t idx = cluster_idx * map->num_core + core_idx;
> >       g_autofree char *name;
> >       g_autofree char *node = NULL;
> > 
> > +    affinity = map->mp_affinity.base;
> > +    affinity |= (cluster_idx & map->mp_affinity.cluster_mask)
> > +        << map->mp_affinity.cluster_shift;
> > +    affinity |= (core_idx & map->mp_affinity.core_mask)
> > +        << map->mp_affinity.core_shift;
> 
> Could we expand/re-use arm_build_mp_affinity() here?

I'm not sure:
   - versal RPU hardcodes Aff1 to 1,
   - versal2 APU uses Aff2 for cluster id and Aff1 for core id,
   - versal2 RPU uses Aff1 for cluster id and Aff0 for core id.

What I can do is use ARM_AFFx_SHIFT defines in my mp_affinity
description struct, and remove the mask as we probably expect it to be
0xff all the time. Something like:

        .mp_affinity = {
            .base = 0x0,
            .core_mask = ARM_AFF1_MASK,
            .cluster_shift = ARM_AFF2_SHIFT,
        },

What do you think?

-- 
Luc