diff mbox series

[v7,7/9] acpi: Put table-setup code in its own function

Message ID 20200419143624.v7.7.I34e9fcd28119cc2fcb87ad8679efb582a4c611df@changeid
State Superseded
Headers show
Series dm: Add programmatic generation of ACPI tables (part A) | expand

Commit Message

Simon Glass April 19, 2020, 8:36 p.m. UTC 
We always write three basic tables to ACPI at the start. Move this into
its own function, along with acpi_fill_header(), so we can write a test
for this code.

Signed-off-by: Simon Glass <sjg at chromium.org>
---

Changes in v7: None
Changes in v5: None
Changes in v4:
- Put back cast on table_compute_checksum()

Changes in v3:
- Fix 'XDST' typo
- Move acpi_align_large() out of dm_test_acpi_setup_base_tables()
- Beef up the comment explaining how the unaligned address is used

Changes in v2: None

 arch/x86/lib/acpi_table.c | 72 +-----------------------------------
 include/acpi/acpi_table.h | 10 +++++
 lib/acpi/acpi_table.c     | 77 +++++++++++++++++++++++++++++++++++++++
 test/dm/acpi.c            | 58 ++++++++++++++++++++++++++++-
 4 files changed, 144 insertions(+), 73 deletions(-)

Comments

Bin Meng April 23, 2020, 9:38 a.m. UTC | #1 
Hi Simon, Wolfgang,

On Mon, Apr 20, 2020 at 4:37 AM Simon Glass <sjg at chromium.org> wrote:
>
> We always write three basic tables to ACPI at the start. Move this into
> its own function, along with acpi_fill_header(), so we can write a test
> for this code.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>

I see Wolfgang gave a RB tag to the v5 patch, so I am going to add
that tag here since no changes between v5 and v7

Reviewed-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com>

But I wonder why this new series is tagged as v7. Do I miss v6?

> ---
>
> Changes in v7: None
> Changes in v5: None
> Changes in v4:
> - Put back cast on table_compute_checksum()
>
> Changes in v3:
> - Fix 'XDST' typo
> - Move acpi_align_large() out of dm_test_acpi_setup_base_tables()
> - Beef up the comment explaining how the unaligned address is used
>
> Changes in v2: None
>
>  arch/x86/lib/acpi_table.c | 72 +-----------------------------------
>  include/acpi/acpi_table.h | 10 +++++
>  lib/acpi/acpi_table.c     | 77 +++++++++++++++++++++++++++++++++++++++
>  test/dm/acpi.c            | 58 ++++++++++++++++++++++++++++-
>  4 files changed, 144 insertions(+), 73 deletions(-)
>

Regards,
Bin
Simon Glass April 26, 2020, 7:45 p.m. UTC | #2 
Hi Bin,

On Thu, 23 Apr 2020 at 03:38, Bin Meng <bmeng.cn at gmail.com> wrote:
>
> Hi Simon, Wolfgang,
>
> On Mon, Apr 20, 2020 at 4:37 AM Simon Glass <sjg at chromium.org> wrote:
> >
> > We always write three basic tables to ACPI at the start. Move this into
> > its own function, along with acpi_fill_header(), so we can write a test
> > for this code.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
>
> I see Wolfgang gave a RB tag to the v5 patch, so I am going to add
> that tag here since no changes between v5 and v7
>
> Reviewed-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com>
>
> But I wonder why this new series is tagged as v7. Do I miss v6?

I'm not sure...but I think I might have skipped a version.

Regards,
SImon
Wolfgang Wallner April 27, 2020, 6:36 a.m. UTC | #3 
Hi Bin, Simon,

-----"Simon Glass" <sjg at chromium.org> schrieb: -----
> Betreff: Re: [PATCH v7 7/9] acpi: Put table-setup code in its own function
>
> Hi Bin,
> 
> On Thu, 23 Apr 2020 at 03:38, Bin Meng <bmeng.cn at gmail.com> wrote:
> >
> > Hi Simon, Wolfgang,
> >
> > On Mon, Apr 20, 2020 at 4:37 AM Simon Glass <sjg at chromium.org> wrote:
> > >
> > > We always write three basic tables to ACPI at the start. Move this into
> > > its own function, along with acpi_fill_header(), so we can write a test
> > > for this code.
> > >
> > > Signed-off-by: Simon Glass <sjg at chromium.org>
> >
> > I see Wolfgang gave a RB tag to the v5 patch, so I am going to add
> > that tag here since no changes between v5 and v7
> >
> > Reviewed-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com>
> >
> > But I wonder why this new series is tagged as v7. Do I miss v6?
> 
> I'm not sure...but I think I might have skipped a version.

