| Message ID | 20201020131440.1090-9-fangying1@huawei.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | hw/arm/virt: Introduce cpu and cache topology support | expand |
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 <fangying1@huawei.com> > Signed-off-by: Henglong Fan <fanhenglong@huawei.com> > --- > 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
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 <fangying1@huawei.com> > Signed-off-by: Henglong Fan <fanhenglong@huawei.com> > --- > 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 > >
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 <fangying1@huawei.com> >> Signed-off-by: Henglong Fan <fanhenglong@huawei.com> >> --- >> 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 >> >> > > . >
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);