All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/1] acpi: cannot have RSDT above 4 GiB
@ 2023-11-11 14:28 Heinrich Schuchardt
  2023-11-12 20:01 ` Simon Glass
  0 siblings, 1 reply; 4+ messages in thread
From: Heinrich Schuchardt @ 2023-11-11 14:28 UTC (permalink / raw)
  To: Simon Glass; +Cc: u-boot, Heinrich Schuchardt

The field RsdtAddress has only 32 bit. The RSDT table cannot be located
beyond 4 GiB.

Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
---
 lib/acpi/base.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/lib/acpi/base.c b/lib/acpi/base.c
index 2057bd2bef..128a76ad39 100644
--- a/lib/acpi/base.c
+++ b/lib/acpi/base.c
@@ -21,10 +21,17 @@ void acpi_write_rsdp(struct acpi_rsdp *rsdp, struct acpi_rsdt *rsdt,
 	memcpy(rsdp->signature, RSDP_SIG, 8);
 	memcpy(rsdp->oem_id, OEM_ID, 6);
 
-	rsdp->length = sizeof(struct acpi_rsdp);
-	rsdp->rsdt_address = map_to_sysmem(rsdt);
+	if (rsdt)
+		rsdp->rsdt_address = map_to_sysmem(rsdt);
+	else
+		rsdp->rsdt_address = 0;
+
+	if (xsdt)
+		rsdp->xsdt_address = map_to_sysmem(xsdt);
+	else
+		rsdp->xsdt_address = 0;
 
-	rsdp->xsdt_address = map_to_sysmem(xsdt);
+	rsdp->length = sizeof(struct acpi_rsdp);
 	rsdp->revision = ACPI_RSDP_REV_ACPI_2_0;
 
 	/* Calculate checksums */
@@ -68,11 +75,15 @@ static void acpi_write_xsdt(struct acpi_xsdt *xsdt)
 static int acpi_write_base(struct acpi_ctx *ctx,
 			   const struct acpi_writer *entry)
 {
-	/* We need at least an RSDP and an RSDT Table */
+	/* We need at least an RSDP and an XSDT Table */
 	ctx->rsdp = ctx->current;
 	acpi_inc_align(ctx, sizeof(struct acpi_rsdp));
-	ctx->rsdt = ctx->current;
-	acpi_inc_align(ctx, sizeof(struct acpi_rsdt));
+	if (map_to_sysmem(ctx->current) < UINT_MAX - 0x10000) {
+		ctx->rsdt = ctx->current;
+		acpi_inc_align(ctx, sizeof(struct acpi_rsdt));
+	} else {
+		ctx->rsdt = 0;
+	}
 	ctx->xsdt = ctx->current;
 	acpi_inc_align(ctx, sizeof(struct acpi_xsdt));
 
@@ -80,7 +91,8 @@ static int acpi_write_base(struct acpi_ctx *ctx,
 	memset(ctx->base, '\0', ctx->current - ctx->base);
 
 	acpi_write_rsdp(ctx->rsdp, ctx->rsdt, ctx->xsdt);
-	acpi_write_rsdt(ctx->rsdt);
+	if (ctx->rsdt)
+		acpi_write_rsdt(ctx->rsdt);
 	acpi_write_xsdt(ctx->xsdt);
 
 	return 0;
-- 
2.40.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] acpi: cannot have RSDT above 4 GiB
  2023-11-11 14:28 [PATCH 1/1] acpi: cannot have RSDT above 4 GiB Heinrich Schuchardt
@ 2023-11-12 20:01 ` Simon Glass
  2023-11-12 20:16   ` Heinrich Schuchardt
  0 siblings, 1 reply; 4+ messages in thread
From: Simon Glass @ 2023-11-12 20:01 UTC (permalink / raw)
  To: Heinrich Schuchardt; +Cc: u-boot

Hi Heinrich,

On Sat, 11 Nov 2023 at 07:28, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
> The field RsdtAddress has only 32 bit. The RSDT table cannot be located
> beyond 4 GiB.
>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> ---
>  lib/acpi/base.c | 26 +++++++++++++++++++-------
>  1 file changed, 19 insertions(+), 7 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

nits / question below

>
> diff --git a/lib/acpi/base.c b/lib/acpi/base.c
> index 2057bd2bef..128a76ad39 100644
> --- a/lib/acpi/base.c
> +++ b/lib/acpi/base.c
> @@ -21,10 +21,17 @@ void acpi_write_rsdp(struct acpi_rsdp *rsdp, struct acpi_rsdt *rsdt,
>         memcpy(rsdp->signature, RSDP_SIG, 8);
>         memcpy(rsdp->oem_id, OEM_ID, 6);
>
> -       rsdp->length = sizeof(struct acpi_rsdp);
> -       rsdp->rsdt_address = map_to_sysmem(rsdt);
> +       if (rsdt)
> +               rsdp->rsdt_address = map_to_sysmem(rsdt);
> +       else
> +               rsdp->rsdt_address = 0;

There is a memset() at the top so this line (and the one below) should
not be needed.

> +
> +       if (xsdt)
> +               rsdp->xsdt_address = map_to_sysmem(xsdt);
> +       else
> +               rsdp->xsdt_address = 0;
>
> -       rsdp->xsdt_address = map_to_sysmem(xsdt);
> +       rsdp->length = sizeof(struct acpi_rsdp);
>         rsdp->revision = ACPI_RSDP_REV_ACPI_2_0;
>
>         /* Calculate checksums */
> @@ -68,11 +75,15 @@ static void acpi_write_xsdt(struct acpi_xsdt *xsdt)
>  static int acpi_write_base(struct acpi_ctx *ctx,
>                            const struct acpi_writer *entry)
>  {
> -       /* We need at least an RSDP and an RSDT Table */
> +       /* We need at least an RSDP and an XSDT Table */
>         ctx->rsdp = ctx->current;
>         acpi_inc_align(ctx, sizeof(struct acpi_rsdp));
> -       ctx->rsdt = ctx->current;
> -       acpi_inc_align(ctx, sizeof(struct acpi_rsdt));
> +       if (map_to_sysmem(ctx->current) < UINT_MAX - 0x10000) {

Won't that do something different on 64-bit machines? It seems that
you want to check against SZ_4G ?

> +               ctx->rsdt = ctx->current;
> +               acpi_inc_align(ctx, sizeof(struct acpi_rsdt));
> +       } else {
> +               ctx->rsdt = 0;
> +       }
>         ctx->xsdt = ctx->current;
>         acpi_inc_align(ctx, sizeof(struct acpi_xsdt));
>
> @@ -80,7 +91,8 @@ static int acpi_write_base(struct acpi_ctx *ctx,
>         memset(ctx->base, '\0', ctx->current - ctx->base);
>
>         acpi_write_rsdp(ctx->rsdp, ctx->rsdt, ctx->xsdt);
> -       acpi_write_rsdt(ctx->rsdt);
> +       if (ctx->rsdt)
> +               acpi_write_rsdt(ctx->rsdt);
>         acpi_write_xsdt(ctx->xsdt);
>
>         return 0;
> --
> 2.40.1
>

Regards,
Simon

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] acpi: cannot have RSDT above 4 GiB
  2023-11-12 20:01 ` Simon Glass
