[v3] Building PPTT with root node and identical implementation flag

[PATCH v3 3/5] hw/acpi/aml-build: Build a root node in the PPTT table

Posted by Alireza Sanaee via 6 months, 3 weeks ago

From: Yicong Yang <yangyicong@hisilicon.com>

Currently we build the PPTT starting from the socket node and each
socket will be a separate tree. For a multi-socket system it'll
be hard for the OS to know the whole system is homogeneous or not
(actually we're in the current implementation) since no parent node
to telling the identical implementation informentation. Add a
root node for indicating this.

Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Signed-off-by: Alireza Sanaee <alireza.sanaee@huawei.com>
---
 hw/acpi/aml-build.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 560cee12a24b..3010325ca423 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -2153,12 +2153,25 @@ void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms,
     int64_t socket_id = -1, cluster_id = -1, core_id = -1;
     uint32_t socket_offset = 0, cluster_offset = 0, core_offset = 0;
     uint32_t pptt_start = table_data->len;
+    uint32_t root_offset;
     int n;
     AcpiTable table = { .sig = "PPTT", .rev = 2,
                         .oem_id = oem_id, .oem_table_id = oem_table_id };
 
     acpi_table_begin(&table, table_data);
 
+    /*
+     * Build a root node for all the processor nodes. Otherwise when
+     * building a multi-socket system each socket tree are separated
+     * and will be hard for the OS like Linux to know whether the
+     * system is homogeneous.
+     */
+    root_offset = table_data->len - pptt_start;
+    build_processor_hierarchy_node(table_data,
+        (1 << 0) | /* Physical package */
+        (1 << 4), /* Identical Implementation */
+        0, 0, NULL, 0);
+
     /*
      * This works with the assumption that cpus[n].props.*_id has been
      * sorted from top to down levels in mc->possible_cpu_arch_ids().
@@ -2175,7 +2188,7 @@ void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms,
             build_processor_hierarchy_node(table_data,
                 (1 << 0) | /* Physical package */
                 (1 << 4), /* Identical Implementation */
-                0, socket_id, NULL, 0);
+                root_offset, socket_id, NULL, 0);
         }
 
         if (mc->smp_props.clusters_supported && mc->smp_props.has_clusters) {
-- 
2.34.1

Re: [PATCH v3 3/5] hw/acpi/aml-build: Build a root node in the PPTT table

Posted by Michael S. Tsirkin 6 months, 3 weeks ago

On Wed, Apr 23, 2025 at 12:41:28PM +0100, Alireza Sanaee wrote:
> From: Yicong Yang <yangyicong@hisilicon.com>
> 
> Currently we build the PPTT starting from the socket node and each
> socket will be a separate tree. For a multi-socket system it'll
> be hard for the OS to know the whole system is homogeneous or not
> (actually we're in the current implementation) since no parent node
> to telling the identical implementation informentation. Add a
> root node for indicating this.
> 
> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> Signed-off-by: Alireza Sanaee <alireza.sanaee@huawei.com>

so how does the topology look before and after this change?


> ---
>  hw/acpi/aml-build.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 560cee12a24b..3010325ca423 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -2153,12 +2153,25 @@ void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms,
>      int64_t socket_id = -1, cluster_id = -1, core_id = -1;
>      uint32_t socket_offset = 0, cluster_offset = 0, core_offset = 0;
>      uint32_t pptt_start = table_data->len;
> +    uint32_t root_offset;
>      int n;
>      AcpiTable table = { .sig = "PPTT", .rev = 2,
>                          .oem_id = oem_id, .oem_table_id = oem_table_id };
>  
>      acpi_table_begin(&table, table_data);
>  
> +    /*
> +     * Build a root node for all the processor nodes. Otherwise when
> +     * building a multi-socket system each socket tree are separated

is separated?

> +     * and will be hard for the OS like Linux to know whether the
> +     * system is homogeneous.
> +     */
> +    root_offset = table_data->len - pptt_start;
> +    build_processor_hierarchy_node(table_data,
> +        (1 << 0) | /* Physical package */
> +        (1 << 4), /* Identical Implementation */
> +        0, 0, NULL, 0);
> +
>      /*
>       * This works with the assumption that cpus[n].props.*_id has been
>       * sorted from top to down levels in mc->possible_cpu_arch_ids().
> @@ -2175,7 +2188,7 @@ void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms,
>              build_processor_hierarchy_node(table_data,
>                  (1 << 0) | /* Physical package */
>                  (1 << 4), /* Identical Implementation */
> -                0, socket_id, NULL, 0);
> +                root_offset, socket_id, NULL, 0);
>          }
>  
>          if (mc->smp_props.clusters_supported && mc->smp_props.has_clusters) {
> -- 
> 2.34.1

Re: [PATCH v3 3/5] hw/acpi/aml-build: Build a root node in the PPTT table

Posted by Yicong Yang via 6 months, 3 weeks ago

On 2025/4/23 20:41, Michael S. Tsirkin wrote:
> On Wed, Apr 23, 2025 at 12:41:28PM +0100, Alireza Sanaee wrote:
>> From: Yicong Yang <yangyicong@hisilicon.com>
>>
>> Currently we build the PPTT starting from the socket node and each
>> socket will be a separate tree. For a multi-socket system it'll
>> be hard for the OS to know the whole system is homogeneous or not
>> (actually we're in the current implementation) since no parent node
>> to telling the identical implementation informentation. Add a
>> root node for indicating this.
>>
>> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com>
>> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>> Signed-off-by: Alireza Sanaee <alireza.sanaee@huawei.com>
> 
> so how does the topology look before and after this change?
> 

for a 2 socket system, the PPTT processor hierarchy tree before this change will be like:
[Socket 0]          [Socket 1]
   ^                    ^
   |-----------\        |-----------\
[CPU 0] ... [CPU 1]  [CPU 0] ... [CPU 1]

after this change there will be a root node in the tree:
[Root Node]
    ^
    |-------------------\
[Socket 0]          [Socket 1]
   ^                    ^
   |-----------\        |-----------\
[CPU 0] ... [CPU 1]  [CPU 0] ... [CPU 1]

> 
>> ---
>>  hw/acpi/aml-build.c | 15 ++++++++++++++-
>>  1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
>> index 560cee12a24b..3010325ca423 100644
>> --- a/hw/acpi/aml-build.c
>> +++ b/hw/acpi/aml-build.c
>> @@ -2153,12 +2153,25 @@ void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms,
>>      int64_t socket_id = -1, cluster_id = -1, core_id = -1;
>>      uint32_t socket_offset = 0, cluster_offset = 0, core_offset = 0;
>>      uint32_t pptt_start = table_data->len;
>> +    uint32_t root_offset;
>>      int n;
>>      AcpiTable table = { .sig = "PPTT", .rev = 2,
>>                          .oem_id = oem_id, .oem_table_id = oem_table_id };
>>  
>>      acpi_table_begin(&table, table_data);
>>  
>> +    /*
>> +     * Build a root node for all the processor nodes. Otherwise when
>> +     * building a multi-socket system each socket tree are separated
> 
> is separated?
> 
>> +     * and will be hard for the OS like Linux to know whether the
>> +     * system is homogeneous.
>> +     */
>> +    root_offset = table_data->len - pptt_start;
>> +    build_processor_hierarchy_node(table_data,
>> +        (1 << 0) | /* Physical package */
>> +        (1 << 4), /* Identical Implementation */
>> +        0, 0, NULL, 0);
>> +
>>      /*
>>       * This works with the assumption that cpus[n].props.*_id has been
>>       * sorted from top to down levels in mc->possible_cpu_arch_ids().
>> @@ -2175,7 +2188,7 @@ void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms,
>>              build_processor_hierarchy_node(table_data,
>>                  (1 << 0) | /* Physical package */
>>                  (1 << 4), /* Identical Implementation */
>> -                0, socket_id, NULL, 0);
>> +                root_offset, socket_id, NULL, 0);
>>          }
>>  
>>          if (mc->smp_props.clusters_supported && mc->smp_props.has_clusters) {
>> -- 
>> 2.34.1
> 
> 
> .
>

[PATCH v3 1/5] tests: virt: Allow changes to PPTT test table
[PATCH v3 2/5] hw/acpi/aml-build: Set identical implementation flag for PPTT processor nodes
[PATCH v3 3/5] hw/acpi/aml-build: Build a root node in the PPTT table
[PATCH v3 4/5] hw/acpi/aml-build: Update the revision of PPTT table
[PATCH v3 5/5] tests: virt: Update expected ACPI tables for virt test