On Fri, 17 Oct 2025 18:56:17 +0000
James Morse [off-list ref] 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 <redacted>
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Reviewed-by: Fenghua Yu <fenghuay@nvidia.com>
Reviewed-by: Gavin Shan <redacted>
Tested-by: Fenghua Yu <fenghuay@nvidia.com>
Signed-off-by: James Morse <james.morse@arm.com>

Hi James,

V2 has dropped out of my memory more or less so to get this back in
I'll do a fresh review (even of ones I've already given an RB on).
Nothing here to change that tag. Just some naming / comment suggestions
that I think would help a little with readability.

Some of these comments come from me forgetting how the spec named
things and so going to take a look.  The spec isn't consistent with the
naming (e.g. ACPI PROCESSOR ID is not necessarily a processor
ID) but I think keeping closer to spec names will help readers. 

Note this may all be in the category of perfect being the enemy of
good + upstream so I don't mind if you ignore.

The spec calls these Processor Hierarchy Node Structures. I think the
addition of the Hierarchy word will help people understand this isn't
finding things below a node specific to a processor but to some higher
level hierarchy structure.

+ * @table_hdr:		A reference to the PPTT table.
+ * @parent_node:	A pointer to the processor node in the @table_hdr.

Likewise, calling this a "processor hierarchy node" would make things
clearer.

+ * @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;

This is definitely a processor hierarchy node.  To my mind
cpu_node doesn't convey this.  See below for more, but perhaps just renaming
it to include hierarchy in the name would help.

+	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);

Here it is indeed a CPU.

+
+		while (cpu_node) {
+			if (cpu_node == parent_node) {
+				cpumask_set_cpu(cpu, cpus);
+				break;
+			}
+			cpu_node = fetch_pptt_node(table_hdr, cpu_node->parent);

But here it is a processor container or a private node.  So
cpu_hieriarchy_node or something along those lines would be more appropriate.

+		}
+	}
+}
+
+/**
+ * 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

I'd go with 'Processor Hierarchy' here
 

+ * 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.
+ */
+void acpi_pptt_get_cpus_from_container(u32 acpi_cpu_id, cpumask_t *cpus)
+{
+	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) {
+
+		if (entry->type == ACPI_PPTT_TYPE_PROCESSOR) {
+			struct acpi_pptt_processor *cpu_node;

similar naming thing here. 

+
+			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);

The double tab indent here is odd.
			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);

Isn't find I think for readability.  I could understand the bonus tab if it was
close to aligning with the line above but it isn't.

+					break;
+			}
+		}
+		entry = ACPI_ADD_PTR(struct acpi_subtable_header, entry,
+				     entry->length);
+	}
+}