| Message ID | 20210225085627.2263-3-fangying1@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/arm/virt: Introduce cpu topology support | expand |
Hi Ying Fang, The only difference between this and what I have in my tree[*] is the removal of the socket node (which has been in the Linux docs since June 2019). Any reason why you removed that node? In any case, I think I deserve a bit more credit for this patch. [*] https://github.com/rhdrjones/qemu/commit/35feecdd43475608c8f55973a0c159eac4aafefd Thanks, drew On Thu, Feb 25, 2021 at 04:56:24PM +0800, Ying Fang wrote: > Support device tree CPU topology descriptions. > > Signed-off-by: Ying Fang <fangying1@huawei.com> > --- > hw/arm/virt.c | 38 +++++++++++++++++++++++++++++++++++++- > include/hw/arm/virt.h | 1 + > 2 files changed, 38 insertions(+), 1 deletion(-) > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index 371147f3ae..c133b342b8 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -351,10 +351,11 @@ static void fdt_add_cpu_nodes(const VirtMachineState *vms) > int cpu; > int addr_cells = 1; > const MachineState *ms = MACHINE(vms); > + const VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms); > int smp_cpus = ms->smp.cpus; > > /* > - * From Documentation/devicetree/bindings/arm/cpus.txt > + * See Linux Documentation/devicetree/bindings/arm/cpus.yaml > * On ARM v8 64-bit systems value should be set to 2, > * that corresponds to the MPIDR_EL1 register size. > * If MPIDR_EL1[63:32] value is equal to 0 on all CPUs > @@ -407,8 +408,42 @@ static void fdt_add_cpu_nodes(const VirtMachineState *vms) > ms->possible_cpus->cpus[cs->cpu_index].props.node_id); > } > > + if (ms->smp.cpus > 1 && !vmc->no_cpu_topology) { > + qemu_fdt_setprop_cell(vms->fdt, nodename, "phandle", > + qemu_fdt_alloc_phandle(vms->fdt)); > + } > + > g_free(nodename); > } > + > + if (ms->smp.cpus > 1 && !vmc->no_cpu_topology) { > + /* > + * See Linux Documentation/devicetree/bindings/cpu/cpu-topology.txt > + */ > + qemu_fdt_add_subnode(vms->fdt, "/cpus/cpu-map"); > + > + for (cpu = ms->smp.cpus - 1; cpu >= 0; cpu--) { > + char *cpu_path = g_strdup_printf("/cpus/cpu@%d", cpu); > + char *map_path; > + > + if (ms->smp.threads > 1) { > + map_path = g_strdup_printf( > + "/cpus/cpu-map/%s%d/%s%d/%s%d", > + "cluster", cpu / (ms->smp.cores * ms->smp.threads), > + "core", (cpu / ms->smp.threads) % ms->smp.cores, > + "thread", cpu % ms->smp.threads); > + } else { > + map_path = g_strdup_printf( > + "/cpus/cpu-map/%s%d/%s%d", > + "cluster", cpu / ms->smp.cores, > + "core", cpu % ms->smp.cores); > + } > + qemu_fdt_add_path(vms->fdt, map_path); > + qemu_fdt_setprop_phandle(vms->fdt, map_path, "cpu", cpu_path); > + g_free(map_path); > + g_free(cpu_path); > + } > + } > } > > static void fdt_add_its_gic_node(VirtMachineState *vms) > @@ -2742,6 +2777,7 @@ static void virt_machine_5_2_options(MachineClass *mc) > virt_machine_6_0_options(mc); > compat_props_add(mc->compat_props, hw_compat_5_2, hw_compat_5_2_len); > vmc->no_secure_gpio = true; > + vmc->no_cpu_topology = true; > } > DEFINE_VIRT_MACHINE(5, 2) > > diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h > index ee9a93101e..7ef6d08ac3 100644 > --- a/include/hw/arm/virt.h > +++ b/include/hw/arm/virt.h > @@ -129,6 +129,7 @@ struct VirtMachineClass { > bool no_kvm_steal_time; > bool acpi_expose_flash; > bool no_secure_gpio; > + bool no_cpu_topology; > }; > > struct VirtMachineState { > -- > 2.23.0 > >
On 2/25/2021 7:16 PM, Andrew Jones wrote: > Hi Ying Fang, > > The only difference between this and what I have in my tree[*] > is the removal of the socket node (which has been in the Linux > docs since June 2019). Any reason why you removed that node? In > any case, I think I deserve a bit more credit for this patch. Sorry, you surely deserve it. I forget to add it here. Should I have a SOB of you here ? The latest linux kernel use a four level cpu topology defined in https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/cpu/cpu-topology.txt?h=v5.11 ie. socket node, cluster node, core node, thread node. The linux kernel 4.19 LTS use a three level cpu topology defined in Documentation/devicetree/bindings/arm/topology.txt ie. cluster node, core node, thread node. Currently Qemu x86 has 4 level of cpu topology as: socket, die, core, thread. Should arm64 active like it here ? Further more, latest linux kernel define the cpu topology struct as. So maybe it only cares about the socket, core, thread topology levels. struct cpu_topology { int thread_id; int core_id; int package_id; int llc_id; cpumask_t thread_sibling; cpumask_t core_sibling; cpumask_t llc_sibling; }; > > [*] https://github.com/rhdrjones/qemu/commit/35feecdd43475608c8f55973a0c159eac4aafefd > > Thanks, > drew > > On Thu, Feb 25, 2021 at 04:56:24PM +0800, Ying Fang wrote: >> Support device tree CPU topology descriptions. >> >> Signed-off-by: Ying Fang <fangying1@huawei.com> >> --- >> hw/arm/virt.c | 38 +++++++++++++++++++++++++++++++++++++- >> include/hw/arm/virt.h | 1 + >> 2 files changed, 38 insertions(+), 1 deletion(-) >> >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >> index 371147f3ae..c133b342b8 100644 >> --- a/hw/arm/virt.c >> +++ b/hw/arm/virt.c >> @@ -351,10 +351,11 @@ static void fdt_add_cpu_nodes(const VirtMachineState *vms) >> int cpu; >> int addr_cells = 1; >> const MachineState *ms = MACHINE(vms); >> + const VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms); >> int smp_cpus = ms->smp.cpus; >> >> /* >> - * From Documentation/devicetree/bindings/arm/cpus.txt >> + * See Linux Documentation/devicetree/bindings/arm/cpus.yaml >> * On ARM v8 64-bit systems value should be set to 2, >> * that corresponds to the MPIDR_EL1 register size. >> * If MPIDR_EL1[63:32] value is equal to 0 on all CPUs >> @@ -407,8 +408,42 @@ static void fdt_add_cpu_nodes(const VirtMachineState *vms) >> ms->possible_cpus->cpus[cs->cpu_index].props.node_id); >> } >> >> + if (ms->smp.cpus > 1 && !vmc->no_cpu_topology) { >> + qemu_fdt_setprop_cell(vms->fdt, nodename, "phandle", >> + qemu_fdt_alloc_phandle(vms->fdt)); >> + } >> + >> g_free(nodename); >> } >> + >> + if (ms->smp.cpus > 1 && !vmc->no_cpu_topology) { >> + /* >> + * See Linux Documentation/devicetree/bindings/cpu/cpu-topology.txt >> + */ >> + qemu_fdt_add_subnode(vms->fdt, "/cpus/cpu-map"); >> + >> + for (cpu = ms->smp.cpus - 1; cpu >= 0; cpu--) { >> + char *cpu_path = g_strdup_printf("/cpus/cpu@%d", cpu); >> + char *map_path; >> + >> + if (ms->smp.threads > 1) { >> + map_path = g_strdup_printf( >> + "/cpus/cpu-map/%s%d/%s%d/%s%d", >> + "cluster", cpu / (ms->smp.cores * ms->smp.threads), a cluster node may be replaced by socket to keep accord with the latest kernel. >> + "core", (cpu / ms->smp.threads) % ms->smp.cores, >> + "thread", cpu % ms->smp.threads); >> + } else { >> + map_path = g_strdup_printf( >> + "/cpus/cpu-map/%s%d/%s%d", >> + "cluster", cpu / ms->smp.cores, >> + "core", cpu % ms->smp.cores); >> + } >> + qemu_fdt_add_path(vms->fdt, map_path); >> + qemu_fdt_setprop_phandle(vms->fdt, map_path, "cpu", cpu_path); >> + g_free(map_path); >> + g_free(cpu_path); >> + } >> + } >> } >> >> static void fdt_add_its_gic_node(VirtMachineState *vms) >> @@ -2742,6 +2777,7 @@ static void virt_machine_5_2_options(MachineClass *mc) >> virt_machine_6_0_options(mc); >> compat_props_add(mc->compat_props, hw_compat_5_2, hw_compat_5_2_len); >> vmc->no_secure_gpio = true; >> + vmc->no_cpu_topology = true; >> } >> DEFINE_VIRT_MACHINE(5, 2) >> >> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h >> index ee9a93101e..7ef6d08ac3 100644 >> --- a/include/hw/arm/virt.h >> +++ b/include/hw/arm/virt.h >> @@ -129,6 +129,7 @@ struct VirtMachineClass { >> bool no_kvm_steal_time; >> bool acpi_expose_flash; >> bool no_secure_gpio; >> + bool no_cpu_topology; >> }; >> >> struct VirtMachineState { >> -- >> 2.23.0 >> >> > > . >
On Thu, Feb 25, 2021 at 09:18:22PM +0800, Ying Fang wrote: > > > On 2/25/2021 7:16 PM, Andrew Jones wrote: > > Hi Ying Fang, > > > > The only difference between this and what I have in my tree[*] > > is the removal of the socket node (which has been in the Linux > > docs since June 2019). Any reason why you removed that node? In > > any case, I think I deserve a bit more credit for this patch. > > Sorry, you surely deserve it. I forget to add it here. > Should I have a SOB of you here ? > > The latest linux kernel use a four level cpu topology defined in > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/cpu/cpu-topology.txt?h=v5.11 > > ie. socket node, cluster node, core node, thread node. Yes, this is why the code I wrote uses the 'socket' node. > > The linux kernel 4.19 LTS use a three level cpu topology defined in > Documentation/devicetree/bindings/arm/topology.txt I don't think we want to target that guest kernel with this new QEMU feature. If we must support that guest kernel, then I would do it under a machine property, like compat_cpumap or something. > > ie. cluster node, core node, thread node. > > Currently Qemu x86 has 4 level of cpu topology as: socket, die, core, > thread. Should arm64 active like it here ? Does the arm64 guest kernel support the concept of 'die'? (I don't think so) Anyway, there's no such concept in the current cpu-map definition. So, if the guest kernel does support die, then what does it map that to from DT and ACPI? > > Further more, latest linux kernel define the cpu topology struct as. > So maybe it only cares about the socket, core, thread topology levels. > > struct cpu_topology { > > int thread_id; > > int core_id; > > int package_id; > > int llc_id; > > cpumask_t thread_sibling; > > cpumask_t core_sibling; > > cpumask_t llc_sibling; > > }; > > > > > [*] https://github.com/rhdrjones/qemu/commit/35feecdd43475608c8f55973a0c159eac4aafefd > > > > Thanks, > > drew > > > > On Thu, Feb 25, 2021 at 04:56:24PM +0800, Ying Fang wrote: > > > Support device tree CPU topology descriptions. > > > > > > Signed-off-by: Ying Fang <fangying1@huawei.com> > > > --- > > > hw/arm/virt.c | 38 +++++++++++++++++++++++++++++++++++++- > > > include/hw/arm/virt.h | 1 + > > > 2 files changed, 38 insertions(+), 1 deletion(-) > > > > > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > > > index 371147f3ae..c133b342b8 100644 > > > --- a/hw/arm/virt.c > > > +++ b/hw/arm/virt.c > > > @@ -351,10 +351,11 @@ static void fdt_add_cpu_nodes(const VirtMachineState *vms) > > > int cpu; > > > int addr_cells = 1; > > > const MachineState *ms = MACHINE(vms); > > > + const VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms); > > > int smp_cpus = ms->smp.cpus; > > > /* > > > - * From Documentation/devicetree/bindings/arm/cpus.txt > > > + * See Linux Documentation/devicetree/bindings/arm/cpus.yaml > > > * On ARM v8 64-bit systems value should be set to 2, > > > * that corresponds to the MPIDR_EL1 register size. > > > * If MPIDR_EL1[63:32] value is equal to 0 on all CPUs > > > @@ -407,8 +408,42 @@ static void fdt_add_cpu_nodes(const VirtMachineState *vms) > > > ms->possible_cpus->cpus[cs->cpu_index].props.node_id); > > > } > > > + if (ms->smp.cpus > 1 && !vmc->no_cpu_topology) { > > > + qemu_fdt_setprop_cell(vms->fdt, nodename, "phandle", > > > + qemu_fdt_alloc_phandle(vms->fdt)); > > > + } > > > + > > > g_free(nodename); > > > } > > > + > > > + if (ms->smp.cpus > 1 && !vmc->no_cpu_topology) { > > > + /* > > > + * See Linux Documentation/devicetree/bindings/cpu/cpu-topology.txt > > > + */ > > > + qemu_fdt_add_subnode(vms->fdt, "/cpus/cpu-map"); > > > + > > > + for (cpu = ms->smp.cpus - 1; cpu >= 0; cpu--) { > > > + char *cpu_path = g_strdup_printf("/cpus/cpu@%d", cpu); > > > + char *map_path; > > > + > > > + if (ms->smp.threads > 1) { > > > + map_path = g_strdup_printf( > > > + "/cpus/cpu-map/%s%d/%s%d/%s%d", > > > + "cluster", cpu / (ms->smp.cores * ms->smp.threads), > > a cluster node may be replaced by socket to keep accord with the latest > kernel. Right, in which case this patch would be identical to the one in my branch. Thanks, drew > > > > + "core", (cpu / ms->smp.threads) % ms->smp.cores, > > > + "thread", cpu % ms->smp.threads); > > > + } else { > > > + map_path = g_strdup_printf( > > > + "/cpus/cpu-map/%s%d/%s%d", > > > + "cluster", cpu / ms->smp.cores, > > > + "core", cpu % ms->smp.cores); > > > + } > > > + qemu_fdt_add_path(vms->fdt, map_path); > > > + qemu_fdt_setprop_phandle(vms->fdt, map_path, "cpu", cpu_path); > > > + g_free(map_path); > > > + g_free(cpu_path); > > > + } > > > + } > > > } > > > static void fdt_add_its_gic_node(VirtMachineState *vms) > > > @@ -2742,6 +2777,7 @@ static void virt_machine_5_2_options(MachineClass *mc) > > > virt_machine_6_0_options(mc); > > > compat_props_add(mc->compat_props, hw_compat_5_2, hw_compat_5_2_len); > > > vmc->no_secure_gpio = true; > > > + vmc->no_cpu_topology = true; > > > } > > > DEFINE_VIRT_MACHINE(5, 2) > > > diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h > > > index ee9a93101e..7ef6d08ac3 100644 > > > --- a/include/hw/arm/virt.h > > > +++ b/include/hw/arm/virt.h > > > @@ -129,6 +129,7 @@ struct VirtMachineClass { > > > bool no_kvm_steal_time; > > > bool acpi_expose_flash; > > > bool no_secure_gpio; > > > + bool no_cpu_topology; > > > }; > > > struct VirtMachineState { > > > -- > > > 2.23.0 > > > > > > > > > > . > > >
diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 371147f3ae..c133b342b8 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -351,10 +351,11 @@ static void fdt_add_cpu_nodes(const VirtMachineState *vms) int cpu; int addr_cells = 1; const MachineState *ms = MACHINE(vms); + const VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms); int smp_cpus = ms->smp.cpus; /* - * From Documentation/devicetree/bindings/arm/cpus.txt + * See Linux Documentation/devicetree/bindings/arm/cpus.yaml * On ARM v8 64-bit systems value should be set to 2, * that corresponds to the MPIDR_EL1 register size. * If MPIDR_EL1[63:32] value is equal to 0 on all CPUs @@ -407,8 +408,42 @@ static void fdt_add_cpu_nodes(const VirtMachineState *vms) ms->possible_cpus->cpus[cs->cpu_index].props.node_id); } + if (ms->smp.cpus > 1 && !vmc->no_cpu_topology) { + qemu_fdt_setprop_cell(vms->fdt, nodename, "phandle", + qemu_fdt_alloc_phandle(vms->fdt)); + } + g_free(nodename); } + + if (ms->smp.cpus > 1 && !vmc->no_cpu_topology) { + /* + * See Linux Documentation/devicetree/bindings/cpu/cpu-topology.txt + */ + qemu_fdt_add_subnode(vms->fdt, "/cpus/cpu-map"); + + for (cpu = ms->smp.cpus - 1; cpu >= 0; cpu--) { + char *cpu_path = g_strdup_printf("/cpus/cpu@%d", cpu); + char *map_path; + + if (ms->smp.threads > 1) { + map_path = g_strdup_printf( + "/cpus/cpu-map/%s%d/%s%d/%s%d", + "cluster", cpu / (ms->smp.cores * ms->smp.threads), + "core", (cpu / ms->smp.threads) % ms->smp.cores, + "thread", cpu % ms->smp.threads); + } else { + map_path = g_strdup_printf( + "/cpus/cpu-map/%s%d/%s%d", + "cluster", cpu / ms->smp.cores, + "core", cpu % ms->smp.cores); + } + qemu_fdt_add_path(vms->fdt, map_path); + qemu_fdt_setprop_phandle(vms->fdt, map_path, "cpu", cpu_path); + g_free(map_path); + g_free(cpu_path); + } + } } static void fdt_add_its_gic_node(VirtMachineState *vms) @@ -2742,6 +2777,7 @@ static void virt_machine_5_2_options(MachineClass *mc) virt_machine_6_0_options(mc); compat_props_add(mc->compat_props, hw_compat_5_2, hw_compat_5_2_len); vmc->no_secure_gpio = true; + vmc->no_cpu_topology = true; } DEFINE_VIRT_MACHINE(5, 2) diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h index ee9a93101e..7ef6d08ac3 100644 --- a/include/hw/arm/virt.h +++ b/include/hw/arm/virt.h @@ -129,6 +129,7 @@ struct VirtMachineClass { bool no_kvm_steal_time; bool acpi_expose_flash; bool no_secure_gpio; + bool no_cpu_topology; }; struct VirtMachineState {
Support device tree CPU topology descriptions. Signed-off-by: Ying Fang <fangying1@huawei.com> --- hw/arm/virt.c | 38 +++++++++++++++++++++++++++++++++++++- include/hw/arm/virt.h | 1 + 2 files changed, 38 insertions(+), 1 deletion(-)