Antwort: [PATCH v2 24/39] acpi: Move acpi_add_table() to generic code

Wolfgang Wallner wolfgang.wallner at br-automation.com
Wed Mar 11 14:43:14 CET 2020


Hi Simon,

-----"Simon Glass" <sjg at chromium.org> schrieb: -----
> 
> Move this code to a generic location so that we can test it with sandbox.
> This requires adding a few new fields to acpi_ctx, so drop the local
> variables used in the original code.
> 
> Also use mapmem to avoid pointer-to-address casts which don't work on
> sandbox.
> 
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
> 
> Changes in v2: None
> 
>  arch/x86/lib/acpi_table.c | 82 +++++----------------------------------
>  include/acpi_table.h      |  9 +++++
>  include/dm/acpi.h         |  5 +++
>  lib/acpi/acpi_table.c     | 64 ++++++++++++++++++++++++++++++
>  test/dm/acpi.c            |  4 ++
>  5 files changed, 92 insertions(+), 72 deletions(-)
> 
> diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c
> index 0757ac3431..9168119547 100644
> --- a/arch/x86/lib/acpi_table.c
> +++ b/arch/x86/lib/acpi_table.c
> @@ -83,66 +83,6 @@ static void acpi_write_xsdt(struct acpi_xsdt *xsdt)
>  			sizeof(struct acpi_xsdt));
>  }
>  
> -/**
> - * Add an ACPI table to the RSDT (and XSDT) structure, recalculate length
> - * and checksum.
> - */
> -static void acpi_add_table(struct acpi_rsdp *rsdp, void *table)
> -{
> -	int i, entries_num;
> -	struct acpi_rsdt *rsdt;
> -	struct acpi_xsdt *xsdt = NULL;
> -
> -	/* The RSDT is mandatory while the XSDT is not */
> -	rsdt = (struct acpi_rsdt *)rsdp->rsdt_address;
> -
> -	if (rsdp->xsdt_address)
> -		xsdt = (struct acpi_xsdt *)((u32)rsdp->xsdt_address);
> -
> -	/* This should always be MAX_ACPI_TABLES */
> -	entries_num = ARRAY_SIZE(rsdt->entry);
> -
> -	for (i = 0; i < entries_num; i++) {
> -		if (rsdt->entry[i] == 0)
> -			break;
> -	}
> -
> -	if (i >= entries_num) {
> -		debug("ACPI: Error: too many tables\n");
> -		return;
> -	}
> -
> -	/* Add table to the RSDT */
> -	rsdt->entry[i] = (u32)table;
> -
> -	/* Fix RSDT length or the kernel will assume invalid entries */
> -	rsdt->header.length = sizeof(struct acpi_table_header) +
> -				(sizeof(u32) * (i + 1));
> -
> -	/* Re-calculate checksum */
> -	rsdt->header.checksum = 0;
> -	rsdt->header.checksum = table_compute_checksum((u8 *)rsdt,
> -			rsdt->header.length);
> -
> -	/*
> -	 * And now the same thing for the XSDT. We use the same index as for
> -	 * now we want the XSDT and RSDT to always be in sync in U-Boot
> -	 */
> -	if (xsdt) {
> -		/* Add table to the XSDT */
> -		xsdt->entry[i] = (u64)(u32)table;
> -
> -		/* Fix XSDT length */
> -		xsdt->header.length = sizeof(struct acpi_table_header) +
> -			(sizeof(u64) * (i + 1));
> -
> -		/* Re-calculate checksum */
> -		xsdt->header.checksum = 0;
> -		xsdt->header.checksum = table_compute_checksum((u8 *)xsdt,
> -				xsdt->header.length);
> -	}
> -}
> -
>  static void acpi_create_facs(struct acpi_facs *facs)
>  {
>  	memset((void *)facs, 0, sizeof(struct acpi_facs));
> @@ -462,8 +402,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_rsdp *rsdp;
> -	struct acpi_rsdt *rsdt;
>  	struct acpi_xsdt *xsdt;
>  	struct acpi_facs *facs;
>  	struct acpi_table_header *dsdt;
> @@ -485,9 +423,9 @@ ulong write_acpi_tables(ulong start_addr)
>  	debug("ACPI: Writing ACPI tables at %lx\n", start_addr);
>  
>  	/* We need at least an RSDP and an RSDT Table */
> -	rsdp = ctx->current;
> +	ctx->rsdp = ctx->current;
>  	acpi_inc_align(ctx, sizeof(struct acpi_rsdp));
> -	rsdt = ctx->current;
> +	ctx->rsdt = ctx->current;
>  	acpi_inc_align(ctx, sizeof(struct acpi_rsdt));
>  	xsdt = ctx->current;
>  	acpi_inc_align(ctx, sizeof(struct acpi_xsdt));
> @@ -500,8 +438,8 @@ ulong write_acpi_tables(ulong start_addr)
>  	/* clear all table memory */
>  	memset((void *)start, 0, ctx->current - start);
>  
> -	acpi_write_rsdp(rsdp, rsdt, xsdt);
> -	acpi_write_rsdt(rsdt);
> +	acpi_write_rsdp(ctx->rsdp, ctx->rsdt, xsdt);
> +	acpi_write_rsdt(ctx->rsdt);
>  	acpi_write_xsdt(xsdt);
>  
>  	debug("ACPI:    * FACS\n");
> @@ -543,38 +481,38 @@ ulong write_acpi_tables(ulong start_addr)
>  	fadt = ctx->current;
>  	acpi_inc_align(ctx, sizeof(struct acpi_fadt));
>  	acpi_create_fadt(fadt, facs, dsdt);
> -	acpi_add_table(rsdp, fadt);
> +	acpi_add_table(ctx, fadt);
>  
>  	debug("ACPI:    * MADT\n");
>  	madt = ctx->current;
>  	acpi_create_madt(madt);
>  	acpi_inc_align(ctx, madt->header.length);
> -	acpi_add_table(rsdp, madt);
> +	acpi_add_table(ctx, madt);
>  
>  	debug("ACPI:    * MCFG\n");
>  	mcfg = ctx->current;
>  	acpi_create_mcfg(mcfg);
>  	acpi_inc_align(ctx, mcfg->header.length);
> -	acpi_add_table(rsdp, mcfg);
> +	acpi_add_table(ctx, mcfg);
>  
>  	debug("ACPI:    * CSRT\n");
>  	csrt = ctx->current;
>  	acpi_create_csrt(csrt);
>  	acpi_inc_align(ctx, csrt->header.length);
> -	acpi_add_table(rsdp, csrt);
> +	acpi_add_table(ctx, csrt);
>  
>  	debug("ACPI:    * SPCR\n");
>  	spcr = ctx->current;
>  	acpi_create_spcr(spcr);
>  	acpi_inc_align(ctx, spcr->header.length);
> -	acpi_add_table(rsdp, spcr);
> +	acpi_add_table(ctx, spcr);
>  
>  	acpi_write_dev_tables(ctx);
>  
>  	addr = map_to_sysmem(ctx->current);
>  	debug("current = %lx\n", addr);
>  
> -	acpi_rsdp_addr = (unsigned long)rsdp;
> +	acpi_rsdp_addr = (unsigned long)ctx->rsdp;
>  	debug("ACPI: done\n");
>  
>  	return addr;
> diff --git a/include/acpi_table.h b/include/acpi_table.h
> index 5fd0fa71a6..2131484880 100644
> --- a/include/acpi_table.h
> +++ b/include/acpi_table.h
> @@ -555,6 +555,15 @@ void acpi_inc(struct acpi_ctx *ctx, uint amount);
>   */
>  void acpi_inc_align(struct acpi_ctx *ctx, uint amount);
>  
> +/**
> + * acpi_add_table() - Add a new table to the RSDP and XSDT
> + *
> + * @ctx: ACPI context
> + * @table: Table to add
> + * @return 0 if OK, -E2BIG if too many tables
> + */
> +int acpi_add_table(struct acpi_ctx *ctx, void *table);
> +
>  #endif /* !__ACPI__*/
>  
>  #include <asm/acpi_table.h>
> diff --git a/include/dm/acpi.h b/include/dm/acpi.h
> index dcfcf5c347..4465e62848 100644
> --- a/include/dm/acpi.h
> +++ b/include/dm/acpi.h
> @@ -30,9 +30,14 @@
>   * This contains a few useful pieces of information used when writing
>   *
>   * @current: Current address for writing
> + * @rsdp: Pointer to the Root System Description Pointer, typically used when
> + *	adding a new table. The RSDP holds pointers to the RSDP and XSDT.

typo: "to the RSDT" (not RSDP)

> + * @rsdt: Pointer to the Root System Description Table
>   */
>  struct acpi_ctx {
>  	void *current;
> +	struct acpi_rsdp *rsdp;
> +	struct acpi_rsdt *rsdt;
>  };
>  
>  /**
> diff --git a/lib/acpi/acpi_table.c b/lib/acpi/acpi_table.c
> index 3d24cc26b6..00e80ac39a 100644
> --- a/lib/acpi/acpi_table.c
> +++ b/lib/acpi/acpi_table.c
> @@ -9,6 +9,8 @@
>  #include <dm.h>
>  #include <acpi_table.h>
>  #include <cpu.h>
> +#include <mapmem.h>
> +#include <tables_csum.h>
>  #include <version.h>
>  #include <dm/acpi.h>
>  
> @@ -116,3 +118,65 @@ void acpi_inc_align(struct acpi_ctx *ctx, uint amount)
>  	ctx->current += amount;
>  	acpi_align(ctx);
>  }
> +
> +/**
> + * Add an ACPI table to the RSDT (and XSDT) structure, recalculate length
> + * and checksum.
> + */
> +int acpi_add_table(struct acpi_ctx *ctx, void *table)
> +{
> +	int i, entries_num;
> +	struct acpi_rsdt *rsdt;
> +	struct acpi_xsdt *xsdt = NULL;
> +
> +	/* The RSDT is mandatory while the XSDT is not */
> +	rsdt = ctx->rsdt;
> +
> +	if (ctx->rsdp->xsdt_address)
> +		xsdt = map_sysmem(ctx->rsdp->xsdt_address, 0);
> +
> +	/* This should always be MAX_ACPI_TABLES */
> +	entries_num = ARRAY_SIZE(rsdt->entry);
> +
> +	for (i = 0; i < entries_num; i++) {
> +		if (rsdt->entry[i] == 0)
> +			break;
> +	}
> +
> +	if (i >= entries_num) {
> +		debug("ACPI: Error: too many tables\n");
> +		return -E2BIG;
> +	}
> +
> +	/* Add table to the RSDT */
> +	rsdt->entry[i] = map_to_sysmem(table);
> +
> +	/* Fix RSDT length or the kernel will assume invalid entries */
> +	rsdt->header.length = sizeof(struct acpi_table_header) +
> +				(sizeof(u32) * (i + 1));
> +
> +	/* Re-calculate checksum */
> +	rsdt->header.checksum = 0;
> +	rsdt->header.checksum = table_compute_checksum((u8 *)rsdt,
> +						       rsdt->header.length);
> +
> +	/*
> +	 * And now the same thing for the XSDT. We use the same index as for
> +	 * now we want the XSDT and RSDT to always be in sync in U-Boot
> +	 */
> +	if (xsdt) {
> +		/* Add table to the XSDT */
> +		xsdt->entry[i] = map_to_sysmem(table);
> +
> +		/* Fix XSDT length */
> +		xsdt->header.length = sizeof(struct acpi_table_header) +
> +			(sizeof(u64) * (i + 1));
> +
> +		/* Re-calculate checksum */
> +		xsdt->header.checksum = 0;
> +		xsdt->header.checksum =
> +			table_compute_checksum((u8 *)xsdt, xsdt->header.length);
> +	}
> +
> +	return 0;
> +}
> diff --git a/test/dm/acpi.c b/test/dm/acpi.c
> index 0bd7e51ac9..a2a57a29a6 100644
> --- a/test/dm/acpi.c
> +++ b/test/dm/acpi.c
> @@ -21,10 +21,14 @@ static int testacpi_write_tables(const struct udevice *dev,
>  				 struct acpi_ctx *ctx)
>  {
>  	struct acpi_dmar *dmar;
> +	int ret;
>  
>  	dmar = (struct acpi_dmar *)ctx->current;
>  	acpi_create_dmar(dmar, DMAR_INTR_REMAP);
>  	ctx->current += sizeof(struct acpi_dmar);
> +	ret = acpi_add_table(ctx, dmar);
> +	if (ret)
> +		return log_msg_ret("add", ret);
>  
>  	return 0;
>  }
> -- 
> 2.25.1.481.gfbce0eb801-goog

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



More information about the U-Boot mailing list