v5 was the last full version of part A of this series.
v6 contained only two fixes for v5, and was applied together with parts of v5.
v7 are the remainig patches of v5.

regards, Wolfgang
Bin Meng April 27, 2020, 7:10 a.m. UTC | #4 
Hi Wolfgang,

On Mon, Apr 27, 2020 at 2:36 PM Wolfgang Wallner
<wolfgang.wallner at br-automation.com> wrote:
>
> Hi Bin, Simon,
>
> -----"Simon Glass" <sjg at chromium.org> schrieb: -----
> > Betreff: Re: [PATCH v7 7/9] acpi: Put table-setup code in its own function
> >
> > Hi Bin,
> >
> > On Thu, 23 Apr 2020 at 03:38, Bin Meng <bmeng.cn at gmail.com> wrote:
> > >
> > > Hi Simon, Wolfgang,
> > >
> > > On Mon, Apr 20, 2020 at 4:37 AM Simon Glass <sjg at chromium.org> wrote:
> > > >
> > > > We always write three basic tables to ACPI at the start. Move this into
> > > > its own function, along with acpi_fill_header(), so we can write a test
> > > > for this code.
> > > >
> > > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > >
> > > I see Wolfgang gave a RB tag to the v5 patch, so I am going to add
> > > that tag here since no changes between v5 and v7
> > >
> > > Reviewed-by: Wolfgang Wallner <wolfgang.wallner at br-automation.com>
> > >
> > > But I wonder why this new series is tagged as v7. Do I miss v6?
> >
> > I'm not sure...but I think I might have skipped a version.
>
> v5 was the last full version of part A of this series.
> v6 contained only two fixes for v5, and was applied together with parts of v5.
> v7 are the remainig patches of v5.

Ah, yes! Thanks for the information.

Regards,
Bin
diff mbox series

Patch

diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c
index ff8cee51d6..600bde2f5f 100644
--- a/arch/x86/lib/acpi_table.c
+++ b/arch/x86/lib/acpi_table.c
@@ -31,58 +31,6 @@  extern const unsigned char AmlCode[];
 /* ACPI RSDP address to be used in boot parameters */
 static ulong acpi_rsdp_addr;
 
