* [PATCH] ACPI: PPTT: Dump PPTT table when error detected
@ 2025-10-14 8:51 Feng Tang
2025-10-14 22:54 ` Jeremy Linton
0 siblings, 1 reply; 3+ messages in thread
From: Feng Tang @ 2025-10-14 8:51 UTC (permalink / raw)
To: Rafael J . Wysocki, Len Brown, linux-acpi, linux-kernel
Cc: Sudeep Holla, James Morse, Jeremy Linton, Feng Tang
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 when there
is warning about it.
The dumped info 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
...
From it, we can see the processor/cache info and the hierarchy relation
between them.
Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com>
---
drivers/acpi/pptt.c | 75 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 75 insertions(+)
diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c
index 54676e3d82dd..e7b48a77a12f 100644
--- a/drivers/acpi/pptt.c
+++ b/drivers/acpi/pptt.c
@@ -498,6 +498,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",
+ 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",
+ 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
@@ -534,6 +607,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);
return -ENOENT;
}
--
2.43.5
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] ACPI: PPTT: Dump PPTT table when error detected 2025-10-14 8:51 [PATCH] ACPI: PPTT: Dump PPTT table when error detected Feng Tang @ 2025-10-14 22:54 ` Jeremy Linton 2025-10-15 1:50 ` Feng Tang 0 siblings, 1 reply; 3+ messages in thread From: Jeremy Linton @ 2025-10-14 22:54 UTC (permalink / raw) To: Feng Tang, Rafael J . Wysocki, Len Brown, linux-acpi, linux-kernel Cc: Sudeep Holla, James Morse Hi, On 10/14/25 3:51 AM, 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 when there > is warning about it. > > The dumped info 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 This is an interesting idea, particularly if the dump is smarter and acts like a linter, but i'm not sure the output right now would have helped with the original problem. In that respect simply sprinkling a lot of pr_debug information around during the parsing might be more helpful to understand the context around parse errors. I was under the impression doing that was frowned on, as I ended up removing quite a number of them during the initial reviews. I wonder if simply print_hex_dump'ing the table on certain errors with some acpi=debug like option is just as useful. Particularly if someone writes a better userspace pptt pretty printer, since the iasl output isn't helpful to understand the tree relationships. > ... > > From it, we can see the processor/cache info and the hierarchy relation > between them. > > Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com> > --- > drivers/acpi/pptt.c | 75 +++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 75 insertions(+) > > diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c > index 54676e3d82dd..e7b48a77a12f 100644 > --- a/drivers/acpi/pptt.c > +++ b/drivers/acpi/pptt.c > @@ -498,6 +498,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", > + 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", > + 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 > @@ -534,6 +607,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); > return -ENOENT; > } > ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] ACPI: PPTT: Dump PPTT table when error detected 2025-10-14 22:54 ` Jeremy Linton @ 2025-10-15 1:50 ` Feng Tang 0 siblings, 0 replies; 3+ messages in thread From: Feng Tang @ 2025-10-15 1:50 UTC (permalink / raw) To: Jeremy Linton Cc: Rafael J . Wysocki, Len Brown, linux-acpi, linux-kernel, Sudeep Holla, James Morse Hi Linton, Thanks for the prompt review! On Tue, Oct 14, 2025 at 05:54:14PM -0500, Jeremy Linton wrote: > Hi, > > On 10/14/25 3:51 AM, 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 when there > > is warning about it. > > > > The dumped info 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 > > > This is an interesting idea, particularly if the dump is smarter and acts > like a linter, but i'm not sure the output right now would have helped with > the original problem. I would say yes :). If the patch was in, it would print out N CPU core info, while the platform actually has 2N CPUs, which made it obvious that the PPTT table was broken and needed firmware team's fix. > In that respect simply sprinkling a lot of pr_debug > information around during the parsing might be more helpful to understand > the context around parse errors. > > I was under the impression doing that was frowned on, as I ended up removing > quite a number of them during the initial reviews. Completely understand. I believe some of the removed stuff can help our case. A big parf of my job is debuging and fixing bugs, so personally I appreciate more debug info :) > > I wonder if simply print_hex_dump'ing the table on certain errors with some > acpi=debug like option is just as useful. Particularly if someone writes a > better userspace pptt pretty printer, since the iasl output isn't helpful to > understand the tree relationships. When I met the issue, I have zero knowledge of PPTT table, and I manually parse the PPTT table in /sys/firmware/acpi/table/PPTT when a printed ACPI spec :) So I think this kind of global dumping can help engineers who are not familiar with PPTT table. Thanks, Feng > > > ... > > > > From it, we can see the processor/cache info and the hierarchy relation > > between them. > > > > Signed-off-by: Feng Tang <feng.tang@linux.alibaba.com> > > --- > > drivers/acpi/pptt.c | 75 +++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 75 insertions(+) > > > > diff --git a/drivers/acpi/pptt.c b/drivers/acpi/pptt.c > > index 54676e3d82dd..e7b48a77a12f 100644 > > --- a/drivers/acpi/pptt.c > > +++ b/drivers/acpi/pptt.c > > @@ -498,6 +498,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", > > + 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", > > + 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 > > @@ -534,6 +607,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); > > return -ENOENT; > > } ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-10-15 1:50 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-10-14 8:51 [PATCH] ACPI: PPTT: Dump PPTT table when error detected Feng Tang 2025-10-14 22:54 ` Jeremy Linton 2025-10-15 1:50 ` Feng Tang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®