|
| From: | wangyanan (Y) |
| Subject: | Re: [RFC PATCH v3 8/9] hw/arm/virt-acpi-build: Generate PPTT table |
| Date: | Mon, 17 May 2021 21:43:34 +0800 |
| User-agent: | Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.4.0 |
Hi Drew, On 2021/5/17 16:02, Andrew Jones wrote:
On Sun, May 16, 2021 at 06:28:59PM +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 guest, we use sequential integers starting from zero for non-leaf nodes without valid ID flag, so that the guest will ignore them and use table offsets as the unique IDs. And we also 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/arm/virt-acpi-build.c | 58 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-)Why aren't we adding build_pptt to aml-build.c, like my original patch does? I don't see anything Arm specific below, at least not if you passed MachineState instead of VirtMachineState, like my original patch did.
I agree to move build_pptt to common code, so that other platforms can also use it if they want. I will do it in next version. BTW, it seems patch 1 and 5 were possibly missed for some review. Any comments for them too? Thanks!
diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 4d64aeb865..b03d57745a 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -435,6 +435,57 @@ build_srat(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) vms->oem_table_id); }+/* ACPI 6.2: 5.2.29 Processor Properties Topology Table (PPTT) */+static void build_pptt(GArray *table_data, BIOSLinker *linker, + VirtMachineState *vms) +{ + MachineState *ms = MACHINE(vms); + int pptt_start = table_data->len; + int uid = 0, 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) {We can't have threads < 1, so this condition should be == 1.
Right, I will fix it. Thanks, Yanan
+ 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); + } else { + 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); + } + } + } + } + + build_header(linker, table_data, + (void *)(table_data->data + pptt_start), "PPTT", + table_data->len - pptt_start, 2, + vms->oem_id, vms->oem_table_id); +} + /* GTDT */ static void build_gtdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms) @@ -719,13 +770,18 @@ 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, vms); + } + acpi_add_table(table_offsets, tables_blob); build_gtdt(tables_blob, tables->linker, vms);--2.19.1Thanks, drew .
| [Prev in Thread] | Current Thread | [Next in Thread] |