@ 2023-11-12 20:16   ` Heinrich Schuchardt
  2023-11-12 21:23     ` Simon Glass
  0 siblings, 1 reply; 4+ messages in thread
From: Heinrich Schuchardt @ 2023-11-12 20:16 UTC (permalink / raw)
  To: Simon Glass; +Cc: U-Boot Mailing List

Simon Glass <sjg@chromium.org> schrieb am So., 12. Nov. 2023, 21:03:

> Hi Heinrich,
>
> On Sat, 11 Nov 2023 at 07:28, Heinrich Schuchardt
> <heinrich.schuchardt@canonical.com> wrote:
> >
> > The field RsdtAddress has only 32 bit. The RSDT table cannot be located
> > beyond 4 GiB.
> >
> > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> > ---
> >  lib/acpi/base.c | 26 +++++++++++++++++++-------
> >  1 file changed, 19 insertions(+), 7 deletions(-)
>
> Reviewed-by: Simon Glass <sjg@chromium.org>
>
> nits / question below
>
> >
> > diff --git a/lib/acpi/base.c b/lib/acpi/base.c
> > index 2057bd2bef..128a76ad39 100644
> > --- a/lib/acpi/base.c
> > +++ b/lib/acpi/base.c
> > @@ -21,10 +21,17 @@ void acpi_write_rsdp(struct acpi_rsdp *rsdp, struct
> acpi_rsdt *rsdt,
> >         memcpy(rsdp->signature, RSDP_SIG, 8);
> >         memcpy(rsdp->oem_id, OEM_ID, 6);
> >
> > -       rsdp->length = sizeof(struct acpi_rsdp);
> > -       rsdp->rsdt_address = map_to_sysmem(rsdt);
> > +       if (rsdt)
> > +               rsdp->rsdt_address = map_to_sysmem(rsdt);
> > +       else
> > +               rsdp->rsdt_address = 0;
>
> There is a memset() at the top so this line (and the one below) should
> not be needed.
>
> > +
> > +       if (xsdt)
> > +               rsdp->xsdt_address = map_to_sysmem(xsdt);
> > +       else
> > +               rsdp->xsdt_address = 0;
> >
> > -       rsdp->xsdt_address = map_to_sysmem(xsdt);
> > +       rsdp->length = sizeof(struct acpi_rsdp);
> >         rsdp->revision = ACPI_RSDP_REV_ACPI_2_0;
> >
> >         /* Calculate checksums */
> > @@ -68,11 +75,15 @@ static void acpi_write_xsdt(struct acpi_xsdt *xsdt)
> >  static int acpi_write_base(struct acpi_ctx *ctx,
> >                            const struct acpi_writer *entry)
> >  {
> > -       /* We need at least an RSDP and an RSDT Table */
> > +       /* We need at least an RSDP and an XSDT Table */
> >         ctx->rsdp = ctx->current;
> >         acpi_inc_align(ctx, sizeof(struct acpi_rsdp));
> > -       ctx->rsdt = ctx->current;
> > -       acpi_inc_align(ctx, sizeof(struct acpi_rsdt));
> > +       if (map_to_sysmem(ctx->current) < UINT_MAX - 0x10000) {
>
> Won't that do something different on 64-bit machines? It seems that
> you want to check against SZ_4G ?
>

For gcc int is 32bit on 64bit systems, so is UINT_MAX. Using SZ_4G could
make this code clearer.

Best regards

Heinrich



> > +               ctx->rsdt = ctx->current;
> > +               acpi_inc_align(ctx, sizeof(struct acpi_rsdt));
> > +       } else {
> > +               ctx->rsdt = 0;
> > +       }
> >         ctx->xsdt = ctx->current;
> >         acpi_inc_align(ctx, sizeof(struct acpi_xsdt));
> >
> > @@ -80,7 +91,8 @@ static int acpi_write_base(struct acpi_ctx *ctx,
> >         memset(ctx->base, '\0', ctx->current - ctx->base);
> >
> >         acpi_write_rsdp(ctx->rsdp, ctx->rsdt, ctx->xsdt);
> > -       acpi_write_rsdt(ctx->rsdt);
> > +       if (ctx->rsdt)
> > +               acpi_write_rsdt(ctx->rsdt);
> >         acpi_write_xsdt(ctx->xsdt);
> >
> >         return 0;
> > --
> > 2.40.1
> >
>
> Regards,
> Simon
>

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] acpi: cannot have RSDT above 4 GiB
  2023-11-12 20:16   ` Heinrich Schuchardt
@ 2023-11-12 21:23     ` Simon Glass
  0 siblings, 0 replies; 4+ messages in thread
From: Simon Glass @ 2023-11-12 21:23 UTC (permalink / raw)
  To: Heinrich Schuchardt; +Cc: U-Boot Mailing List

Hi Heinrich,

On Sun, 12 Nov 2023 at 13:16, Heinrich Schuchardt
<heinrich.schuchardt@canonical.com> wrote:
>
>
>
> Simon Glass <sjg@chromium.org> schrieb am So., 12. Nov. 2023, 21:03:
>>
>> Hi Heinrich,
>>
>> On Sat, 11 Nov 2023 at 07:28, Heinrich Schuchardt
>> <heinrich.schuchardt@canonical.com> wrote:
>> >
>> > The field RsdtAddress has only 32 bit. The RSDT table cannot be located
>> > beyond 4 GiB.
>> >
>> > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
>> > ---
>> >  lib/acpi/base.c | 26 +++++++++++++++++++-------
>> >  1 file changed, 19 insertions(+), 7 deletions(-)
>>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>
>> nits / question below
>>
>> >
>> > diff --git a/lib/acpi/base.c b/lib/acpi/base.c
>> > index 2057bd2bef..128a76ad39 100644
>> > --- a/lib/acpi/base.c
>> > +++ b/lib/acpi/base.c
>> > @@ -21,10 +21,17 @@ void acpi_write_rsdp(struct acpi_rsdp *rsdp, struct acpi_rsdt *rsdt,
>> >         memcpy(rsdp->signature, RSDP_SIG, 8);
>> >         memcpy(rsdp->oem_id, OEM_ID, 6);
>> >
>> > -       rsdp->length = sizeof(struct acpi_rsdp);
>> > -       rsdp->rsdt_address = map_to_sysmem(rsdt);
>> > +       if (rsdt)
>> > +               rsdp->rsdt_address = map_to_sysmem(rsdt);
>> > +       else
>> > +               rsdp->rsdt_address = 0;
>>
>> There is a memset() at the top so this line (and the one below) should
>> not be needed.
>>
>> > +
>> > +       if (xsdt)
>> > +               rsdp->xsdt_address = map_to_sysmem(xsdt);
>> > +       else
>> > +               rsdp->xsdt_address = 0;
>> >
>> > -       rsdp->xsdt_address = map_to_sysmem(xsdt);
>> > +       rsdp->length = sizeof(struct acpi_rsdp);
>> >         rsdp->revision = ACPI_RSDP_REV_ACPI_2_0;
>> >
>> >         /* Calculate checksums */
>> > @@ -68,11 +75,15 @@ static void acpi_write_xsdt(struct acpi_xsdt *xsdt)
>> >  static int acpi_write_base(struct acpi_ctx *ctx,
>> >                            const struct acpi_writer *entry)
>> >  {
>> > -       /* We need at least an RSDP and an RSDT Table */
>> > +       /* We need at least an RSDP and an XSDT Table */
>> >         ctx->rsdp = ctx->current;
>> >         acpi_inc_align(ctx, sizeof(struct acpi_rsdp));
>> > -       ctx->rsdt = ctx->current;
>> > -       acpi_inc_align(ctx, sizeof(struct acpi_rsdt));
>> > +       if (map_to_sysmem(ctx->current) < UINT_MAX - 0x10000) {
>>
>> Won't that do something different on 64-bit machines? It seems that
>> you want to check against SZ_4G ?
>
>
> For gcc int is 32bit on 64bit systems, so is UINT_MAX. Using SZ_4G could make this code clearer.

Oh I didn't know that...I just assumed that uint would be 64-bit on a
64-machine machine, since it is the natural reg size! Yes, that is
very very confusing. I vaguely recall some compiler setting for that
and that Linux did it a particular way.

>
> Best regards
>
> Heinrich
>
>
>>
>> > +               ctx->rsdt = ctx->current;
>> > +               acpi_inc_align(ctx, sizeof(struct acpi_rsdt));
>> > +       } else {
>> > +               ctx->rsdt = 0;
>> > +       }
>> >         ctx->xsdt = ctx->current;
>> >         acpi_inc_align(ctx, sizeof(struct acpi_xsdt));
>> >
>> > @@ -80,7 +91,8 @@ static int acpi_write_base(struct acpi_ctx *ctx,
>> >         memset(ctx->base, '\0', ctx->current - ctx->base);
>> >
>> >         acpi_write_rsdp(ctx->rsdp, ctx->rsdt, ctx->xsdt);
>> > -       acpi_write_rsdt(ctx->rsdt);
>> > +       if (ctx->rsdt)
>> > +               acpi_write_rsdt(ctx->rsdt);
>> >         acpi_write_xsdt(ctx->xsdt);
>> >
>> >         return 0;
>> > --
>> > 2.40.1
>> >
>>
>> Regards,
>> Simon

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2023-11-12 21:23 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-11 14:28 [PATCH 1/1] acpi: cannot have RSDT above 4 GiB Heinrich Schuchardt
2023-11-12 20:01 ` Simon Glass
2023-11-12 20:16   ` Heinrich Schuchardt
2023-11-12 21:23     ` Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.