Re: [PATCH v1] efi: make sure EFI disk controllers are connected when discovering devices

[Alexander Graf]

On 09.02.22 18:20, Renaud Métrich wrote:
Hi,
I observed no slowdown at all with the new code.

I was thinking of the same, i.e. trying to find the device and perform 
a discovery if not found but there is some sort of caching mechanism 
inside Grub regarding devices which prevents Grub from working at all 
if the device list already have been initialized. So in the end, this 
would require modifying the code too much.

If you fear there is some impact on performances, then I may introduce 
a compilation option instead and not ship the code by default.


I don't think a compile time option is going to help too much - this is 
a real problem that will happen in the wild. And especially when you 
start looking at signed grub binaries, you want to make sure that the 
distribution provided grub binary contains all functionality you need.

Let's ask some UEFI experts like Leif, Ard and Heinrich whether they can 
disregard my fear for slowdown when we go and connect all controllers on 
startup. Or if they have good ideas for alternatives.


Thanks!

Alex



Renaud.

Le 2/9/22 à 14:16, Alexander Graf a écrit :

On 08.02.22 11:56, Renaud Métrich wrote:
When efi.quickboot is enabled on VMWare (which is the default for
hardware release 16 and later), it may happen that not all EFI devices
are connected. Due to this, browsing the devices in make_devices() just
fails to find devices, in particular partitions for a given disk.
This typically happens when network booting, then trying to 
chainload to
local disk (this is used in deployment tools such as Red Hat 
Satellite).

This patch makes sure all controllers are connected before enumerating
the disks. It blindly calls the EFI ConnectController function with
recursive option and just ignores the result, which is 0 (EFI_SUCCESS)
or 14 (EFI_NOT_FOUND) when the handle is not a controller.

Signed-off-by: Renaud Métrich <rmetr...@redhat.com>


Isn't this going to slow down most normal boots? Is there a way we 
can only try harder to find devices once the "fast path" is 
exhausted? This could be useful for other systems as well, where we 
could delay probing for floppy drives until such a point.

I'm thinking of something akin to udev_settle. You typically search 
for devices by UUID. If you find the UUID, all is good. If you don't, 
invoke the "try to find additional devices" path.


Alex


---
  grub-core/disk/efi/efidisk.c | 10 ++++++++++
  grub-core/kern/efi/efi.c     | 13 +++++++++++++
  include/grub/efi/efi.h       |  5 +++++
  3 files changed, 28 insertions(+)

diff --git a/grub-core/disk/efi/efidisk.c 
b/grub-core/disk/efi/efidisk.c
index f077b5f55..0b16c3868 100644
--- a/grub-core/disk/efi/efidisk.c
+++ b/grub-core/disk/efi/efidisk.c
@@ -387,6 +387,16 @@ static void
  enumerate_disks (void)
  {
    struct grub_efidisk_data *devices;
+  unsigned i;
+  grub_efi_handle_t *handles;
+  grub_efi_uintn_t num_handles;
+
+  /* Prior to enumerating, make sure all EFI devices are connected */
+
+  handles = grub_efi_locate_handle (GRUB_EFI_ALL_HANDLES,
+                    NULL, NULL, &num_handles);
+  for (i = 0; i < num_handles; i++)
+    (void) grub_efi_connect_controller(handles[i], NULL, NULL, 1);
      devices = make_devices ();
    if (! devices)
diff --git a/grub-core/kern/efi/efi.c b/grub-core/kern/efi/efi.c
index 18858c327..bdfff3dd0 100644
--- a/grub-core/kern/efi/efi.c
+++ b/grub-core/kern/efi/efi.c
@@ -95,6 +95,19 @@ grub_efi_locate_handle 
(grub_efi_locate_search_type_t search_type,
    return buffer;
  }
  +grub_efi_status_t
+grub_efi_connect_controller (grub_efi_handle_t controller_handle,
+                 grub_efi_handle_t *driver_image_handle,
+                 grub_efi_device_path_protocol_t 
*remaining_device_path,
+                 grub_efi_boolean_t recursive)
+{
+  grub_efi_boot_services_t *b;
+
+  b = grub_efi_system_table->boot_services;
+  return efi_call_4 (b->connect_controller, controller_handle,
+             driver_image_handle, remaining_device_path, recursive);
+}
+
  void *
  grub_efi_open_protocol (grub_efi_handle_t handle,
              grub_efi_guid_t *protocol,
diff --git a/include/grub/efi/efi.h b/include/grub/efi/efi.h
index fc723962d..62dbd9788 100644
--- a/include/grub/efi/efi.h
+++ b/include/grub/efi/efi.h
@@ -32,6 +32,11 @@ EXPORT_FUNC(grub_efi_locate_handle) 
(grub_efi_locate_search_type_t search_type,
                       grub_efi_guid_t *protocol,
                       void *search_key,
                       grub_efi_uintn_t *num_handles);
+grub_efi_status_t
+EXPORT_FUNC(grub_efi_connect_controller) (grub_efi_handle_t 
controller_handle,
+                      grub_efi_handle_t *driver_image_handle,
+                      grub_efi_device_path_protocol_t 
*remaining_device_path,
+                      grub_efi_boolean_t recursive);
  void *EXPORT_FUNC(grub_efi_open_protocol) (grub_efi_handle_t handle,
                         grub_efi_guid_t *protocol,
                         grub_efi_uint32_t attributes);



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