Re: [PATCH v2] ACPI: PPTT: Dump PPTT table when error detected

Feng Tang
2026-01-10T15:04:49Z

Hi Hanjun,

Thanks for the review!

On Sat, Jan 10, 2026 at 12:29:43PM +0800, Hanjun Guo wrote:
> Hi Feng Tang,
>
> On 2025/12/31 18:49, Feng Tang wrote:
> > There was warning message about PPTT table:
> >
> > "ACPI PPTT: PPTT table found, but unable to locate core 1 (1)",
> >
> > and it in turn caused scheduler warnings when building up the system.
> > It took a while to root cause the problem be related a broken PPTT
> > table which has wrong cache information.
> >
> > To speedup debugging similar issues, dump the PPTT table, which makes
> > the warning more noticeable and helps bug hunting.
>
> Agreed, I think it was useful for debugging.
>
> >
> > The dumped info format on a ARM server is like:
> >
> > ACPI PPTT: Processors:
> > P[ 0][0x0024]: parent=0x0000 acpi_proc_id= 0 num_res=1 flags=0x11(package)
> > P[ 1][0x005a]: parent=0x0024 acpi_proc_id= 0 num_res=1 flags=0x12()
> > P[ 2][0x008a]: parent=0x005a acpi_proc_id= 0 num_res=3 flags=0x1a(leaf)
> > P[ 3][0x00f2]: parent=0x005a acpi_proc_id= 1 num_res=3 flags=0x1a(leaf)
> > P[ 4][0x015a]: parent=0x005a acpi_proc_id= 2 num_res=3 flags=0x1a(leaf)
> > ...
> > ACPI PPTT: Caches:
> > C[ 0][0x0072]: flags=0x7f next_level=0x0000 size=0x4000000 sets=65536 way=16 attribute=0xa line_size=64
> > C[ 1][0x00aa]: flags=0x7f next_level=0x00da size=0x10000 sets=256 way=4 attribute=0x4 line_size=64
> > C[ 2][0x00c2]: flags=0x7f next_level=0x00da size=0x10000 sets=256 way=4 attribute=0x2 line_size=64
> > C[ 3][0x00da]: flags=0x7f next_level=0x0000 size=0x100000 sets=2048 way=8 attribute=0xa line_size=64
> > ...
> >
> > It provides a global and straightforward view of the hierarchy of the
> > processor and caches info of the platform, and from the offset info
> > (the 3rd column), the child-parent relation could be checked.
> >
> > With this, the root cause of the original issue was pretty obvious,
> > that there were some caches items missing which caused the issue when
> > building up scheduler domain.
>
> Just a discussion, can we just dump the raw PPTT table via acpidump
> in user space when we meet the problem? With the raw PPTT table, we
> can go though the content to see if we have problems.

Good point! We can use iasl to decode the PPTT table. And this dump
is still useful as:
* when enabling new silicon or new firmware (APCI tables), sometimes it
can't make to boot to user space when the issue happens.
* This dump shows the processor and cache items separately and cleanly,
while the P[]/C[] index imply the numbers. In an 128 core product ARM
sever, the print with this patch is about 500 line, while the acpidump
is about 10,000 lines and harder to parse.

