Hi Dave,

On 09/09/2025 11:14, Dave Martin wrote:

On Thu, Aug 28, 2025 at 04:58:16PM +0100, James Morse wrote:

quoted

On 27/08/2025 11:53, Dave Martin wrote:

quoted

On Fri, Aug 22, 2025 at 03:29:47PM +0000, James Morse wrote:

quoted

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.

quoted

quoted

quoted

diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
index 660457644a5b..cb93a9a7f9b6 100644
--- a/drivers/acpi/pptt.c
+++ b/drivers/acpi/pptt.c

@@ -971,3 +971,65 @@ int find_acpi_cache_level_from_id(u32 cache_id)

[...]

quoted

quoted

quoted

+ * acpi_pptt_get_cpumask_from_cache_id() - Get the cpus associated with the
+ *					   specified cache
+ * @cache_id: The id field of the unified 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)
+{

[...]

quoted

quoted

quoted

+	/*
+	 * If we found the cache first, we'd still need to walk from each cpu.
+	 */
+	for_each_possible_cpu(cpu) {

[...]

quoted

quoted

Again, it feels like we are repeating the same walk multiple times to
determine how deep the table is (on which point the table is self-
describing anyway), and then again to derive some static property, and
then we are then doing all of that work multiple times to derive
different static properties, etc.

Can we not just walk over the tables once and stash the derived
properties somewhere?

That is possible - but its a more invasive change to the PPTT parsing code.
Before the introduction of the leaf flag, the search for a processor also included a
search to check if the discovered node was a leaf.

I think this is trading time - walking over the table multiple times, against the memory
you'd need to de-serialise the tree to find the necessary properties quickly. I think the
reason Jeremy L went this way was because there may never be another request into this
code, so being ready with a quick answer was a waste of memory.

MPAM doesn't change this - all these things are done up front during driver probing, and
the values are cached by the driver.

I guess that's true.

quoted

quoted

I'm still getting my head around this parsing code, so I'm not saying
that the approach is incorrect here -- just wondering whether there is
a way to make it simpler.

It's walked at boot, and on cpu-hotplug. Neither are particularly performance critical.

Do we do this only for unknown late secondaries (e.g., that haven't
previously come online?) 

No, each time a CPU comes online.

I haven't gone to track this down but, if not,
this cuts across the assertion that "there may never be another request
into this code".

CPU hotplug is optional - you don't have to bounce CPUs. It's very common on mobile parts
for power saving. I think its fairly unusual on server parts, once CPUs are online they
stay online.

The cacheinfo code doesn't cache this, it re-reads it every time. That turns out to be
because of PowerPC where some of these properties can be changed while a CPU is offline.
Sure, we could have a Kconfig thing to say ARCH_STATIC_TABLES_ARE_STATIC, but that would
be a different piece of work.
(I've had a couple of stabs at this, but cacheinfo is the shape it needs to be)

cpu hotlug is slow in practice, but gratuitous cost on this path should
still be avoided where feasible.

quoted

I agree that as platforms get bigger, there will be a tipping point ... I don't think
anyone has complained yet!

Ack -- when in ACPI, do as the ACPI folks do, I guess.

Thanks,

James