Antwort: [PATCH v2 21/39] acpi: Convert part of acpi_table to use acpi_ctx
Wolfgang Wallner
wolfgang.wallner at br-automation.com
Wed Mar 11 13:58:48 CET 2020
Hi Simon,
-----"Simon Glass" <sjg at chromium.org> schrieb: -----
>
> The current code uses an address but a pointer would result in fewer
> casts. Also it repeats the alignment code in a lot of places so this would
> be better done in a helper function.
>
> Update write_acpi_tables() to make use of the new acpi_ctx structure,
> adding a few helpers to clean things up.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
> Changes in v2: None
>
> arch/x86/lib/acpi_table.c | 88 +++++++++++++++++++--------------------
> include/acpi_table.h | 36 ++++++++++++++++
> lib/acpi/acpi_table.c | 22 ++++++++++
> test/dm/acpi.c | 28 +++++++++++++
> 4 files changed, 129 insertions(+), 45 deletions(-)
>
> diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c
> index 487fef87f2..8e13d6a3e6 100644
> --- a/arch/x86/lib/acpi_table.c
> +++ b/arch/x86/lib/acpi_table.c
> @@ -10,6 +10,7 @@
> #include <cpu.h>
> #include <dm.h>
> #include <dm/uclass-internal.h>
> +#include <mapmem.h>
> #include <serial.h>
> #include <version.h>
> #include <asm/acpi/global_nvs.h>
> @@ -19,6 +20,7 @@
> #include <asm/mpspec.h>
> #include <asm/tables.h>
> #include <asm/arch/global_nvs.h>
> +#include <dm/acpi.h>
>
> /*
> * IASL compiles the dsdt entries and writes the hex values
> @@ -468,9 +470,9 @@ static void acpi_create_spcr(struct acpi_spcr *spcr)
> /*
> * QEMU's version of write_acpi_tables is defined in drivers/misc/qfw.c
> */
> -ulong write_acpi_tables(ulong start)
> +ulong write_acpi_tables(ulong start_addr)
> {
> - u32 current;
> + struct acpi_ctx sctx, *ctx = &sctx;
> struct acpi_rsdp *rsdp;
> struct acpi_rsdt *rsdt;
> struct acpi_xsdt *xsdt;
> @@ -481,60 +483,61 @@ ulong write_acpi_tables(ulong start)
> struct acpi_madt *madt;
> struct acpi_csrt *csrt;
> struct acpi_spcr *spcr;
> + void *start;
> + ulong addr;
> int i;
>
> - current = start;
> + start = map_sysmem(start_addr, 0);
> + ctx->current = start;
>
> /* Align ACPI tables to 16 byte */
> - current = ALIGN(current, 16);
> + acpi_align(ctx);
>
> - debug("ACPI: Writing ACPI tables at %lx\n", start);
> + debug("ACPI: Writing ACPI tables at %lx\n", start_addr);
>
> /* We need at least an RSDP and an RSDT Table */
> - rsdp = (struct acpi_rsdp *)current;
> - current += sizeof(struct acpi_rsdp);
> - current = ALIGN(current, 16);
> - rsdt = (struct acpi_rsdt *)current;
> - current += sizeof(struct acpi_rsdt);
> - current = ALIGN(current, 16);
> - xsdt = (struct acpi_xsdt *)current;
> - current += sizeof(struct acpi_xsdt);
> + rsdp = ctx->current;
> + acpi_inc_align(ctx, sizeof(struct acpi_rsdp));
> + rsdt = ctx->current;
> + acpi_inc_align(ctx, sizeof(struct acpi_rsdt));
> + xsdt = ctx->current;
> + acpi_inc_align(ctx, sizeof(struct acpi_xsdt));
> /*
> * Per ACPI spec, the FACS table address must be aligned to a 64 byte
> * boundary (Windows checks this, but Linux does not).
> */
> - current = ALIGN(current, 64);
> + acpi_align_large(ctx);
>
> /* clear all table memory */
> - memset((void *)start, 0, current - start);
> + memset((void *)start, 0, ctx->current - start);
>
> acpi_write_rsdp(rsdp, rsdt, xsdt);
> acpi_write_rsdt(rsdt);
> acpi_write_xsdt(xsdt);
>
> debug("ACPI: * FACS\n");
> - facs = (struct acpi_facs *)current;
> - current += sizeof(struct acpi_facs);
> - current = ALIGN(current, 16);
> + facs = ctx->current;
> + acpi_inc_align(ctx, sizeof(struct acpi_facs));
>
> acpi_create_facs(facs);
>
> debug("ACPI: * DSDT\n");
> - dsdt = (struct acpi_table_header *)current;
> + dsdt = ctx->current;
> memcpy(dsdt, &AmlCode, sizeof(struct acpi_table_header));
> - current += sizeof(struct acpi_table_header);
> - memcpy((char *)current,
> + acpi_inc(ctx, sizeof(struct acpi_table_header));
> + memcpy(ctx->current,
> (char *)&AmlCode + sizeof(struct acpi_table_header),
> dsdt->length - sizeof(struct acpi_table_header));
> - current += dsdt->length - sizeof(struct acpi_table_header);
> - current = ALIGN(current, 16);
> + acpi_inc_align(ctx, dsdt->length - sizeof(struct acpi_table_header));
>
> /* Pack GNVS into the ACPI table area */
> for (i = 0; i < dsdt->length; i++) {
> u32 *gnvs = (u32 *)((u32)dsdt + i);
> if (*gnvs == ACPI_GNVS_ADDR) {
> - debug("Fix up global NVS in DSDT to 0x%08x\n", current);
> - *gnvs = current;
> + ulong addr = (ulong)map_to_sysmem(ctx->current);
> +
> + debug("Fix up global NVS in DSDT to %#08lx\n", addr);
> + *gnvs = addr;
> break;
> }
> }
> @@ -544,51 +547,46 @@ ulong write_acpi_tables(ulong start)
> dsdt->checksum = table_compute_checksum((void *)dsdt, dsdt->length);
>
> /* Fill in platform-specific global NVS variables */
> - acpi_create_gnvs((struct acpi_global_nvs *)current);
> - current += sizeof(struct acpi_global_nvs);
> - current = ALIGN(current, 16);
> + acpi_create_gnvs(ctx->current);
> + acpi_inc_align(ctx, sizeof(struct acpi_global_nvs));
>
> debug("ACPI: * FADT\n");
> - fadt = (struct acpi_fadt *)current;
> - current += sizeof(struct acpi_fadt);
> - current = ALIGN(current, 16);
> + fadt = ctx->current;
> + acpi_inc_align(ctx, sizeof(struct acpi_fadt));
> acpi_create_fadt(fadt, facs, dsdt);
> acpi_add_table(rsdp, fadt);
>
> debug("ACPI: * MADT\n");
> - madt = (struct acpi_madt *)current;
> + madt = ctx->current;
> acpi_create_madt(madt);
> - current += madt->header.length;
> + acpi_inc_align(ctx, madt->header.length);
> acpi_add_table(rsdp, madt);
> - current = ALIGN(current, 16);
>
> debug("ACPI: * MCFG\n");
> - mcfg = (struct acpi_mcfg *)current;
> + mcfg = ctx->current;
> acpi_create_mcfg(mcfg);
> - current += mcfg->header.length;
> + acpi_inc_align(ctx, mcfg->header.length);
> acpi_add_table(rsdp, mcfg);
> - current = ALIGN(current, 16);
>
> debug("ACPI: * CSRT\n");
> - csrt = (struct acpi_csrt *)current;
> + csrt = ctx->current;
> acpi_create_csrt(csrt);
> - current += csrt->header.length;
> + acpi_inc_align(ctx, csrt->header.length);
> acpi_add_table(rsdp, csrt);
> - current = ALIGN(current, 16);
>
> debug("ACPI: * SPCR\n");
> - spcr = (struct acpi_spcr *)current;
> + spcr = ctx->current;
> acpi_create_spcr(spcr);
> - current += spcr->header.length;
> + acpi_inc_align(ctx, spcr->header.length);
> acpi_add_table(rsdp, spcr);
> - current = ALIGN(current, 16);
>
> - debug("current = %x\n", current);
> + addr = map_to_sysmem(ctx->current);
> + debug("current = %lx\n", addr);
>
> acpi_rsdp_addr = (unsigned long)rsdp;
> debug("ACPI: done\n");
>
> - return current;
> + return addr;
> }
>
> ulong acpi_get_rsdp_addr(void)
> diff --git a/include/acpi_table.h b/include/acpi_table.h
> index 3fd2ef16b0..5fd0fa71a6 100644
> --- a/include/acpi_table.h
> +++ b/include/acpi_table.h
> @@ -26,6 +26,8 @@
>
> #if !defined(__ACPI__)
>
> +struct acpi_ctx;
> +
> /*
> * RSDP (Root System Description Pointer)
> * Note: ACPI 1.0 didn't have length, xsdt_address, and ext_checksum
> @@ -519,6 +521,40 @@ int acpi_create_dmar(struct acpi_dmar *dmar, enum dmar_flags flags);
> */
> void acpi_fill_header(struct acpi_table_header *header, char *signature);
>
> +/**
> + * acpi_align() - Align the ACPI output pointer to a 16-byte boundary
> + *
> + * @ctx: ACPI context
> + */
> +void acpi_align(struct acpi_ctx *ctx);
Nit: The function names acpi_align() and acpi_align_large() are both vague
on the exact alignment that is used.
How about acpi_align16() and acpi_align64() ?
> +
> +/**
> + * acpi_align_large() - Align the ACPI output pointer to a 64-byte boundary
> + *
> + * @ctx: ACPI context
> + */
> +void acpi_align_large(struct acpi_ctx *ctx);
> +
> +/**
> + * acpi_inc() - Increment the ACPI output pointer by a bit
> + *
> + * The pointer is NOT aligned afterwards.
> + *
> + * @ctx: ACPI context
> + * @amount: Amount to increment by
> + */
> +void acpi_inc(struct acpi_ctx *ctx, uint amount);
> +
> +/**
> + * acpi_inc_align() - Increment the ACPI output pointer by a bit and align
> + *
> + * The pointer is aligned afterwards.
> + *
> + * @ctx: ACPI context
> + * @amount: Amount to increment by
> + */
> +void acpi_inc_align(struct acpi_ctx *ctx, uint amount);
Similar nit as above: acpi_inc_align16() ?
> +
> #endif /* !__ACPI__*/
>
> #include <asm/acpi_table.h>
> diff --git a/lib/acpi/acpi_table.c b/lib/acpi/acpi_table.c
> index a86bfa6187..3d24cc26b6 100644
> --- a/lib/acpi/acpi_table.c
> +++ b/lib/acpi/acpi_table.c
> @@ -10,6 +10,7 @@
> #include <acpi_table.h>
> #include <cpu.h>
> #include <version.h>
> +#include <dm/acpi.h>
>
> int acpi_create_dmar(struct acpi_dmar *dmar, enum dmar_flags flags)
> {
> @@ -94,3 +95,24 @@ void acpi_fill_header(struct acpi_table_header *header, char *signature)
> header->oem_revision = U_BOOT_BUILD_DATE;
> memcpy(header->aslc_id, ASLC_ID, 4);
> }
> +
> +void acpi_align(struct acpi_ctx *ctx)
> +{
> + ctx->current = (void *)ALIGN((ulong)ctx->current, 16);
> +}
> +
> +void acpi_align_large(struct acpi_ctx *ctx)
> +{
> + ctx->current = (void *)ALIGN((ulong)ctx->current, 64);
> +}
> +
> +void acpi_inc(struct acpi_ctx *ctx, uint amount)
> +{
> + ctx->current += amount;
> +}
> +
> +void acpi_inc_align(struct acpi_ctx *ctx, uint amount)
> +{
> + ctx->current += amount;
> + acpi_align(ctx);
> +}
> diff --git a/test/dm/acpi.c b/test/dm/acpi.c
> index b87fbd16b0..0bd7e51ac9 100644
> --- a/test/dm/acpi.c
> +++ b/test/dm/acpi.c
> @@ -152,3 +152,31 @@ static int dm_test_acpi_write_tables(struct unit_test_state *uts)
> return 0;
> }
> DM_TEST(dm_test_acpi_write_tables, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
> +
> +/* Test basic ACPI functions */
> +static int dm_test_acpi_basic(struct unit_test_state *uts)
> +{
> + struct acpi_ctx ctx;
> +
> + /* Check align works */
> + ctx.current = (void *)5;
> + acpi_align(&ctx);
> + ut_asserteq_ptr((void *)16, ctx.current);
> +
> + /* Check that align does nothing if already aligned */
> + acpi_align(&ctx);
> + ut_asserteq_ptr((void *)16, ctx.current);
> + acpi_align_large(&ctx);
> + ut_asserteq_ptr((void *)64, ctx.current);
> + acpi_align_large(&ctx);
> + ut_asserteq_ptr((void *)64, ctx.current);
> +
> + /* Check incrementing */
> + acpi_inc(&ctx, 3);
> + ut_asserteq_ptr((void *)67, ctx.current);
> + acpi_inc_align(&ctx, 3);
> + ut_asserteq_ptr((void *)80, ctx.current);
> +
> + return 0;
> +}
> +DM_TEST(dm_test_acpi_basic, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
> --
> 2.25.1.481.gfbce0eb801-goog
Apart from the nits above:
Reviewed-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com>
More information about the U-Boot
mailing list