* [PATCH for-6.2 v5 0/5] hw/arm/virt: Introduce cpu topology support
@ 2021-08-05 12:39 Yanan Wang
2021-08-05 12:39 ` [PATCH v5 1/5] hw/arm/virt: Only describe cpu topology to guest since virt 6.2 Yanan Wang
` (5 more replies)
0 siblings, 6 replies; 13+ messages in thread
From: Yanan Wang @ 2021-08-05 12:39 UTC (permalink / raw)
To: Peter Maydell, Andrew Jones, Michael S . Tsirkin, Igor Mammedov
Cc: Salil Mehta, qemu-devel, Yanan Wang, Shannon Zhao, qemu-arm,
Alistair Francis, wanghaibin.wang, David Gibson
Hi,
This is a new version (v5) of the series [1] that I posted to introduce
support for generating cpu topology descriptions to virt machine guest.
Once the view of an accurate virtual cpu topology is provided to guest,
with a well-designed vCPU pinning to the pCPU we may get a huge benefit,
e.g., the scheduling performance improvement. See Dario Faggioli's
research and the related performance tests in [2] for reference.
This patch series introduces cpu topology support for ARM platform.
Both cpu-map in DT and ACPI PPTT table are introduced to store the
topology information. And we only describe the topology information
to 6.2 and newer virt machines, considering compatibility.
[1] https://lore.kernel.org/qemu-devel/20210622093413.13360-1-wangyanan55@huawei.com/
[2] https://kvmforum2020.sched.com/event/eE1y/virtual-topology-for-virtual-machines
-friend-or-foe-dario-faggioli-suse
Some tests:
1) -smp 16,sockets=2,cores=4,threads=2,maxcpus=16
lscpu:
Architecture: aarch64
Byte Order: Little Endian
CPU(s): 16
On-line CPU(s) list: 0-15
Thread(s) per core: 2
Core(s) per socket: 4
Socket(s): 2
NUMA node(s): 1
Vendor ID: ARM
Model: 2
Model name: Cortex-A72
Stepping: r0p2
BogoMIPS: 100.00
NUMA node0 CPU(s): 0-15
cat /sys/devices/system/cpu/present --> 0-15
cat /sys/devices/system/cpu/possible --> 0-15
2) -smp 8,sockets=2,cores=4,threads=2,maxcpus=16
lscpu:
Architecture: aarch64
Byte Order: Little Endian
CPU(s): 8
On-line CPU(s) list: 0-7
Thread(s) per core: 2
Core(s) per socket: 4
Socket(s): 1
NUMA node(s): 1
Vendor ID: ARM
Model: 2
Model name: Cortex-A72
Stepping: r0p2
BogoMIPS: 100.00
NUMA node0 CPU(s): 0-7
cat /sys/devices/system/cpu/present --> 0-7
cat /sys/devices/system/cpu/possible --> 0-7
---
Changelogs:
v4->v5:
- drop the added -smp "expose=on|off" parameter and only describe topology
for 6.2 and newer machines
- rebased the code on patch series [3] which has introduced some fix and
improvement for smp parsing
- [3]: https://lore.kernel.org/qemu-devel/20210803080527.156556-1-wangyanan55@huawei.com/
v3->v4:
- add new -smp parameter "expose=on|off" for users to enable/disable the feature
- add stricter -smp cmdline parsing rules on "expose=on" case
- move build_pptt to generic aml-build.c
- add default cluster node in the cpu-map
- rebase on top of latest upstream master
- v3: https://lore.kernel.org/qemu-devel/20210516102900.28036-1-wangyanan55@huawei.com/
v2->v3:
- address comments from David, Philippe, and Andrew. Thanks!
- split some change into separate commits for ease of review
- adjust parsing rules of virt_smp_parse to be more strict
(after discussion with Andrew)
- adjust author credit for the patches
v1->v2:
- Address Andrew Jones's comments
- Address Michael S. Tsirkin's comments
---
Andrew Jones (2):
hw/arm/virt: Add cpu-map to device tree
hw/acpi/aml-build: Generate PPTT table
Yanan Wang (3):
hw/arm/virt: Only describe cpu topology to guest since virt 6.2
device_tree: Add qemu_fdt_add_path
hw/acpi/aml-build: Add Processor hierarchy node structure
hw/acpi/aml-build.c | 76 ++++++++++++++++++++++++++++++++++++
hw/arm/virt-acpi-build.c | 8 +++-
hw/arm/virt.c | 62 ++++++++++++++++++++++++-----
include/hw/acpi/aml-build.h | 7 ++++
include/hw/arm/virt.h | 4 +-
include/sysemu/device_tree.h | 1 +
softmmu/device_tree.c | 44 ++++++++++++++++++++-
7 files changed, 188 insertions(+), 14 deletions(-)
--
2.19.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v5 1/5] hw/arm/virt: Only describe cpu topology to guest since virt 6.2
2021-08-05 12:39 [PATCH for-6.2 v5 0/5] hw/arm/virt: Introduce cpu topology support Yanan Wang
@ 2021-08-05 12:39 ` Yanan Wang
2021-08-05 12:39 ` [PATCH for-6.2 v5 2/5] device_tree: Add qemu_fdt_add_path Yanan Wang
` (4 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Yanan Wang @ 2021-08-05 12:39 UTC (permalink / raw)
To: Peter Maydell, Andrew Jones, Michael S . Tsirkin, Igor Mammedov
Cc: Salil Mehta, qemu-devel, Yanan Wang, Shannon Zhao, qemu-arm,
Alistair Francis, wanghaibin.wang, David Gibson
On existing older machine types, without cpu topology described
in ACPI or DT, the guest will populate one by default. With the
topology described, it will read the information and set up its
topology as instructed, but that may not be the same as what was
getting used by default. It's possible that a user application
has a dependency on the default topology and if the default one
gets changed it will probably behave differently. So we should
only describe cpu topology information to the guest on 6.2 and
later machine types.
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
---
hw/arm/virt.c | 3 +++
include/hw/arm/virt.h | 4 +++-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index ae029680da..82f2eba6bd 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2795,9 +2795,12 @@ DEFINE_VIRT_MACHINE_AS_LATEST(6, 2)
static void virt_machine_6_1_options(MachineClass *mc)
{
+ VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
+
virt_machine_6_2_options(mc);
compat_props_add(mc->compat_props, hw_compat_6_1, hw_compat_6_1_len);
mc->smp_props.prefer_sockets = true;
+ vmc->no_cpu_topology = true;
}
DEFINE_VIRT_MACHINE(6, 1)
diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
index 9661c46699..6ab21ea90e 100644
--- a/include/hw/arm/virt.h
+++ b/include/hw/arm/virt.h
@@ -124,11 +124,13 @@ struct VirtMachineClass {
bool claim_edge_triggered_timers;
bool smbios_old_sys_ver;
bool no_highmem_ecam;
- bool no_ged; /* Machines < 4.2 has no support for ACPI GED device */
+ bool no_ged; /* Machines < 4.2 have no support for ACPI GED device */
bool kvm_no_adjvtime;
bool no_kvm_steal_time;
bool acpi_expose_flash;
bool no_secure_gpio;
+ /* Machines < 6.2 have no support for describing cpu topology to guest */
+ bool no_cpu_topology;
};
struct VirtMachineState {
--
2.19.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH for-6.2 v5 2/5] device_tree: Add qemu_fdt_add_path
2021-08-05 12:39 [PATCH for-6.2 v5 0/5] hw/arm/virt: Introduce cpu topology support Yanan Wang
2021-08-05 12:39 ` [PATCH v5 1/5] hw/arm/virt: Only describe cpu topology to guest since virt 6.2 Yanan Wang
@ 2021-08-05 12:39 ` Yanan Wang
2021-08-05 12:39 ` [PATCH for-6.2 v5 3/5] hw/arm/virt: Add cpu-map to device tree Yanan Wang
` (3 subsequent siblings)
5 siblings, 0 replies; 13+ messages in thread
From: Yanan Wang @ 2021-08-05 12:39 UTC (permalink / raw)
To: Peter Maydell, Andrew Jones, Michael S . Tsirkin, Igor Mammedov
Cc: Salil Mehta, qemu-devel, Yanan Wang, Shannon Zhao, qemu-arm,
Alistair Francis, wanghaibin.wang, David Gibson
qemu_fdt_add_path() works like qemu_fdt_add_subnode(), except it
also adds all missing subnodes from the given path. We'll use it
in a coming patch where we will add cpu-map to the device tree.
And we also tweak an error message of qemu_fdt_add_subnode().
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
Cc: David Gibson <david@gibson.dropbear.id.au>
Cc: Alistair Francis <alistair.francis@wdc.com>
Co-developed-by: Andrew Jones <drjones@redhat.com>
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
---
include/sysemu/device_tree.h | 1 +
softmmu/device_tree.c | 44 ++++++++++++++++++++++++++++++++++--
2 files changed, 43 insertions(+), 2 deletions(-)
diff --git a/include/sysemu/device_tree.h b/include/sysemu/device_tree.h
index 8a2fe55622..ef060a9759 100644
--- a/include/sysemu/device_tree.h
+++ b/include/sysemu/device_tree.h
@@ -121,6 +121,7 @@ uint32_t qemu_fdt_get_phandle(void *fdt, const char *path);
uint32_t qemu_fdt_alloc_phandle(void *fdt);
int qemu_fdt_nop_node(void *fdt, const char *node_path);
int qemu_fdt_add_subnode(void *fdt, const char *name);
+int qemu_fdt_add_path(void *fdt, const char *path);
#define qemu_fdt_setprop_cells(fdt, node_path, property, ...) \
do { \
diff --git a/softmmu/device_tree.c b/softmmu/device_tree.c
index b621f63fba..3965c834ca 100644
--- a/softmmu/device_tree.c
+++ b/softmmu/device_tree.c
@@ -540,8 +540,8 @@ int qemu_fdt_add_subnode(void *fdt, const char *name)
retval = fdt_add_subnode(fdt, parent, basename);
if (retval < 0) {
- error_report("FDT: Failed to create subnode %s: %s", name,
- fdt_strerror(retval));
+ error_report("%s: Failed to create subnode %s: %s",
+ __func__, name, fdt_strerror(retval));
exit(1);
}
@@ -549,6 +549,46 @@ int qemu_fdt_add_subnode(void *fdt, const char *name)
return retval;
}
+/*
+ * qemu_fdt_add_path: Like qemu_fdt_add_subnode(), but will add
+ * all missing subnodes from the given path.
+ */
+int qemu_fdt_add_path(void *fdt, const char *path)
+{
+ const char *name;
+ const char *p = path;
+ int namelen, retval;
+ int parent = 0;
+
+ if (path[0] != '/') {
+ return -1;
+ }
+
+ while (p) {
+ name = p + 1;
+ p = strchr(name, '/');
+ namelen = p != NULL ? p - name : strlen(name);
+
+ retval = fdt_subnode_offset_namelen(fdt, parent, name, namelen);
+ if (retval < 0 && retval != -FDT_ERR_NOTFOUND) {
+ error_report("%s: Unexpected error in finding subnode %.*s: %s",
+ __func__, namelen, name, fdt_strerror(retval));
+ exit(1);
+ } else if (retval == -FDT_ERR_NOTFOUND) {
+ retval = fdt_add_subnode_namelen(fdt, parent, name, namelen);
+ if (retval < 0) {
+ error_report("%s: Failed to create subnode %.*s: %s",
+ __func__, namelen, name, fdt_strerror(retval));
+ exit(1);
+ }
+ }
+
+ parent = retval;
+ }
+
+ return retval;
+}
+
void qemu_fdt_dumpdtb(void *fdt, int size)
{
const char *dumpdtb = current_machine->dumpdtb;
--
2.19.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH for-6.2 v5 3/5] hw/arm/virt: Add cpu-map to device tree
2021-08-05 12:39 [PATCH for-6.2 v5 0/5] hw/arm/virt: Introduce cpu topology support Yanan Wang
2021-08-05 12:39 ` [PATCH v5 1/5] hw/arm/virt: Only describe cpu topology to guest since virt 6.2 Yanan Wang
2021-08-05 12:39 ` [PATCH for-6.2 v5 2/5] device_tree: Add qemu_fdt_add_path Yanan Wang
@ 2021-08-05 12:39 ` Yanan Wang
2021-08-17 2:10 ` wangyanan (Y)
2021-08-05 12:39 ` [PATCH for-6.2 v5 4/5] hw/acpi/aml-build: Add Processor hierarchy node structure Yanan Wang
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Yanan Wang @ 2021-08-05 12:39 UTC (permalink / raw)
To: Peter Maydell, Andrew Jones, Michael S . Tsirkin, Igor Mammedov
Cc: Salil Mehta, qemu-devel, Yanan Wang, Shannon Zhao, qemu-arm,
Alistair Francis, wanghaibin.wang, David Gibson
From: Andrew Jones <drjones@redhat.com>
Support device tree CPU topology descriptions.
In accordance with the Devicetree Specification, the Linux Doc
"arm/cpus.yaml" requires that cpus and cpu nodes in the DT are
present. And we have already met the requirement by generating
/cpus/cpu@* nodes for members within ms->smp.cpus. Accordingly,
we should also create subnodes in cpu-map for the present cpus,
each of which relates to an unique cpu node.
The Linux Doc "cpu/cpu-topology.txt" states that the hierarchy
of CPUs in a SMP system is defined through four entities and
they are socket/cluster/core/thread. It is also required that
a socket node's child nodes must be one or more cluster nodes.
Given that currently we are only provided with information of
socket/core/thread, we assume there is one cluster child node
in each socket node when creating cpu-map.
Signed-off-by: Andrew Jones <drjones@redhat.com>
Co-developed-by: Yanan Wang <wangyanan55@huawei.com>
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
---
hw/arm/virt.c | 59 ++++++++++++++++++++++++++++++++++++++++++---------
1 file changed, 49 insertions(+), 10 deletions(-)
diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 82f2eba6bd..d1e294be95 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -350,20 +350,21 @@ 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
- * 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
- * in the system, #address-cells can be set to 1, since
- * MPIDR_EL1[63:32] bits are not used for CPUs
- * identification.
+ * 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
+ * in the system, #address-cells can be set to 1, since
+ * MPIDR_EL1[63:32] bits are not used for CPUs
+ * identification.
*
- * Here we actually don't know whether our system is 32- or 64-bit one.
- * The simplest way to go is to examine affinity IDs of all our CPUs. If
- * at least one of them has Aff3 populated, we set #address-cells to 2.
+ * Here we actually don't know whether our system is 32- or 64-bit one.
+ * The simplest way to go is to examine affinity IDs of all our CPUs. If
+ * at least one of them has Aff3 populated, we set #address-cells to 2.
*/
for (cpu = 0; cpu < smp_cpus; cpu++) {
ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(cpu));
@@ -406,8 +407,46 @@ static void fdt_add_cpu_nodes(const VirtMachineState *vms)
ms->possible_cpus->cpus[cs->cpu_index].props.node_id);
}
+ if (!vmc->no_cpu_topology) {
+ qemu_fdt_setprop_cell(ms->fdt, nodename, "phandle",
+ qemu_fdt_alloc_phandle(ms->fdt));
+ }
+
g_free(nodename);
}
+
+ if (!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 = 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/socket%d/cluster0/core%d/thread%d",
+ cpu / (ms->smp.cores * ms->smp.threads),
+ (cpu / ms->smp.threads) % ms->smp.cores,
+ cpu % ms->smp.threads);
+ } else {
+ map_path = g_strdup_printf(
+ "/cpus/cpu-map/socket%d/cluster0/core%d",
+ cpu / ms->smp.cores,
+ 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)
--
2.19.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH for-6.2 v5 4/5] hw/acpi/aml-build: Add Processor hierarchy node structure
2021-08-05 12:39 [PATCH for-6.2 v5 0/5] hw/arm/virt: Introduce cpu topology support Yanan Wang
` (2 preceding siblings ...)
2021-08-05 12:39 ` [PATCH for-6.2 v5 3/5] hw/arm/virt: Add cpu-map to device tree Yanan Wang
@ 2021-08-05 12:39 ` Yanan Wang
2021-08-05 12:39 ` [PATCH for-6.2 v5 5/5] hw/acpi/aml-build: Generate PPTT table Yanan Wang
2021-08-23 23:53 ` [PATCH for-6.2 v5 0/5] hw/arm/virt: Introduce cpu topology support Michael S. Tsirkin
5 siblings, 0 replies; 13+ messages in thread
From: Yanan Wang @ 2021-08-05 12:39 UTC (permalink / raw)
To: Peter Maydell, Andrew Jones, Michael S . Tsirkin, Igor Mammedov
Cc: Salil Mehta, qemu-devel, Yanan Wang, Shannon Zhao, qemu-arm,
Alistair Francis, wanghaibin.wang, David Gibson
Add a generic API to build Processor hierarchy node structure (Type 0),
which is strictly consistent with descriptions in ACPI 6.2: 5.2.29.1.
This function will be used to build ACPI PPTT table for cpu topology.
Reviewed-by: Andrew Jones <drjones@redhat.com>
Co-developed-by: Ying Fang <fangying1@huawei.com>
Co-developed-by: Henglong Fan <fanhenglong@huawei.com>
Co-developed-by: Yanan Wang <wangyanan55@huawei.com>
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
---
hw/acpi/aml-build.c | 26 ++++++++++++++++++++++++++
include/hw/acpi/aml-build.h | 4 ++++
2 files changed, 30 insertions(+)
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index d5103e6d7b..9fa5024414 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1920,6 +1920,32 @@ void build_slit(GArray *table_data, BIOSLinker *linker, MachineState *ms,
table_data->len - slit_start, 1, oem_id, oem_table_id);
}
+/* ACPI 6.2: 5.2.29.1 Processor hierarchy node structure (Type 0) */
+void build_processor_hierarchy_node(GArray *tbl, uint32_t flags,
+ uint32_t parent, uint32_t id,
+ uint32_t *priv_rsrc, uint32_t priv_num)
+{
+ int i;
+
+ build_append_byte(tbl, 0); /* Type 0 - processor */
+ build_append_byte(tbl, 20 + priv_num * 4); /* Length */
+ build_append_int_noprefix(tbl, 0, 2); /* Reserved */
+ build_append_int_noprefix(tbl, flags, 4); /* Flags */
+ build_append_int_noprefix(tbl, parent, 4); /* Parent */
+ build_append_int_noprefix(tbl, id, 4); /* ACPI Processor ID */
+
+ /* Number of private resources */
+ build_append_int_noprefix(tbl, priv_num, 4);
+
+ /* Private resources[N] */
+ if (priv_num > 0) {
+ assert(priv_rsrc);
+ for (i = 0; i < priv_num; i++) {
+ build_append_int_noprefix(tbl, priv_rsrc[i], 4);
+ }
+ }
+}
+
/* build rev1/rev3/rev5.1 FADT */
void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
const char *oem_id, const char *oem_table_id)
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index 471266d739..ea74b8f6ed 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -462,6 +462,10 @@ void build_srat_memory(AcpiSratMemoryAffinity *numamem, uint64_t base,
void build_slit(GArray *table_data, BIOSLinker *linker, MachineState *ms,
const char *oem_id, const char *oem_table_id);
+void build_processor_hierarchy_node(GArray *tbl, uint32_t flags,
+ uint32_t parent, uint32_t id,
+ uint32_t *priv_rsrc, uint32_t priv_num);
+
void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
const char *oem_id, const char *oem_table_id);
--
2.19.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH for-6.2 v5 5/5] hw/acpi/aml-build: Generate PPTT table
2021-08-05 12:39 [PATCH for-6.2 v5 0/5] hw/arm/virt: Introduce cpu topology support Yanan Wang
` (3 preceding siblings ...)
2021-08-05 12:39 ` [PATCH for-6.2 v5 4/5] hw/acpi/aml-build: Add Processor hierarchy node structure Yanan Wang
@ 2021-08-05 12:39 ` Yanan Wang
2021-08-23 23:52 ` Michael S. Tsirkin
2021-08-23 23:53 ` [PATCH for-6.2 v5 0/5] hw/arm/virt: Introduce cpu topology support Michael S. Tsirkin
5 siblings, 1 reply; 13+ messages in thread
From: Yanan Wang @ 2021-08-05 12:39 UTC (permalink / raw)
To: Peter Maydell, Andrew Jones, Michael S . Tsirkin, Igor Mammedov
Cc: Salil Mehta, qemu-devel, Yanan Wang, Shannon Zhao, qemu-arm,
Alistair Francis, wanghaibin.wang, David Gibson
From: Andrew Jones <drjones@redhat.com>
Add the Processor Properties Topology Table (PPTT) to expose
CPU topology information defined by users to ACPI guests.
Note, a DT-boot Linux guest with a non-flat CPU topology will
see socket and core IDs being sequential integers starting
from zero, which is different from ACPI-boot Linux guest,
e.g. with -smp 4,sockets=2,cores=2,threads=1
a DT boot produces:
cpu: 0 package_id: 0 core_id: 0
cpu: 1 package_id: 0 core_id: 1
cpu: 2 package_id: 1 core_id: 0
cpu: 3 package_id: 1 core_id: 1
an ACPI boot produces:
cpu: 0 package_id: 36 core_id: 0
cpu: 1 package_id: 36 core_id: 1
cpu: 2 package_id: 96 core_id: 2
cpu: 3 package_id: 96 core_id: 3
This is due to several reasons:
1) DT cpu nodes do not have an equivalent field to what the PPTT
ACPI Processor ID must be, i.e. something equal to the MADT CPU
UID or equal to the UID of an ACPI processor container. In both
ACPI cases those are platform dependant IDs assigned by the
vendor.
2) While QEMU is the vendor for a guest, if the topology specifies
SMT (> 1 thread), then, with ACPI, it is impossible to assign a
core-id the same value as a package-id, thus it is not possible
to have package-id=0 and core-id=0. This is because package and
core containers must be in the same ACPI namespace and therefore
must have unique UIDs.
3) ACPI processor containers are not mandatorily required for PPTT
tables to be used and, due to the limitations of which IDs are
selected described above in (2), they are not helpful for QEMU,
so we don't build them with this patch. In the absence of them,
Linux assigns its own unique IDs. The maintainers have chosen not
to use counters from zero, but rather ACPI table offsets, which
explains why the numbers are so much larger than with DT.
4) When there is no SMT (threads=1) the core IDs for ACPI boot guests
match the logical CPU IDs, because these IDs must be equal to the
MADT CPU UID (as no processor containers are present), and QEMU
uses the logical CPU ID for these MADT IDs.
So in summary, with QEMU as vender for the guest, we use sequential
integers starting from zero for non-leaf nodes without valid ID flag,
so that guest will ignore them and use table offsets as unique IDs.
And we use logical CPU IDs for leaf nodes to be consistent with MADT.
Signed-off-by: Andrew Jones <drjones@redhat.com>
Co-developed-by: Yanan Wang <wangyanan55@huawei.com>
Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
---
hw/acpi/aml-build.c | 50 +++++++++++++++++++++++++++++++++++++
hw/arm/virt-acpi-build.c | 8 +++++-
include/hw/acpi/aml-build.h | 3 +++
3 files changed, 60 insertions(+), 1 deletion(-)
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 9fa5024414..aa61c9651e 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1946,6 +1946,56 @@ void build_processor_hierarchy_node(GArray *tbl, uint32_t flags,
}
}
+/* ACPI 6.2: 5.2.29 Processor Properties Topology Table (PPTT) */
+void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms,
+ const char *oem_id, const char *oem_table_id)
+{
+ int pptt_start = table_data->len;
+ int uid = 0;
+ int socket;
+
+ acpi_data_push(table_data, sizeof(AcpiTableHeader));
+
+ for (socket = 0; socket < ms->smp.sockets; socket++) {
+ uint32_t socket_offset = table_data->len - pptt_start;
+ int core;
+
+ build_processor_hierarchy_node(
+ table_data,
+ (1 << 0), /* ACPI 6.2 - Physical package */
+ 0, socket, NULL, 0);
+
+ for (core = 0; core < ms->smp.cores; core++) {
+ uint32_t core_offset = table_data->len - pptt_start;
+ int thread;
+
+ if (ms->smp.threads > 1) {
+ build_processor_hierarchy_node(table_data, 0, socket_offset,
+ core, NULL, 0);
+
+ for (thread = 0; thread < ms->smp.threads; thread++) {
+ build_processor_hierarchy_node(
+ table_data,
+ (1 << 1) | /* ACPI 6.2 - ACPI Processor ID valid */
+ (1 << 2) | /* ACPI 6.3 - Processor is a Thread */
+ (1 << 3), /* ACPI 6.3 - Node is a Leaf */
+ core_offset, uid++, NULL, 0);
+ }
+ } else {
+ build_processor_hierarchy_node(
+ table_data,
+ (1 << 1) | /* ACPI 6.2 - ACPI Processor ID valid */
+ (1 << 3), /* ACPI 6.3 - Node is a Leaf */
+ socket_offset, uid++, NULL, 0);
+ }
+ }
+ }
+
+ build_header(linker, table_data,
+ (void *)(table_data->data + pptt_start), "PPTT",
+ table_data->len - pptt_start, 2, oem_id, oem_table_id);
+}
+
/* build rev1/rev3/rev5.1 FADT */
void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
const char *oem_id, const char *oem_table_id)
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 037cc1fd82..db23306a06 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -790,13 +790,19 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
dsdt = tables_blob->len;
build_dsdt(tables_blob, tables->linker, vms);
- /* FADT MADT GTDT MCFG SPCR pointed to by RSDT */
+ /* FADT MADT PPTT GTDT MCFG SPCR pointed to by RSDT */
acpi_add_table(table_offsets, tables_blob);
build_fadt_rev5(tables_blob, tables->linker, vms, dsdt);
acpi_add_table(table_offsets, tables_blob);
build_madt(tables_blob, tables->linker, vms);
+ if (!vmc->no_cpu_topology) {
+ acpi_add_table(table_offsets, tables_blob);
+ build_pptt(tables_blob, tables->linker, ms,
+ vms->oem_id, vms->oem_table_id);
+ }
+
acpi_add_table(table_offsets, tables_blob);
build_gtdt(tables_blob, tables->linker, vms);
diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
index ea74b8f6ed..6c29f853cd 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -466,6 +466,9 @@ void build_processor_hierarchy_node(GArray *tbl, uint32_t flags,
uint32_t parent, uint32_t id,
uint32_t *priv_rsrc, uint32_t priv_num);
+void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms,
+ const char *oem_id, const char *oem_table_id);
+
void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
const char *oem_id, const char *oem_table_id);
--
2.19.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH for-6.2 v5 3/5] hw/arm/virt: Add cpu-map to device tree
2021-08-05 12:39 ` [PATCH for-6.2 v5 3/5] hw/arm/virt: Add cpu-map to device tree Yanan Wang
@ 2021-08-17 2:10 ` wangyanan (Y)
2021-08-17 11:51 ` Andrew Jones
0 siblings, 1 reply; 13+ messages in thread
From: wangyanan (Y) @ 2021-08-17 2:10 UTC (permalink / raw)
To: Peter Maydell, Andrew Jones, Michael S . Tsirkin, Igor Mammedov
Cc: Salil Mehta, qemu-devel, Shannon Zhao, qemu-arm,
Alistair Francis, wanghaibin.wang, David Gibson
Hi,
On 2021/8/5 20:39, Yanan Wang wrote:
> From: Andrew Jones <drjones@redhat.com>
>
> Support device tree CPU topology descriptions.
>
> In accordance with the Devicetree Specification, the Linux Doc
> "arm/cpus.yaml" requires that cpus and cpu nodes in the DT are
> present. And we have already met the requirement by generating
> /cpus/cpu@* nodes for members within ms->smp.cpus. Accordingly,
> we should also create subnodes in cpu-map for the present cpus,
> each of which relates to an unique cpu node.
>
> The Linux Doc "cpu/cpu-topology.txt" states that the hierarchy
> of CPUs in a SMP system is defined through four entities and
> they are socket/cluster/core/thread. It is also required that
> a socket node's child nodes must be one or more cluster nodes.
> Given that currently we are only provided with information of
> socket/core/thread, we assume there is one cluster child node
> in each socket node when creating cpu-map.
>
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> Co-developed-by: Yanan Wang <wangyanan55@huawei.com>
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> ---
> hw/arm/virt.c | 59 ++++++++++++++++++++++++++++++++++++++++++---------
> 1 file changed, 49 insertions(+), 10 deletions(-)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 82f2eba6bd..d1e294be95 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -350,20 +350,21 @@ 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
> - * 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
> - * in the system, #address-cells can be set to 1, since
> - * MPIDR_EL1[63:32] bits are not used for CPUs
> - * identification.
> + * 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
> + * in the system, #address-cells can be set to 1, since
> + * MPIDR_EL1[63:32] bits are not used for CPUs
> + * identification.
> *
> - * Here we actually don't know whether our system is 32- or 64-bit one.
> - * The simplest way to go is to examine affinity IDs of all our CPUs. If
> - * at least one of them has Aff3 populated, we set #address-cells to 2.
> + * Here we actually don't know whether our system is 32- or 64-bit one.
> + * The simplest way to go is to examine affinity IDs of all our CPUs. If
> + * at least one of them has Aff3 populated, we set #address-cells to 2.
> */
> for (cpu = 0; cpu < smp_cpus; cpu++) {
> ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(cpu));
> @@ -406,8 +407,46 @@ static void fdt_add_cpu_nodes(const VirtMachineState *vms)
> ms->possible_cpus->cpus[cs->cpu_index].props.node_id);
> }
>
> + if (!vmc->no_cpu_topology) {
> + qemu_fdt_setprop_cell(ms->fdt, nodename, "phandle",
> + qemu_fdt_alloc_phandle(ms->fdt));
> + }
> +
> g_free(nodename);
> }
> +
> + if (!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 = 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/socket%d/cluster0/core%d/thread%d",
> + cpu / (ms->smp.cores * ms->smp.threads),
> + (cpu / ms->smp.threads) % ms->smp.cores,
> + cpu % ms->smp.threads);
It seems that there is some discrepancy between the documentation
(Documentation/devicetree/bindings/cpu/cpu-topology.txt) and the
actual implementation of DT topology parser for ARM64
(function parse_dt_topology() in drivers/base/arch_topology.c).
The doc says the cpu-map node's child nodes can be:
- one or more cluster nodes or
- one or more socket nodes in a multi-socket system
which means a cpu-map can be defined as two formats such as:
1) cpu-map
socket0
cluster0
core0
core1
cluster1
core0
core1
socket1
cluster0
core0
core1
cluster1
core0
core1
2) cpu-map
cluster0
cluster0
core0
core1
cluster1
core0
core1
cluster1
cluster0
core0
core1
cluster1
core0
core1
But current parser only assumes that there are nested clusters within
cpu-map and is unaware of socket, the parser also ignore any information
about the nesting of clusters and present the scheduler with a flat list of
them. So based on current parser, we will get 4 packages (sockets) totally,
2 cores per package, 1 threads per core from 2), but will get nothing
useful from 1).
I think the ARM64 kernel DT parser should be optimized so that it's
also aware of sockets and can parse both formats of cpu-map. But
before this, I think we still have to build the cpu-map in format 2) if
we hope to describe topology successfully through DT. :)
Thanks,
Yanan
.
> + } else {
> + map_path = g_strdup_printf(
> + "/cpus/cpu-map/socket%d/cluster0/core%d",
> + cpu / ms->smp.cores,
> + 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)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH for-6.2 v5 3/5] hw/arm/virt: Add cpu-map to device tree
2021-08-17 2:10 ` wangyanan (Y)
@ 2021-08-17 11:51 ` Andrew Jones
2021-08-17 13:54 ` wangyanan (Y)
0 siblings, 1 reply; 13+ messages in thread
From: Andrew Jones @ 2021-08-17 11:51 UTC (permalink / raw)
To: wangyanan (Y)
Cc: Peter Maydell, Salil Mehta, Michael S . Tsirkin, qemu-devel,
Shannon Zhao, qemu-arm, Alistair Francis, wanghaibin.wang,
Igor Mammedov, David Gibson
On Tue, Aug 17, 2021 at 10:10:44AM +0800, wangyanan (Y) wrote:
> Hi,
> On 2021/8/5 20:39, Yanan Wang wrote:
> > From: Andrew Jones <drjones@redhat.com>
> >
> > Support device tree CPU topology descriptions.
> >
> > In accordance with the Devicetree Specification, the Linux Doc
> > "arm/cpus.yaml" requires that cpus and cpu nodes in the DT are
> > present. And we have already met the requirement by generating
> > /cpus/cpu@* nodes for members within ms->smp.cpus. Accordingly,
> > we should also create subnodes in cpu-map for the present cpus,
> > each of which relates to an unique cpu node.
> >
> > The Linux Doc "cpu/cpu-topology.txt" states that the hierarchy
> > of CPUs in a SMP system is defined through four entities and
> > they are socket/cluster/core/thread. It is also required that
> > a socket node's child nodes must be one or more cluster nodes.
> > Given that currently we are only provided with information of
> > socket/core/thread, we assume there is one cluster child node
> > in each socket node when creating cpu-map.
> >
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > Co-developed-by: Yanan Wang <wangyanan55@huawei.com>
> > Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> > ---
> > hw/arm/virt.c | 59 ++++++++++++++++++++++++++++++++++++++++++---------
> > 1 file changed, 49 insertions(+), 10 deletions(-)
> >
> > diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> > index 82f2eba6bd..d1e294be95 100644
> > --- a/hw/arm/virt.c
> > +++ b/hw/arm/virt.c
> > @@ -350,20 +350,21 @@ 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
> > - * 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
> > - * in the system, #address-cells can be set to 1, since
> > - * MPIDR_EL1[63:32] bits are not used for CPUs
> > - * identification.
> > + * 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
> > + * in the system, #address-cells can be set to 1, since
> > + * MPIDR_EL1[63:32] bits are not used for CPUs
> > + * identification.
> > *
> > - * Here we actually don't know whether our system is 32- or 64-bit one.
> > - * The simplest way to go is to examine affinity IDs of all our CPUs. If
> > - * at least one of them has Aff3 populated, we set #address-cells to 2.
> > + * Here we actually don't know whether our system is 32- or 64-bit one.
> > + * The simplest way to go is to examine affinity IDs of all our CPUs. If
> > + * at least one of them has Aff3 populated, we set #address-cells to 2.
> > */
> > for (cpu = 0; cpu < smp_cpus; cpu++) {
> > ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(cpu));
> > @@ -406,8 +407,46 @@ static void fdt_add_cpu_nodes(const VirtMachineState *vms)
> > ms->possible_cpus->cpus[cs->cpu_index].props.node_id);
> > }
> > + if (!vmc->no_cpu_topology) {
> > + qemu_fdt_setprop_cell(ms->fdt, nodename, "phandle",
> > + qemu_fdt_alloc_phandle(ms->fdt));
> > + }
> > +
> > g_free(nodename);
> > }
> > +
> > + if (!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 = 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/socket%d/cluster0/core%d/thread%d",
> > + cpu / (ms->smp.cores * ms->smp.threads),
> > + (cpu / ms->smp.threads) % ms->smp.cores,
> > + cpu % ms->smp.threads);
> It seems that there is some discrepancy between the documentation
> (Documentation/devicetree/bindings/cpu/cpu-topology.txt) and the
> actual implementation of DT topology parser for ARM64
> (function parse_dt_topology() in drivers/base/arch_topology.c).
>
> The doc says the cpu-map node's child nodes can be:
> - one or more cluster nodes or
> - one or more socket nodes in a multi-socket system
> which means a cpu-map can be defined as two formats such as:
> 1) cpu-map
> socket0
> cluster0
> core0
> core1
> cluster1
> core0
> core1
> socket1
> cluster0
> core0
> core1
> cluster1
> core0
> core1
>
> 2) cpu-map
> cluster0
> cluster0
> core0
> core1
> cluster1
> core0
> core1
> cluster1
> cluster0
> core0
> core1
> cluster1
> core0
> core1
>
> But current parser only assumes that there are nested clusters within
> cpu-map and is unaware of socket, the parser also ignore any information
> about the nesting of clusters and present the scheduler with a flat list of
> them. So based on current parser, we will get 4 packages (sockets) totally,
> 2 cores per package, 1 threads per core from 2), but will get nothing
> useful from 1).
>
> I think the ARM64 kernel DT parser should be optimized so that it's
> also aware of sockets and can parse both formats of cpu-map. But
> before this, I think we still have to build the cpu-map in format 2) if
> we hope to describe topology successfully through DT. :)
I would stick to implementing this per the documentation and as similarly
to the ACPI version as possible. I doubt there will be many cases where a
mach-virt VM is launched with a complex topology description and with DT
instead of ACPI anyway. And, if there is a use for that, then the Linux
parser needs to be fixed to match the documentation.
Thanks,
drew
>
> Thanks,
> Yanan
> .
> > + } else {
> > + map_path = g_strdup_printf(
> > + "/cpus/cpu-map/socket%d/cluster0/core%d",
> > + cpu / ms->smp.cores,
> > + 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)
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH for-6.2 v5 3/5] hw/arm/virt: Add cpu-map to device tree
2021-08-17 11:51 ` Andrew Jones
@ 2021-08-17 13:54 ` wangyanan (Y)
0 siblings, 0 replies; 13+ messages in thread
From: wangyanan (Y) @ 2021-08-17 13:54 UTC (permalink / raw)
To: Andrew Jones
Cc: Peter Maydell, Salil Mehta, Michael S . Tsirkin, qemu-devel,
Shannon Zhao, qemu-arm, Alistair Francis, wanghaibin.wang,
Igor Mammedov, David Gibson
On 2021/8/17 19:51, Andrew Jones wrote:
> On Tue, Aug 17, 2021 at 10:10:44AM +0800, wangyanan (Y) wrote:
>> Hi,
>> On 2021/8/5 20:39, Yanan Wang wrote:
>>> From: Andrew Jones <drjones@redhat.com>
>>>
>>> Support device tree CPU topology descriptions.
>>>
>>> In accordance with the Devicetree Specification, the Linux Doc
>>> "arm/cpus.yaml" requires that cpus and cpu nodes in the DT are
>>> present. And we have already met the requirement by generating
>>> /cpus/cpu@* nodes for members within ms->smp.cpus. Accordingly,
>>> we should also create subnodes in cpu-map for the present cpus,
>>> each of which relates to an unique cpu node.
>>>
>>> The Linux Doc "cpu/cpu-topology.txt" states that the hierarchy
>>> of CPUs in a SMP system is defined through four entities and
>>> they are socket/cluster/core/thread. It is also required that
>>> a socket node's child nodes must be one or more cluster nodes.
>>> Given that currently we are only provided with information of
>>> socket/core/thread, we assume there is one cluster child node
>>> in each socket node when creating cpu-map.
>>>
>>> Signed-off-by: Andrew Jones <drjones@redhat.com>
>>> Co-developed-by: Yanan Wang <wangyanan55@huawei.com>
>>> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
>>> ---
>>> hw/arm/virt.c | 59 ++++++++++++++++++++++++++++++++++++++++++---------
>>> 1 file changed, 49 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>>> index 82f2eba6bd..d1e294be95 100644
>>> --- a/hw/arm/virt.c
>>> +++ b/hw/arm/virt.c
>>> @@ -350,20 +350,21 @@ 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
>>> - * 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
>>> - * in the system, #address-cells can be set to 1, since
>>> - * MPIDR_EL1[63:32] bits are not used for CPUs
>>> - * identification.
>>> + * 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
>>> + * in the system, #address-cells can be set to 1, since
>>> + * MPIDR_EL1[63:32] bits are not used for CPUs
>>> + * identification.
>>> *
>>> - * Here we actually don't know whether our system is 32- or 64-bit one.
>>> - * The simplest way to go is to examine affinity IDs of all our CPUs. If
>>> - * at least one of them has Aff3 populated, we set #address-cells to 2.
>>> + * Here we actually don't know whether our system is 32- or 64-bit one.
>>> + * The simplest way to go is to examine affinity IDs of all our CPUs. If
>>> + * at least one of them has Aff3 populated, we set #address-cells to 2.
>>> */
>>> for (cpu = 0; cpu < smp_cpus; cpu++) {
>>> ARMCPU *armcpu = ARM_CPU(qemu_get_cpu(cpu));
>>> @@ -406,8 +407,46 @@ static void fdt_add_cpu_nodes(const VirtMachineState *vms)
>>> ms->possible_cpus->cpus[cs->cpu_index].props.node_id);
>>> }
>>> + if (!vmc->no_cpu_topology) {
>>> + qemu_fdt_setprop_cell(ms->fdt, nodename, "phandle",
>>> + qemu_fdt_alloc_phandle(ms->fdt));
>>> + }
>>> +
>>> g_free(nodename);
>>> }
>>> +
>>> + if (!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 = 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/socket%d/cluster0/core%d/thread%d",
>>> + cpu / (ms->smp.cores * ms->smp.threads),
>>> + (cpu / ms->smp.threads) % ms->smp.cores,
>>> + cpu % ms->smp.threads);
>> It seems that there is some discrepancy between the documentation
>> (Documentation/devicetree/bindings/cpu/cpu-topology.txt) and the
>> actual implementation of DT topology parser for ARM64
>> (function parse_dt_topology() in drivers/base/arch_topology.c).
>>
>> The doc says the cpu-map node's child nodes can be:
>> - one or more cluster nodes or
>> - one or more socket nodes in a multi-socket system
>> which means a cpu-map can be defined as two formats such as:
>> 1) cpu-map
>> socket0
>> cluster0
>> core0
>> core1
>> cluster1
>> core0
>> core1
>> socket1
>> cluster0
>> core0
>> core1
>> cluster1
>> core0
>> core1
>>
>> 2) cpu-map
>> cluster0
>> cluster0
>> core0
>> core1
>> cluster1
>> core0
>> core1
>> cluster1
>> cluster0
>> core0
>> core1
>> cluster1
>> core0
>> core1
>>
>> But current parser only assumes that there are nested clusters within
>> cpu-map and is unaware of socket, the parser also ignore any information
>> about the nesting of clusters and present the scheduler with a flat list of
>> them. So based on current parser, we will get 4 packages (sockets) totally,
>> 2 cores per package, 1 threads per core from 2), but will get nothing
>> useful from 1).
>>
>> I think the ARM64 kernel DT parser should be optimized so that it's
>> also aware of sockets and can parse both formats of cpu-map. But
>> before this, I think we still have to build the cpu-map in format 2) if
>> we hope to describe topology successfully through DT. :)
> I would stick to implementing this per the documentation and as similarly
> to the ACPI version as possible. I doubt there will be many cases where a
> mach-virt VM is launched with a complex topology description and with DT
> instead of ACPI anyway. And, if there is a use for that, then the Linux
> parser needs to be fixed to match the documentation.
Thanks for the suggestion!
I agree at this point. We may need to keep consistence with the Doc.
The guest kernel should try to enure itself can parse both cpu-map
formats and should also accept the result if it can not get anything
useful from a well defined cpu-map.
Thanks,
Yanan
.
> Thanks,
> drew
>
>> Thanks,
>> Yanan
>> .
>>> + } else {
>>> + map_path = g_strdup_printf(
>>> + "/cpus/cpu-map/socket%d/cluster0/core%d",
>>> + cpu / ms->smp.cores,
>>> + 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)
> .
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH for-6.2 v5 5/5] hw/acpi/aml-build: Generate PPTT table
2021-08-05 12:39 ` [PATCH for-6.2 v5 5/5] hw/acpi/aml-build: Generate PPTT table Yanan Wang
@ 2021-08-23 23:52 ` Michael S. Tsirkin
2021-08-24 3:19 ` wangyanan (Y)
0 siblings, 1 reply; 13+ messages in thread
From: Michael S. Tsirkin @ 2021-08-23 23:52 UTC (permalink / raw)
To: Yanan Wang
Cc: Peter Maydell, Andrew Jones, qemu-devel, Shannon Zhao, qemu-arm,
Alistair Francis, wanghaibin.wang, Igor Mammedov, Salil Mehta,
David Gibson
On Thu, Aug 05, 2021 at 08:39:21PM +0800, Yanan Wang wrote:
> From: Andrew Jones <drjones@redhat.com>
>
> Add the Processor Properties Topology Table (PPTT) to expose
> CPU topology information defined by users to ACPI guests.
>
> Note, a DT-boot Linux guest with a non-flat CPU topology will
> see socket and core IDs being sequential integers starting
> from zero, which is different from ACPI-boot Linux guest,
> e.g. with -smp 4,sockets=2,cores=2,threads=1
>
> a DT boot produces:
>
> cpu: 0 package_id: 0 core_id: 0
> cpu: 1 package_id: 0 core_id: 1
> cpu: 2 package_id: 1 core_id: 0
> cpu: 3 package_id: 1 core_id: 1
>
> an ACPI boot produces:
>
> cpu: 0 package_id: 36 core_id: 0
> cpu: 1 package_id: 36 core_id: 1
> cpu: 2 package_id: 96 core_id: 2
> cpu: 3 package_id: 96 core_id: 3
>
> This is due to several reasons:
>
> 1) DT cpu nodes do not have an equivalent field to what the PPTT
> ACPI Processor ID must be, i.e. something equal to the MADT CPU
> UID or equal to the UID of an ACPI processor container. In both
> ACPI cases those are platform dependant IDs assigned by the
> vendor.
>
> 2) While QEMU is the vendor for a guest, if the topology specifies
> SMT (> 1 thread), then, with ACPI, it is impossible to assign a
> core-id the same value as a package-id, thus it is not possible
> to have package-id=0 and core-id=0. This is because package and
> core containers must be in the same ACPI namespace and therefore
> must have unique UIDs.
>
> 3) ACPI processor containers are not mandatorily required for PPTT
> tables to be used and, due to the limitations of which IDs are
> selected described above in (2), they are not helpful for QEMU,
> so we don't build them with this patch. In the absence of them,
> Linux assigns its own unique IDs. The maintainers have chosen not
> to use counters from zero, but rather ACPI table offsets, which
> explains why the numbers are so much larger than with DT.
>
> 4) When there is no SMT (threads=1) the core IDs for ACPI boot guests
> match the logical CPU IDs, because these IDs must be equal to the
> MADT CPU UID (as no processor containers are present), and QEMU
> uses the logical CPU ID for these MADT IDs.
>
> So in summary, with QEMU as vender for the guest, we use sequential
vendor?
> integers starting from zero for non-leaf nodes without valid ID flag,
> so that guest will ignore them and use table offsets as unique IDs.
> And we use logical CPU IDs for leaf nodes to be consistent with MADT.
>
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> Co-developed-by: Yanan Wang <wangyanan55@huawei.com>
> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
> ---
> hw/acpi/aml-build.c | 50 +++++++++++++++++++++++++++++++++++++
> hw/arm/virt-acpi-build.c | 8 +++++-
> include/hw/acpi/aml-build.h | 3 +++
> 3 files changed, 60 insertions(+), 1 deletion(-)
>
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 9fa5024414..aa61c9651e 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -1946,6 +1946,56 @@ void build_processor_hierarchy_node(GArray *tbl, uint32_t flags,
> }
> }
>
> +/* ACPI 6.2: 5.2.29 Processor Properties Topology Table (PPTT) */
> +void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms,
> + const char *oem_id, const char *oem_table_id)
> +{
> + int pptt_start = table_data->len;
> + int uid = 0;
> + int socket;
> +
> + acpi_data_push(table_data, sizeof(AcpiTableHeader));
> +
> + for (socket = 0; socket < ms->smp.sockets; socket++) {
> + uint32_t socket_offset = table_data->len - pptt_start;
> + int core;
> +
> + build_processor_hierarchy_node(
> + table_data,
> + (1 << 0), /* ACPI 6.2 - Physical package */
A bit better to be detailed:
/* Physical package - represents the boundary of a physical package */
> + 0, socket, NULL, 0);
> +
> + for (core = 0; core < ms->smp.cores; core++) {
> + uint32_t core_offset = table_data->len - pptt_start;
> + int thread;
> +
> + if (ms->smp.threads > 1) {
> + build_processor_hierarchy_node(table_data, 0, socket_offset,
and here:
/* Physical package - does not represent the boundary of a physical package */
> + core, NULL, 0);
> +
> + for (thread = 0; thread < ms->smp.threads; thread++) {
> + build_processor_hierarchy_node(
> + table_data,
> + (1 << 1) | /* ACPI 6.2 - ACPI Processor ID valid */
> + (1 << 2) | /* ACPI 6.3 - Processor is a Thread */
> + (1 << 3), /* ACPI 6.3 - Node is a Leaf */
> + core_offset, uid++, NULL, 0);
> + }
> + } else {
> + build_processor_hierarchy_node(
> + table_data,
> + (1 << 1) | /* ACPI 6.2 - ACPI Processor ID valid */
> + (1 << 3), /* ACPI 6.3 - Node is a Leaf */
> + socket_offset, uid++, NULL, 0);
> + }
> + }
> + }
> +
> + build_header(linker, table_data,
> + (void *)(table_data->data + pptt_start), "PPTT",
> + table_data->len - pptt_start, 2, oem_id, oem_table_id);
> +}
> +
> /* build rev1/rev3/rev5.1 FADT */
> void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
> const char *oem_id, const char *oem_table_id)
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 037cc1fd82..db23306a06 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -790,13 +790,19 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
> dsdt = tables_blob->len;
> build_dsdt(tables_blob, tables->linker, vms);
>
> - /* FADT MADT GTDT MCFG SPCR pointed to by RSDT */
> + /* FADT MADT PPTT GTDT MCFG SPCR pointed to by RSDT */
> acpi_add_table(table_offsets, tables_blob);
> build_fadt_rev5(tables_blob, tables->linker, vms, dsdt);
>
> acpi_add_table(table_offsets, tables_blob);
> build_madt(tables_blob, tables->linker, vms);
>
> + if (!vmc->no_cpu_topology) {
> + acpi_add_table(table_offsets, tables_blob);
> + build_pptt(tables_blob, tables->linker, ms,
> + vms->oem_id, vms->oem_table_id);
> + }
> +
> acpi_add_table(table_offsets, tables_blob);
> build_gtdt(tables_blob, tables->linker, vms);
>
> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
> index ea74b8f6ed..6c29f853cd 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -466,6 +466,9 @@ void build_processor_hierarchy_node(GArray *tbl, uint32_t flags,
> uint32_t parent, uint32_t id,
> uint32_t *priv_rsrc, uint32_t priv_num);
>
> +void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms,
> + const char *oem_id, const char *oem_table_id);
> +
> void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
> const char *oem_id, const char *oem_table_id);
>
> --
> 2.19.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH for-6.2 v5 0/5] hw/arm/virt: Introduce cpu topology support
2021-08-05 12:39 [PATCH for-6.2 v5 0/5] hw/arm/virt: Introduce cpu topology support Yanan Wang
` (4 preceding siblings ...)
2021-08-05 12:39 ` [PATCH for-6.2 v5 5/5] hw/acpi/aml-build: Generate PPTT table Yanan Wang
@ 2021-08-23 23:53 ` Michael S. Tsirkin
2021-08-24 3:19 ` wangyanan (Y)
5 siblings, 1 reply; 13+ messages in thread
From: Michael S. Tsirkin @ 2021-08-23 23:53 UTC (permalink / raw)
To: Yanan Wang
Cc: Peter Maydell, Andrew Jones, qemu-devel, Shannon Zhao, qemu-arm,
Alistair Francis, wanghaibin.wang, Igor Mammedov, Salil Mehta,
David Gibson
On Thu, Aug 05, 2021 at 08:39:16PM +0800, Yanan Wang wrote:
> Hi,
>
> This is a new version (v5) of the series [1] that I posted to introduce
> support for generating cpu topology descriptions to virt machine guest.
>
> Once the view of an accurate virtual cpu topology is provided to guest,
> with a well-designed vCPU pinning to the pCPU we may get a huge benefit,
> e.g., the scheduling performance improvement. See Dario Faggioli's
> research and the related performance tests in [2] for reference.
>
> This patch series introduces cpu topology support for ARM platform.
> Both cpu-map in DT and ACPI PPTT table are introduced to store the
> topology information. And we only describe the topology information
> to 6.2 and newer virt machines, considering compatibility.
ACPI things:
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
pls merge through ARM tree.
> [1] https://lore.kernel.org/qemu-devel/20210622093413.13360-1-wangyanan55@huawei.com/
> [2] https://kvmforum2020.sched.com/event/eE1y/virtual-topology-for-virtual-machines
> -friend-or-foe-dario-faggioli-suse
>
> Some tests:
> 1) -smp 16,sockets=2,cores=4,threads=2,maxcpus=16
> lscpu:
> Architecture: aarch64
> Byte Order: Little Endian
> CPU(s): 16
> On-line CPU(s) list: 0-15
> Thread(s) per core: 2
> Core(s) per socket: 4
> Socket(s): 2
> NUMA node(s): 1
> Vendor ID: ARM
> Model: 2
> Model name: Cortex-A72
> Stepping: r0p2
> BogoMIPS: 100.00
> NUMA node0 CPU(s): 0-15
>
> cat /sys/devices/system/cpu/present --> 0-15
> cat /sys/devices/system/cpu/possible --> 0-15
>
> 2) -smp 8,sockets=2,cores=4,threads=2,maxcpus=16
> lscpu:
> Architecture: aarch64
> Byte Order: Little Endian
> CPU(s): 8
> On-line CPU(s) list: 0-7
> Thread(s) per core: 2
> Core(s) per socket: 4
> Socket(s): 1
> NUMA node(s): 1
> Vendor ID: ARM
> Model: 2
> Model name: Cortex-A72
> Stepping: r0p2
> BogoMIPS: 100.00
> NUMA node0 CPU(s): 0-7
>
> cat /sys/devices/system/cpu/present --> 0-7
> cat /sys/devices/system/cpu/possible --> 0-7
>
> ---
>
> Changelogs:
>
> v4->v5:
> - drop the added -smp "expose=on|off" parameter and only describe topology
> for 6.2 and newer machines
> - rebased the code on patch series [3] which has introduced some fix and
> improvement for smp parsing
> - [3]: https://lore.kernel.org/qemu-devel/20210803080527.156556-1-wangyanan55@huawei.com/
>
> v3->v4:
> - add new -smp parameter "expose=on|off" for users to enable/disable the feature
> - add stricter -smp cmdline parsing rules on "expose=on" case
> - move build_pptt to generic aml-build.c
> - add default cluster node in the cpu-map
> - rebase on top of latest upstream master
> - v3: https://lore.kernel.org/qemu-devel/20210516102900.28036-1-wangyanan55@huawei.com/
>
> v2->v3:
> - address comments from David, Philippe, and Andrew. Thanks!
> - split some change into separate commits for ease of review
> - adjust parsing rules of virt_smp_parse to be more strict
> (after discussion with Andrew)
> - adjust author credit for the patches
>
> v1->v2:
> - Address Andrew Jones's comments
> - Address Michael S. Tsirkin's comments
>
> ---
>
> Andrew Jones (2):
> hw/arm/virt: Add cpu-map to device tree
> hw/acpi/aml-build: Generate PPTT table
>
> Yanan Wang (3):
> hw/arm/virt: Only describe cpu topology to guest since virt 6.2
> device_tree: Add qemu_fdt_add_path
> hw/acpi/aml-build: Add Processor hierarchy node structure
>
> hw/acpi/aml-build.c | 76 ++++++++++++++++++++++++++++++++++++
> hw/arm/virt-acpi-build.c | 8 +++-
> hw/arm/virt.c | 62 ++++++++++++++++++++++++-----
> include/hw/acpi/aml-build.h | 7 ++++
> include/hw/arm/virt.h | 4 +-
> include/sysemu/device_tree.h | 1 +
> softmmu/device_tree.c | 44 ++++++++++++++++++++-
> 7 files changed, 188 insertions(+), 14 deletions(-)
>
> --
> 2.19.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH for-6.2 v5 5/5] hw/acpi/aml-build: Generate PPTT table
2021-08-23 23:52 ` Michael S. Tsirkin
@ 2021-08-24 3:19 ` wangyanan (Y)
0 siblings, 0 replies; 13+ messages in thread
From: wangyanan (Y) @ 2021-08-24 3:19 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Peter Maydell, Andrew Jones, qemu-devel, Shannon Zhao, qemu-arm,
Alistair Francis, wanghaibin.wang, Igor Mammedov, Salil Mehta,
David Gibson
On 2021/8/24 7:52, Michael S. Tsirkin wrote:
> On Thu, Aug 05, 2021 at 08:39:21PM +0800, Yanan Wang wrote:
>> From: Andrew Jones <drjones@redhat.com>
>>
>> Add the Processor Properties Topology Table (PPTT) to expose
>> CPU topology information defined by users to ACPI guests.
>>
>> Note, a DT-boot Linux guest with a non-flat CPU topology will
>> see socket and core IDs being sequential integers starting
>> from zero, which is different from ACPI-boot Linux guest,
>> e.g. with -smp 4,sockets=2,cores=2,threads=1
>>
>> a DT boot produces:
>>
>> cpu: 0 package_id: 0 core_id: 0
>> cpu: 1 package_id: 0 core_id: 1
>> cpu: 2 package_id: 1 core_id: 0
>> cpu: 3 package_id: 1 core_id: 1
>>
>> an ACPI boot produces:
>>
>> cpu: 0 package_id: 36 core_id: 0
>> cpu: 1 package_id: 36 core_id: 1
>> cpu: 2 package_id: 96 core_id: 2
>> cpu: 3 package_id: 96 core_id: 3
>>
>> This is due to several reasons:
>>
>> 1) DT cpu nodes do not have an equivalent field to what the PPTT
>> ACPI Processor ID must be, i.e. something equal to the MADT CPU
>> UID or equal to the UID of an ACPI processor container. In both
>> ACPI cases those are platform dependant IDs assigned by the
>> vendor.
>>
>> 2) While QEMU is the vendor for a guest, if the topology specifies
>> SMT (> 1 thread), then, with ACPI, it is impossible to assign a
>> core-id the same value as a package-id, thus it is not possible
>> to have package-id=0 and core-id=0. This is because package and
>> core containers must be in the same ACPI namespace and therefore
>> must have unique UIDs.
>>
>> 3) ACPI processor containers are not mandatorily required for PPTT
>> tables to be used and, due to the limitations of which IDs are
>> selected described above in (2), they are not helpful for QEMU,
>> so we don't build them with this patch. In the absence of them,
>> Linux assigns its own unique IDs. The maintainers have chosen not
>> to use counters from zero, but rather ACPI table offsets, which
>> explains why the numbers are so much larger than with DT.
>>
>> 4) When there is no SMT (threads=1) the core IDs for ACPI boot guests
>> match the logical CPU IDs, because these IDs must be equal to the
>> MADT CPU UID (as no processor containers are present), and QEMU
>> uses the logical CPU ID for these MADT IDs.
>>
>> So in summary, with QEMU as vender for the guest, we use sequential
> vendor?
Yes, will fix the typo.
>> integers starting from zero for non-leaf nodes without valid ID flag,
>> so that guest will ignore them and use table offsets as unique IDs.
>> And we use logical CPU IDs for leaf nodes to be consistent with MADT.
>>
>> Signed-off-by: Andrew Jones <drjones@redhat.com>
>> Co-developed-by: Yanan Wang <wangyanan55@huawei.com>
>> Signed-off-by: Yanan Wang <wangyanan55@huawei.com>
>> ---
>> hw/acpi/aml-build.c | 50 +++++++++++++++++++++++++++++++++++++
>> hw/arm/virt-acpi-build.c | 8 +++++-
>> include/hw/acpi/aml-build.h | 3 +++
>> 3 files changed, 60 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
>> index 9fa5024414..aa61c9651e 100644
>> --- a/hw/acpi/aml-build.c
>> +++ b/hw/acpi/aml-build.c
>> @@ -1946,6 +1946,56 @@ void build_processor_hierarchy_node(GArray *tbl, uint32_t flags,
>> }
>> }
>>
>> +/* ACPI 6.2: 5.2.29 Processor Properties Topology Table (PPTT) */
>> +void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms,
>> + const char *oem_id, const char *oem_table_id)
>> +{
>> + int pptt_start = table_data->len;
>> + int uid = 0;
>> + int socket;
>> +
>> + acpi_data_push(table_data, sizeof(AcpiTableHeader));
>> +
>> + for (socket = 0; socket < ms->smp.sockets; socket++) {
>> + uint32_t socket_offset = table_data->len - pptt_start;
>> + int core;
>> +
>> + build_processor_hierarchy_node(
>> + table_data,
>> + (1 << 0), /* ACPI 6.2 - Physical package */
> A bit better to be detailed:
>
> /* Physical package - represents the boundary of a physical package */
>
Ok. I will change this place and below mentioned to be more detailed
as you suggested if it's preferred.
>> + 0, socket, NULL, 0);
>> +
>> + for (core = 0; core < ms->smp.cores; core++) {
>> + uint32_t core_offset = table_data->len - pptt_start;
>> + int thread;
>> +
>> + if (ms->smp.threads > 1) {
>> + build_processor_hierarchy_node(table_data, 0, socket_offset,
> and here:
> /* Physical package - does not represent the boundary of a physical package */
Thanks,
Yanan
.
>> + core, NULL, 0);
>> +
>> + for (thread = 0; thread < ms->smp.threads; thread++) {
>> + build_processor_hierarchy_node(
>> + table_data,
>> + (1 << 1) | /* ACPI 6.2 - ACPI Processor ID valid */
>> + (1 << 2) | /* ACPI 6.3 - Processor is a Thread */
>> + (1 << 3), /* ACPI 6.3 - Node is a Leaf */
>> + core_offset, uid++, NULL, 0);
>> + }
>> + } else {
>> + build_processor_hierarchy_node(
>> + table_data,
>> + (1 << 1) | /* ACPI 6.2 - ACPI Processor ID valid */
>> + (1 << 3), /* ACPI 6.3 - Node is a Leaf */
>> + socket_offset, uid++, NULL, 0);
>> + }
>> + }
>> + }
>> +
>> + build_header(linker, table_data,
>> + (void *)(table_data->data + pptt_start), "PPTT",
>> + table_data->len - pptt_start, 2, oem_id, oem_table_id);
>> +}
>> +
>> /* build rev1/rev3/rev5.1 FADT */
>> void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
>> const char *oem_id, const char *oem_table_id)
>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>> index 037cc1fd82..db23306a06 100644
>> --- a/hw/arm/virt-acpi-build.c
>> +++ b/hw/arm/virt-acpi-build.c
>> @@ -790,13 +790,19 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables *tables)
>> dsdt = tables_blob->len;
>> build_dsdt(tables_blob, tables->linker, vms);
>>
>> - /* FADT MADT GTDT MCFG SPCR pointed to by RSDT */
>> + /* FADT MADT PPTT GTDT MCFG SPCR pointed to by RSDT */
>> acpi_add_table(table_offsets, tables_blob);
>> build_fadt_rev5(tables_blob, tables->linker, vms, dsdt);
>>
>> acpi_add_table(table_offsets, tables_blob);
>> build_madt(tables_blob, tables->linker, vms);
>>
>> + if (!vmc->no_cpu_topology) {
>> + acpi_add_table(table_offsets, tables_blob);
>> + build_pptt(tables_blob, tables->linker, ms,
>> + vms->oem_id, vms->oem_table_id);
>> + }
>> +
>> acpi_add_table(table_offsets, tables_blob);
>> build_gtdt(tables_blob, tables->linker, vms);
>>
>> diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h
>> index ea74b8f6ed..6c29f853cd 100644
>> --- a/include/hw/acpi/aml-build.h
>> +++ b/include/hw/acpi/aml-build.h
>> @@ -466,6 +466,9 @@ void build_processor_hierarchy_node(GArray *tbl, uint32_t flags,
>> uint32_t parent, uint32_t id,
>> uint32_t *priv_rsrc, uint32_t priv_num);
>>
>> +void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms,
>> + const char *oem_id, const char *oem_table_id);
>> +
>> void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
>> const char *oem_id, const char *oem_table_id);
>>
>> --
>> 2.19.1
> .
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH for-6.2 v5 0/5] hw/arm/virt: Introduce cpu topology support
2021-08-23 23:53 ` [PATCH for-6.2 v5 0/5] hw/arm/virt: Introduce cpu topology support Michael S. Tsirkin
@ 2021-08-24 3:19 ` wangyanan (Y)
0 siblings, 0 replies; 13+ messages in thread
From: wangyanan (Y) @ 2021-08-24 3:19 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Peter Maydell, Andrew Jones, qemu-devel, Shannon Zhao, qemu-arm,
Alistair Francis, Igor Mammedov, wanghaibin.wang, Salil Mehta,
David Gibson
On 2021/8/24 7:53, Michael S. Tsirkin wrote:
> On Thu, Aug 05, 2021 at 08:39:16PM +0800, Yanan Wang wrote:
>> Hi,
>>
>> This is a new version (v5) of the series [1] that I posted to introduce
>> support for generating cpu topology descriptions to virt machine guest.
>>
>> Once the view of an accurate virtual cpu topology is provided to guest,
>> with a well-designed vCPU pinning to the pCPU we may get a huge benefit,
>> e.g., the scheduling performance improvement. See Dario Faggioli's
>> research and the related performance tests in [2] for reference.
>>
>> This patch series introduces cpu topology support for ARM platform.
>> Both cpu-map in DT and ACPI PPTT table are introduced to store the
>> topology information. And we only describe the topology information
>> to 6.2 and newer virt machines, considering compatibility.
> ACPI things:
>
> Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Thanks for the reviewing.
> pls merge through ARM tree.
Sure, got it.
Yanan
.
>
>> [1] https://lore.kernel.org/qemu-devel/20210622093413.13360-1-wangyanan55@huawei.com/
>> [2] https://kvmforum2020.sched.com/event/eE1y/virtual-topology-for-virtual-machines
>> -friend-or-foe-dario-faggioli-suse
>>
>> Some tests:
>> 1) -smp 16,sockets=2,cores=4,threads=2,maxcpus=16
>> lscpu:
>> Architecture: aarch64
>> Byte Order: Little Endian
>> CPU(s): 16
>> On-line CPU(s) list: 0-15
>> Thread(s) per core: 2
>> Core(s) per socket: 4
>> Socket(s): 2
>> NUMA node(s): 1
>> Vendor ID: ARM
>> Model: 2
>> Model name: Cortex-A72
>> Stepping: r0p2
>> BogoMIPS: 100.00
>> NUMA node0 CPU(s): 0-15
>>
>> cat /sys/devices/system/cpu/present --> 0-15
>> cat /sys/devices/system/cpu/possible --> 0-15
>>
>> 2) -smp 8,sockets=2,cores=4,threads=2,maxcpus=16
>> lscpu:
>> Architecture: aarch64
>> Byte Order: Little Endian
>> CPU(s): 8
>> On-line CPU(s) list: 0-7
>> Thread(s) per core: 2
>> Core(s) per socket: 4
>> Socket(s): 1
>> NUMA node(s): 1
>> Vendor ID: ARM
>> Model: 2
>> Model name: Cortex-A72
>> Stepping: r0p2
>> BogoMIPS: 100.00
>> NUMA node0 CPU(s): 0-7
>>
>> cat /sys/devices/system/cpu/present --> 0-7
>> cat /sys/devices/system/cpu/possible --> 0-7
>>
>> ---
>>
>> Changelogs:
>>
>> v4->v5:
>> - drop the added -smp "expose=on|off" parameter and only describe topology
>> for 6.2 and newer machines
>> - rebased the code on patch series [3] which has introduced some fix and
>> improvement for smp parsing
>> - [3]: https://lore.kernel.org/qemu-devel/20210803080527.156556-1-wangyanan55@huawei.com/
>>
>> v3->v4:
>> - add new -smp parameter "expose=on|off" for users to enable/disable the feature
>> - add stricter -smp cmdline parsing rules on "expose=on" case
>> - move build_pptt to generic aml-build.c
>> - add default cluster node in the cpu-map
>> - rebase on top of latest upstream master
>> - v3: https://lore.kernel.org/qemu-devel/20210516102900.28036-1-wangyanan55@huawei.com/
>>
>> v2->v3:
>> - address comments from David, Philippe, and Andrew. Thanks!
>> - split some change into separate commits for ease of review
>> - adjust parsing rules of virt_smp_parse to be more strict
>> (after discussion with Andrew)
>> - adjust author credit for the patches
>>
>> v1->v2:
>> - Address Andrew Jones's comments
>> - Address Michael S. Tsirkin's comments
>>
>> ---
>>
>> Andrew Jones (2):
>> hw/arm/virt: Add cpu-map to device tree
>> hw/acpi/aml-build: Generate PPTT table
>>
>> Yanan Wang (3):
>> hw/arm/virt: Only describe cpu topology to guest since virt 6.2
>> device_tree: Add qemu_fdt_add_path
>> hw/acpi/aml-build: Add Processor hierarchy node structure
>>
>> hw/acpi/aml-build.c | 76 ++++++++++++++++++++++++++++++++++++
>> hw/arm/virt-acpi-build.c | 8 +++-
>> hw/arm/virt.c | 62 ++++++++++++++++++++++++-----
>> include/hw/acpi/aml-build.h | 7 ++++
>> include/hw/arm/virt.h | 4 +-
>> include/sysemu/device_tree.h | 1 +
>> softmmu/device_tree.c | 44 ++++++++++++++++++++-
>> 7 files changed, 188 insertions(+), 14 deletions(-)
>>
>> --
>> 2.19.1
>
> .
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-08-24 3:20 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-05 12:39 [PATCH for-6.2 v5 0/5] hw/arm/virt: Introduce cpu topology support Yanan Wang
2021-08-05 12:39 ` [PATCH v5 1/5] hw/arm/virt: Only describe cpu topology to guest since virt 6.2 Yanan Wang
2021-08-05 12:39 ` [PATCH for-6.2 v5 2/5] device_tree: Add qemu_fdt_add_path Yanan Wang
2021-08-05 12:39 ` [PATCH for-6.2 v5 3/5] hw/arm/virt: Add cpu-map to device tree Yanan Wang
2021-08-17 2:10 ` wangyanan (Y)
2021-08-17 11:51 ` Andrew Jones
2021-08-17 13:54 ` wangyanan (Y)
2021-08-05 12:39 ` [PATCH for-6.2 v5 4/5] hw/acpi/aml-build: Add Processor hierarchy node structure Yanan Wang
2021-08-05 12:39 ` [PATCH for-6.2 v5 5/5] hw/acpi/aml-build: Generate PPTT table Yanan Wang
2021-08-23 23:52 ` Michael S. Tsirkin
2021-08-24 3:19 ` wangyanan (Y)
2021-08-23 23:53 ` [PATCH for-6.2 v5 0/5] hw/arm/virt: Introduce cpu topology support Michael S. Tsirkin
2021-08-24 3:19 ` wangyanan (Y)
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.