Re: [RFC PATCH v2 09/13] hw/arm/virt-acpi-build: add PPTT table

2020-11-02 Thread Ying Fang 




On 10/30/2020 12:56 AM, Andrew Jones wrote:

On Tue, Oct 20, 2020 at 09:14:36PM +0800, Ying Fang wrote:

Add the Processor Properties Topology Table (PPTT) to present CPU topology
information to the guest.

Signed-off-by: Andrew Jones 


I don't know why I have an s-o-b here. I guess it's because this code
looks nearly identical to what I wrote, except for using the new and,
IMO, unnecessary build_socket_hierarchy and build_smt_hierarchy functions.

IMHO, you should drop the last patch and just take

https://github.com/rhdrjones/qemu/commit/439b38d67ca1f2cbfa5b9892a822b651ebd05c11

as it is, unless it needs to be fixed somehow

Thanks,
drew


This patch is based on your branch however it is slightly modified.
As described in:

[RFC,v2,08/13] hw/acpi/aml-build: add processor hierarchy node structure

The wrapper function build_socket_hierarchy and build_smt_hierarchy are
introduced to make later patch much more readable and make preparations 
for cache hierarchy.


Hope it won't make you confused. I will drop your branch patch and
give details in the commit message in the next post.

Thanks,
Ying



Signed-off-by: Ying Fang 
---
  hw/arm/virt-acpi-build.c | 42 
  1 file changed, 42 insertions(+)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index fae5a26741..e1f3ea50ad 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -429,6 +429,42 @@ build_srat(GArray *table_data, BIOSLinker *linker, 
VirtMachineState *vms)
   "SRAT", table_data->len - srat_start, 3, NULL, NULL);
  }
  
+static void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState *ms)

+{
+int pptt_start = table_data->len;
+int uid = 0, cpus = 0, socket;
+unsigned int smp_cores = ms->smp.cores;
+unsigned int smp_threads = ms->smp.threads;
+
+acpi_data_push(table_data, sizeof(AcpiTableHeader));
+
+for (socket = 0; cpus < ms->possible_cpus->len; socket++) {
+uint32_t socket_offset = table_data->len - pptt_start;
+int core;
+
+build_socket_hierarchy(table_data, 0, socket);
+
+for (core = 0; core < smp_cores; core++) {
+uint32_t core_offset = table_data->len - pptt_start;
+int thread;
+
+if (smp_threads <= 1) {
+build_processor_hierarchy(table_data, 2, socket_offset, uid++);
+ } else {
+build_processor_hierarchy(table_data, 0, socket_offset, core);
+for (thread = 0; thread < smp_threads; thread++) {
+build_smt_hierarchy(table_data, core_offset, uid++);
+}
+ }
+}
+cpus += smp_cores * smp_threads;
+}
+
+build_header(linker, table_data,
+ (void *)(table_data->data + pptt_start), "PPTT",
+ table_data->len - pptt_start, 2, NULL, NULL);
+}
+
  /* GTDT */
  static void
  build_gtdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
@@ -669,6 +705,7 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables 
*tables)
  unsigned dsdt, xsdt;
  GArray *tables_blob = tables->table_data;
  MachineState *ms = MACHINE(vms);
+bool cpu_topology_enabled = !vmc->ignore_cpu_topology;
  
  table_offsets = g_array_new(false, true /* clear */,

  sizeof(uint32_t));
@@ -688,6 +725,11 @@ void virt_acpi_build(VirtMachineState *vms, 
AcpiBuildTables *tables)
  acpi_add_table(table_offsets, tables_blob);
  build_madt(tables_blob, tables->linker, vms);
  
+if (cpu_topology_enabled) {

+acpi_add_table(table_offsets, tables_blob);
+build_pptt(tables_blob, tables->linker, ms);
+}
+
  acpi_add_table(table_offsets, tables_blob);
  build_gtdt(tables_blob, tables->linker, vms);
  
--

2.23.0




.

Re: [RFC PATCH v2 09/13] hw/arm/virt-acpi-build: add PPTT table

2020-10-29 Thread Andrew Jones 
On Tue, Oct 20, 2020 at 09:14:36PM +0800, Ying Fang wrote:
> Add the Processor Properties Topology Table (PPTT) to present CPU topology
> information to the guest.
> 
> Signed-off-by: Andrew Jones 

I don't know why I have an s-o-b here. I guess it's because this code
looks nearly identical to what I wrote, except for using the new and,
IMO, unnecessary build_socket_hierarchy and build_smt_hierarchy functions.

IMHO, you should drop the last patch and just take 

https://github.com/rhdrjones/qemu/commit/439b38d67ca1f2cbfa5b9892a822b651ebd05c11

as it is, unless it needs to be fixed somehow.

Thanks,
drew

