* [PATCH 1/2] x86: acpi: Fix calculation of DSDT length
@ 2020-09-09 12:33 Wolfgang Wallner
2020-09-09 12:33 ` [PATCH 2/2] x86: acpi: Add memset to initialize SPCR table Wolfgang Wallner
` (2 more replies)
0 siblings, 3 replies; 9+ messages in thread
From: Wolfgang Wallner @ 2020-09-09 12:33 UTC (permalink / raw)
To: u-boot
Currently, the calculation for the length of the DSDT table includes any
bytes that are added for alignment, but those bytes are not initialized.
This is because the DSDT length is calculated after a call to
acpi_inc_align(). Split this up into the following sequence:
* acpi_inc()
* Calculate DSDT length
* acpi_align()
Signed-off-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
---
arch/x86/lib/acpi_table.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c
index 3a93fedfc3..6b827bfa3f 100644
--- a/arch/x86/lib/acpi_table.c
+++ b/arch/x86/lib/acpi_table.c
@@ -427,7 +427,7 @@ ulong write_acpi_tables(ulong start_addr)
(char *)&AmlCode + sizeof(struct acpi_table_header),
dsdt->length - sizeof(struct acpi_table_header));
- acpi_inc_align(ctx, dsdt->length - sizeof(struct acpi_table_header));
+ acpi_inc(ctx, dsdt->length - sizeof(struct acpi_table_header));
/* Pack GNVS into the ACPI table area */
for (i = 0; i < dsdt->length; i++) {
@@ -450,6 +450,8 @@ ulong write_acpi_tables(ulong start_addr)
dsdt->checksum = 0;
dsdt->checksum = table_compute_checksum((void *)dsdt, dsdt->length);
+ acpi_align(ctx);
+
/*
* Fill in platform-specific global NVS variables. If this fails we
* cannot return the error but this should only happen while debugging.
--
2.28.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH 2/2] x86: acpi: Add memset to initialize SPCR table
2020-09-09 12:33 [PATCH 1/2] x86: acpi: Fix calculation of DSDT length Wolfgang Wallner
@ 2020-09-09 12:33 ` Wolfgang Wallner
2020-09-09 13:09 ` Andy Shevchenko
2020-09-09 14:35 ` Simon Glass
2020-09-09 13:08 ` [PATCH 1/2] x86: acpi: Fix calculation of DSDT length Andy Shevchenko
2020-09-09 14:35 ` Simon Glass
2 siblings, 2 replies; 9+ messages in thread
From: Wolfgang Wallner @ 2020-09-09 12:33 UTC (permalink / raw)
To: u-boot
Add a missing memset to acpi_create_spcr().
The other acpi_create_xxxx() functions perform a memset on their
structures, acpi_create_spcr() does not and as a result the contents of
this table are partly uninitialized (and thus random after every reset).
Signed-off-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
---
arch/x86/lib/acpi_table.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c
index 6b827bfa3f..054235843e 100644
--- a/arch/x86/lib/acpi_table.c
+++ b/arch/x86/lib/acpi_table.c
@@ -252,6 +252,8 @@ static void acpi_create_spcr(struct acpi_spcr *spcr)
int space_id;
int ret = -ENODEV;
+ memset((void *)spcr, 0, sizeof(struct acpi_spcr));
+
/* Fill out header fields */
acpi_fill_header(header, "SPCR");
header->length = sizeof(struct acpi_spcr);
@@ -359,6 +361,7 @@ static void acpi_create_spcr(struct acpi_spcr *spcr)
spcr->baud_rate = 0;
/* Fix checksum */
+
header->checksum = table_compute_checksum((void *)spcr, header->length);
}
--
2.28.0
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] x86: acpi: Add memset to initialize SPCR table
2020-09-09 12:33 ` [PATCH 2/2] x86: acpi: Add memset to initialize SPCR table Wolfgang Wallner
@ 2020-09-09 13:09 ` Andy Shevchenko
2020-09-09 14:35 ` Simon Glass
1 sibling, 0 replies; 9+ messages in thread
From: Andy Shevchenko @ 2020-09-09 13:09 UTC (permalink / raw)
To: u-boot
On Wed, Sep 09, 2020 at 02:33:21PM +0200, Wolfgang Wallner wrote:
> Add a missing memset to acpi_create_spcr().
>
> The other acpi_create_xxxx() functions perform a memset on their
> structures, acpi_create_spcr() does not and as a result the contents of
> this table are partly uninitialized (and thus random after every reset).
Fixes: tag?
In any case, after addressing below,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
> ---
>
> arch/x86/lib/acpi_table.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c
> index 6b827bfa3f..054235843e 100644
> --- a/arch/x86/lib/acpi_table.c
> +++ b/arch/x86/lib/acpi_table.c
> @@ -252,6 +252,8 @@ static void acpi_create_spcr(struct acpi_spcr *spcr)
> int space_id;
> int ret = -ENODEV;
>
> + memset((void *)spcr, 0, sizeof(struct acpi_spcr));
> +
> /* Fill out header fields */
> acpi_fill_header(header, "SPCR");
> header->length = sizeof(struct acpi_spcr);
> @@ -359,6 +361,7 @@ static void acpi_create_spcr(struct acpi_spcr *spcr)
> spcr->baud_rate = 0;
>
> /* Fix checksum */
> +
> header->checksum = table_compute_checksum((void *)spcr, header->length);
> }
Unrelated change.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 2/2] x86: acpi: Add memset to initialize SPCR table
2020-09-09 12:33 ` [PATCH 2/2] x86: acpi: Add memset to initialize SPCR table Wolfgang Wallner
2020-09-09 13:09 ` Andy Shevchenko
@ 2020-09-09 14:35 ` Simon Glass
1 sibling, 0 replies; 9+ messages in thread
From: Simon Glass @ 2020-09-09 14:35 UTC (permalink / raw)
To: u-boot
On Wed, 9 Sep 2020 at 06:33, Wolfgang Wallner
<wolfgang.wallner@br-automation.com> wrote:
>
> Add a missing memset to acpi_create_spcr().
>
> The other acpi_create_xxxx() functions perform a memset on their
> structures, acpi_create_spcr() does not and as a result the contents of
> this table are partly uninitialized (and thus random after every reset).
>
> Signed-off-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
> ---
>
> arch/x86/lib/acpi_table.c | 3 +++
> 1 file changed, 3 insertions(+)
Reviewed-by: Simon Glass <sjg@chromium.org>
For further thought, I do wonder if acpi_fill_header() should do the
memset, if sizeof() could be passed as a parameter?
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] x86: acpi: Fix calculation of DSDT length
2020-09-09 12:33 [PATCH 1/2] x86: acpi: Fix calculation of DSDT length Wolfgang Wallner
2020-09-09 12:33 ` [PATCH 2/2] x86: acpi: Add memset to initialize SPCR table Wolfgang Wallner
@ 2020-09-09 13:08 ` Andy Shevchenko
2020-09-09 13:11 ` Andy Shevchenko
2020-09-09 14:35 ` Simon Glass
2 siblings, 1 reply; 9+ messages in thread
From: Andy Shevchenko @ 2020-09-09 13:08 UTC (permalink / raw)
To: u-boot
On Wed, Sep 09, 2020 at 02:33:20PM +0200, Wolfgang Wallner wrote:
> Currently, the calculation for the length of the DSDT table includes any
> bytes that are added for alignment, but those bytes are not initialized.
>
> This is because the DSDT length is calculated after a call to
> acpi_inc_align(). Split this up into the following sequence:
>
> * acpi_inc()
> * Calculate DSDT length
> * acpi_align()
Perhaps Fixes: tag?
In any case,
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> Signed-off-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
>
> ---
>
> arch/x86/lib/acpi_table.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c
> index 3a93fedfc3..6b827bfa3f 100644
> --- a/arch/x86/lib/acpi_table.c
> +++ b/arch/x86/lib/acpi_table.c
> @@ -427,7 +427,7 @@ ulong write_acpi_tables(ulong start_addr)
> (char *)&AmlCode + sizeof(struct acpi_table_header),
> dsdt->length - sizeof(struct acpi_table_header));
>
> - acpi_inc_align(ctx, dsdt->length - sizeof(struct acpi_table_header));
> + acpi_inc(ctx, dsdt->length - sizeof(struct acpi_table_header));
>
> /* Pack GNVS into the ACPI table area */
> for (i = 0; i < dsdt->length; i++) {
> @@ -450,6 +450,8 @@ ulong write_acpi_tables(ulong start_addr)
> dsdt->checksum = 0;
> dsdt->checksum = table_compute_checksum((void *)dsdt, dsdt->length);
>
> + acpi_align(ctx);
> +
> /*
> * Fill in platform-specific global NVS variables. If this fails we
> * cannot return the error but this should only happen while debugging.
> --
> 2.28.0
>
>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 9+ messages in thread* [PATCH 1/2] x86: acpi: Fix calculation of DSDT length
2020-09-09 13:08 ` [PATCH 1/2] x86: acpi: Fix calculation of DSDT length Andy Shevchenko
@ 2020-09-09 13:11 ` Andy Shevchenko
2020-09-16 15:00 ` Wolfgang Wallner
0 siblings, 1 reply; 9+ messages in thread
From: Andy Shevchenko @ 2020-09-09 13:11 UTC (permalink / raw)
To: u-boot
On Wed, Sep 09, 2020 at 04:08:17PM +0300, Andy Shevchenko wrote:
> On Wed, Sep 09, 2020 at 02:33:20PM +0200, Wolfgang Wallner wrote:
> > Currently, the calculation for the length of the DSDT table includes any
> > bytes that are added for alignment, but those bytes are not initialized.
> >
> > This is because the DSDT length is calculated after a call to
> > acpi_inc_align(). Split this up into the following sequence:
> >
> > * acpi_inc()
> > * Calculate DSDT length
> > * acpi_align()
>
> Perhaps Fixes: tag?
Ah, it is against series not yet applied?
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] x86: acpi: Fix calculation of DSDT length
2020-09-09 13:11 ` Andy Shevchenko
@ 2020-09-16 15:00 ` Wolfgang Wallner
2020-09-16 15:53 ` Andy Shevchenko
0 siblings, 1 reply; 9+ messages in thread
From: Wolfgang Wallner @ 2020-09-16 15:00 UTC (permalink / raw)
To: u-boot
Hi Andy,
-----"Andy Shevchenko" <andriy.shevchenko@linux.intel.com> schrieb: -----
> Betreff: Re: [PATCH 1/2] x86: acpi: Fix calculation of DSDT length
>
> On Wed, Sep 09, 2020 at 04:08:17PM +0300, Andy Shevchenko wrote:
> > On Wed, Sep 09, 2020 at 02:33:20PM +0200, Wolfgang Wallner wrote:
> > > Currently, the calculation for the length of the DSDT table includes any
> > > bytes that are added for alignment, but those bytes are not initialized.
> > >
> > > This is because the DSDT length is calculated after a call to
> > > acpi_inc_align(). Split this up into the following sequence:
> > >
> > > * acpi_inc()
> > > * Calculate DSDT length
> > > * acpi_align()
> >
> > Perhaps Fixes: tag?
>
> Ah, it is against series not yet applied?
No, the patch is against master.
But I could not point to a specific commit that is fixed, the code in question
is a result of multiple commits.
Because of this I have also sent V2 without a Fixes tags.
regards, Wolfgang
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] x86: acpi: Fix calculation of DSDT length
2020-09-16 15:00 ` Wolfgang Wallner
@ 2020-09-16 15:53 ` Andy Shevchenko
0 siblings, 0 replies; 9+ messages in thread
From: Andy Shevchenko @ 2020-09-16 15:53 UTC (permalink / raw)
To: u-boot
On Wed, Sep 16, 2020 at 05:00:26PM +0200, Wolfgang Wallner wrote:
> -----"Andy Shevchenko" <andriy.shevchenko@linux.intel.com> schrieb: -----
> > Betreff: Re: [PATCH 1/2] x86: acpi: Fix calculation of DSDT length
> >
> > On Wed, Sep 09, 2020 at 04:08:17PM +0300, Andy Shevchenko wrote:
> > > On Wed, Sep 09, 2020 at 02:33:20PM +0200, Wolfgang Wallner wrote:
> > > > Currently, the calculation for the length of the DSDT table includes any
> > > > bytes that are added for alignment, but those bytes are not initialized.
> > > >
> > > > This is because the DSDT length is calculated after a call to
> > > > acpi_inc_align(). Split this up into the following sequence:
> > > >
> > > > * acpi_inc()
> > > > * Calculate DSDT length
> > > > * acpi_align()
> > >
> > > Perhaps Fixes: tag?
> >
> > Ah, it is against series not yet applied?
>
> No, the patch is against master.
> But I could not point to a specific commit that is fixed, the code in question
> is a result of multiple commits.
>
> Because of this I have also sent V2 without a Fixes tags.
Hmm... Usually in kernel we put a chain of Fixes in such cases.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/2] x86: acpi: Fix calculation of DSDT length
2020-09-09 12:33 [PATCH 1/2] x86: acpi: Fix calculation of DSDT length Wolfgang Wallner
2020-09-09 12:33 ` [PATCH 2/2] x86: acpi: Add memset to initialize SPCR table Wolfgang Wallner
2020-09-09 13:08 ` [PATCH 1/2] x86: acpi: Fix calculation of DSDT length Andy Shevchenko
@ 2020-09-09 14:35 ` Simon Glass
2 siblings, 0 replies; 9+ messages in thread
From: Simon Glass @ 2020-09-09 14:35 UTC (permalink / raw)
To: u-boot
On Wed, 9 Sep 2020 at 06:33, Wolfgang Wallner
<wolfgang.wallner@br-automation.com> wrote:
>
> Currently, the calculation for the length of the DSDT table includes any
> bytes that are added for alignment, but those bytes are not initialized.
>
> This is because the DSDT length is calculated after a call to
> acpi_inc_align(). Split this up into the following sequence:
>
> * acpi_inc()
> * Calculate DSDT length
> * acpi_align()
>
> Signed-off-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
>
> ---
>
> arch/x86/lib/acpi_table.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
Reviewed-by: Simon Glass <sjg@chromium.org>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-09-16 15:53 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-09-09 12:33 [PATCH 1/2] x86: acpi: Fix calculation of DSDT length Wolfgang Wallner
2020-09-09 12:33 ` [PATCH 2/2] x86: acpi: Add memset to initialize SPCR table Wolfgang Wallner
2020-09-09 13:09 ` Andy Shevchenko
2020-09-09 14:35 ` Simon Glass
2020-09-09 13:08 ` [PATCH 1/2] x86: acpi: Fix calculation of DSDT length Andy Shevchenko
2020-09-09 13:11 ` Andy Shevchenko
2020-09-16 15:00 ` Wolfgang Wallner
2020-09-16 15:53 ` Andy Shevchenko
2020-09-09 14:35 ` Simon Glass
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox