| Message ID | 20210413080745.33004-3-wangyanan55@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/arm/virt: Introduce cpu topology support | expand |
Hi Yanan, Drew, On 4/13/21 10:07 AM, Yanan Wang wrote: > From: Andrew Jones <drjones@redhat.com> > > Support device tree CPU topology descriptions. > > Signed-off-by: Andrew Jones <drjones@redhat.com> > Signed-off-by: Yanan Wang <wangyanan55@huawei.com> > --- > hw/arm/virt.c | 41 ++++++++++++++++++++++++++++++++++++++++- > include/hw/arm/virt.h | 1 + > 2 files changed, 41 insertions(+), 1 deletion(-) > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index 9f01d9041b..f4ae60ded9 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -352,10 +352,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 > @@ -408,8 +409,45 @@ 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(ms->fdt, nodename, "phandle", > + qemu_fdt_alloc_phandle(ms->fdt)); > + } > + > g_free(nodename); > } > + > + if (ms->smp.cpus > 1 && !vmc->no_cpu_topology) { > + /* > + * See Linux Documentation/devicetree/bindings/cpu/cpu-topology.txt > + * In a SMP system, the hierarchy of CPUs is defined through four > + * entities that are used to describe the layout of physical CPUs > + * in the system: socket/cluster/core/thread. > + */ > + qemu_fdt_add_subnode(ms->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", > + "socket", 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", > + "socket", cpu / ms->smp.cores, > + "core", cpu % ms->smp.cores); > + } > + qemu_fdt_add_path(ms->fdt, map_path); > + qemu_fdt_setprop_phandle(ms->fdt, map_path, "cpu", cpu_path); > + g_free(map_path); > + g_free(cpu_path); > + } > + } > } > > static void fdt_add_its_gic_node(VirtMachineState *vms) > @@ -2769,6 +2807,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; Bare with me because "machine versioning" is something new to me, I was expecting it to be only related to migrated fields. Why do we need to care about not adding the FDT node in older machines? Shouldn't the guest skip unknown FDT nodes? Thanks, Phil. > } > DEFINE_VIRT_MACHINE(5, 2) > > diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h > index 921416f918..4a4b98e4a7 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 { >
On Tue, Apr 27, 2021 at 11:47:17AM +0200, Philippe Mathieu-Daudé wrote: > Hi Yanan, Drew, > > On 4/13/21 10:07 AM, Yanan Wang wrote: > > From: Andrew Jones <drjones@redhat.com> > > > > Support device tree CPU topology descriptions. > > > > Signed-off-by: Andrew Jones <drjones@redhat.com> > > Signed-off-by: Yanan Wang <wangyanan55@huawei.com> > > --- > > hw/arm/virt.c | 41 ++++++++++++++++++++++++++++++++++++++++- > > include/hw/arm/virt.h | 1 + > > 2 files changed, 41 insertions(+), 1 deletion(-) > > > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > > index 9f01d9041b..f4ae60ded9 100644 > > --- a/hw/arm/virt.c > > +++ b/hw/arm/virt.c > > @@ -352,10 +352,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 > > @@ -408,8 +409,45 @@ 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(ms->fdt, nodename, "phandle", > > + qemu_fdt_alloc_phandle(ms->fdt)); > > + } > > + > > g_free(nodename); > > } > > + > > + if (ms->smp.cpus > 1 && !vmc->no_cpu_topology) { > > + /* > > + * See Linux Documentation/devicetree/bindings/cpu/cpu-topology.txt > > + * In a SMP system, the hierarchy of CPUs is defined through four > > + * entities that are used to describe the layout of physical CPUs > > + * in the system: socket/cluster/core/thread. > > + */ > > + qemu_fdt_add_subnode(ms->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", > > + "socket", 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", > > + "socket", cpu / ms->smp.cores, > > + "core", cpu % ms->smp.cores); > > + } > > + qemu_fdt_add_path(ms->fdt, map_path); > > + qemu_fdt_setprop_phandle(ms->fdt, map_path, "cpu", cpu_path); > > + g_free(map_path); > > + g_free(cpu_path); > > + } > > + } > > } > > > > static void fdt_add_its_gic_node(VirtMachineState *vms) > > @@ -2769,6 +2807,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; > > Bare with me because "machine versioning" is something new to me, I was > expecting it to be only related to migrated fields. > Why do we need to care about not adding the FDT node in older machines? > Shouldn't the guest skip unknown FDT nodes? It probably should, the question is whether it would. Also, the nodes may not be unknown, so the guest will read the information and set up its topology as instructed. That topology may not be the same as what was getting used by default without the topology description. It's possible that a user's application has a dependency on the topology and if that topology gets changed under its feat it'll behave differently. In short, machine versioning isn't just about vmstate, it's also about keeping a machine type looking the same to the guest. Now, it's possible that we're being overly cautious here, but this compat variable doesn't complicate code too much. So I think I'd prefer to use it than not. Thanks, drew
On 4/27/21 12:04 PM, Andrew Jones wrote: > On Tue, Apr 27, 2021 at 11:47:17AM +0200, Philippe Mathieu-Daudé wrote: >> Hi Yanan, Drew, >> >> On 4/13/21 10:07 AM, Yanan Wang wrote: >>> From: Andrew Jones <drjones@redhat.com> >>> >>> Support device tree CPU topology descriptions. >>> >>> Signed-off-by: Andrew Jones <drjones@redhat.com> >>> Signed-off-by: Yanan Wang <wangyanan55@huawei.com> >>> --- >>> hw/arm/virt.c | 41 ++++++++++++++++++++++++++++++++++++++++- >>> include/hw/arm/virt.h | 1 + >>> 2 files changed, 41 insertions(+), 1 deletion(-) >>> >>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >>> index 9f01d9041b..f4ae60ded9 100644 >>> --- a/hw/arm/virt.c >>> +++ b/hw/arm/virt.c >>> @@ -352,10 +352,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 >>> @@ -408,8 +409,45 @@ 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(ms->fdt, nodename, "phandle", >>> + qemu_fdt_alloc_phandle(ms->fdt)); >>> + } >>> + >>> g_free(nodename); >>> } >>> + >>> + if (ms->smp.cpus > 1 && !vmc->no_cpu_topology) { >>> + /* >>> + * See Linux Documentation/devicetree/bindings/cpu/cpu-topology.txt >>> + * In a SMP system, the hierarchy of CPUs is defined through four >>> + * entities that are used to describe the layout of physical CPUs >>> + * in the system: socket/cluster/core/thread. >>> + */ >>> + qemu_fdt_add_subnode(ms->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", >>> + "socket", 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", >>> + "socket", cpu / ms->smp.cores, >>> + "core", cpu % ms->smp.cores); >>> + } >>> + qemu_fdt_add_path(ms->fdt, map_path); >>> + qemu_fdt_setprop_phandle(ms->fdt, map_path, "cpu", cpu_path); >>> + g_free(map_path); >>> + g_free(cpu_path); >>> + } >>> + } >>> } >>> >>> static void fdt_add_its_gic_node(VirtMachineState *vms) >>> @@ -2769,6 +2807,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; >> >> Bare with me because "machine versioning" is something new to me, I was >> expecting it to be only related to migrated fields. >> Why do we need to care about not adding the FDT node in older machines? >> Shouldn't the guest skip unknown FDT nodes? > > It probably should, the question is whether it would. Also, the nodes may > not be unknown, so the guest will read the information and set up its > topology as instructed. That topology may not be the same as what was > getting used by default without the topology description. It's possible > that a user's application has a dependency on the topology and if that > topology gets changed under its feat it'll behave differently. [*] I see. > In short, machine versioning isn't just about vmstate, it's also about > keeping a machine type looking the same to the guest. Yes, TIL. > Now, it's possible that we're being overly cautious here, but this compat > variable doesn't complicate code too much. So I think I'd prefer to use it > than not. No problem. Could you or Yanan add your first paragraph ([*], reworded in the commit description? I don't think a comment in the code is useful, but having it in the commit is helpful IMO. Thanks, Phil.
On 2021/4/27 20:36, Philippe Mathieu-Daudé wrote: > On 4/27/21 12:04 PM, Andrew Jones wrote: >> On Tue, Apr 27, 2021 at 11:47:17AM +0200, Philippe Mathieu-Daudé wrote: >>> Hi Yanan, Drew, >>> >>> On 4/13/21 10:07 AM, Yanan Wang wrote: >>>> From: Andrew Jones <drjones@redhat.com> >>>> >>>> Support device tree CPU topology descriptions. >>>> >>>> Signed-off-by: Andrew Jones <drjones@redhat.com> >>>> Signed-off-by: Yanan Wang <wangyanan55@huawei.com> >>>> --- >>>> hw/arm/virt.c | 41 ++++++++++++++++++++++++++++++++++++++++- >>>> include/hw/arm/virt.h | 1 + >>>> 2 files changed, 41 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >>>> index 9f01d9041b..f4ae60ded9 100644 >>>> --- a/hw/arm/virt.c >>>> +++ b/hw/arm/virt.c >>>> @@ -352,10 +352,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 >>>> @@ -408,8 +409,45 @@ 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(ms->fdt, nodename, "phandle", >>>> + qemu_fdt_alloc_phandle(ms->fdt)); >>>> + } >>>> + >>>> g_free(nodename); >>>> } >>>> + >>>> + if (ms->smp.cpus > 1 && !vmc->no_cpu_topology) { >>>> + /* >>>> + * See Linux Documentation/devicetree/bindings/cpu/cpu-topology.txt >>>> + * In a SMP system, the hierarchy of CPUs is defined through four >>>> + * entities that are used to describe the layout of physical CPUs >>>> + * in the system: socket/cluster/core/thread. >>>> + */ >>>> + qemu_fdt_add_subnode(ms->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", >>>> + "socket", 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", >>>> + "socket", cpu / ms->smp.cores, >>>> + "core", cpu % ms->smp.cores); >>>> + } >>>> + qemu_fdt_add_path(ms->fdt, map_path); >>>> + qemu_fdt_setprop_phandle(ms->fdt, map_path, "cpu", cpu_path); >>>> + g_free(map_path); >>>> + g_free(cpu_path); >>>> + } >>>> + } >>>> } >>>> >>>> static void fdt_add_its_gic_node(VirtMachineState *vms) >>>> @@ -2769,6 +2807,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; >>> Bare with me because "machine versioning" is something new to me, I was >>> expecting it to be only related to migrated fields. >>> Why do we need to care about not adding the FDT node in older machines? >>> Shouldn't the guest skip unknown FDT nodes? >> It probably should, the question is whether it would. Also, the nodes may >> not be unknown, so the guest will read the information and set up its >> topology as instructed. That topology may not be the same as what was >> getting used by default without the topology description. It's possible >> that a user's application has a dependency on the topology and if that >> topology gets changed under its feat it'll behave differently. > [*] > > I see. > >> In short, machine versioning isn't just about vmstate, it's also about >> keeping a machine type looking the same to the guest. > Yes, TIL. > >> Now, it's possible that we're being overly cautious here, but this compat >> variable doesn't complicate code too much. So I think I'd prefer to use it >> than not. > No problem. Could you or Yanan add your first paragraph ([*], reworded > in the commit description? I don't think a comment in the code is > useful, but having it in the commit is helpful IMO. Hi Philippe, Of course. I think I can do this for the commit description. Thanks, Yanan > Thanks, > > Phil. > > .
On Tue, Apr 13, 2021 at 04:07:41PM +0800, Yanan Wang wrote: > From: Andrew Jones <drjones@redhat.com> > > Support device tree CPU topology descriptions. > > Signed-off-by: Andrew Jones <drjones@redhat.com> > Signed-off-by: Yanan Wang <wangyanan55@huawei.com> > --- > hw/arm/virt.c | 41 ++++++++++++++++++++++++++++++++++++++++- > include/hw/arm/virt.h | 1 + > 2 files changed, 41 insertions(+), 1 deletion(-) > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index 9f01d9041b..f4ae60ded9 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -352,10 +352,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 > @@ -408,8 +409,45 @@ 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) { We should probably always generate the cpu-map, like we agreed to always generate the PPTT. If, for some reason, we don't want to generate the cpu-map for uniprocessor systems, then we should actually be checking ms->smp.maxcpus here (and below) to be sure it's uniprocessor. Thanks, drew > + qemu_fdt_setprop_cell(ms->fdt, nodename, "phandle", > + qemu_fdt_alloc_phandle(ms->fdt)); > + } > + > g_free(nodename); > } > + > + if (ms->smp.cpus > 1 && !vmc->no_cpu_topology) { > + /* > + * See Linux Documentation/devicetree/bindings/cpu/cpu-topology.txt > + * In a SMP system, the hierarchy of CPUs is defined through four > + * entities that are used to describe the layout of physical CPUs > + * in the system: socket/cluster/core/thread. > + */ > + qemu_fdt_add_subnode(ms->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", > + "socket", 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", > + "socket", cpu / ms->smp.cores, > + "core", cpu % ms->smp.cores); > + } > + qemu_fdt_add_path(ms->fdt, map_path); > + qemu_fdt_setprop_phandle(ms->fdt, map_path, "cpu", cpu_path); > + g_free(map_path); > + g_free(cpu_path); > + } > + } > } > > static void fdt_add_its_gic_node(VirtMachineState *vms) > @@ -2769,6 +2807,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 921416f918..4a4b98e4a7 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.19.1 >
On 2021/5/13 14:58, Andrew Jones wrote: > On Tue, Apr 13, 2021 at 04:07:41PM +0800, Yanan Wang wrote: >> From: Andrew Jones <drjones@redhat.com> >> >> Support device tree CPU topology descriptions. >> >> Signed-off-by: Andrew Jones <drjones@redhat.com> >> Signed-off-by: Yanan Wang <wangyanan55@huawei.com> >> --- >> hw/arm/virt.c | 41 ++++++++++++++++++++++++++++++++++++++++- >> include/hw/arm/virt.h | 1 + >> 2 files changed, 41 insertions(+), 1 deletion(-) >> >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >> index 9f01d9041b..f4ae60ded9 100644 >> --- a/hw/arm/virt.c >> +++ b/hw/arm/virt.c >> @@ -352,10 +352,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 >> @@ -408,8 +409,45 @@ 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) { > We should probably always generate the cpu-map, like we agreed to always > generate the PPTT. Ok, I will remove smp.cpus check for cpu-map too. Single cpu node, corresponding to single cpu-map path also works. > If, for some reason, we don't want to generate the > cpu-map for uniprocessor systems, then we should actually be checking > ms->smp.maxcpus here (and below) to be sure it's uniprocessor. Right, it's max cpus that ought to be checked but not smp cpus. Thanks, Yanan > Thanks, > drew > >> + qemu_fdt_setprop_cell(ms->fdt, nodename, "phandle", >> + qemu_fdt_alloc_phandle(ms->fdt)); >> + } >> + >> g_free(nodename); >> } >> + >> + if (ms->smp.cpus > 1 && !vmc->no_cpu_topology) { >> + /* >> + * See Linux Documentation/devicetree/bindings/cpu/cpu-topology.txt >> + * In a SMP system, the hierarchy of CPUs is defined through four >> + * entities that are used to describe the layout of physical CPUs >> + * in the system: socket/cluster/core/thread. >> + */ >> + qemu_fdt_add_subnode(ms->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", >> + "socket", 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", >> + "socket", cpu / ms->smp.cores, >> + "core", cpu % ms->smp.cores); >> + } >> + qemu_fdt_add_path(ms->fdt, map_path); >> + qemu_fdt_setprop_phandle(ms->fdt, map_path, "cpu", cpu_path); >> + g_free(map_path); >> + g_free(cpu_path); >> + } >> + } >> } >> >> static void fdt_add_its_gic_node(VirtMachineState *vms) >> @@ -2769,6 +2807,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 921416f918..4a4b98e4a7 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.19.1 >> > .
diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 9f01d9041b..f4ae60ded9 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -352,10 +352,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 @@ -408,8 +409,45 @@ 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(ms->fdt, nodename, "phandle", + qemu_fdt_alloc_phandle(ms->fdt)); + } + g_free(nodename); } + + if (ms->smp.cpus > 1 && !vmc->no_cpu_topology) { + /* + * See Linux Documentation/devicetree/bindings/cpu/cpu-topology.txt + * In a SMP system, the hierarchy of CPUs is defined through four + * entities that are used to describe the layout of physical CPUs + * in the system: socket/cluster/core/thread. + */ + qemu_fdt_add_subnode(ms->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", + "socket", 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", + "socket", cpu / ms->smp.cores, + "core", cpu % ms->smp.cores); + } + qemu_fdt_add_path(ms->fdt, map_path); + qemu_fdt_setprop_phandle(ms->fdt, map_path, "cpu", cpu_path); + g_free(map_path); + g_free(cpu_path); + } + } } static void fdt_add_its_gic_node(VirtMachineState *vms) @@ -2769,6 +2807,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 921416f918..4a4b98e4a7 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 {