[PATCH v1] cmd: acpi: Print revisions in hex format
The revisions are usually dates in hex-decimal format representing YYYYmmdd. Print them in hex to see this clearly.
Before: ... FACP 000e5420 0000f4 (v06 U-BOOT U-BOOTBL 538970376 INTL 0) DSDT 000e4780 000ba0 (v02 U-BOOT U-BOOTBL 65536 INTL 538968870) ... After: ... FACP 000e5420 0000f4 (v06 U-BOOT U-BOOTBL 20200908 INTL 0) DSDT 000e4780 000ba0 (v02 U-BOOT U-BOOTBL 10000 INTL 20200326) ...
Fixes: 0b885bcfd9b0 ("acpi: Add an acpi command") Cc: Wolfgang Wallner wolfgang.wallner@br-automation.com Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com --- cmd/acpi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cmd/acpi.c b/cmd/acpi.c index 085a3a650d1a..a3419b42b556 100644 --- a/cmd/acpi.c +++ b/cmd/acpi.c @@ -26,7 +26,7 @@ static void dump_hdr(struct acpi_table_header *hdr) printf("%.*s %08lx %06x", ACPI_NAME_LEN, hdr->signature, (ulong)map_to_sysmem(hdr), hdr->length); if (has_hdr) { - printf(" (v%02d %.6s %.8s %u %.4s %d)\n", hdr->revision, + printf(" (v%02d %.6s %.8s %x %.4s %x)\n", hdr->revision, hdr->oem_id, hdr->oem_table_id, hdr->oem_revision, hdr->aslc_id, hdr->aslc_revision); } else {
Hi Andy,
-----"Andy Shevchenko" andriy.shevchenko@linux.intel.com schrieb: -----
Betreff: [PATCH v1] cmd: acpi: Print revisions in hex format
The revisions are usually dates in hex-decimal format representing YYYYmmdd. Print them in hex to see this clearly.
Before: ... FACP 000e5420 0000f4 (v06 U-BOOT U-BOOTBL 538970376 INTL 0) DSDT 000e4780 000ba0 (v02 U-BOOT U-BOOTBL 65536 INTL 538968870) ... After: ... FACP 000e5420 0000f4 (v06 U-BOOT U-BOOTBL 20200908 INTL 0) DSDT 000e4780 000ba0 (v02 U-BOOT U-BOOTBL 10000 INTL 20200326) ...
Fixes: 0b885bcfd9b0 ("acpi: Add an acpi command") Cc: Wolfgang Wallner wolfgang.wallner@br-automation.com Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
cmd/acpi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com Tested-by: Wolfgang Wallner wolfgang.wallner@br-automation.com Tested on a custom Apollolake board.
Related to "acpi list": During my recent ACPI debugging I found it very useful to have the checksum printed for each table with "acpi list". Would there be interest to have that upstream? If so I would send a patch.
regards, Wolfgang
On Tue, Sep 8, 2020 at 5:58 PM Wolfgang Wallner wolfgang.wallner@br-automation.com wrote:
-----"Andy Shevchenko" andriy.shevchenko@linux.intel.com schrieb: -----
Betreff: [PATCH v1] cmd: acpi: Print revisions in hex format
The revisions are usually dates in hex-decimal format representing YYYYmmdd. Print them in hex to see this clearly.
Before: ... FACP 000e5420 0000f4 (v06 U-BOOT U-BOOTBL 538970376 INTL 0) DSDT 000e4780 000ba0 (v02 U-BOOT U-BOOTBL 65536 INTL 538968870) ... After: ... FACP 000e5420 0000f4 (v06 U-BOOT U-BOOTBL 20200908 INTL 0) DSDT 000e4780 000ba0 (v02 U-BOOT U-BOOTBL 10000 INTL 20200326) ...
Fixes: 0b885bcfd9b0 ("acpi: Add an acpi command") Cc: Wolfgang Wallner wolfgang.wallner@br-automation.com Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
cmd/acpi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com Tested-by: Wolfgang Wallner wolfgang.wallner@br-automation.com Tested on a custom Apollolake board.
Thanks!
Related to "acpi list": During my recent ACPI debugging I found it very useful to have the checksum printed for each table with "acpi list". Would there be interest to have that upstream? If so I would send a patch.
Can you elaborate what was the problem that checksum helped?
Hi Andy,
-----"Andy Shevchenko" andy.shevchenko@gmail.com schrieb: -----
Betreff: Re: [PATCH v1] cmd: acpi: Print revisions in hex format
On Tue, Sep 8, 2020 at 5:58 PM Wolfgang Wallner wolfgang.wallner@br-automation.com wrote:
-----"Andy Shevchenko" andriy.shevchenko@linux.intel.com schrieb: -----
Betreff: [PATCH v1] cmd: acpi: Print revisions in hex format
The revisions are usually dates in hex-decimal format representing YYYYmmdd. Print them in hex to see this clearly.
Before: ... FACP 000e5420 0000f4 (v06 U-BOOT U-BOOTBL 538970376 INTL 0) DSDT 000e4780 000ba0 (v02 U-BOOT U-BOOTBL 65536 INTL 538968870) ... After: ... FACP 000e5420 0000f4 (v06 U-BOOT U-BOOTBL 20200908 INTL 0) DSDT 000e4780 000ba0 (v02 U-BOOT U-BOOTBL 10000 INTL 20200326) ...
Fixes: 0b885bcfd9b0 ("acpi: Add an acpi command") Cc: Wolfgang Wallner wolfgang.wallner@br-automation.com Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
cmd/acpi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Wolfgang Wallner wolfgang.wallner@br-automation.com Tested-by: Wolfgang Wallner wolfgang.wallner@br-automation.com Tested on a custom Apollolake board.
Thanks!
You're welcome.
Related to "acpi list": During my recent ACPI debugging I found it very useful to have the checksum printed for each table with "acpi list". Would there be interest to have that upstream? If so I would send a patch.
Can you elaborate what was the problem that checksum helped?
Sure. I saw two strange things with the ACPI checksums:
1) The DSDT length included uninitialized bytes from alignment. This is described in the following link:
https://lists.denx.de/pipermail/u-boot/2020-September/425378.html
This was the actual bug I was looking for.
2) acpi_create_spcr() is missing a memset(). The other acpi_create_xxxx() functions perform a memset on their structure, acpi_create_spcr() does not and as a result the contents of this table are party uninitialized.
I plan to send a patch for both of them.
Regards, Wolfgang
On Tue, Sep 08, 2020 at 05:32:08PM +0200, Wolfgang Wallner wrote:
-----"Andy Shevchenko" andy.shevchenko@gmail.com schrieb: -----
On Tue, Sep 8, 2020 at 5:58 PM Wolfgang Wallner wolfgang.wallner@br-automation.com wrote:
-----"Andy Shevchenko" andriy.shevchenko@linux.intel.com schrieb: -----
...
Related to "acpi list": During my recent ACPI debugging I found it very useful to have the checksum printed for each table with "acpi list". Would there be interest to have that upstream? If so I would send a patch.
Can you elaborate what was the problem that checksum helped?
Sure. I saw two strange things with the ACPI checksums:
- The DSDT length included uninitialized bytes from alignment. This is
described in the following link:
https://lists.denx.de/pipermail/u-boot/2020-September/425378.html
This was the actual bug I was looking for.
- acpi_create_spcr() is missing a memset(). The other acpi_create_xxxx()
functions perform a memset on their structure, acpi_create_spcr() does not and as a result the contents of this table are party uninitialized.
I plan to send a patch for both of them.
I'm not sure I understood how checksum pointed to uninitialized data?
Hi Andy,
-----"Andy Shevchenko" andy.shevchenko@gmail.com schrieb: -----
Betreff: Re: [PATCH v1] cmd: acpi: Print revisions in hex format
On Tue, Sep 08, 2020 at 05:32:08PM +0200, Wolfgang Wallner wrote:
-----"Andy Shevchenko" andy.shevchenko@gmail.com schrieb: -----
On Tue, Sep 8, 2020 at 5:58 PM Wolfgang Wallner wolfgang.wallner@br-automation.com wrote:
-----"Andy Shevchenko" andriy.shevchenko@linux.intel.com schrieb: -----
...
Related to "acpi list": During my recent ACPI debugging I found it very useful to have the checksum printed for each table with "acpi list". Would there be interest to have that upstream? If so I would send a patch.
Can you elaborate what was the problem that checksum helped?
Sure. I saw two strange things with the ACPI checksums:
- The DSDT length included uninitialized bytes from alignment. This is
described in the following link:
https://lists.denx.de/pipermail/u-boot/2020-September/425378.html
This was the actual bug I was looking for.
- acpi_create_spcr() is missing a memset(). The other acpi_create_xxxx()
functions perform a memset on their structure, acpi_create_spcr() does not and as a result the contents of this table are party uninitialized.
I plan to send a patch for both of them.
I'm not sure I understood how checksum pointed to uninitialized data?
After adding the checksums to "acpi list" I realized that the checksums for DSDT and SPCR where different after every reset. Looking at the code each turned out to be somehow related to uninitialized memory.
regards, Wolfgang
PS: My mail client has somehow corrupted your last mail, and I can't open it. A colleague has forwarded it to me so I could reply. This is why I reply to my own mail.
On Wed, Sep 09, 2020 at 09:15:22AM +0200, Wolfgang Wallner wrote:
-----"Andy Shevchenko" andy.shevchenko@gmail.com schrieb: -----
Betreff: Re: [PATCH v1] cmd: acpi: Print revisions in hex format On Tue, Sep 08, 2020 at 05:32:08PM +0200, Wolfgang Wallner wrote:
-----"Andy Shevchenko" andy.shevchenko@gmail.com schrieb: -----
On Tue, Sep 8, 2020 at 5:58 PM Wolfgang Wallner wolfgang.wallner@br-automation.com wrote:
-----"Andy Shevchenko" andriy.shevchenko@linux.intel.com schrieb: -----
...
Related to "acpi list": During my recent ACPI debugging I found it very useful to have the checksum printed for each table with "acpi list". Would there be interest to have that upstream? If so I would send a patch.
Can you elaborate what was the problem that checksum helped?
Sure. I saw two strange things with the ACPI checksums:
- The DSDT length included uninitialized bytes from alignment. This is
described in the following link:
https://lists.denx.de/pipermail/u-boot/2020-September/425378.html
This was the actual bug I was looking for.
- acpi_create_spcr() is missing a memset(). The other acpi_create_xxxx()
functions perform a memset on their structure, acpi_create_spcr() does not and as a result the contents of this table are party uninitialized.
I plan to send a patch for both of them.
I'm not sure I understood how checksum pointed to uninitialized data?
After adding the checksums to "acpi list" I realized that the checksums for DSDT and SPCR where different after every reset. Looking at the code each turned out to be somehow related to uninitialized memory.
I see. I would rather to have it as a separate (sub-)command. But I leave this to Simon.
On Tue, Sep 8, 2020 at 10:30 PM Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
The revisions are usually dates in hex-decimal format representing YYYYmmdd. Print them in hex to see this clearly.
Before: ... FACP 000e5420 0000f4 (v06 U-BOOT U-BOOTBL 538970376 INTL 0) DSDT 000e4780 000ba0 (v02 U-BOOT U-BOOTBL 65536 INTL 538968870) ... After: ... FACP 000e5420 0000f4 (v06 U-BOOT U-BOOTBL 20200908 INTL 0) DSDT 000e4780 000ba0 (v02 U-BOOT U-BOOTBL 10000 INTL 20200326) ...
Fixes: 0b885bcfd9b0 ("acpi: Add an acpi command") Cc: Wolfgang Wallner wolfgang.wallner@br-automation.com Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
cmd/acpi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
On Mon, Sep 21, 2020 at 9:31 AM Bin Meng bmeng.cn@gmail.com wrote:
On Tue, Sep 8, 2020 at 10:30 PM Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
The revisions are usually dates in hex-decimal format representing YYYYmmdd. Print them in hex to see this clearly.
Before: ... FACP 000e5420 0000f4 (v06 U-BOOT U-BOOTBL 538970376 INTL 0) DSDT 000e4780 000ba0 (v02 U-BOOT U-BOOTBL 65536 INTL 538968870) ... After: ... FACP 000e5420 0000f4 (v06 U-BOOT U-BOOTBL 20200908 INTL 0) DSDT 000e4780 000ba0 (v02 U-BOOT U-BOOTBL 10000 INTL 20200326) ...
Fixes: 0b885bcfd9b0 ("acpi: Add an acpi command") Cc: Wolfgang Wallner wolfgang.wallner@br-automation.com Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
cmd/acpi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
applied to u-boot-x86, thanks!
Hi Andy,
On Tue, Sep 8, 2020 at 10:30 PM Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
The revisions are usually dates in hex-decimal format representing YYYYmmdd. Print them in hex to see this clearly.
Before: ... FACP 000e5420 0000f4 (v06 U-BOOT U-BOOTBL 538970376 INTL 0) DSDT 000e4780 000ba0 (v02 U-BOOT U-BOOTBL 65536 INTL 538968870) ... After: ... FACP 000e5420 0000f4 (v06 U-BOOT U-BOOTBL 20200908 INTL 0) DSDT 000e4780 000ba0 (v02 U-BOOT U-BOOTBL 10000 INTL 20200326) ...
Fixes: 0b885bcfd9b0 ("acpi: Add an acpi command") Cc: Wolfgang Wallner wolfgang.wallner@br-automation.com Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
cmd/acpi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cmd/acpi.c b/cmd/acpi.c index 085a3a650d1a..a3419b42b556 100644 --- a/cmd/acpi.c +++ b/cmd/acpi.c @@ -26,7 +26,7 @@ static void dump_hdr(struct acpi_table_header *hdr) printf("%.*s %08lx %06x", ACPI_NAME_LEN, hdr->signature, (ulong)map_to_sysmem(hdr), hdr->length); if (has_hdr) {
printf(" (v%02d %.6s %.8s %u %.4s %d)\n", hdr->revision,
printf(" (v%02d %.6s %.8s %x %.4s %x)\n", hdr->revision, hdr->oem_id, hdr->oem_table_id, hdr->oem_revision, hdr->aslc_id, hdr->aslc_revision); } else {--
This unfortunately breaks the unit test.
/u/test/dm/acpi.c:371, dm_test_acpi_cmd_list(): console: Expected 'RSDT 0570ebf0 000030 (v01 U-BOOT U-BOOTBL 538970401 INTL 0)', got 'RSDT 0570ebf0 000030 (v01 U-BOOT U-BOOTBL 20200921 INTL 0)' Failures: 2
Would you please resend v2 to fix this?
Regards, Bin
On Mon, Sep 21, 2020 at 10:20:18AM +0800, Bin Meng wrote:
On Tue, Sep 8, 2020 at 10:30 PM Andy Shevchenko andriy.shevchenko@linux.intel.com wrote:
The revisions are usually dates in hex-decimal format representing YYYYmmdd. Print them in hex to see this clearly.
Before: ... FACP 000e5420 0000f4 (v06 U-BOOT U-BOOTBL 538970376 INTL 0) DSDT 000e4780 000ba0 (v02 U-BOOT U-BOOTBL 65536 INTL 538968870) ... After: ... FACP 000e5420 0000f4 (v06 U-BOOT U-BOOTBL 20200908 INTL 0) DSDT 000e4780 000ba0 (v02 U-BOOT U-BOOTBL 10000 INTL 20200326) ...
Fixes: 0b885bcfd9b0 ("acpi: Add an acpi command") Cc: Wolfgang Wallner wolfgang.wallner@br-automation.com Signed-off-by: Andy Shevchenko andriy.shevchenko@linux.intel.com
cmd/acpi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cmd/acpi.c b/cmd/acpi.c index 085a3a650d1a..a3419b42b556 100644 --- a/cmd/acpi.c +++ b/cmd/acpi.c @@ -26,7 +26,7 @@ static void dump_hdr(struct acpi_table_header *hdr) printf("%.*s %08lx %06x", ACPI_NAME_LEN, hdr->signature, (ulong)map_to_sysmem(hdr), hdr->length); if (has_hdr) {
printf(" (v%02d %.6s %.8s %u %.4s %d)\n", hdr->revision,
printf(" (v%02d %.6s %.8s %x %.4s %x)\n", hdr->revision, hdr->oem_id, hdr->oem_table_id, hdr->oem_revision, hdr->aslc_id, hdr->aslc_revision); } else {--
This unfortunately breaks the unit test.
/u/test/dm/acpi.c:371, dm_test_acpi_cmd_list(): console: Expected 'RSDT 0570ebf0 000030 (v01 U-BOOT U-BOOTBL 538970401 INTL 0)', got 'RSDT 0570ebf0 000030 (v01 U-BOOT U-BOOTBL 20200921 INTL 0)' Failures: 2
Would you please resend v2 to fix this?
v2 has been just sent. Thanks!
participants (4)
-
Andy Shevchenko -
Andy Shevchenko -
Bin Meng -
Wolfgang Wallner