[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ba2d4852-dd38-4d98-96fe-2716875ff199@arm.com>
Date: Wed, 12 Nov 2025 10:14:04 +0000
From: Ben Horgan <ben.horgan@....com>
To: Gavin Shan <gshan@...hat.com>, james.morse@....com
Cc: amitsinght@...vell.com, baisheng.gao@...soc.com,
baolin.wang@...ux.alibaba.com, bobo.shaobowang@...wei.com,
carl@...amperecomputing.com, catalin.marinas@....com, dakr@...nel.org,
dave.martin@....com, david@...hat.com, dfustini@...libre.com,
fenghuay@...dia.com, gregkh@...uxfoundation.org, guohanjun@...wei.com,
jeremy.linton@....com, jonathan.cameron@...wei.com, kobak@...dia.com,
lcherian@...vell.com, lenb@...nel.org, linux-acpi@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
lpieralisi@...nel.org, peternewman@...gle.com, quic_jiles@...cinc.com,
rafael@...nel.org, robh@...nel.org, rohit.mathew@....com,
scott@...amperecomputing.com, sdonthineni@...dia.com, sudeep.holla@....com,
tan.shaopeng@...itsu.com, will@...nel.org, xhao@...ux.alibaba.com,
Shaopeng Tan <tan.shaopeng@...fujitsu.com>
Subject: Re: [PATCH 01/33] ACPI / PPTT: Add a helper to fill a cpumask from a
processor container
Hi Gavin,
On 11/8/25 04:31, Gavin Shan wrote:
> Hi Ben,
>
> On 11/7/25 10:34 PM, Ben Horgan wrote:
>> From: James Morse <james.morse@....com>
>>
>> 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@....com>
>> Reviewed-by: Sudeep Holla <sudeep.holla@....com>
>> Reviewed-by: Jonathan Cameron <jonathan.cameron@...wei.com>
>> Reviewed-by: Fenghua Yu <fenghuay@...dia.com>
>> Reviewed-by: Gavin Shan <gshan@...hat.com>
>> Tested-by: Fenghua Yu <fenghuay@...dia.com>
>> Tested-by: Shaopeng Tan <tan.shaopeng@...fujitsu.com>
>> Tested-by: Peter Newman <peternewman@...gle.com>
>> Signed-off-by: James Morse <james.morse@....com>
>> Signed-off-by: Ben Horgan <ben.horgan@....com>
>> ---
>> Changes since v3:
>> Refer to processor hierarchy in comments (Jonathan)
>> Fix indent (Jonathan)
>> ---
>> drivers/acpi/pptt.c | 85 ++++++++++++++++++++++++++++++++++++++++++++
>> include/linux/acpi.h | 3 ++
>> 2 files changed, 88 insertions(+)
>>
>
> Two nitpicks below...
>
>> diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
>> index 54676e3d82dd..69917cc6bd2f 100644
>> --- a/drivers/acpi/pptt.c
>> +++ b/drivers/acpi/pptt.c
>> @@ -817,3 +817,88 @@ 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 hierarchy node
>> + *
>> + * @table_hdr: A reference to the PPTT table
>> + * @parent_node: A pointer to the processor hierarchy 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 Hierarchy' 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.
>> + */
>> +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) {
>> +
>
> Unnecessary blank line here.
Ack
>
>> + if (entry->type == ACPI_PPTT_TYPE_PROCESSOR) {
>> + struct acpi_pptt_processor *cpu_node;
>> +
>> + 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);
>
> Need we to check if @cpu_node does crosses the boundary (@table_end), as
> what's
> doing in acpi_find_processor_node()? Actually, the similar hunk of code
> from
> the function can be reused here.
As acpi_find_processor_node() is called from acpi_pptt_get_child_cpus()
I don't think we need to duplicate the checking here.
>
> static struct acpi_pptt_processor *acpi_find_processor_node(struct
> acpi_table_header *table_hdr,
> u32
> acpi_cpu_id)
> {
> :
> while ((unsigned long)entry + proc_sz <= table_end) {
> cpu_node = (struct acpi_pptt_processor *)entry;
>
> if (entry->length == 0) {
> pr_warn("Invalid zero length subtable\n");
> break;
> }
>
> /* entry->length may not equal proc_sz, revalidate the
> processor structure length */
> if (entry->type == ACPI_PPTT_TYPE_PROCESSOR &&
> acpi_cpu_id == cpu_node->acpi_processor_id &&
> (unsigned long)entry + entry->length <= table_end &&
> entry->length == proc_sz + cpu_node-
>>number_of_priv_resources * sizeof(u32) &&
> acpi_pptt_leaf_node(table_hdr, cpu_node)) {
> return (struct acpi_pptt_processor *)entry;
> }
>
> entry = ACPI_ADD_PTR(struct acpi_subtable_header, entry,
> entry->length);
> }
>
> :
> }
>
>
>> + }
>> +}
>> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
>> index 5ff5d99f6ead..4752ebd48132 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);
>
> Thanks,
> Gavin
>
Thanks,
Ben
Powered by blists - more mailing lists