[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <733018f5-3d1c-4238-849c-e253a778c81f@arm.com>
Date: Fri, 25 Jul 2025 18:06:03 +0100
From: James Morse <james.morse@....com>
To: "Shaopeng Tan (Fujitsu)" <tan.shaopeng@...itsu.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>
Cc: Rob Herring <robh@...nel.org>, Ben Horgan <ben.horgan@....com>,
Rohit Mathew <rohit.mathew@....com>,
Shanker Donthineni <sdonthineni@...dia.com>, Zeng Heng
<zengheng4@...wei.com>, Lecopzer Chen <lecopzerc@...dia.com>,
Carl Worth <carl@...amperecomputing.com>,
"shameerali.kolothum.thodi@...wei.com"
<shameerali.kolothum.thodi@...wei.com>,
D Scott Phillips OS <scott@...amperecomputing.com>,
"lcherian@...vell.com" <lcherian@...vell.com>,
"bobo.shaobowang@...wei.com" <bobo.shaobowang@...wei.com>,
"baolin.wang@...ux.alibaba.com" <baolin.wang@...ux.alibaba.com>,
Jamie Iles <quic_jiles@...cinc.com>, Xin Hao <xhao@...ux.alibaba.com>,
"peternewman@...gle.com" <peternewman@...gle.com>,
"dfustini@...libre.com" <dfustini@...libre.com>,
"amitsinght@...vell.com" <amitsinght@...vell.com>,
David Hildenbrand <david@...hat.com>, Rex Nie <rex.nie@...uarmicro.com>,
Dave Martin <dave.martin@....com>, Koba Ko <kobak@...dia.com>,
Sudeep Holla <sudeep.holla@....com>
Subject: Re: [RFC PATCH 05/36] ACPI / PPTT: Add a helper to fill a cpumask
from a processor container
Hi Shaopeng,
On 17/07/2025 08:58, Shaopeng Tan (Fujitsu) wrote:
> Hello James,
>
>> The PPTT describes CPUs and caches, as well as processor containers.
>> The ACPI table for MPAM describes the set of CPUs that can access an MSC
>> with the UID of a processor container.
>>
>> 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.
>> diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c index
>> 54676e3d82dd..13619b1b821b 100644
>> --- a/drivers/acpi/pptt.c
>> +++ b/drivers/acpi/pptt.c
>> @@ -298,6 +298,99 @@ static struct acpi_pptt_processor
>> +/**
>> + * acpi_pptt_get_cpus_from_container() - Populate a cpumask with all CPUs
>> in a
>> + * processor containers
>> + * @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.
>> + */
>> +int 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;
>> + bool leaf_flag, has_leaf_flag = false;
>> + unsigned long table_end;
>> + acpi_status status;
>> + u32 proc_sz;
>> + int ret = 0;
>> +
>> + cpumask_clear(cpus);
>> +
>> + status = acpi_get_table(ACPI_SIG_PPTT, 0, &table_hdr);
>> + if (ACPI_FAILURE(status))
>> + return 0;
> If pptt table cannot be got, should -ENODEV be returned?
In general its not an error for the PPTT to be missing, there are plenty of platforms
where that is the case. I think in this case the caller has to be working with some
information that means there has to be a PPTT, so this isn't an error that needs handling.
In MPAM's case, the ACPI table references things in the PPTT, if that table were missing
the platform description is unusable. I don't think this is something we need to help
debug - just ensure we don't cause a panic() that would make it harder to debug!
>> + if (table_hdr->revision > 1)
>> + has_leaf_flag = true;
>> +
>> + 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) {
>> + leaf_flag = cpu_node->flags &
>> ACPI_PPTT_ACPI_LEAF_NODE;
>> + if ((has_leaf_flag && !leaf_flag) ||
>> + (!has_leaf_flag
>> && !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);
>> + }
>> + }
>> + entry = ACPI_ADD_PTR(struct acpi_subtable_header, entry,
>> + entry->length);
>> + }
>> +
>> + acpi_put_table(table_hdr);
>> +
>> + return ret;
> Only 0 is returned here.
Good spot! I think this allocated memory in the past, it can probably be made 'void',
which will make your above point easier too.
> There is no action to be taken when the mask is incomplete.
I don't think there needs to be. General callers should be using cacheinfo for this
information. This only exists as the MPAM driver needs to know about the topology of the
system before 'all' the CPUs are online. (which could be never).
Thanks,
James
Powered by blists - more mailing lists