-static void acpi_write_rsdp(struct acpi_rsdp *rsdp, struct acpi_rsdt *rsdt,
-			    struct acpi_xsdt *xsdt)
-{
-	memset(rsdp, 0, sizeof(struct acpi_rsdp));
-
-	memcpy(rsdp->signature, RSDP_SIG, 8);
-	memcpy(rsdp->oem_id, OEM_ID, 6);
-
-	rsdp->length = sizeof(struct acpi_rsdp);
-	rsdp->rsdt_address = (u32)rsdt;
-
-	rsdp->xsdt_address = (u64)(u32)xsdt;
-	rsdp->revision = ACPI_RSDP_REV_ACPI_2_0;
-
-	/* Calculate checksums */
-	rsdp->checksum = table_compute_checksum((void *)rsdp, 20);
-	rsdp->ext_checksum = table_compute_checksum((void *)rsdp,
-			sizeof(struct acpi_rsdp));
-}
-
-static void acpi_write_rsdt(struct acpi_rsdt *rsdt)
-{
-	struct acpi_table_header *header = &(rsdt->header);
-
-	/* Fill out header fields */
-	acpi_fill_header(header, "RSDT");
-	header->length = sizeof(struct acpi_rsdt);
-	header->revision = 1;
-
-	/* Entries are filled in later, we come with an empty set */
-
-	/* Fix checksum */
-	header->checksum = table_compute_checksum((void *)rsdt,
-			sizeof(struct acpi_rsdt));
-}
-
-static void acpi_write_xsdt(struct acpi_xsdt *xsdt)
-{
-	struct acpi_table_header *header = &(xsdt->header);
-
-	/* Fill out header fields */
-	acpi_fill_header(header, "XSDT");
-	header->length = sizeof(struct acpi_xsdt);
-	header->revision = 1;
-
-	/* Entries are filled in later, we come with an empty set */
-
-	/* Fix checksum */
-	header->checksum = table_compute_checksum((void *)xsdt,
-			sizeof(struct acpi_xsdt));
-}
-
 static void acpi_create_facs(struct acpi_facs *facs)
 {
 	memset((void *)facs, 0, sizeof(struct acpi_facs));
@@ -411,7 +359,6 @@  static void acpi_create_spcr(struct acpi_spcr *spcr)
 ulong write_acpi_tables(ulong start_addr)
 {
 	struct acpi_ctx sctx, *ctx = &sctx;
-	struct acpi_xsdt *xsdt;
 	struct acpi_facs *facs;
 	struct acpi_table_header *dsdt;
 	struct acpi_fadt *fadt;
@@ -424,33 +371,16 @@  ulong write_acpi_tables(ulong start_addr)
 	int i;
 
 	start = map_sysmem(start_addr, 0);
-	ctx->current = start;
-
-	/* Align ACPI tables to 16 byte */
-	acpi_align(ctx);
 
 	debug("ACPI: Writing ACPI tables at %lx\n", start_addr);
 
-	/* We need at least an RSDP and an RSDT 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));
-	xsdt = ctx->current;
-	acpi_inc_align(ctx, sizeof(struct acpi_xsdt));
+	acpi_setup_base_tables(ctx, start);
 	/*
 	 * Per ACPI spec, the FACS table address must be aligned to a 64 byte
 	 * boundary (Windows checks this, but Linux does not).
 	 */
 	acpi_align64(ctx);
 
-	/* clear all table memory */
-	memset((void *)start, 0, ctx->current - start);
-
-	acpi_write_rsdp(ctx->rsdp, ctx->rsdt, xsdt);
-	acpi_write_rsdt(ctx->rsdt);
-	acpi_write_xsdt(xsdt);
-
 	debug("ACPI:    * FACS\n");
 	facs = ctx->current;
 	acpi_inc_align(ctx, sizeof(struct acpi_facs));
diff --git a/include/acpi/acpi_table.h b/include/acpi/acpi_table.h
index 55349c0bb6..3681c5c8ed 100644
--- a/include/acpi/acpi_table.h
+++ b/include/acpi/acpi_table.h
@@ -560,6 +560,16 @@  void acpi_inc_align(struct acpi_ctx *ctx, uint amount);
  */
 int acpi_add_table(struct acpi_ctx *ctx, void *table);
 
+/**
+ * acpi_setup_base_tables() - Set up context along with RSDP, RSDT and XSDT
+ *
+ * Set up the context with the given start position. Some basic tables are
+ * always needed, so set them up as well.
+ *
+ * @ctx: Context to set up
+ */
+void acpi_setup_base_tables(struct acpi_ctx *ctx, void *start);
+
 #endif /* !__ACPI__*/
 
 #include <asm/acpi_table.h>
diff --git a/lib/acpi/acpi_table.c b/lib/acpi/acpi_table.c
index 5311ae9368..85193c49e4 100644
--- a/lib/acpi/acpi_table.c
+++ b/lib/acpi/acpi_table.c
@@ -181,3 +181,80 @@  int acpi_add_table(struct acpi_ctx *ctx, void *table)
 
 	return 0;
 }
+
+static void acpi_write_rsdp(struct acpi_rsdp *rsdp, struct acpi_rsdt *rsdt,
+			    struct acpi_xsdt *xsdt)
+{
+	memset(rsdp, 0, sizeof(struct acpi_rsdp));
+
+	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);
+
+	rsdp->xsdt_address = map_to_sysmem(xsdt);
+	rsdp->revision = ACPI_RSDP_REV_ACPI_2_0;
+
+	/* Calculate checksums */
+	rsdp->checksum = table_compute_checksum(rsdp, 20);
+	rsdp->ext_checksum = table_compute_checksum(rsdp,
+						    sizeof(struct acpi_rsdp));
+}
+
+static void acpi_write_rsdt(struct acpi_rsdt *rsdt)
+{
+	struct acpi_table_header *header = &rsdt->header;
+
+	/* Fill out header fields */
+	acpi_fill_header(header, "RSDT");
+	header->length = sizeof(struct acpi_rsdt);
+	header->revision = 1;
+
+	/* Entries are filled in later, we come with an empty set */
+
+	/* Fix checksum */
+	header->checksum = table_compute_checksum(rsdt,
+						  sizeof(struct acpi_rsdt));
+}
+
+static void acpi_write_xsdt(struct acpi_xsdt *xsdt)
+{
+	struct acpi_table_header *header = &xsdt->header;
+
+	/* Fill out header fields */
+	acpi_fill_header(header, "XSDT");
+	header->length = sizeof(struct acpi_xsdt);
+	header->revision = 1;
+
+	/* Entries are filled in later, we come with an empty set */
+
+	/* Fix checksum */
+	header->checksum = table_compute_checksum(xsdt,
+						  sizeof(struct acpi_xsdt));
+}
+
+void acpi_setup_base_tables(struct acpi_ctx *ctx, void *start)
+{
+	struct acpi_xsdt *xsdt;
+
+	ctx->current = start;
+
+	/* Align ACPI tables to 16 byte */
+	acpi_align(ctx);
+
+	/* We need at least an RSDP and an RSDT 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));
+	xsdt = ctx->current;
+	acpi_inc_align(ctx, sizeof(struct acpi_xsdt));
+
+	/* clear all table memory */
+	memset((void *)start, '\0', ctx->current - start);
+
+	acpi_write_rsdp(ctx->rsdp, ctx->rsdt, xsdt);
+	acpi_write_rsdt(ctx->rsdt);
+	acpi_write_xsdt(xsdt);
+}
diff --git a/test/dm/acpi.c b/test/dm/acpi.c
index ffc611efb8..beb1b6da73 100644
--- a/test/dm/acpi.c
+++ b/test/dm/acpi.c
@@ -8,6 +8,9 @@ 
 
 #include <common.h>
 #include <dm.h>
