[PATCH] hw/arm/sbsa-ref: add "reg" property to DT cpu nodes

Leif Lindholm posted 1 patch 3 years, 8 months ago
Test docker-quick@centos7 failed
Test docker-mingw@fedora failed
Test checkpatch failed
Test FreeBSD failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200825165217.7379-1-leif@nuviainc.com
Maintainers: Radoslaw Biernacki <rad@semihalf.com>, Leif Lindholm <leif@nuviainc.com>, Peter Maydell <peter.maydell@linaro.org>
There is a newer version of this series
hw/arm/sbsa-ref.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
[PATCH] hw/arm/sbsa-ref: add "reg" property to DT cpu nodes
Posted by Leif Lindholm 3 years, 8 months ago
The sbsa-ref platform uses a minimal device tree to pass amount of memory
as well as number of cpus to the firmware. However, when dumping that
minimal dtb (with -M sbsa-virt,dumpdtb=<file>), the resulting blob
generates a warning when decompiled by dtc due to lack of reg property.

Add a simple reg property per cpu, representing a 64-bit MPIDR_EL1.

Signed-off-by: Leif Lindholm <leif@nuviainc.com>
---
 hw/arm/sbsa-ref.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
index f030a416fd..167c57a08b 100644
--- a/hw/arm/sbsa-ref.c
+++ b/hw/arm/sbsa-ref.c
@@ -183,7 +183,22 @@ static void create_fdt(SBSAMachineState *sms)
         g_free(matrix);
     }
 
+    /*
+     * From Documentation/devicetree/bindings/arm/cpus.yaml
+     *  On ARM v8 64-bit systems this property is required
+     *    and matches the MPIDR_EL1 register affinity bits.
+     *
+     *    * If cpus node's #address-cells property is set to 2
+     *
+     *      The first reg cell bits [7:0] must be set to
+     *      bits [39:32] of MPIDR_EL1.
+     *
+     *      The second reg cell bits [23:0] must be set to
+     *      bits [23:0] of MPIDR_EL1.
+     */
     qemu_fdt_add_subnode(sms->fdt, "/cpus");
+    qemu_fdt_setprop_cell(sms->fdt, "/cpus", "#address-cells", 2);
+    qemu_fdt_setprop_cell(sms->fdt, "/cpus", "#size-cells", 0x0);
 
     for (cpu = sms->smp_cpus - 1; cpu >= 0; cpu--) {
         char *nodename = g_strdup_printf("/cpus/cpu@%d", cpu);
@@ -191,6 +206,7 @@ static void create_fdt(SBSAMachineState *sms)
         CPUState *cs = CPU(armcpu);
 
         qemu_fdt_add_subnode(sms->fdt, nodename);
+        qemu_fdt_setprop_u64(sms->fdt, nodename, "reg", cpu);
 
         if (ms->possible_cpus->cpus[cs->cpu_index].props.has_node_id) {
             qemu_fdt_setprop_cell(sms->fdt, nodename, "numa-node-id",
-- 
2.20.1


Re: [PATCH] hw/arm/sbsa-ref: add "reg" property to DT cpu nodes
Posted by Graeme Gregory 3 years, 8 months ago
On Tue, Aug 25, 2020 at 05:52:17PM +0100, Leif Lindholm wrote:
> The sbsa-ref platform uses a minimal device tree to pass amount of memory
> as well as number of cpus to the firmware. However, when dumping that
> minimal dtb (with -M sbsa-virt,dumpdtb=<file>), the resulting blob
> generates a warning when decompiled by dtc due to lack of reg property.
> 
> Add a simple reg property per cpu, representing a 64-bit MPIDR_EL1.
> 
> Signed-off-by: Leif Lindholm <leif@nuviainc.com>
> ---
>  hw/arm/sbsa-ref.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c
> index f030a416fd..167c57a08b 100644
> --- a/hw/arm/sbsa-ref.c
> +++ b/hw/arm/sbsa-ref.c
> @@ -183,7 +183,22 @@ static void create_fdt(SBSAMachineState *sms)
>          g_free(matrix);
>      }
>  
> +    /*
> +     * From Documentation/devicetree/bindings/arm/cpus.yaml
> +     *  On ARM v8 64-bit systems this property is required
> +     *    and matches the MPIDR_EL1 register affinity bits.
> +     *
> +     *    * If cpus node's #address-cells property is set to 2
> +     *
> +     *      The first reg cell bits [7:0] must be set to
> +     *      bits [39:32] of MPIDR_EL1.
> +     *
> +     *      The second reg cell bits [23:0] must be set to
> +     *      bits [23:0] of MPIDR_EL1.
> +     */
>      qemu_fdt_add_subnode(sms->fdt, "/cpus");
> +    qemu_fdt_setprop_cell(sms->fdt, "/cpus", "#address-cells", 2);
> +    qemu_fdt_setprop_cell(sms->fdt, "/cpus", "#size-cells", 0x0);
>  
>      for (cpu = sms->smp_cpus - 1; cpu >= 0; cpu--) {
>          char *nodename = g_strdup_printf("/cpus/cpu@%d", cpu);
> @@ -191,6 +206,7 @@ static void create_fdt(SBSAMachineState *sms)
>          CPUState *cs = CPU(armcpu);
>  
>          qemu_fdt_add_subnode(sms->fdt, nodename);
> +        qemu_fdt_setprop_u64(sms->fdt, nodename, "reg", cpu);

To obey the statement earlier shouldnt this be taken from the mp_affinity
variable in armcpu?

Graeme

>  
>          if (ms->possible_cpus->cpus[cs->cpu_index].props.has_node_id) {
>              qemu_fdt_setprop_cell(sms->fdt, nodename, "numa-node-id",
> -- 
> 2.20.1
>