[PATCH 07/32] x86: Store the FADT so we can avoid searching for it

Bin Meng bmeng.cn at gmail.com
Fri Oct 16 11:55:39 CEST 2020


Hi Simon,

On Mon, Sep 28, 2020 at 12:26 PM Simon Glass <sjg at chromium.org> wrote:
>
> Put the FADT in global_data and access it as needed. Drop the functions
> which search for it.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
>  arch/x86/cpu/cpu.c          |  6 ++--
>  arch/x86/include/asm/acpi.h |  9 ------
>  arch/x86/lib/acpi.c         | 57 -------------------------------------
>  arch/x86/lib/acpi_table.c   |  1 +
>  include/dm/acpi.h           |  2 ++
>  5 files changed, 6 insertions(+), 69 deletions(-)
>
> diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c
> index 5c33f02f034..6cbfc0c8488 100644
> --- a/arch/x86/cpu/cpu.c
> +++ b/arch/x86/cpu/cpu.c
> @@ -199,13 +199,13 @@ __weak void board_final_cleanup(void)
>
>  int last_stage_init(void)
>  {
> -       struct acpi_fadt __maybe_unused *fadt;
> +       struct acpi_fadt *fadt;
>         int ret;
>
>         board_final_init();
>
>         if (IS_ENABLED(CONFIG_HAVE_ACPI_RESUME)) {
> -               fadt = acpi_find_fadt();
> +               fadt = gd->acpi_ctx->fadt;

NAK, this breaks the ACPI resume. On the resume path, the FADT address
has to be recovered from the system RAM.

>
>                 if (fadt && gd->arch.prev_sleep_state == ACPI_S3)
>                         acpi_resume(fadt);
> @@ -218,7 +218,7 @@ int last_stage_init(void)
>         }
>
>         if (IS_ENABLED(CONFIG_GENERATE_ACPI_TABLE)) {
> -               fadt = acpi_find_fadt();
> +               fadt = gd->acpi_ctx->fadt;

This one is okay

>
>                 /* Don't touch ACPI hardware on HW reduced platforms */
>                 if (fadt && !(fadt->flags & ACPI_FADT_HW_REDUCED_ACPI)) {
> diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h
> index 4475d046e8c..2023e2b0460 100644
> --- a/arch/x86/include/asm/acpi.h
> +++ b/arch/x86/include/asm/acpi.h
> @@ -8,15 +8,6 @@
>
>  struct acpi_fadt;
>
> -/**
> - * acpi_find_fadt() - find ACPI FADT table in the system memory
> - *
> - * This routine parses the ACPI table to locate the ACPI FADT table.
> - *
> - * @return:    a pointer to the ACPI FADT table in the system memory
> - */
> -struct acpi_fadt *acpi_find_fadt(void);
> -
>  /**
>   * acpi_find_wakeup_vector() - find OS installed wake up vector address
>   *
> diff --git a/arch/x86/lib/acpi.c b/arch/x86/lib/acpi.c
> index 155fffabf08..20f1bb79b35 100644
> --- a/arch/x86/lib/acpi.c
> +++ b/arch/x86/lib/acpi.c
> @@ -9,63 +9,6 @@
>  #include <asm/io.h>
>  #include <asm/tables.h>
>
> -static struct acpi_rsdp *acpi_valid_rsdp(struct acpi_rsdp *rsdp)
> -{
> -       if (strncmp((char *)rsdp, RSDP_SIG, sizeof(RSDP_SIG) - 1) != 0)
> -               return NULL;
> -
> -       debug("Looking on %p for valid checksum\n", rsdp);
> -
> -       if (table_compute_checksum((void *)rsdp, 20) != 0)
> -               return NULL;
> -       debug("acpi rsdp checksum 1 passed\n");
> -
> -       if ((rsdp->revision > 1) &&
> -           (table_compute_checksum((void *)rsdp, rsdp->length) != 0))
> -               return NULL;
> -       debug("acpi rsdp checksum 2 passed\n");
> -
> -       return rsdp;
> -}
> -
> -struct acpi_fadt *acpi_find_fadt(void)
> -{
> -       char *p, *end;
> -       struct acpi_rsdp *rsdp = NULL;
> -       struct acpi_rsdt *rsdt;
> -       struct acpi_fadt *fadt = NULL;
> -       int i;
> -
> -       /* Find RSDP */
> -       for (p = (char *)ROM_TABLE_ADDR; p < (char *)ROM_TABLE_END; p += 16) {
> -               rsdp = acpi_valid_rsdp((struct acpi_rsdp *)p);
> -               if (rsdp)
> -                       break;
> -       }
> -
> -       if (!rsdp)
> -               return NULL;
> -
> -       debug("RSDP found at %p\n", rsdp);
> -       rsdt = (struct acpi_rsdt *)(uintptr_t)rsdp->rsdt_address;
> -
> -       end = (char *)rsdt + rsdt->header.length;
> -       debug("RSDT found at %p ends at %p\n", rsdt, end);
> -
> -       for (i = 0; ((char *)&rsdt->entry[i]) < end; i++) {
> -               fadt = (struct acpi_fadt *)(uintptr_t)rsdt->entry[i];
> -               if (strncmp((char *)fadt, "FACP", 4) == 0)
> -                       break;
> -               fadt = NULL;
> -       }
> -
> -       if (!fadt)
> -               return NULL;
> -
> -       debug("FADT found at %p\n", fadt);
> -       return fadt;
> -}
> -

The above codes should not be removed for the resume path.

>  void *acpi_find_wakeup_vector(struct acpi_fadt *fadt)
>  {
>         struct acpi_facs *facs;
> diff --git a/arch/x86/lib/acpi_table.c b/arch/x86/lib/acpi_table.c
> index f0f342d8935..e0130ad5230 100644
> --- a/arch/x86/lib/acpi_table.c
> +++ b/arch/x86/lib/acpi_table.c
> @@ -590,6 +590,7 @@ ulong write_acpi_tables(ulong start_addr)
>
>         debug("ACPI:    * FADT\n");
>         fadt = ctx->current;
> +       ctx->fadt = fadt;
>         acpi_inc_align(ctx, sizeof(struct acpi_fadt));
>         acpi_create_fadt(fadt, facs, dsdt);
>         acpi_add_table(ctx, fadt);
> diff --git a/include/dm/acpi.h b/include/dm/acpi.h
> index e8b0336f6d8..a200305e640 100644
> --- a/include/dm/acpi.h
> +++ b/include/dm/acpi.h
> @@ -46,6 +46,7 @@ enum acpi_dump_option {
>   *     adding a new table. The RSDP holds pointers to the RSDT and XSDT.
>   * @rsdt: Pointer to the Root System Description Table
>   * @xsdt: Pointer to the Extended System Description Table
> + * @fadt: Pointer to the Fixed ACPI Description Table
>   * @nhlt: Intel Non-High-Definition-Audio Link Table (NHLT) pointer, used to
>   *     build up information that audio codecs need to provide in the NHLT ACPI
>   *     table
> @@ -58,6 +59,7 @@ struct acpi_ctx {
>         struct acpi_rsdp *rsdp;
>         struct acpi_rsdt *rsdt;
>         struct acpi_xsdt *xsdt;
> +       struct acpi_fadt *fadt;
>         struct nhlt *nhlt;
>         char *len_stack[ACPIGEN_LENSTACK_SIZE];
>         int ltop;
> --

Given the FADT only needs to be located once in either normal boot
flow or resume flow, I think this patch is not necessary.

Regards,
Bin


More information about the U-Boot mailing list