| 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
| ||
|
Message-ID: <20251024152256.00003f8e@huawei.com>
Date: Fri, 24 Oct 2025 15:22:56 +0100
From: Jonathan Cameron <jonathan.cameron@...wei.com>
To: Jeremy Linton <jeremy.linton@....com>
CC: James Morse <james.morse@....com>, <linux-kernel@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>, <linux-acpi@...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>, Dave Martin <dave.martin@....com>, Koba Ko
<kobak@...dia.com>, Shanker Donthineni <sdonthineni@...dia.com>,
<fenghuay@...dia.com>, <baisheng.gao@...soc.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>, Catalin Marinas <catalin.marinas@....com>, "Will
Deacon" <will@...nel.org>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Danilo Krummrich <dakr@...nel.org>, Gavin Shan <gshan@...hat.com>
Subject: Re: [PATCH v3 04/29] ACPI / PPTT: Add a helper to fill a cpumask
from a cache_id
On Wed, 22 Oct 2025 07:58:36 -0500
Jeremy Linton <jeremy.linton@....com> wrote:
> Hi,
>
> This is largely looking pretty solid, but..
>
>
> On 10/17/25 1:56 PM, James Morse wrote:
> > MPAM identifies CPUs by the cache_id in the PPTT cache structure.
> >
> > The driver needs to know which CPUs are associated with the cache.
> > The CPUs may not all be online, so cacheinfo does not have the
> > information.
> >
> > Add a helper to pull this information out of the PPTT.
> >
> > CC: Rohit Mathew <Rohit.Mathew@....com>
> > Signed-off-by: James Morse <james.morse@....com>
> > Reviewed-by: Sudeep Holla <sudeep.holla@....com>
> > Tested-by: Fenghua Yu <fenghuay@...dia.com>
> > ---
> > Changes since v2:
> > * Removed stray cleanup useage in preference for acpi_get_pptt().
> > * Removed WARN_ON_ONCE() for symmetry with other helpers.
> > * Dropped restriction on unified caches.
> >
> > Changes since v1:
> > * Added punctuation to the commit message.
> > * Removed a comment about an alternative implementaion.
> > * Made the loop continue with a warning if a CPU is missing from the PPTT.
> >
> > Changes since RFC:
> > * acpi_count_levels() now returns a value.
> > * Converted the table-get stuff to use Jonathan's cleanup helper.
> > * Dropped Sudeep's Review tag due to the cleanup change.
> > ---
> > drivers/acpi/pptt.c | 64 ++++++++++++++++++++++++++++++++++++++++++++
> > include/linux/acpi.h | 6 +++++
> > 2 files changed, 70 insertions(+)
> >
> > diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
> > index 50c8f2a3c927..2f86f58699a6 100644
> > --- a/drivers/acpi/pptt.c
> > +++ b/drivers/acpi/pptt.c
> > @@ -985,3 +985,67 @@ int find_acpi_cache_level_from_id(u32 cache_id)
> >
> > return -ENOENT;
> > }
> > +
> > +/**
> > + * acpi_pptt_get_cpumask_from_cache_id() - Get the cpus associated with the
> > + * specified cache
> > + * @cache_id: The id field of the cache
> > + * @cpus: Where to build the cpumask
> > + *
> > + * Determine which CPUs are below this cache in the PPTT. This allows the property
> > + * to be found even if the CPUs are offline.
> > + *
> > + * The PPTT table must be rev 3 or later,
> > + *
> > + * Return: -ENOENT if the PPTT doesn't exist, or the cache cannot be found.
> > + * Otherwise returns 0 and sets the cpus in the provided cpumask.
> > + */
> > +int acpi_pptt_get_cpumask_from_cache_id(u32 cache_id, cpumask_t *cpus)
> > +{
> > + int level, cpu;
> > + u32 acpi_cpu_id;
> > + struct acpi_pptt_cache *cache;
> > + struct acpi_table_header *table;
> > + struct acpi_pptt_cache_v1 *cache_v1;
> > + struct acpi_pptt_processor *cpu_node;
> > +
> > + cpumask_clear(cpus);
> > +
> > + table = acpi_get_pptt();
> > + if (!table)
> > + return -ENOENT;
> > +
> > + if (table->revision < 3)
> > + return -ENOENT;
> > +
> > + for_each_possible_cpu(cpu) {
> > + acpi_cpu_id = get_acpi_id_for_cpu(cpu);
> > + cpu_node = acpi_find_processor_node(table, acpi_cpu_id);
> > + if (!cpu_node)
> > + continue;
> > +
> > + /* Start at 1 for L1 */
> > + level = 1;
> > + cache = acpi_find_any_type_cache_node(table, acpi_cpu_id, level,
> > + &cpu_node);
> > + while (cache) {
> > + cache_v1 = ACPI_ADD_PTR(struct acpi_pptt_cache_v1,
> > + cache, sizeof(*cache));
>
> Is the core acpi definition in actbl2.h correct? Shouldn't it be
> something along the lines of:
>
> struct acpi_pptt_cache_v1 {
> struct acpi_subtable_header header;
> u16 reservedd;
> u32 flags;
> u32 next_level_of_cache;
> u32 size;
> u32 number_of_sets;
> u8 associativity;
> u8 attributes;
> u16 lines_size;
> u32 cache_id;
> }
>
>
> Then that solves the detection of the additional field problem correctly
> because the length (24 vs 28) of the subtable then tells you which
> version your dealing with. (and goes back to why much of this is coded
> to use ACPI_ADD_PTR rather than structure+ logic.)
>
Do we want to deal with arguing the change in ACPICA?
I fully agree that it would be much nicer if that didn't use this weird
bits of structures approach :(
https://github.com/acpica/acpica/blob/master/source/include/actbl2.h#L3497
is where this is coming from.
Maybe can do it in parallel. Rafael, what do you think is best way forwards
with this?
Jonathan
>
> Thanks,
>
>
>
>
>
>
> > + if (!cache)
> > + continue;
> > +
> > + cache_v1 = ACPI_ADD_PTR(struct acpi_pptt_cache_v1,
> > + cache, sizeof(*cache));
> > +
> > + if (cache->flags & ACPI_PPTT_CACHE_ID_VALID &&
> > + cache_v1->cache_id == cache_id)
> > + cpumask_set_cpu(cpu, cpus);
> > +
> > + level++;
> > + cache = acpi_find_any_type_cache_node(table, acpi_cpu_id,
> > + level, &cpu_node);
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> > index be074bdfd4d1..a9dbacabdf89 100644
> > --- a/include/linux/acpi.h
> > +++ b/include/linux/acpi.h
> > @@ -1543,6 +1543,7 @@ 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);
> > int find_acpi_cache_level_from_id(u32 cache_id);
> > +int acpi_pptt_get_cpumask_from_cache_id(u32 cache_id, cpumask_t *cpus);
> > #else
> > static inline int acpi_pptt_cpu_is_thread(unsigned int cpu)
> > {
> > @@ -1570,6 +1571,11 @@ static inline int find_acpi_cache_level_from_id(u32 cache_id)
> > {
> > return -ENOENT;
> > }
> > +static inline int acpi_pptt_get_cpumask_from_cache_id(u32 cache_id,
> > + cpumask_t *cpus)
> > +{
> > + return -ENOENT;
> > +}
> > #endif
> >
> > void acpi_arch_init(void);
>
>
Powered by blists - more mailing lists