linux-kernel - Re: [PATCH 03/33] ACPI / PPTT: Add a helper to fill a cpumask from a processor container

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ff91d387-7fe6-4461-a95c-a46b73e4ec73@arm.com>
Date: Wed, 10 Sep 2025 20:29:36 +0100
From: James Morse <james.morse@....com>
To: Dave Martin <Dave.Martin@....com>
Cc: linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-acpi@...r.kernel.org, devicetree@...r.kernel.org,
 D Scott Phillips OS <scott@...amperecomputing.com>,
 carl@...amperecomputing.com, lcherian@...vell.com,
 bobo.shaobowang@...wei.com, tan.shaopeng@...itsu.com,
 baolin.wang@...ux.alibaba.com, Jamie Iles <quic_jiles@...cinc.com>,
 Xin Hao <xhao@...ux.alibaba.com>, peternewman@...gle.com,
 dfustini@...libre.com, amitsinght@...vell.com,
 David Hildenbrand <david@...hat.com>, Rex Nie <rex.nie@...uarmicro.com>,
 Koba Ko <kobak@...dia.com>, Shanker Donthineni <sdonthineni@...dia.com>,
 fenghuay@...dia.com, baisheng.gao@...soc.com,
 Jonathan Cameron <jonathan.cameron@...wei.com>, Rob Herring
 <robh@...nel.org>, Rohit Mathew <rohit.mathew@....com>,
 Rafael Wysocki <rafael@...nel.org>, Len Brown <lenb@...nel.org>,
 Lorenzo Pieralisi <lpieralisi@...nel.org>, Hanjun Guo
 <guohanjun@...wei.com>, Sudeep Holla <sudeep.holla@....com>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Catalin Marinas <catalin.marinas@....com>,
 Will Deacon <will@...nel.org>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 Danilo Krummrich <dakr@...nel.org>
Subject: Re: [PATCH 03/33] ACPI / PPTT: Add a helper to fill a cpumask from a
 processor container

Hi Dave,

On 05/09/2025 17:24, Dave Martin wrote:
> On Thu, Aug 28, 2025 at 04:57:06PM +0100, James Morse wrote:
>> On 27/08/2025 11:48, Dave Martin wrote:
>>> On Fri, Aug 22, 2025 at 03:29:44PM +0000, James Morse wrote:
>>>> 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.

>>>> diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
>>>> @@ -298,6 +298,92 @@ static struct acpi_pptt_processor *acpi_find_processor_node(struct acpi_table_he

>>>>  +void acpi_pptt_get_cpus_from_container(u32 acpi_cpu_id, cpumask_t *cpus)
>>>>  +{

>>>> +		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 = acpi_pptt_leaf_node(table_hdr, cpu_node);
>>>> +			if (!leaf_flag) {
>>>> +				if (cpu_node->acpi_processor_id == acpi_cpu_id)
>>
>>
>>> Is there any need to distinguish processor containers from (leaf) CPU
>>> nodes, here?  If not, dropping the distinction might simplify the code
>>> here (even if callers do not care).
>>
>> In the namespace the object types are different, so I assumed they have their own UID
>> space. The PPTT holds both - hence the check for which kind of thing it is. The risk is
>> looking for processor-container-4 and finding CPU-4 instead...
>>
>> The relevant ACPI bit is "8.4.2.1 Processor Container Device", its says:
>> | A processor container declaration must supply a _UID method returning an ID that is
>> | unique in the processor container hierarchy.
>>
>> Which doesn't quite let me combine them here.
> 
> I was going by the PPTT spec, where the types are not distinct --
> you're probably right, though.

This way round is at least robust to this happening.


> According to that, isn't it the "ACPI Processor ID valid" flag, not the
> "Node is a Leaf" flag, that says whether this field is meaningful?

ACPI_PPTT_ACPI_PROCESSOR_ID_VALID was checked a few lines earlier. We're looking for
processors, hence also checking the leaf.


> It's reasonable not to bother to try to enumerate the children of a
> node that claims to be a leaf (even if there actually are children),
> but I wonder what happens if acpi_processor_id is not declared to be
> valid and matches by accident.  That's probably not a valid table (?)
> but does anything bad happen on the kernel side?

The type and flag are both checked earlier, so this can't happen.
You could certainly but junk nodes in the table that would be skipped over, and those
could point to a parent that is a leaf - I can't spot anything in the table parsing code
that would care about that.


>>> Otherwise, maybe eliminate leaf_flag and collapse these into a single
>>> if(), as suggested by Ben [1].
>>>
>>>> +					acpi_pptt_get_child_cpus(table_hdr, cpu_node, cpus);
>>>
>>> Can there ever be multiple matches?
>>>
>>> The possibility of duplicate processor IDs in the PPTT sounds weird to
>>> me, but then I'm not an ACPI expert.
>>
>> Multiple processor-containers with the same ID? That would be a corrupt table.
>> acpi_pptt_get_child_cpus() then walks the tree again to find the CPUs below this
>> processor-container - those have a different kind of id.

> Does anything bad happen if we encounter duplicates?
> 
> (Other then the MPAM driver never getting enabled, or not working as
> advertised, that is.)
> 
> I haven't tried to think through all the implications, here.

It would be unpredictable which node linux finds when it goes looking for CPUs. I don't
think anything would notice. Messing up the cache hierarchy is a different story!


Thanks,

James