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

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;

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

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

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
participants (2)
-
Heinrich Schuchardt
-
Simon Glass