| Message ID | 20200827124335.30586-1-leif@nuviainc.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] hw/arm/sbsa-ref: add "reg" property to DT cpu nodes | expand |
Le jeu. 27 août 2020 14:45, Leif Lindholm <leif@nuviainc.com> a écrit : > 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. > > This also ends up being cleaner than having the firmware calculating its > own IDs for generating APCI. > > Signed-off-by: Leif Lindholm <leif@nuviainc.com> > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> --- > > As per Graeme's feedback, properly represent the MPIDR topology info > in the reg property rather than just counting cores (and update the > commit message on why this is useful). > I'm using the local helper function sbsa_ref_cpu_mp_affinity() for this, > and moving it up somewhat rather than adding a forward declaration. > > hw/arm/sbsa-ref.c | 29 +++++++++++++++++++++++------ > 1 file changed, 23 insertions(+), 6 deletions(-) > > diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c > index f030a416fd..3e65ded9a0 100644 > --- a/hw/arm/sbsa-ref.c > +++ b/hw/arm/sbsa-ref.c > @@ -138,6 +138,12 @@ static const int sbsa_ref_irqmap[] = { > [SBSA_EHCI] = 11, > }; > > +static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx) > +{ > + uint8_t clustersz = ARM_DEFAULT_CPUS_PER_CLUSTER; > + return arm_cpu_mp_affinity(idx, clustersz); > +} > + > /* > * Firmware on this machine only uses ACPI table to load OS, these limited > * device tree nodes are just to let firmware know the info which varies > from > @@ -183,14 +189,31 @@ 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); > ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(cpu)); > CPUState *cs = CPU(armcpu); > + uint64_t mpidr = sbsa_ref_cpu_mp_affinity(sms, cpu); > > qemu_fdt_add_subnode(sms->fdt, nodename); > + qemu_fdt_setprop_u64(sms->fdt, nodename, "reg", mpidr); > > if (ms->possible_cpus->cpus[cs->cpu_index].props.has_node_id) { > qemu_fdt_setprop_cell(sms->fdt, nodename, "numa-node-id", > @@ -717,12 +740,6 @@ static void sbsa_ref_init(MachineState *machine) > arm_load_kernel(ARM_CPU(first_cpu), machine, &sms->bootinfo); > } > > -static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx) > -{ > - uint8_t clustersz = ARM_DEFAULT_CPUS_PER_CLUSTER; > - return arm_cpu_mp_affinity(idx, clustersz); > -} > - > static const CPUArchIdList *sbsa_ref_possible_cpu_arch_ids(MachineState > *ms) > { > unsigned int max_cpus = ms->smp.max_cpus; > -- > 2.20.1 > > > <div dir="auto"><div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Le jeu. 27 août 2020 14:45, Leif Lindholm <<a href="mailto:leif@nuviainc.com">leif@nuviainc.com</a>> a écrit :<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">The sbsa-ref platform uses a minimal device tree to pass amount of memory<br> as well as number of cpus to the firmware. However, when dumping that<br> minimal dtb (with -M sbsa-virt,dumpdtb=<file>), the resulting blob<br> generates a warning when decompiled by dtc due to lack of reg property.<br> <br> Add a simple reg property per cpu, representing a 64-bit MPIDR_EL1.<br> <br> This also ends up being cleaner than having the firmware calculating its<br> own IDs for generating APCI.<br> <br> Signed-off-by: Leif Lindholm <<a href="mailto:leif@nuviainc.com" target="_blank" rel="noreferrer">leif@nuviainc.com</a>><br></blockquote></div></div><div dir="auto"><br></div><div dir="auto"><span style="font-family:sans-serif;font-size:13.696px">Reviewed-by: Philippe Mathieu-Daudé <</span><a href="mailto:f4bug@amsat.org" style="text-decoration:none;color:rgb(66,133,244);font-family:sans-serif;font-size:13.696px">f4bug@amsat.org</a><span style="font-family:sans-serif;font-size:13.696px">></span><br></div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> ---<br> <br> As per Graeme's feedback, properly represent the MPIDR topology info<br> in the reg property rather than just counting cores (and update the<br> commit message on why this is useful).<br> I'm using the local helper function sbsa_ref_cpu_mp_affinity() for this,<br> and moving it up somewhat rather than adding a forward declaration.<br> <br> hw/arm/sbsa-ref.c | 29 +++++++++++++++++++++++------<br> 1 file changed, 23 insertions(+), 6 deletions(-)<br> <br> diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c<br> index f030a416fd..3e65ded9a0 100644<br> --- a/hw/arm/sbsa-ref.c<br> +++ b/hw/arm/sbsa-ref.c<br> @@ -138,6 +138,12 @@ static const int sbsa_ref_irqmap[] = {<br> [SBSA_EHCI] = 11,<br> };<br> <br> +static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx)<br> +{<br> + uint8_t clustersz = ARM_DEFAULT_CPUS_PER_CLUSTER;<br> + return arm_cpu_mp_affinity(idx, clustersz);<br> +}<br> +<br> /*<br> * Firmware on this machine only uses ACPI table to load OS, these limited<br> * device tree nodes are just to let firmware know the info which varies from<br> @@ -183,14 +189,31 @@ static void create_fdt(SBSAMachineState *sms)<br> g_free(matrix);<br> }<br> <br> + /*<br> + * From Documentation/devicetree/bindings/arm/cpus.yaml<br> + * On ARM v8 64-bit systems this property is required<br> + * and matches the MPIDR_EL1 register affinity bits.<br> + *<br> + * * If cpus node's #address-cells property is set to 2<br> + *<br> + * The first reg cell bits [7:0] must be set to<br> + * bits [39:32] of MPIDR_EL1.<br> + *<br> + * The second reg cell bits [23:0] must be set to<br> + * bits [23:0] of MPIDR_EL1.<br> + */<br> qemu_fdt_add_subnode(sms->fdt, "/cpus");<br> + qemu_fdt_setprop_cell(sms->fdt, "/cpus", "#address-cells", 2);<br> + qemu_fdt_setprop_cell(sms->fdt, "/cpus", "#size-cells", 0x0);<br> <br> for (cpu = sms->smp_cpus - 1; cpu >= 0; cpu--) {<br> char *nodename = g_strdup_printf("/cpus/cpu@%d", cpu);<br> ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(cpu));<br> CPUState *cs = CPU(armcpu);<br> + uint64_t mpidr = sbsa_ref_cpu_mp_affinity(sms, cpu);<br> <br> qemu_fdt_add_subnode(sms->fdt, nodename);<br> + qemu_fdt_setprop_u64(sms->fdt, nodename, "reg", mpidr);<br> <br> if (ms->possible_cpus->cpus[cs->cpu_index].props.has_node_id) {<br> qemu_fdt_setprop_cell(sms->fdt, nodename, "numa-node-id",<br> @@ -717,12 +740,6 @@ static void sbsa_ref_init(MachineState *machine)<br> arm_load_kernel(ARM_CPU(first_cpu), machine, &sms->bootinfo);<br> }<br> <br> -static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx)<br> -{<br> - uint8_t clustersz = ARM_DEFAULT_CPUS_PER_CLUSTER;<br> - return arm_cpu_mp_affinity(idx, clustersz);<br> -}<br> -<br> static const CPUArchIdList *sbsa_ref_possible_cpu_arch_ids(MachineState *ms)<br> {<br> unsigned int max_cpus = ms->smp.max_cpus;<br> -- <br> 2.20.1<br> <br> <br> </blockquote></div></div></div>
diff --git a/hw/arm/sbsa-ref.c b/hw/arm/sbsa-ref.c index f030a416fd..3e65ded9a0 100644 --- a/hw/arm/sbsa-ref.c +++ b/hw/arm/sbsa-ref.c @@ -138,6 +138,12 @@ static const int sbsa_ref_irqmap[] = { [SBSA_EHCI] = 11, }; +static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx) +{ + uint8_t clustersz = ARM_DEFAULT_CPUS_PER_CLUSTER; + return arm_cpu_mp_affinity(idx, clustersz); +} + /* * Firmware on this machine only uses ACPI table to load OS, these limited * device tree nodes are just to let firmware know the info which varies from @@ -183,14 +189,31 @@ 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); ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(cpu)); CPUState *cs = CPU(armcpu); + uint64_t mpidr = sbsa_ref_cpu_mp_affinity(sms, cpu); qemu_fdt_add_subnode(sms->fdt, nodename); + qemu_fdt_setprop_u64(sms->fdt, nodename, "reg", mpidr); if (ms->possible_cpus->cpus[cs->cpu_index].props.has_node_id) { qemu_fdt_setprop_cell(sms->fdt, nodename, "numa-node-id", @@ -717,12 +740,6 @@ static void sbsa_ref_init(MachineState *machine) arm_load_kernel(ARM_CPU(first_cpu), machine, &sms->bootinfo); } -static uint64_t sbsa_ref_cpu_mp_affinity(SBSAMachineState *sms, int idx) -{ - uint8_t clustersz = ARM_DEFAULT_CPUS_PER_CLUSTER; - return arm_cpu_mp_affinity(idx, clustersz); -} - static const CPUArchIdList *sbsa_ref_possible_cpu_arch_ids(MachineState *ms) { unsigned int max_cpus = ms->smp.max_cpus;
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. This also ends up being cleaner than having the firmware calculating its own IDs for generating APCI. Signed-off-by: Leif Lindholm <leif@nuviainc.com> --- As per Graeme's feedback, properly represent the MPIDR topology info in the reg property rather than just counting cores (and update the commit message on why this is useful). I'm using the local helper function sbsa_ref_cpu_mp_affinity() for this, and moving it up somewhat rather than adding a forward declaration. hw/arm/sbsa-ref.c | 29 +++++++++++++++++++++++------ 1 file changed, 23 insertions(+), 6 deletions(-)