Antwort: [PATCH v2 21/39] acpi: Convert part of acpi_table to use acpi_ctx

Hi Simon,

-----"Simon Glass" sjg@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@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@br-automation.com