[PATCH 1/1] acpi: cannot have RSDT above 4 GiB

[Heinrich Schuchardt]

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

> Hi Heinrich,
>
> On Sat, 11 Nov 2023 at 07:28, Heinrich Schuchardt
> <heinrich.schuchardt at 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 at canonical.com>
> > ---
> >  lib/acpi/base.c | 26 +++++++++++++++++++-------
> >  1 file changed, 19 insertions(+), 7 deletions(-)
>
> Reviewed-by: Simon Glass <sjg at 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
>