[PATCH v2 14/28] efi_loader: Install ACPI tables

[Patrick Rudolph]

On Fri, Sep 6, 2024 at 12:15 PM Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> Hi Patrick,
>
> On Fri, Sep 06, 2024 at 09:22:16AM +0200, Patrick Rudolph wrote:
> > Install ACPI tables inside the efi_loader similar to SMBIOS tables.
> > When ACPI is enabled and wasn't installed in other places, install
> > the ACPI table in EFI. Since EFI is necessary to pass the ACPI
> > table location when FDT isn't used, there's no need to install it
> > separately.
> >
> > When CONFIG_BLOBLIST_TABLES is set the tables will be stored in a
> > bloblist.
> >
> > TEST: Booted QEMU SBSA using EFI and ACPI only.
> >
>
> Why none of the EFI maintainers were CC'ed into that?
>
> > Signed-off-by: Patrick Rudolph <patrick.rudolph at 9elements.com>
> > Cc: Simon Glass <sjg at chromium.org>
> > Cc: Tom Rini <trini at konsulko.com>
> > ---
> >  lib/efi_loader/efi_acpi.c        | 57 ++++++++++++++++++++++++++++++--
> >  test/py/tests/test_event_dump.py |  1 +
> >  2 files changed, 56 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_acpi.c b/lib/efi_loader/efi_acpi.c
> > index 67bd7f8ca2..9262f21ea6 100644
> > --- a/lib/efi_loader/efi_acpi.c
> > +++ b/lib/efi_loader/efi_acpi.c
> > @@ -6,15 +6,23 @@
> >   */
> >
> >  #include <efi_loader.h>
> > -#include <log.h>
> > -#include <mapmem.h>
> >  #include <acpi/acpi_table.h>
> >  #include <asm/global_data.h>
> > +#include <asm/io.h>
> > +#include <bloblist.h>
> > +#include <linux/sizes.h>
> > +#include <linux/log2.h>
> > +#include <log.h>
> > +#include <malloc.h>
> > +#include <mapmem.h>
> >
> >  DECLARE_GLOBAL_DATA_PTR;
> >
> >  static const efi_guid_t acpi_guid = EFI_ACPI_TABLE_GUID;
> >
> > +enum {
> > +     TABLE_SIZE      = SZ_64K,
> > +};
> >  /*
> >   * Install the ACPI table as a configuration table.
> >   *
> > @@ -47,3 +55,48 @@ efi_status_t efi_acpi_register(void)
> >       return efi_install_configuration_table(&acpi_guid,
> >                                              (void *)(ulong)addr);
> >  }
> > +
> > +static int install_acpi_table(void)
>
> This function doesn't install anything.  efi_acpi_register()
> nstalls the config tables for ACPI. This is just preparing and setting up
> correctly table_start_high/table_end_high AFAICT
>
It was copied from efi_smbios.c.
Please fix that file as well if you think it has the wrong name.

> > +{
> > +     u64 rom_addr, rom_table_end;
> > +     void *addr;
> > +
> > +     if (!IS_ENABLED(CONFIG_GENERATE_ACPI_TABLE) ||
> > +         IS_ENABLED(CONFIG_X86) ||
> > +         IS_ENABLED(CONFIG_QFW_ACPI))
> > +             return 0;
> > +
> > +     /* Align the table to a 4KB boundary to keep EFI happy */
> > +     if (IS_ENABLED(CONFIG_BLOBLIST_TABLES))
> > +             addr = bloblist_add(BLOBLISTT_ACPI_TABLES, TABLE_SIZE,
> > +                                 ilog2(SZ_4K));
> > +     else
> > +             addr = memalign(SZ_4K, TABLE_SIZE);
>
> The alignment is already taken care of in efi_acpi_register(). Also an
>
When the caller does not align the memory it could happen that
efi_acpi_register() changes the
page attribute of an existing mapping, which might break on the OS side.

Please fix efi_acpi_register() to not assume anything and do not
silently modify mem-ranges.
An assert should be used instead to enforce proper alignment on page boundary.

> > +
> > +     if (!addr)
> > +             return log_msg_ret("mem", -ENOBUFS);
> > +
> > +     rom_addr = virt_to_phys(addr);
> > +
> > +     gd->arch.table_start_high = rom_addr;
> > +
> > +     rom_table_end = write_acpi_tables(rom_addr);
> > +     if (!rom_table_end) {
> > +             log_err("Can't create ACPI configuration table\n");
> > +             return -EINTR;
> > +     }
> > +
> > +     debug("- wrote 'acpi' to %llx, end %llx\n", rom_addr, rom_table_end);
> > +     if (rom_table_end - rom_addr > TABLE_SIZE) {
> > +             log_err("Out of space for configuration tables: need %llx, have %x\n",
> > +                     rom_table_end - rom_addr, TABLE_SIZE);
> > +             return log_msg_ret("acpi", -ENOSPC);
> > +     }
> > +     gd->arch.table_end_high = rom_table_end;
> > +
> > +     debug("- done writing tables\n");
> > +
> > +     return 0;
> > +}
> > +
> > +EVENT_SPY_SIMPLE(EVT_LAST_STAGE_INIT, install_acpi_table);
>
> Why is this an event and how do you make sure it runs before
> efi_acpi_register()?. This should just run explicitly before that to prepare
> the ACPI tables and if it fails we shouldn't even try to install the config
> table
>
It's reusing the same mechanism as existing code to make sure it runs before
efi_acpi_register(). Existing code uses
EVENT_SPY_SIMPLE(EVT_LAST_STAGE_INIT,...); to make sure it runs before
the tables are registered.

> /Ilias
> > diff --git a/test/py/tests/test_event_dump.py b/test/py/tests/test_event_dump.py
> > index e282c67335..88bbf34f71 100644
> > --- a/test/py/tests/test_event_dump.py
> > +++ b/test/py/tests/test_event_dump.py
> > @@ -18,6 +18,7 @@ def test_event_dump(u_boot_console):
> >  --------------------  ------------------------------  ------------------------------
> >  EVT_FT_FIXUP          bootmeth_vbe_ft_fixup           .*boot/vbe_request.c:.*
> >  EVT_FT_FIXUP          bootmeth_vbe_simple_ft_fixup    .*boot/vbe_simple_os.c:.*
> > +EVT_LAST_STAGE_INIT   install_acpi_table              .*lib/efi_loader/efi_acpi.c:.*
> >  EVT_LAST_STAGE_INIT   install_smbios_table            .*lib/efi_loader/efi_smbios.c:.*
> >  EVT_MISC_INIT_F       sandbox_early_getopt_check      .*arch/sandbox/cpu/start.c:.*
> >  EVT_TEST              h_adder_simple                  .*test/common/event.c:'''
> > --
> > 2.45.2
> >