>
> >
> > Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com>
> > ---
> > Changelog:
> >
> > v2
> > * rebase againt 6.19 and refine the commit log
> >
> > drivers/acpi/pptt.c | 75 +++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 75 insertions(+)
> >
> > diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
> > index de5f8c018333..e00abedcd786 100644
> > --- a/drivers/acpi/pptt.c
> > +++ b/drivers/acpi/pptt.c
> > @@ -529,6 +529,79 @@ static void acpi_pptt_warn_missing(void)
> > pr_warn_once("No PPTT table found, CPU and cache topology may be inaccurate\n");
> > }
> > +static void acpi_dump_pptt_table(struct acpi_table_header *table_hdr)
> > +{
> > + struct acpi_subtable_header *entry, *entry_start;
> > + unsigned long end;
> > + struct acpi_pptt_processor *cpu;
> > + struct acpi_pptt_cache *cache;
> > + u32 entry_sz, i;
> > + u8 len;
> > + static bool dumped;
> > +
> > + /* PPTT table could be pretty big, no need to dump it twice */
> > + if (dumped)
> > + return;
> > + dumped = true;
> > +
> > + end = (unsigned long)table_hdr + table_hdr->length;
> > + entry_start = ACPI_ADD_PTR(struct acpi_subtable_header, table_hdr,
> > + sizeof(struct acpi_table_pptt));
> > +
> > + pr_info("Processors:\n");
> > + entry_sz = sizeof(struct acpi_pptt_processor);
> > + entry = entry_start;
> > + i = 0;
> > + while ((unsigned long)entry + entry_sz <= end) {
> > + len = entry->length;
> > + if (!len) {
> > + pr_warn("Invalid zero length subtable\n");
> > + return;
> > + }
> > +
> > + cpu = (struct acpi_pptt_processor *)entry;
> > + entry = ACPI_ADD_PTR(struct acpi_subtable_header, entry, len);
> > +
> > + if (cpu->header.type != ACPI_PPTT_TYPE_PROCESSOR)
> > + continue;
> > +
> > + printk(KERN_INFO "P[%3d][0x%04lx]: parent=0x%04x acpi_proc_id=%3d num_res=%d flags=0x%02x(%s%s%s)\n",
>
> pr_info() please.

Will change.

> > + i++, (unsigned long)cpu - (unsigned long)table_hdr,
> > + cpu->parent, cpu->acpi_processor_id,
> > + cpu->number_of_priv_resources, cpu->flags,
> > + cpu->flags & ACPI_PPTT_PHYSICAL_PACKAGE ? "package" : "",
> > + cpu->flags & ACPI_PPTT_ACPI_LEAF_NODE ? "leaf" : "",
> > + cpu->flags & ACPI_PPTT_ACPI_PROCESSOR_IS_THREAD ? ", thread" : ""
> > + );
> > +
> > + }
> > +
> > + pr_info("Caches:\n");
> > + entry_sz = sizeof(struct acpi_pptt_cache);
> > + entry = entry_start;
> > + i = 0;
> > + while ((unsigned long)entry + entry_sz <= end) {
> > + len = entry->length;
> > + if (!len) {
> > + pr_warn("Invalid zero length subtable\n");
> > + return;
> > + }
> > +
> > + cache = (struct acpi_pptt_cache *)entry;
> > + entry = ACPI_ADD_PTR(struct acpi_subtable_header, entry, len);
> > +
> > + if (cache->header.type != ACPI_PPTT_TYPE_CACHE)
> > + continue;
> > +
> > + printk(KERN_INFO "C[%4d][0x%04lx]: flags=0x%02x next_level=0x%04x size=0x%-8x sets=%-6d way=%-2d attribute=0x%-2x line_size=%d\n",
>
> Same here.

Yes.

> > + i++, (unsigned long)cache - (unsigned long)table_hdr,
> > + cache->flags, cache->next_level_of_cache, cache->size,
> > + cache->number_of_sets, cache->associativity,
> > + cache->attributes, cache->line_size
> > + );
> > + }
> > +}
> > +
> > /**
> > * topology_get_acpi_cpu_tag() - Find a unique topology value for a feature
> > * @table: Pointer to the head of the PPTT table
> > @@ -565,6 +638,8 @@ static int topology_get_acpi_cpu_tag(struct acpi_table_header *table,
> > }
> > pr_warn_once("PPTT table found, but unable to locate core %d (%d)\n",
> > cpu, acpi_cpu_id);
> > +
> > + acpi_dump_pptt_table(table);
>
> I think it would be good to dump it as needed, as a debug feature.

Makes sense to me. Should I add a kernel config option or a module
parameter for it, or just change the pr_info to pr_debug (it's in
a unlikely error path)?

Thanks,
Feng

> Thanks
> Hanjun