The ACPI MPAM table uses the UID of a processor container specified in
the PPTT to indicate the subset of CPUs and cache topology that can
access each MPAM System Component (MSC).
This information is not directly useful to the kernel. The equivalent
cpumask is needed instead.
Add a helper to find the processor container by its id, then walk
the possible CPUs to fill a cpumask with the CPUs that have this
processor container as a parent.
CC: Dave Martin <dave.martin@arm.com>
Reviewed-by: Sudeep Holla <sudeep.holla@arm.com>
Signed-off-by: James Morse <james.morse@arm.com>
---
Changes since v1:
* Replaced commit message with wording from Dave.
* Fixed a stray plural.
* Moved further down in the file to make use of get_pptt() helper.
* Added a break to exit the loop early.
Changes since RFC:
* Removed leaf_flag local variable from acpi_pptt_get_cpus_from_container()
Changes since RFC:
* Dropped has_leaf_flag dodging of acpi_pptt_leaf_node()
* Added missing : in kernel-doc
* Made helper return void as this never actually returns an error.
---
drivers/acpi/pptt.c | 83 ++++++++++++++++++++++++++++++++++++++++++++
include/linux/acpi.h | 3 ++
2 files changed, 86 insertions(+)
diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
index 54676e3d82dd..1728545d90b2 100644
--- a/drivers/acpi/pptt.c
+++ b/drivers/acpi/pptt.c
@@ -817,3 +817,86 @@ int find_acpi_cpu_topology_hetero_id(unsigned int cpu)
return find_acpi_cpu_topology_tag(cpu, PPTT_ABORT_PACKAGE,
ACPI_PPTT_ACPI_IDENTICAL);
}
+
+/**
+ * acpi_pptt_get_child_cpus() - Find all the CPUs below a PPTT processor node
+ * @table_hdr: A reference to the PPTT table.
+ * @parent_node: A pointer to the processor node in the @table_hdr.
+ * @cpus: A cpumask to fill with the CPUs below @parent_node.
+ *
+ * Walks up the PPTT from every possible CPU to find if the provided
+ * @parent_node is a parent of this CPU.
+ */
+static void acpi_pptt_get_child_cpus(struct acpi_table_header *table_hdr,
+ struct acpi_pptt_processor *parent_node,
+ cpumask_t *cpus)
+{
+ struct acpi_pptt_processor *cpu_node;
+ u32 acpi_id;
+ int cpu;
+
+ cpumask_clear(cpus);
+
+ for_each_possible_cpu(cpu) {
+ acpi_id = get_acpi_id_for_cpu(cpu);
+ cpu_node = acpi_find_processor_node(table_hdr, acpi_id);
+
+ while (cpu_node) {
+ if (cpu_node == parent_node) {
+ cpumask_set_cpu(cpu, cpus);
+ break;
+ }
+ cpu_node = fetch_pptt_node(table_hdr, cpu_node->parent);
+ }
+ }
+}
+
+/**
+ * acpi_pptt_get_cpus_from_container() - Populate a cpumask with all CPUs in a
+ * processor container
+ * @acpi_cpu_id: The UID of the processor container.
+ * @cpus: The resulting CPU mask.
+ *
+ * Find the specified Processor Container, and fill @cpus with all the cpus
+ * below it.
+ *
+ * Not all 'Processor' entries in the PPTT are either a CPU or a Processor
+ * Container, they may exist purely to describe a Private resource. CPUs
+ * have to be leaves, so a Processor Container is a non-leaf that has the
+ * 'ACPI Processor ID valid' flag set.
+ *
+ * Return: 0 for a complete walk, or an error if the mask is incomplete.
+ */
+void acpi_pptt_get_cpus_from_container(u32 acpi_cpu_id, cpumask_t *cpus)
+{
+ struct acpi_pptt_processor *cpu_node;
+ struct acpi_table_header *table_hdr;
+ struct acpi_subtable_header *entry;
+ unsigned long table_end;
+ u32 proc_sz;
+
+ cpumask_clear(cpus);
+
+ table_hdr = acpi_get_pptt();
+ if (!table_hdr)
+ return;
+
+ table_end = (unsigned long)table_hdr + table_hdr->length;
+ entry = ACPI_ADD_PTR(struct acpi_subtable_header, table_hdr,
+ sizeof(struct acpi_table_pptt));
+ proc_sz = sizeof(struct acpi_pptt_processor);
+ while ((unsigned long)entry + proc_sz <= table_end) {
+ cpu_node = (struct acpi_pptt_processor *)entry;
+ if (entry->type == ACPI_PPTT_TYPE_PROCESSOR &&
+ cpu_node->flags & ACPI_PPTT_ACPI_PROCESSOR_ID_VALID) {
+ if (!acpi_pptt_leaf_node(table_hdr, cpu_node)) {
+ if (cpu_node->acpi_processor_id == acpi_cpu_id) {
+ acpi_pptt_get_child_cpus(table_hdr, cpu_node, cpus);
+ break;
+ }
+ }
+ }
+ entry = ACPI_ADD_PTR(struct acpi_subtable_header, entry,
+ entry->length);
+ }
+}
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 1c5bb1e887cd..f97a9ff678cc 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1541,6 +1541,7 @@ int find_acpi_cpu_topology(unsigned int cpu, int level);
int find_acpi_cpu_topology_cluster(unsigned int cpu);
int find_acpi_cpu_topology_package(unsigned int cpu);
int find_acpi_cpu_topology_hetero_id(unsigned int cpu);
+void acpi_pptt_get_cpus_from_container(u32 acpi_cpu_id, cpumask_t *cpus);
#else
static inline int acpi_pptt_cpu_is_thread(unsigned int cpu)
{
@@ -1562,6 +1563,8 @@ static inline int find_acpi_cpu_topology_hetero_id(unsigned int cpu)
{
return -EINVAL;
}
+static inline void acpi_pptt_get_cpus_from_container(u32 acpi_cpu_id,
+ cpumask_t *cpus) { }
#endif
void acpi_arch_init(void);
--
2.39.5
On 9/10/25 13:42, James Morse wrote: > The ACPI MPAM table uses the UID of a processor container specified in > the PPTT to indicate the subset of CPUs and cache topology that can > access each MPAM System Component (MSC). > > This information is not directly useful to the kernel. The equivalent > cpumask is needed instead. > > Add a helper to find the processor container by its id, then walk > the possible CPUs to fill a cpumask with the CPUs that have this > processor container as a parent. > > CC: Dave Martin <dave.martin@arm.com> > Reviewed-by: Sudeep Holla <sudeep.holla@arm.com> > Signed-off-by: James Morse <james.morse@arm.com> Reviewed-by: Fenghua Yu <fenghuay@nvidia.com> Thanks. -Fenghua
On Wed, 10 Sep 2025 20:42:41 +0000 James Morse <james.morse@arm.com> wrote: > The ACPI MPAM table uses the UID of a processor container specified in > the PPTT to indicate the subset of CPUs and cache topology that can > access each MPAM System Component (MSC). > > This information is not directly useful to the kernel. The equivalent > cpumask is needed instead. > > Add a helper to find the processor container by its id, then walk > the possible CPUs to fill a cpumask with the CPUs that have this > processor container as a parent. > > CC: Dave Martin <dave.martin@arm.com> > Reviewed-by: Sudeep Holla <sudeep.holla@arm.com> > Signed-off-by: James Morse <james.morse@arm.com> Hi James, Sorry I missed v1. Busy few weeks. I think one resource leak plus a few suggested changes that I'm not that bothered about. Jonathan > --- > Changes since v1: > * Replaced commit message with wording from Dave. > * Fixed a stray plural. > * Moved further down in the file to make use of get_pptt() helper. > * Added a break to exit the loop early. > > Changes since RFC: > * Removed leaf_flag local variable from acpi_pptt_get_cpus_from_container() > > Changes since RFC: > * Dropped has_leaf_flag dodging of acpi_pptt_leaf_node() > * Added missing : in kernel-doc > * Made helper return void as this never actually returns an error. > --- > drivers/acpi/pptt.c | 83 ++++++++++++++++++++++++++++++++++++++++++++ > include/linux/acpi.h | 3 ++ > 2 files changed, 86 insertions(+) > > diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c > index 54676e3d82dd..1728545d90b2 100644 > --- a/drivers/acpi/pptt.c > +++ b/drivers/acpi/pptt.c > @@ -817,3 +817,86 @@ int find_acpi_cpu_topology_hetero_id(unsigned int cpu) > return find_acpi_cpu_topology_tag(cpu, PPTT_ABORT_PACKAGE, > ACPI_PPTT_ACPI_IDENTICAL); > } > +/** > + * acpi_pptt_get_cpus_from_container() - Populate a cpumask with all CPUs in a > + * processor container > + * @acpi_cpu_id: The UID of the processor container. > + * @cpus: The resulting CPU mask. > + * > + * Find the specified Processor Container, and fill @cpus with all the cpus > + * below it. > + * > + * Not all 'Processor' entries in the PPTT are either a CPU or a Processor > + * Container, they may exist purely to describe a Private resource. CPUs > + * have to be leaves, so a Processor Container is a non-leaf that has the > + * 'ACPI Processor ID valid' flag set. > + * > + * Return: 0 for a complete walk, or an error if the mask is incomplete. > + */ > +void acpi_pptt_get_cpus_from_container(u32 acpi_cpu_id, cpumask_t *cpus) > +{ > + struct acpi_pptt_processor *cpu_node; > + struct acpi_table_header *table_hdr; > + struct acpi_subtable_header *entry; > + unsigned long table_end; > + u32 proc_sz; > + > + cpumask_clear(cpus); > + > + table_hdr = acpi_get_pptt(); This calls acpi_get_table() so you need to put it again or every call to this leaks a reference count. I messed around with DEFINE_FREE() for this but it doesn't fit that well as the underlying call doesn't return the table. This one does though so you could do a pptt specific one. Or just acpi_put_table(table_hdr); at exit path from this function. > + if (!table_hdr) > + return; > + > + table_end = (unsigned long)table_hdr + table_hdr->length; > + entry = ACPI_ADD_PTR(struct acpi_subtable_header, table_hdr, > + sizeof(struct acpi_table_pptt)); Hmm. Not related to this patch but I have no idea why acpi_get_pptt() doesn't return a struct acpi_table_pptt as if it did this would be a simple + 1 and not require those who only sometimes deal with ACPI code to go check what that macro actually does! > + proc_sz = sizeof(struct acpi_pptt_processor); Maybe sizeof (*cpu_node) is more helpful to reader. Also shorter so you could do while ((unsigned long)entry + sizeof(*cpu_node) <= table_end) > + while ((unsigned long)entry + proc_sz <= table_end) { > + cpu_node = (struct acpi_pptt_processor *)entry; For me, assigning this before checking the type is inelegant. but the nesting does get deep without it so I guess this is ok maybe, though I wonder if better reorganized to combine a different bunch of conditions. I think this is functionally identival. if (entry->type == ACPI_PTT_TYPE_PROCESSOR) { struct acpi_pptt_processor *cpu_node = (struct acpi_pptt_processor *)entry; if ((cpu_node->flags & ACPI_PPTT_ACPI_PROCESSOR_ID_VALID) && (!acpi_pptt_leaf_node(table_hdr, cpu_node) && (cpu_node->acpi_processor_id == acpi_cpu_id)) { acpi_pptt_get_child_cpus(table_hdr, cpu_node, cpus); break; } } entry = ACPI_ADD_PTR(struct acpi_subtable_header, entry, entry->length); More generally I wonder if it is worth adding a for_each_acpi_pptt_entry() macro. There is some precedence in drivers acpi such as for_each_nhlt_endpoint() That's probably material for another day though unless you think it brings enough benefits to do it here. > + if (entry->type == ACPI_PPTT_TYPE_PROCESSOR && > + cpu_node->flags & ACPI_PPTT_ACPI_PROCESSOR_ID_VALID) { > + if (!acpi_pptt_leaf_node(table_hdr, cpu_node)) { > + if (cpu_node->acpi_processor_id == acpi_cpu_id) { > + acpi_pptt_get_child_cpus(table_hdr, cpu_node, cpus); > + break; > + } > + } > + } > + entry = ACPI_ADD_PTR(struct acpi_subtable_header, entry, > + entry->length); > + } > +} > diff --git a/include/linux/acpi.h b/include/linux/acpi.h > index 1c5bb1e887cd..f97a9ff678cc 100644 > --- a/include/linux/acpi.h > +++ b/include/linux/acpi.h > @@ -1541,6 +1541,7 @@ int find_acpi_cpu_topology(unsigned int cpu, int level); > int find_acpi_cpu_topology_cluster(unsigned int cpu); > int find_acpi_cpu_topology_package(unsigned int cpu); > int find_acpi_cpu_topology_hetero_id(unsigned int cpu); > +void acpi_pptt_get_cpus_from_container(u32 acpi_cpu_id, cpumask_t *cpus); > #else > static inline int acpi_pptt_cpu_is_thread(unsigned int cpu) > { > @@ -1562,6 +1563,8 @@ static inline int find_acpi_cpu_topology_hetero_id(unsigned int cpu) > { > return -EINVAL; > } > +static inline void acpi_pptt_get_cpus_from_container(u32 acpi_cpu_id, > + cpumask_t *cpus) { } > #endif > > void acpi_arch_init(void);
Hi Jonathan, On 11/09/2025 11:43, Jonathan Cameron wrote: > On Wed, 10 Sep 2025 20:42:41 +0000 > James Morse <james.morse@arm.com> wrote: > >> The ACPI MPAM table uses the UID of a processor container specified in >> the PPTT to indicate the subset of CPUs and cache topology that can >> access each MPAM System Component (MSC). >> >> This information is not directly useful to the kernel. The equivalent >> cpumask is needed instead. >> >> Add a helper to find the processor container by its id, then walk >> the possible CPUs to fill a cpumask with the CPUs that have this >> processor container as a parent. >> >> CC: Dave Martin <dave.martin@arm.com> >> Reviewed-by: Sudeep Holla <sudeep.holla@arm.com> >> Signed-off-by: James Morse <james.morse@arm.com> > > Hi James, > > Sorry I missed v1. Busy few weeks. > > I think one resource leak plus a few suggested changes that > I'm not that bothered about. >> diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c >> index 54676e3d82dd..1728545d90b2 100644 >> --- a/drivers/acpi/pptt.c >> +++ b/drivers/acpi/pptt.c >> @@ -817,3 +817,86 @@ int find_acpi_cpu_topology_hetero_id(unsigned int cpu) >> return find_acpi_cpu_topology_tag(cpu, PPTT_ABORT_PACKAGE, >> ACPI_PPTT_ACPI_IDENTICAL); >> } > >> +/** >> + * acpi_pptt_get_cpus_from_container() - Populate a cpumask with all CPUs in a >> + * processor container >> + * @acpi_cpu_id: The UID of the processor container. >> + * @cpus: The resulting CPU mask. >> + * >> + * Find the specified Processor Container, and fill @cpus with all the cpus >> + * below it. >> + * >> + * Not all 'Processor' entries in the PPTT are either a CPU or a Processor >> + * Container, they may exist purely to describe a Private resource. CPUs >> + * have to be leaves, so a Processor Container is a non-leaf that has the >> + * 'ACPI Processor ID valid' flag set. >> + * >> + * Return: 0 for a complete walk, or an error if the mask is incomplete. >> + */ >> +void acpi_pptt_get_cpus_from_container(u32 acpi_cpu_id, cpumask_t *cpus) >> +{ >> + struct acpi_pptt_processor *cpu_node; >> + struct acpi_table_header *table_hdr; >> + struct acpi_subtable_header *entry; >> + unsigned long table_end; >> + u32 proc_sz; >> + >> + cpumask_clear(cpus); >> + >> + table_hdr = acpi_get_pptt(); > > This calls acpi_get_table() so you need to put it again or every call > to this leaks a reference count. I messed around with DEFINE_FREE() for this > but it doesn't fit that well as the underlying call doesn't return the table. > This one does though so you could do a pptt specific one. > > Or just acpi_put_table(table_hdr); at exit path from this function. This is a strange one - you spotted it on your follow up email. >> + if (!table_hdr) >> + return; >> + >> + table_end = (unsigned long)table_hdr + table_hdr->length; >> + entry = ACPI_ADD_PTR(struct acpi_subtable_header, table_hdr, >> + sizeof(struct acpi_table_pptt)); > Hmm. Not related to this patch but I have no idea why acpi_get_pptt() > doesn't return a struct acpi_table_pptt as if it did this would be a simple > + 1 and not require those who only sometimes deal with ACPI code to go > check what that macro actually does! Looks like that would have a knock on effect for the types in: | topology_get_acpi_cpu_tag() | acpi_find_processor_node() | cache_setup_acpi_cpu() | fetch_pptt_node() ... basically everywhere this file uses struct acpi_table_header... It's simpler to cast it here if you're keen on the '+1' approach, but this pattern is how this file does this. This is the same as acpi_pptt_leaf_node(). >> + proc_sz = sizeof(struct acpi_pptt_processor); > Maybe sizeof (*cpu_node) is more helpful to reader. (again, same as acpi_pptt_leaf_node()) > Also shorter so you could do > while ((unsigned long)entry + sizeof(*cpu_node) <= table_end) > >> + while ((unsigned long)entry + proc_sz <= table_end) { >> + cpu_node = (struct acpi_pptt_processor *)entry; > > For me, assigning this before checking the type is inelegant. I agree, but when in pptt.c ... > but the nesting does get deep without it so I guess this is ok maybe, though > I wonder if better reorganized to combine a different bunch of conditions. > I think this is functionally identival. > > if (entry->type == ACPI_PTT_TYPE_PROCESSOR) { > struct acpi_pptt_processor *cpu_node = > (struct acpi_pptt_processor *)entry; > if ((cpu_node->flags & ACPI_PPTT_ACPI_PROCESSOR_ID_VALID) && > (!acpi_pptt_leaf_node(table_hdr, cpu_node) && > (cpu_node->acpi_processor_id == acpi_cpu_id)) { > acpi_pptt_get_child_cpus(table_hdr, cpu_node, cpus); > break; > > } > } > entry = ACPI_ADD_PTR(struct acpi_subtable_header, entry, > entry->length); It is the same - I think this is better as it reduces the scope of cpu_node. > More generally I wonder if it is worth adding a for_each_acpi_pptt_entry() macro. > There is some precedence in drivers acpi such as for_each_nhlt_endpoint() When this series had support for ACPI PPI-Partitions there were more PPTT helpers and I had a function pointer based call_on_every_entry() kind of thing. Jeremy L though it obscured the flow. A custom for_each_ is probably better. I may float it as an RFC after these are done. I don't want to make this series any bigger. It certainly makes the users easier on the eye. | drivers/acpi/pptt.c | 43 +++++++++++++++---------------------------- | 1 file changed, 15 insertions(+), 28 deletions(-) Stashed here for posterity: https://git.kernel.org/pub/scm/linux/kernel/git/morse/linux.git/commit/?h=pptt/for_each_pptt_entry/v0&id=353ceeba3d39c6b6a10eeb1a59c49649cdf719d8 > That's probably material for another day though unless you think it brings > enough benefits to do it here. > > >> + if (entry->type == ACPI_PPTT_TYPE_PROCESSOR && >> + cpu_node->flags & ACPI_PPTT_ACPI_PROCESSOR_ID_VALID) { >> + if (!acpi_pptt_leaf_node(table_hdr, cpu_node)) { >> + if (cpu_node->acpi_processor_id == acpi_cpu_id) { >> + acpi_pptt_get_child_cpus(table_hdr, cpu_node, cpus); >> + break; >> + } >> + } >> + } >> + entry = ACPI_ADD_PTR(struct acpi_subtable_header, entry, >> + entry->length); >> + } >> +} >> diff --git a/include/linux/acpi.h b/include/linux/acpi.h >> index 1c5bb1e887cd..f97a9ff678cc 100644 >> --- a/include/linux/acpi.h >> +++ b/include/linux/acpi.h >> @@ -1541,6 +1541,7 @@ int find_acpi_cpu_topology(unsigned int cpu, int level); >> int find_acpi_cpu_topology_cluster(unsigned int cpu); >> int find_acpi_cpu_topology_package(unsigned int cpu); >> int find_acpi_cpu_topology_hetero_id(unsigned int cpu); >> +void acpi_pptt_get_cpus_from_container(u32 acpi_cpu_id, cpumask_t *cpus); >> #else >> static inline int acpi_pptt_cpu_is_thread(unsigned int cpu) >> { >> @@ -1562,6 +1563,8 @@ static inline int find_acpi_cpu_topology_hetero_id(unsigned int cpu) >> { >> return -EINVAL; >> } >> +static inline void acpi_pptt_get_cpus_from_container(u32 acpi_cpu_id, >> + cpumask_t *cpus) { } >> #endif >> >> void acpi_arch_init(void); >
On Thu, 11 Sep 2025 11:43:37 +0100 Jonathan Cameron <jonathan.cameron@huawei.com> wrote: > On Wed, 10 Sep 2025 20:42:41 +0000 > James Morse <james.morse@arm.com> wrote: > > > The ACPI MPAM table uses the UID of a processor container specified in > > the PPTT to indicate the subset of CPUs and cache topology that can > > access each MPAM System Component (MSC). > > > > This information is not directly useful to the kernel. The equivalent > > cpumask is needed instead. > > > > Add a helper to find the processor container by its id, then walk > > the possible CPUs to fill a cpumask with the CPUs that have this > > processor container as a parent. > > > > CC: Dave Martin <dave.martin@arm.com> > > Reviewed-by: Sudeep Holla <sudeep.holla@arm.com> > > Signed-off-by: James Morse <james.morse@arm.com> > > Hi James, > > Sorry I missed v1. Busy few weeks. > > I think one resource leak plus a few suggested changes that > I'm not that bothered about. Ignore the resource leak. I didn't read acpi_get_pptt() properly. No bug there. So consider the comments below, but I'm fine with this as is. Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com> > > Jonathan > > > > --- > > Changes since v1: > > * Replaced commit message with wording from Dave. > > * Fixed a stray plural. > > * Moved further down in the file to make use of get_pptt() helper. > > * Added a break to exit the loop early. > > > > Changes since RFC: > > * Removed leaf_flag local variable from acpi_pptt_get_cpus_from_container() > > > > Changes since RFC: > > * Dropped has_leaf_flag dodging of acpi_pptt_leaf_node() > > * Added missing : in kernel-doc > > * Made helper return void as this never actually returns an error. > > --- > > drivers/acpi/pptt.c | 83 ++++++++++++++++++++++++++++++++++++++++++++ > > include/linux/acpi.h | 3 ++ > > 2 files changed, 86 insertions(+) > > > > diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c > > index 54676e3d82dd..1728545d90b2 100644 > > --- a/drivers/acpi/pptt.c > > +++ b/drivers/acpi/pptt.c > > @@ -817,3 +817,86 @@ int find_acpi_cpu_topology_hetero_id(unsigned int cpu) > > return find_acpi_cpu_topology_tag(cpu, PPTT_ABORT_PACKAGE, > > ACPI_PPTT_ACPI_IDENTICAL); > > } > > > +/** > > + * acpi_pptt_get_cpus_from_container() - Populate a cpumask with all CPUs in a > > + * processor container > > + * @acpi_cpu_id: The UID of the processor container. > > + * @cpus: The resulting CPU mask. > > + * > > + * Find the specified Processor Container, and fill @cpus with all the cpus > > + * below it. > > + * > > + * Not all 'Processor' entries in the PPTT are either a CPU or a Processor > > + * Container, they may exist purely to describe a Private resource. CPUs > > + * have to be leaves, so a Processor Container is a non-leaf that has the > > + * 'ACPI Processor ID valid' flag set. > > + * > > + * Return: 0 for a complete walk, or an error if the mask is incomplete. > > + */ > > +void acpi_pptt_get_cpus_from_container(u32 acpi_cpu_id, cpumask_t *cpus) > > +{ > > + struct acpi_pptt_processor *cpu_node; > > + struct acpi_table_header *table_hdr; > > + struct acpi_subtable_header *entry; > > + unsigned long table_end; > > + u32 proc_sz; > > + > > + cpumask_clear(cpus); > > + > > + table_hdr = acpi_get_pptt(); > > This calls acpi_get_table() so you need to put it again or every call > to this leaks a reference count. I messed around with DEFINE_FREE() for this > but it doesn't fit that well as the underlying call doesn't return the table. > This one does though so you could do a pptt specific one. > > Or just acpi_put_table(table_hdr); at exit path from this function. > > > > + if (!table_hdr) > > + return; > > + > > + table_end = (unsigned long)table_hdr + table_hdr->length; > > + entry = ACPI_ADD_PTR(struct acpi_subtable_header, table_hdr, > > + sizeof(struct acpi_table_pptt)); > Hmm. Not related to this patch but I have no idea why acpi_get_pptt() > doesn't return a struct acpi_table_pptt as if it did this would be a simple > + 1 and not require those who only sometimes deal with ACPI code to go > check what that macro actually does! > > > > + proc_sz = sizeof(struct acpi_pptt_processor); > Maybe sizeof (*cpu_node) is more helpful to reader. > Also shorter so you could do > while ((unsigned long)entry + sizeof(*cpu_node) <= table_end) > > > + while ((unsigned long)entry + proc_sz <= table_end) { > > + cpu_node = (struct acpi_pptt_processor *)entry; > > For me, assigning this before checking the type is inelegant. > but the nesting does get deep without it so I guess this is ok maybe, though > I wonder if better reorganized to combine a different bunch of conditions. > I think this is functionally identival. > > if (entry->type == ACPI_PTT_TYPE_PROCESSOR) { > struct acpi_pptt_processor *cpu_node = > (struct acpi_pptt_processor *)entry; > if ((cpu_node->flags & ACPI_PPTT_ACPI_PROCESSOR_ID_VALID) && > (!acpi_pptt_leaf_node(table_hdr, cpu_node) && > (cpu_node->acpi_processor_id == acpi_cpu_id)) { > acpi_pptt_get_child_cpus(table_hdr, cpu_node, cpus); > break; > > } > } > entry = ACPI_ADD_PTR(struct acpi_subtable_header, entry, > entry->length); > > More generally I wonder if it is worth adding a for_each_acpi_pptt_entry() macro. > There is some precedence in drivers acpi such as for_each_nhlt_endpoint() > > That's probably material for another day though unless you think it brings > enough benefits to do it here. > > > > + if (entry->type == ACPI_PPTT_TYPE_PROCESSOR && > > + cpu_node->flags & ACPI_PPTT_ACPI_PROCESSOR_ID_VALID) { > > + if (!acpi_pptt_leaf_node(table_hdr, cpu_node)) { > > + if (cpu_node->acpi_processor_id == acpi_cpu_id) { > > + acpi_pptt_get_child_cpus(table_hdr, cpu_node, cpus); > > + break; > > + } > > + } > > + } > > + entry = ACPI_ADD_PTR(struct acpi_subtable_header, entry, > > + entry->length); > > + } > > +} > > diff --git a/include/linux/acpi.h b/include/linux/acpi.h > > index 1c5bb1e887cd..f97a9ff678cc 100644 > > --- a/include/linux/acpi.h > > +++ b/include/linux/acpi.h > > @@ -1541,6 +1541,7 @@ int find_acpi_cpu_topology(unsigned int cpu, int level); > > int find_acpi_cpu_topology_cluster(unsigned int cpu); > > int find_acpi_cpu_topology_package(unsigned int cpu); > > int find_acpi_cpu_topology_hetero_id(unsigned int cpu); > > +void acpi_pptt_get_cpus_from_container(u32 acpi_cpu_id, cpumask_t *cpus); > > #else > > static inline int acpi_pptt_cpu_is_thread(unsigned int cpu) > > { > > @@ -1562,6 +1563,8 @@ static inline int find_acpi_cpu_topology_hetero_id(unsigned int cpu) > > { > > return -EINVAL; > > } > > +static inline void acpi_pptt_get_cpus_from_container(u32 acpi_cpu_id, > > + cpumask_t *cpus) { } > > #endif > > > > void acpi_arch_init(void); >
© 2016 - 2025 Red Hat, Inc.