[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 1/4] efi: Deduplicate configuration table search function
|
From: |
John Paul Adrian Glaubitz |
|
Subject: |
Re: [PATCH 1/4] efi: Deduplicate configuration table search function |
|
Date: |
Tue, 31 Oct 2023 22:41:04 +0100 |
|
User-agent: |
Evolution 3.50.1 |
On Tue, 2023-10-31 at 20:22 +0100, Vladimir 'phcoder' Serbinenko wrote:
> We do table search in many places doing exactly the same algorithm.
> The only minor variance in users is which table is used if several entries
> are present. As specification mandates uniqueness and even if it ever isn't,
> first entry is good enough, unify this code and always use the first entry.
>
> Signed-off-by: Vladimir Serbinenko <phcoder@gmail.com>
> ---
> grub-core/commands/efi/loadbios.c | 37 ++++++++-----------------------
> grub-core/commands/efi/lssal.c | 18 +++++----------
> grub-core/commands/efi/smbios.c | 28 ++---------------------
> grub-core/kern/efi/acpi.c | 26 ++--------------------
> grub-core/kern/efi/efi.c | 18 +++++++++++++++
> grub-core/kern/efi/fdt.c | 20 +++++------------
> include/grub/efi/efi.h | 3 +++
> 7 files changed, 46 insertions(+), 104 deletions(-)
>
> diff --git a/grub-core/commands/efi/loadbios.c
> b/grub-core/commands/efi/loadbios.c
> index 8f6b0ecfc..8e042095a 100644
> --- a/grub-core/commands/efi/loadbios.c
> +++ b/grub-core/commands/efi/loadbios.c
> @@ -92,7 +92,6 @@ lock_rom_area (void)
> static void
> fake_bios_data (int use_rom)
> {
> - unsigned i;
> void *acpi, *smbios;
> grub_uint16_t *ebda_seg_ptr, *low_mem_ptr;
>
> @@ -101,33 +100,15 @@ fake_bios_data (int use_rom)
> if ((*ebda_seg_ptr) || (*low_mem_ptr))
> return;
>
> - acpi = 0;
> - smbios = 0;
> - for (i = 0; i < grub_efi_system_table->num_table_entries; i++)
> - {
> - grub_guid_t *guid =
> - &grub_efi_system_table->configuration_table[i].vendor_guid;
> -
> - if (! grub_memcmp (guid, &acpi2_guid, sizeof (grub_guid_t)))
> - {
> - acpi = grub_efi_system_table->configuration_table[i].vendor_table;
> - grub_dprintf ("efi", "ACPI2: %p\n", acpi);
> - }
> - else if (! grub_memcmp (guid, &acpi_guid, sizeof (grub_guid_t)))
> - {
> - void *t;
> -
> - t = grub_efi_system_table->configuration_table[i].vendor_table;
> - if (! acpi)
> - acpi = t;
> - grub_dprintf ("efi", "ACPI: %p\n", t);
> - }
> - else if (! grub_memcmp (guid, &smbios_guid, sizeof (grub_guid_t)))
> - {
> - smbios = grub_efi_system_table->configuration_table[i].vendor_table;
> - grub_dprintf ("efi", "SMBIOS: %p\n", smbios);
> - }
> - }
> + acpi = grub_efi_find_configuration_table (&acpi2_guid);
> + grub_dprintf ("efi", "ACPI2: %p\n", acpi);
> + if (!acpi) {
> + acpi = grub_efi_find_configuration_table (&acpi_guid);
> + grub_dprintf ("efi", "ACPI: %p\n", acpi);
> + }
> +
> + smbios = grub_efi_find_configuration_table (&smbios_guid);
> + grub_dprintf ("efi", "SMBIOS: %p\n", smbios);
>
> *ebda_seg_ptr = FAKE_EBDA_SEG;
> *low_mem_ptr = (FAKE_EBDA_SEG >> 6);
> diff --git a/grub-core/commands/efi/lssal.c b/grub-core/commands/efi/lssal.c
> index fd6085f1b..7248bdc29 100644
> --- a/grub-core/commands/efi/lssal.c
> +++ b/grub-core/commands/efi/lssal.c
> @@ -136,22 +136,16 @@ grub_cmd_lssal (struct grub_command *cmd __attribute__
> ((unused)),
> int argc __attribute__ ((unused)),
> char **args __attribute__ ((unused)))
> {
> - const grub_efi_system_table_t *st = grub_efi_system_table;
> - grub_efi_configuration_table_t *t = st->configuration_table;
> - unsigned int i;
> static grub_guid_t guid = GRUB_EFI_SAL_TABLE_GUID;
> + void *table = grub_efi_find_configuration_table (&guid);
>
> - for (i = 0; i < st->num_table_entries; i++)
> + if (table == NULL)
> {
> - if (grub_memcmp (&guid, &t->vendor_guid,
> - sizeof (grub_guid_t)) == 0)
> - {
> - disp_sal (t->vendor_table);
> - return GRUB_ERR_NONE;
> - }
> - t++;
> + grub_printf ("SAL not found\n");
> + return GRUB_ERR_NONE;
> }
> - grub_printf ("SAL not found\n");
> +
> + disp_sal (table);
> return GRUB_ERR_NONE;
> }
>
> diff --git a/grub-core/commands/efi/smbios.c b/grub-core/commands/efi/smbios.c
> index d77239732..717e5fc1d 100644
> --- a/grub-core/commands/efi/smbios.c
> +++ b/grub-core/commands/efi/smbios.c
> @@ -18,44 +18,20 @@
> */
>
> #include <grub/smbios.h>
> -#include <grub/misc.h>
> #include <grub/efi/efi.h>
> -#include <grub/efi/api.h>
>
> struct grub_smbios_eps *
> grub_machine_smbios_get_eps (void)
> {
> - unsigned i;
> static grub_guid_t smbios_guid = GRUB_EFI_SMBIOS_TABLE_GUID;
>
> - for (i = 0; i < grub_efi_system_table->num_table_entries; i++)
> - {
> - grub_guid_t *guid =
> - &grub_efi_system_table->configuration_table[i].vendor_guid;
> -
> - if (! grub_memcmp (guid, &smbios_guid, sizeof (grub_guid_t)))
> - return (struct grub_smbios_eps *)
> - grub_efi_system_table->configuration_table[i].vendor_table;
> - }
> -
> - return 0;
> + return (struct grub_smbios_eps *) grub_efi_find_configuration_table
> (&smbios_guid);
> }
>
> struct grub_smbios_eps3 *
> grub_machine_smbios_get_eps3 (void)
> {
> - unsigned i;
> static grub_guid_t smbios3_guid = GRUB_EFI_SMBIOS3_TABLE_GUID;
>
> - for (i = 0; i < grub_efi_system_table->num_table_entries; i++)
> - {
> - grub_guid_t *guid =
> - &grub_efi_system_table->configuration_table[i].vendor_guid;
> -
> - if (! grub_memcmp (guid, &smbios3_guid, sizeof (grub_guid_t)))
> - return (struct grub_smbios_eps3 *)
> - grub_efi_system_table->configuration_table[i].vendor_table;
> - }
> -
> - return 0;
> + return (struct grub_smbios_eps3 *) grub_efi_find_configuration_table
> (&smbios3_guid);
> }
> diff --git a/grub-core/kern/efi/acpi.c b/grub-core/kern/efi/acpi.c
> index 461c77c33..828e6dbb2 100644
> --- a/grub-core/kern/efi/acpi.c
> +++ b/grub-core/kern/efi/acpi.c
> @@ -18,42 +18,20 @@
> */
>
> #include <grub/acpi.h>
> -#include <grub/misc.h>
> #include <grub/efi/efi.h>
> -#include <grub/efi/api.h>
>
> struct grub_acpi_rsdp_v10 *
> grub_machine_acpi_get_rsdpv1 (void)
> {
> - unsigned i;
> static grub_guid_t acpi_guid = GRUB_EFI_ACPI_TABLE_GUID;
>
> - for (i = 0; i < grub_efi_system_table->num_table_entries; i++)
> - {
> - grub_guid_t *guid =
> - &grub_efi_system_table->configuration_table[i].vendor_guid;
> -
> - if (! grub_memcmp (guid, &acpi_guid, sizeof (grub_guid_t)))
> - return (struct grub_acpi_rsdp_v10 *)
> - grub_efi_system_table->configuration_table[i].vendor_table;
> - }
> - return 0;
> + return (struct grub_acpi_rsdp_v10 *) grub_efi_find_configuration_table
> (&acpi_guid);
> }
>
> struct grub_acpi_rsdp_v20 *
> grub_machine_acpi_get_rsdpv2 (void)
> {
> - unsigned i;
> static grub_guid_t acpi20_guid = GRUB_EFI_ACPI_20_TABLE_GUID;
>
> - for (i = 0; i < grub_efi_system_table->num_table_entries; i++)
> - {
> - grub_guid_t *guid =
> - &grub_efi_system_table->configuration_table[i].vendor_guid;
> -
> - if (! grub_memcmp (guid, &acpi20_guid, sizeof (grub_guid_t)))
> - return (struct grub_acpi_rsdp_v20 *)
> - grub_efi_system_table->configuration_table[i].vendor_table;
> - }
> - return 0;
> + return (struct grub_acpi_rsdp_v20 *) grub_efi_find_configuration_table
> (&acpi20_guid);
> }
> diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
> index a2afd8de9..e53808307 100644
> --- a/grub-core/kern/efi/efi.c
> +++ b/grub-core/kern/efi/efi.c
> @@ -1031,3 +1031,21 @@ grub_efi_compare_device_paths (const
> grub_efi_device_path_t *dp1,
>
> return 0;
> }
> +
> +void *
> +grub_efi_find_configuration_table (const grub_guid_t *target_guid)
> +{
> + unsigned i;
> +
> + for (i = 0; i < grub_efi_system_table->num_table_entries; i++)
> + {
> + grub_guid_t *guid =
> + &grub_efi_system_table->configuration_table[i].vendor_guid;
> +
> + if (! grub_memcmp (guid, target_guid, sizeof (grub_guid_t)))
> + return (void *)
> + grub_efi_system_table->configuration_table[i].vendor_table;
> + }
> +
> + return 0;
> +}
> diff --git a/grub-core/kern/efi/fdt.c b/grub-core/kern/efi/fdt.c
> index 8fcf43f1b..15a495a34 100644
> --- a/grub-core/kern/efi/fdt.c
> +++ b/grub-core/kern/efi/fdt.c
> @@ -23,21 +23,13 @@
> void *
> grub_efi_get_firmware_fdt (void)
> {
> - grub_efi_configuration_table_t *tables;
> static grub_guid_t fdt_guid = GRUB_EFI_DEVICE_TREE_GUID;
> - void *firmware_fdt = NULL;
> - unsigned int i;
> -
> - /* Look for FDT in UEFI config tables. */
> - tables = grub_efi_system_table->configuration_table;
> -
> - for (i = 0; i < grub_efi_system_table->num_table_entries; i++)
> - if (grub_memcmp (&tables[i].vendor_guid, &fdt_guid, sizeof (fdt_guid))
> == 0)
> - {
> - firmware_fdt = tables[i].vendor_table;
> - grub_dprintf ("linux", "found registered FDT @ %p\n", firmware_fdt);
> - break;
> - }
> + void *firmware_fdt = grub_efi_find_configuration_table (&fdt_guid);
>
> + if (firmware_fdt) {
> + grub_dprintf ("linux", "found registered FDT @ %p\n", firmware_fdt);
> + } else {
> + grub_dprintf ("linux", "not found registered FDT\n");
> + }
> return firmware_fdt;
> }
> diff --git a/include/grub/efi/efi.h b/include/grub/efi/efi.h
> index 572f7135f..a5cd99e5a 100644
> --- a/include/grub/efi/efi.h
> +++ b/include/grub/efi/efi.h
> @@ -119,6 +119,9 @@ extern void (*EXPORT_VAR(grub_efi_net_config))
> (grub_efi_handle_t hnd,
> char **device,
> char **path);
>
> +void *
> +EXPORT_FUNC (grub_efi_find_configuration_table) (const grub_guid_t
> *target_guid);
> +
> #if defined(__arm__) || defined(__aarch64__) || defined(__riscv) ||
> defined(__loongarch__)
> void *EXPORT_FUNC(grub_efi_get_firmware_fdt)(void);
> grub_err_t EXPORT_FUNC(grub_efi_get_ram_base)(grub_addr_t *);
> --
> 2.39.2
Tested-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>
--
.''`. John Paul Adrian Glaubitz
: :' : Debian Developer
`. `' Physicist
`- GPG: 62FF 8A75 84E0 2956 9546 0006 7426 3B37 F5B5 F913