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

["Philippe Mathieu-Daudé" <f4bug@amsat.org>]

[-- Attachment #1: Type: text/plain, Size: 3665 bytes --]

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
>
>
>

[-- Attachment #2: Type: text/html, Size: 4977 bytes --]