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

2020-11-02 Thread Ying Fang 




On 10/30/2020 1:24 AM, Andrew Jones wrote:

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

Add the processor hierarchy node structures to build ACPI information
for CPU topology. Three helpers are introduced:

(1) build_socket_hierarchy for socket description structure
(2) build_processor_hierarchy for processor description structure
(3) build_smt_hierarchy for thread (logic processor) description structure


I see now the reason to introduce three functions is because the last
patch adds different private resources. You should point that plan out
in this commit message.


Yes, the private resources are used to describe cache hierarchy
and it is variable among different topology level. I will point it
out in the commit message to avoid any confusion.

Thanks,
Ying



Thanks,
drew



Signed-off-by: Ying Fang 
Signed-off-by: Henglong Fan 
---
  hw/acpi/aml-build.c | 37 +
  include/hw/acpi/aml-build.h |  7 +++
  2 files changed, 44 insertions(+)

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 3792ba96ce..da3b41b514 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1770,6 +1770,43 @@ void build_slit(GArray *table_data, BIOSLinker *linker, 
MachineState *ms)
   table_data->len - slit_start, 1, NULL, NULL);
  }
  
+/*

+ * ACPI 6.3: 5.2.29.1 Processor hierarchy node structure (Type 0)
+ */
+void build_socket_hierarchy(GArray *tbl, uint32_t parent, uint32_t id)
+{
+build_append_byte(tbl, 0);  /* Type 0 - processor */
+build_append_byte(tbl, 20); /* Length, no private resources */
+build_append_int_noprefix(tbl, 0, 2);  /* Reserved */
+build_append_int_noprefix(tbl, 1, 4);  /* Flags: Physical package */
+build_append_int_noprefix(tbl, parent, 4);  /* Parent */
+build_append_int_noprefix(tbl, id, 4); /* ACPI processor ID */
+build_append_int_noprefix(tbl, 0, 4);  /* Number of private resources */
+}
+
+void build_processor_hierarchy(GArray *tbl, uint32_t flags,
+   uint32_t parent, uint32_t id)
+{
+build_append_byte(tbl, 0);  /* Type 0 - processor */
+build_append_byte(tbl, 20); /* Length, no private resources */
+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 */
+build_append_int_noprefix(tbl, 0, 4);  /* Number of private resources */
+}
+
+void build_smt_hierarchy(GArray *tbl, uint32_t parent, uint32_t id)
+{
+build_append_byte(tbl, 0);/* Type 0 - processor */
+build_append_byte(tbl, 20);   /* Length, add private resources */
+build_append_int_noprefix(tbl, 0, 2); /* Reserved */
+build_append_int_noprefix(tbl, 0x0e, 4);/* Processor is a thread */
+build_append_int_noprefix(tbl, parent , 4); /* parent */
+build_append_int_noprefix(tbl, id, 4);  /* ACPI processor ID */
+build_append_int_noprefix(tbl, 0, 4);   /* Num of private resources */
+}
+
  /* 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 fe0055fffb..56474835a7 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -437,6 +437,13 @@ void build_srat_memory(AcpiSratMemoryAffinity *numamem, 
uint64_t base,
  
  void build_slit(GArray *table_data, BIOSLinker *linker, MachineState *ms);
  
+void build_socket_hierarchy(GArray *tbl, uint32_t parent, uint32_t id);

+
+void build_processor_hierarchy(GArray *tbl, uint32_t flags,
+   uint32_t parent, uint32_t id);
+
+void build_smt_hierarchy(GArray *tbl, uint32_t parent, uint32_t id);
+
  void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
  const char *oem_id, const char *oem_table_id);
  
--

2.23.0




.

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

2020-10-29 Thread Andrew Jones 
On Tue, Oct 20, 2020 at 09:14:35PM +0800, Ying Fang wrote:
> Add the processor hierarchy node structures to build ACPI information
> for CPU topology. Three helpers are introduced:
> 
> (1) build_socket_hierarchy for socket description structure
> (2) build_processor_hierarchy for processor description structure
> (3) build_smt_hierarchy for thread (logic processor) description structure

I see now the reason to introduce three functions is because the last
patch adds different private resources. You should point that plan out
in this commit message.

Thanks,
drew

> 
> Signed-off-by: Ying Fang 
> Signed-off-by: Henglong Fan 
> ---
>  hw/acpi/aml-build.c | 37 +
>  include/hw/acpi/aml-build.h |  7 +++
>  2 files changed, 44 insertions(+)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 3792ba96ce..da3b41b514 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -1770,6 +1770,43 @@ void build_slit(GArray *table_data, BIOSLinker 
> *linker, MachineState *ms)
>   table_data->len - slit_start, 1, NULL, NULL);
>  }
>  
> +/*
> + * ACPI 6.3: 5.2.29.1 Processor hierarchy node structure (Type 0)
> + */
> +void build_socket_hierarchy(GArray *tbl, uint32_t parent, uint32_t id)
> +{
> +build_append_byte(tbl, 0);  /* Type 0 - processor */
> +build_append_byte(tbl, 20); /* Length, no private resources */
> +build_append_int_noprefix(tbl, 0, 2);  /* Reserved */
> +build_append_int_noprefix(tbl, 1, 4);  /* Flags: Physical package */
> +build_append_int_noprefix(tbl, parent, 4);  /* Parent */
> +build_append_int_noprefix(tbl, id, 4); /* ACPI processor ID */
> +build_append_int_noprefix(tbl, 0, 4);  /* Number of private resources */
> +}
> +
> +void build_processor_hierarchy(GArray *tbl, uint32_t flags,
> +   uint32_t parent, uint32_t id)
> +{
> +build_append_byte(tbl, 0);  /* Type 0 - processor */
> +build_append_byte(tbl, 20); /* Length, no private resources */
> +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 */
> +build_append_int_noprefix(tbl, 0, 4);  /* Number of private resources */
> +}
> +
> +void build_smt_hierarchy(GArray *tbl, uint32_t parent, uint32_t id)
> +{
> +build_append_byte(tbl, 0);/* Type 0 - processor */
> +build_append_byte(tbl, 20);   /* Length, add private resources */
> +build_append_int_noprefix(tbl, 0, 2); /* Reserved */
> +build_append_int_noprefix(tbl, 0x0e, 4);/* Processor is a thread */
> +build_append_int_noprefix(tbl, parent , 4); /* parent */
> +build_append_int_noprefix(tbl, id, 4);  /* ACPI processor ID */
> +build_append_int_noprefix(tbl, 0, 4);   /* Num of private resources 
> */
> +}
> +
>  /* 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 fe0055fffb..56474835a7 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -437,6 +437,13 @@ void build_srat_memory(AcpiSratMemoryAffinity *numamem, 
> uint64_t base,
>  
>  void build_slit(GArray *table_data, BIOSLinker *linker, MachineState *ms);
>  
> +void build_socket_hierarchy(GArray *tbl, uint32_t parent, uint32_t id);
> +
> +void build_processor_hierarchy(GArray *tbl, uint32_t flags,
> +   uint32_t parent, uint32_t id);
> +
> +void build_smt_hierarchy(GArray *tbl, uint32_t parent, uint32_t id);
> +
>  void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
>  const char *oem_id, const char *oem_table_id);
>  
> -- 
> 2.23.0
> 
>

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

2020-10-29 Thread Andrew Jones 
On Tue, Oct 20, 2020 at 09:14:35PM +0800, Ying Fang wrote:
> Add the processor hierarchy node structures to build ACPI information
> for CPU topology. Three helpers are introduced:
> 
> (1) build_socket_hierarchy for socket description structure
> (2) build_processor_hierarchy for processor description structure
> (3) build_smt_hierarchy for thread (logic processor) description structure
> 
> Signed-off-by: Ying Fang 
> Signed-off-by: Henglong Fan 
> ---
>  hw/acpi/aml-build.c | 37 +
>  include/hw/acpi/aml-build.h |  7 +++
>  2 files changed, 44 insertions(+)
> 
> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
> index 3792ba96ce..da3b41b514 100644
> --- a/hw/acpi/aml-build.c
> +++ b/hw/acpi/aml-build.c
> @@ -1770,6 +1770,43 @@ void build_slit(GArray *table_data, BIOSLinker 
> *linker, MachineState *ms)
>   table_data->len - slit_start, 1, NULL, NULL);
>  }
>  
> +/*
> + * ACPI 6.3: 5.2.29.1 Processor hierarchy node structure (Type 0)
> + */
> +void build_socket_hierarchy(GArray *tbl, uint32_t parent, uint32_t id)
> +{
> +build_append_byte(tbl, 0);  /* Type 0 - processor */
> +build_append_byte(tbl, 20); /* Length, no private resources */
> +build_append_int_noprefix(tbl, 0, 2);  /* Reserved */
> +build_append_int_noprefix(tbl, 1, 4);  /* Flags: Physical package */
> +build_append_int_noprefix(tbl, parent, 4);  /* Parent */
> +build_append_int_noprefix(tbl, id, 4); /* ACPI processor ID */
> +build_append_int_noprefix(tbl, 0, 4);  /* Number of private resources */
> +}
> +
> +void build_processor_hierarchy(GArray *tbl, uint32_t flags,
> +   uint32_t parent, uint32_t id)
> +{
> +build_append_byte(tbl, 0);  /* Type 0 - processor */
> +build_append_byte(tbl, 20); /* Length, no private resources */
> +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 */
> +build_append_int_noprefix(tbl, 0, 4);  /* Number of private resources */
> +}
> +
> +void build_smt_hierarchy(GArray *tbl, uint32_t parent, uint32_t id)
> +{
> +build_append_byte(tbl, 0);/* Type 0 - processor */
> +build_append_byte(tbl, 20);   /* Length, add private resources */
> +build_append_int_noprefix(tbl, 0, 2); /* Reserved */
> +build_append_int_noprefix(tbl, 0x0e, 4);/* Processor is a thread */
> +build_append_int_noprefix(tbl, parent , 4); /* parent */
> +build_append_int_noprefix(tbl, id, 4);  /* ACPI processor ID */
> +build_append_int_noprefix(tbl, 0, 4);   /* Num of private resources 
> */
> +}
> +
>  /* 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 fe0055fffb..56474835a7 100644
> --- a/include/hw/acpi/aml-build.h
> +++ b/include/hw/acpi/aml-build.h
> @@ -437,6 +437,13 @@ void build_srat_memory(AcpiSratMemoryAffinity *numamem, 
> uint64_t base,
>  
>  void build_slit(GArray *table_data, BIOSLinker *linker, MachineState *ms);
>  
> +void build_socket_hierarchy(GArray *tbl, uint32_t parent, uint32_t id);
> +
> +void build_processor_hierarchy(GArray *tbl, uint32_t flags,
> +   uint32_t parent, uint32_t id);
> +
> +void build_smt_hierarchy(GArray *tbl, uint32_t parent, uint32_t id);
> +
>  void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
>  const char *oem_id, const char *oem_table_id);
>  
> -- 
> 2.23.0
> 
>

I don't think we need three 99% identical functions that only differ by a
flags field, especially when one of the functions is the generic form that
takes flags as an argument. At the very least this should be

void build_processor_hierarchy(tbl, flags, parent id)
{
  ...
}

void build_socket_hierarchy(tbl, parent, id)
{
  build_processor_hierarchy(tbl, 1, parent, id);
}

void build_smt_hierarchy(tbl, parent, id)
{
  build_processor_hierarchy(tbl, 0xe, parent, id);
}

Thanks,
drew

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

2020-10-20 Thread Ying Fang 
Add the processor hierarchy node structures to build ACPI information
for CPU topology. Three helpers are introduced:

(1) build_socket_hierarchy for socket description structure
(2) build_processor_hierarchy for processor description structure
(3) build_smt_hierarchy for thread (logic processor) description structure

Signed-off-by: Ying Fang 
Signed-off-by: Henglong Fan 
---
 hw/acpi/aml-build.c | 37 +
 include/hw/acpi/aml-build.h |  7 +++
 2 files changed, 44 insertions(+)

diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
index 3792ba96ce..da3b41b514 100644
--- a/hw/acpi/aml-build.c
+++ b/hw/acpi/aml-build.c
@@ -1770,6 +1770,43 @@ void build_slit(GArray *table_data, BIOSLinker *linker, 
MachineState *ms)
  table_data->len - slit_start, 1, NULL, NULL);
 }
 
+/*
+ * ACPI 6.3: 5.2.29.1 Processor hierarchy node structure (Type 0)
+ */
+void build_socket_hierarchy(GArray *tbl, uint32_t parent, uint32_t id)
+{
+build_append_byte(tbl, 0);  /* Type 0 - processor */
+build_append_byte(tbl, 20); /* Length, no private resources */
+build_append_int_noprefix(tbl, 0, 2);  /* Reserved */
+build_append_int_noprefix(tbl, 1, 4);  /* Flags: Physical package */
+build_append_int_noprefix(tbl, parent, 4);  /* Parent */
+build_append_int_noprefix(tbl, id, 4); /* ACPI processor ID */
+build_append_int_noprefix(tbl, 0, 4);  /* Number of private resources */
+}
+
+void build_processor_hierarchy(GArray *tbl, uint32_t flags,
+   uint32_t parent, uint32_t id)
+{
+build_append_byte(tbl, 0);  /* Type 0 - processor */
+build_append_byte(tbl, 20); /* Length, no private resources */
+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 */
+build_append_int_noprefix(tbl, 0, 4);  /* Number of private resources */
+}
+
+void build_smt_hierarchy(GArray *tbl, uint32_t parent, uint32_t id)
+{
+build_append_byte(tbl, 0);/* Type 0 - processor */
+build_append_byte(tbl, 20);   /* Length, add private resources */
+build_append_int_noprefix(tbl, 0, 2); /* Reserved */
+build_append_int_noprefix(tbl, 0x0e, 4);/* Processor is a thread */
+build_append_int_noprefix(tbl, parent , 4); /* parent */
+build_append_int_noprefix(tbl, id, 4);  /* ACPI processor ID */
+build_append_int_noprefix(tbl, 0, 4);   /* Num of private resources */
+}
+
 /* 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 fe0055fffb..56474835a7 100644
--- a/include/hw/acpi/aml-build.h
+++ b/include/hw/acpi/aml-build.h
@@ -437,6 +437,13 @@ void build_srat_memory(AcpiSratMemoryAffinity *numamem, 
uint64_t base,
 
 void build_slit(GArray *table_data, BIOSLinker *linker, MachineState *ms);
 
+void build_socket_hierarchy(GArray *tbl, uint32_t parent, uint32_t id);
+
+void build_processor_hierarchy(GArray *tbl, uint32_t flags,
+   uint32_t parent, uint32_t id);
+
+void build_smt_hierarchy(GArray *tbl, uint32_t parent, uint32_t id);
+
 void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f,
 const char *oem_id, const char *oem_table_id);
 
-- 
2.23.0