diff mbox series

[1/2] x86: acpi: Fix calculation of DSDT length

Message ID 20200909123322.1684889-1-wolfgang.wallner@br-automation.com
State Superseded
Delegated to: Bin Meng
Headers show
Series [1/2] x86: acpi: Fix calculation of DSDT length | expand

Commit Message

Wolfgang Wallner Sept. 9, 2020, 12:33 p.m. UTC
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(-)

Comments

Andy Shevchenko Sept. 9, 2020, 1:08 p.m. UTC | #1
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
> 
>
Andy Shevchenko Sept. 9, 2020, 1:11 p.m. UTC | #2
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?
Simon Glass Sept. 9, 2020, 2:35 p.m. UTC | #3
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>
Wolfgang Wallner Sept. 16, 2020, 3 p.m. UTC | #4
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
Andy Shevchenko Sept. 16, 2020, 3:53 p.m. UTC | #5
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.
diff mbox series

Patch

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.