> Signed-off-by: Ying Fang 
> ---
>  hw/arm/virt-acpi-build.c | 42 
>  1 file changed, 42 insertions(+)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index fae5a26741..e1f3ea50ad 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -429,6 +429,42 @@ build_srat(GArray *table_data, BIOSLinker *linker, 
> VirtMachineState *vms)
>   "SRAT", table_data->len - srat_start, 3, NULL, NULL);
>  }
>  
> +static void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState 
> *ms)
> +{
> +int pptt_start = table_data->len;
> +int uid = 0, cpus = 0, socket;
> +unsigned int smp_cores = ms->smp.cores;
> +unsigned int smp_threads = ms->smp.threads;
> +
> +acpi_data_push(table_data, sizeof(AcpiTableHeader));
> +
> +for (socket = 0; cpus < ms->possible_cpus->len; socket++) {
> +uint32_t socket_offset = table_data->len - pptt_start;
> +int core;
> +
> +build_socket_hierarchy(table_data, 0, socket);
> +
> +for (core = 0; core < smp_cores; core++) {
> +uint32_t core_offset = table_data->len - pptt_start;
> +int thread;
> +
> +if (smp_threads <= 1) {
> +build_processor_hierarchy(table_data, 2, socket_offset, 
> uid++);
> + } else {
> +build_processor_hierarchy(table_data, 0, socket_offset, 
> core);
> +for (thread = 0; thread < smp_threads; thread++) {
> +build_smt_hierarchy(table_data, core_offset, uid++);
> +}
> + }
> +}
> +cpus += smp_cores * smp_threads;
> +}
> +
> +build_header(linker, table_data,
> + (void *)(table_data->data + pptt_start), "PPTT",
> + table_data->len - pptt_start, 2, NULL, NULL);
> +}
> +
>  /* GTDT */
>  static void
>  build_gtdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> @@ -669,6 +705,7 @@ void virt_acpi_build(VirtMachineState *vms, 
> AcpiBuildTables *tables)
>  unsigned dsdt, xsdt;
>  GArray *tables_blob = tables->table_data;
>  MachineState *ms = MACHINE(vms);
> +bool cpu_topology_enabled = !vmc->ignore_cpu_topology;
>  
>  table_offsets = g_array_new(false, true /* clear */,
>  sizeof(uint32_t));
> @@ -688,6 +725,11 @@ void virt_acpi_build(VirtMachineState *vms, 
> AcpiBuildTables *tables)
>  acpi_add_table(table_offsets, tables_blob);
>  build_madt(tables_blob, tables->linker, vms);
>  
> +if (cpu_topology_enabled) {
> +acpi_add_table(table_offsets, tables_blob);
> +build_pptt(tables_blob, tables->linker, ms);
> +}
> +
>  acpi_add_table(table_offsets, tables_blob);
>  build_gtdt(tables_blob, tables->linker, vms);
>  
> -- 
> 2.23.0
> 
>

[RFC PATCH v2 09/13] hw/arm/virt-acpi-build: add PPTT table

2020-10-20 Thread Ying Fang 
Add the Processor Properties Topology Table (PPTT) to present CPU topology
information to the guest.

Signed-off-by: Andrew Jones 
Signed-off-by: Ying Fang 
---
 hw/arm/virt-acpi-build.c | 42 
 1 file changed, 42 insertions(+)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index fae5a26741..e1f3ea50ad 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -429,6 +429,42 @@ build_srat(GArray *table_data, BIOSLinker *linker, 
VirtMachineState *vms)
  "SRAT", table_data->len - srat_start, 3, NULL, NULL);
 }
 
+static void build_pptt(GArray *table_data, BIOSLinker *linker, MachineState 
*ms)
+{
+int pptt_start = table_data->len;
+int uid = 0, cpus = 0, socket;
+unsigned int smp_cores = ms->smp.cores;
+unsigned int smp_threads = ms->smp.threads;
+
+acpi_data_push(table_data, sizeof(AcpiTableHeader));
+
+for (socket = 0; cpus < ms->possible_cpus->len; socket++) {
+uint32_t socket_offset = table_data->len - pptt_start;
+int core;
+
+build_socket_hierarchy(table_data, 0, socket);
+
+for (core = 0; core < smp_cores; core++) {
+uint32_t core_offset = table_data->len - pptt_start;
+int thread;
+
+if (smp_threads <= 1) {
+build_processor_hierarchy(table_data, 2, socket_offset, uid++);
+ } else {
+build_processor_hierarchy(table_data, 0, socket_offset, core);
+for (thread = 0; thread < smp_threads; thread++) {
+build_smt_hierarchy(table_data, core_offset, uid++);
+}
+ }
+}
+cpus += smp_cores * smp_threads;
+}
+
+build_header(linker, table_data,
+ (void *)(table_data->data + pptt_start), "PPTT",
+ table_data->len - pptt_start, 2, NULL, NULL);
+}
+
 /* GTDT */
 static void
 build_gtdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
@@ -669,6 +705,7 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables 
*tables)
 unsigned dsdt, xsdt;
 GArray *tables_blob = tables->table_data;
 MachineState *ms = MACHINE(vms);
+bool cpu_topology_enabled = !vmc->ignore_cpu_topology;
 
 table_offsets = g_array_new(false, true /* clear */,
 sizeof(uint32_t));
@@ -688,6 +725,11 @@ void virt_acpi_build(VirtMachineState *vms, 
AcpiBuildTables *tables)
 acpi_add_table(table_offsets, tables_blob);
 build_madt(tables_blob, tables->linker, vms);
 
+if (cpu_topology_enabled) {
+acpi_add_table(table_offsets, tables_blob);
+build_pptt(tables_blob, tables->linker, ms);
+}
+
 acpi_add_table(table_offsets, tables_blob);
 build_gtdt(tables_blob, tables->linker, vms);
 
-- 
2.23.0