Re: [Qemu-riscv] [Qemu-devel] [PATCH] riscv: virt: Add cpu-topology DT node.

[Philippe Mathieu-Daudé]

On 6/25/19 1:24 AM, Alistair Francis wrote:
> On Mon, Jun 24, 2019 at 3:57 PM Atish Patra <address@hidden> wrote:
>>
>> Currently, there is no cpu topology defined in RISC-V.
>> Define a device tree node that clearly describes the
>> entire topology. This saves the trouble of scanning individual
>> cache to figure out the topology.
>>
>> Here is the linux kernel patch series that enables topology
>> for RISC-V.
>>
>> http://lists.infradead.org/pipermail/linux-riscv/2019-June/005072.html
>>
>> CPU topology after applying this patch in QEMU & above series in kernel
>>
>> / # cat /sys/devices/system/cpu/cpu2/topology/thread_siblings_list
>> 2
>> / # cat /sys/devices/system/cpu/cpu2/topology/physical_package_id
>> 0
>> / # cat /sys/devices/system/cpu/cpu2/topology/core_siblings_list
>> 0-7
>>
>> Signed-off-by: Atish Patra <address@hidden>
>> ---
>>  hw/riscv/virt.c | 21 +++++++++++++++++++--
>>  1 file changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/riscv/virt.c b/hw/riscv/virt.c
>> index 84d94d0c42d8..da0b8aa18747 100644
>> --- a/hw/riscv/virt.c
>> +++ b/hw/riscv/virt.c
>> @@ -203,9 +203,12 @@ static void *create_fdt(RISCVVirtState *s, const struct 
>> MemmapEntry *memmap,
>>          qemu_fdt_setprop_string(fdt, nodename, "status", "okay");
>>          qemu_fdt_setprop_cell(fdt, nodename, "reg", cpu);
>>          qemu_fdt_setprop_string(fdt, nodename, "device_type", "cpu");
>> +        qemu_fdt_setprop_cell(fdt, nodename, "phandle", cpu_phandle);
>> +        qemu_fdt_setprop_cell(fdt, nodename, "linux,phandle", cpu_phandle);
>> +        int intc_phandle = phandle++;
> 
> Don't declare variables in the middle of code. The variable must be
> declared at the start of a block.

I guess this has been relaxed since we allow GNU C99:

https://git.qemu.org/?p=qemu.git;a=commit;h=7be41675f7cb16be7c8d2554add7a63fa43781a8

> 
> With that fixed:
> 
> Reviewed-by: Alistair Francis <address@hidden>
> 
> Alistair
> 
>>          qemu_fdt_add_subnode(fdt, intc);
>> -        qemu_fdt_setprop_cell(fdt, intc, "phandle", cpu_phandle);
>> -        qemu_fdt_setprop_cell(fdt, intc, "linux,phandle", cpu_phandle);
>> +        qemu_fdt_setprop_cell(fdt, intc, "phandle", intc_phandle);
>> +        qemu_fdt_setprop_cell(fdt, intc, "linux,phandle", intc_phandle);
>>          qemu_fdt_setprop_string(fdt, intc, "compatible", "riscv,cpu-intc");
>>          qemu_fdt_setprop(fdt, intc, "interrupt-controller", NULL, 0);
>>          qemu_fdt_setprop_cell(fdt, intc, "#interrupt-cells", 1);
>> @@ -214,6 +217,20 @@ static void *create_fdt(RISCVVirtState *s, const struct 
>> MemmapEntry *memmap,
>>          g_free(nodename);
>>      }
>>
>> +    /* Add cpu-topology node */
>> +    qemu_fdt_add_subnode(fdt, "/cpus/cpu-map");
>> +    qemu_fdt_add_subnode(fdt, "/cpus/cpu-map/cluster0");
>> +    for (cpu = s->soc.num_harts - 1; cpu >= 0; cpu--) {
>> +        char *core_nodename = 
>> g_strdup_printf("/cpus/cpu-map/cluster0/core%d",
>> +                                              cpu);
>> +        char *cpu_nodename = g_strdup_printf("/cpus/cpu@%d", cpu);
>> +        uint32_t intc_phandle = qemu_fdt_get_phandle(fdt, cpu_nodename);
>> +        qemu_fdt_add_subnode(fdt, core_nodename);
>> +        qemu_fdt_setprop_cell(fdt, core_nodename, "cpu", intc_phandle);
>> +        g_free(core_nodename);
>> +        g_free(cpu_nodename);
>> +    }
>> +
>>      cells =  g_new0(uint32_t, s->soc.num_harts * 4);
>>      for (cpu = 0; cpu < s->soc.num_harts; cpu++) {
>>          nodename =
>> --
>> 2.21.0
>>
>>
>