On 9/6/24 09:22, 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.

The UEFI specification clearly describes how to hand over ACPI tables. I
can't see why bloblists should be of any relevance in the UEFI context.


TEST: Booted QEMU SBSA using EFI and ACPI only.

With this description it is unclear if you used QFW to install ACPI
tables which would not test the code below.


Signed-off-by: Patrick Rudolph <[email protected]>
Cc: Simon Glass <[email protected]>
Cc: Tom Rini <[email protected]>

Please, cc maintainers as reported by scripts/get_maintainers in future.

---
  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);
  }
+

Please, provide a function description.

+static int install_acpi_table(void)
+{
+       u64 rom_addr, rom_table_end;

There is no ROM involved here. Please, use meaningful variable names.

+       void *addr;
+
+       if (!IS_ENABLED(CONFIG_GENERATE_ACPI_TABLE) ||
+           IS_ENABLED(CONFIG_X86) ||
+           IS_ENABLED(CONFIG_QFW_ACPI))

Why would you want to call write_acpi_tables() twice on the sandbox? See

board/sandbox/sandbox.c:180:
            end = write_acpi_tables(addr);

We should remove the redundant code from board/sandbox/sandbox.c with
this patch.

+               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 UEFI specification wants ACPI tables to reside in
EfiACPIReclaimMemory. Neither bloblist_add() nor memalign() should be
used here.

+
+       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);

Please, use log_debug().

+       if (rom_table_end - rom_addr > TABLE_SIZE) {
+               log_err("Out of space for configuration tables: need %llx, have 
%x\n",

This check is too late. We must ensure that write_acpi_table() does not
overwrite unallocated memory.

+                       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");

ditto

Best regards

Heinrich

+
+       return 0;
+}
+
+EVENT_SPY_SIMPLE(EVT_LAST_STAGE_INIT, install_acpi_table);
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:'''

Reply via email to