grub-devel.gnu.org archive mirror
 help / color / mirror / Atom feed
From: "Vladimir 'phcoder' Serbinenko" <phcoder@gmail.com>
To: The development of GRUB 2 <grub-devel@gnu.org>
Subject: [PATCH 1/4] efi: Deduplicate configuration table search function
Date: Tue, 31 Oct 2023 20:22:50 +0100	[thread overview]
Message-ID: <CAEaD8JN1saWQMqfZXC7w-pLGFv=_UntiXm965LuHDKFdgHWwog@mail.gmail.com> (raw)

[-- Attachment #1: Type: text/plain, Size: 339 bytes --]

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.

-- 
Regards
Vladimir 'phcoder' Serbinenko

[-- Attachment #2: 0001-efi-Deduplicate-configuration-table-search-function.patch --]
[-- Type: text/x-patch, Size: 9163 bytes --]

From ef8f6f068b275e3c010d3793048ac1d702d3f0fa Mon Sep 17 00:00:00 2001
From: Vladimir Serbinenko <phcoder@gmail.com>
Date: Sun, 13 Aug 2023 09:15:02 +0200
Subject: [PATCH 1/4] efi: Deduplicate configuration table search function

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


[-- Attachment #3: Type: text/plain, Size: 141 bytes --]

_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel

             reply	other threads:[~2023-10-31 19:23 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-31 19:22 Vladimir 'phcoder' Serbinenko [this message]
2023-10-31 21:41 ` [PATCH 1/4] efi: Deduplicate configuration table search function John Paul Adrian Glaubitz
  -- strict thread matches above, loose matches on Subject: below --
2023-10-07 14:57 Vladimir 'phcoder' Serbinenko
2023-10-17 14:38 ` Daniel Kiper

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAEaD8JN1saWQMqfZXC7w-pLGFv=_UntiXm965LuHDKFdgHWwog@mail.gmail.com' \
    --to=phcoder@gmail.com \
    --cc=grub-devel@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).