Re: [PATCH 2/2] efi_loader: remove non vital devices first

13 Nov 2024

Am 13. November 2024 17:03:03 MEZ schrieb Simon Glass sjg@chromium.org:
...
Hi Heinrich,
On Wed, 13 Nov 2024 at 08:17, Heinrich Schuchardt xypron.glpk@gmx.de
wrote:
...
Am 13. November 2024 15:39:22 MEZ schrieb Simon Glass sjg@chromium.org:
...
Hi,
On Wed, 13 Nov 2024 at 05:52, Heinrich Schuchardt xypron.glpk@gmx.de
wrote:
...
...
...
On 11/1/24 21:29, Mark Kettenis wrote:
...
...
From: Janne Grunau j@jannau.net
Date: Thu, 31 Oct 2024 23:48:02 +0100
DM_FLAG_VITAL marks devices which are essential for the operation of
other devices. Removing these devices before their users can result
in
...
...
...
...
...
hangs or crashes.
This potentially fixes EFI boot of Renesas rcar3 devices. Their
clock
...
...
...
...
...
devices (and with this series the dart iommu) are the only devices
markes as vital.
The arm boot code already handles devioce removal in this way.
There is a typo in that last sentence of the commit message
(devioce).
...
...
...
...
Otherwise:
...
Signed-off-by: Janne Grunau j@jannau.net
Reviewed-by: Mark Kettenis kettenis@openbsd.org
...

lib/efi_loader/efi_boottime.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/lib/efi_loader/efi_boottime.c
b/lib/efi_loader/efi_boottime.c
...
...
...
...
...
index
4f52284b4c653c252b0ed6c0c87da8901448d4b4..7db3c95782970f8c06a970a8ee86b1804cd848b6
100644
...
...
...
...
...
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -2234,6 +2234,7 @@ static efi_status_t EFIAPI
efi_exit_boot_services(efi_handle_t image_handle,
...
...
...
...
...
         if (IS_ENABLED(CONFIG_USB_DEVICE))
                 udc_disconnect();
         board_quiesce_devices();


       dm_remove_devices_flags(DM_REMOVE_ACTIVE_ALL |



DM_REMOVE_NON_VITAL);
...
...
...
...
...
         dm_remove_devices_flags(DM_REMOVE_ACTIVE_ALL);

Simon's patch 6224dc9ba428 ("arm: Remove vital devices last") addressed
the same issue for bootm on arm. But what about about other
architectures?
...
...
...
This logic should be moved to drivers/core/root.c instead of
replicating
...
...
...
code.
We could have a common helper, but it should not be in driver/core as
this ordering is more of a policy decision. Unless we can add a
parameter telling dm exactly what to do...
BTW, Heinrich, this behaviour is exactly what my bootflow_efi() test
was supposed to check. But since it doesn't have the
exit-boot-services piece at your request...
Regards,
Simon
Why can't we generally remove non-vital devices first if all are to be
removed?
...
I cannot see anything device specific here.
No objection to that...but it needs to be in a new function, not become the
default behaviour of dm_remove_devices_flags(), which is supposed to do
what it is told and in one pass.
dm_remove_device_flags() makes no specific promises concerning the deletion sequence and its internal working. But obviously it will fail if non-vital redources are not deleted first.
Leaving this broken function exported and writing a new one has no user benefit.
We could copy the old function to a new static function that we call as needed from the old function name.
Best regards
Heinruch
...
Regards,
Simon

    