+#include <malloc.h>
+#include <mapmem.h>
+#include <tables_csum.h>
 #include <version.h>
 #include <acpi/acpi_table.h>
 #include <dm/acpi.h>
@@ -137,9 +140,9 @@  static int dm_test_acpi_write_tables(struct unit_test_state *uts)
 	buf = malloc(BUF_SIZE);
 	ut_assertnonnull(buf);
 
-	ctx.current = buf;
+	acpi_setup_base_tables(&ctx, buf);
+	dmar = ctx.current;
 	ut_assertok(acpi_write_dev_tables(&ctx));
-	dmar = buf;
 
 	/*
 	 * We should have two dmar tables, one for each "denx,u-boot-acpi-test"
@@ -152,6 +155,11 @@  static int dm_test_acpi_write_tables(struct unit_test_state *uts)
 	ut_asserteq(DMAR_INTR_REMAP, dmar[1].flags);
 	ut_asserteq(32 - 1, dmar[1].host_address_width);
 
+	/* Check that the pointers were added correctly */
+	ut_asserteq(map_to_sysmem(dmar), ctx.rsdt->entry[0]);
+	ut_asserteq(map_to_sysmem(dmar + 1), ctx.rsdt->entry[1]);
+	ut_asserteq(0, ctx.rsdt->entry[2]);
+
 	return 0;
 }
 DM_TEST(dm_test_acpi_write_tables, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
@@ -183,3 +191,49 @@  static int dm_test_acpi_basic(struct unit_test_state *uts)
 	return 0;
 }
 DM_TEST(dm_test_acpi_basic, DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);
+
+/* Test acpi_setup_base_tables */
+static int dm_test_acpi_setup_base_tables(struct unit_test_state *uts)
+{
+	struct acpi_rsdp *rsdp;
+	struct acpi_rsdt *rsdt;
+	struct acpi_xsdt *xsdt;
+	struct acpi_ctx ctx;
+	void *buf, *end;
+
+	/*
+	 * Use an unaligned address deliberately, by allocating an aligned
+	 * address and then adding 4 to it
+	 */
+	buf = memalign(64, BUF_SIZE);
+	ut_assertnonnull(buf);
+	acpi_setup_base_tables(&ctx, buf + 4);
+
+	rsdp = buf + 16;
+	ut_asserteq_ptr(rsdp, ctx.rsdp);
+	ut_assertok(memcmp(RSDP_SIG, rsdp->signature, sizeof(rsdp->signature)));
+	ut_asserteq(sizeof(*rsdp), rsdp->length);
+	ut_assertok(table_compute_checksum(rsdp, 20));
+	ut_assertok(table_compute_checksum(rsdp, sizeof(*rsdp)));
+
+	rsdt = PTR_ALIGN((void *)rsdp + sizeof(*rsdp), 16);
+	ut_asserteq_ptr(rsdt, ctx.rsdt);
+	ut_assertok(memcmp("RSDT", rsdt->header.signature, ACPI_NAME_LEN));
+	ut_asserteq(sizeof(*rsdt), rsdt->header.length);
+	ut_assertok(table_compute_checksum(rsdt, sizeof(*rsdt)));
+
+	xsdt = PTR_ALIGN((void *)rsdt + sizeof(*rsdt), 16);
+	ut_assertok(memcmp("XSDT", xsdt->header.signature, ACPI_NAME_LEN));
+	ut_asserteq(sizeof(*xsdt), xsdt->header.length);
+	ut_assertok(table_compute_checksum(xsdt, sizeof(*xsdt)));
+
+	end = PTR_ALIGN((void *)xsdt + sizeof(*xsdt), 64);
+	ut_asserteq_ptr(end, ctx.current);
+
+	ut_asserteq(map_to_sysmem(rsdt), rsdp->rsdt_address);
+	ut_asserteq(map_to_sysmem(xsdt), rsdp->xsdt_address);
+
+	return 0;
+}
+DM_TEST(dm_test_acpi_setup_base_tables,
+	DM_TESTF_SCAN_PDATA | DM_TESTF_SCAN_FDT);