mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [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®