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

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