On 21.10.20 13:41, Ilias Apalodimas wrote: > Hi Heinrich, > > On Wed, Oct 21, 2020 at 12:17:29PM +0200, Heinrich Schuchardt wrote: >> On 10/21/20 9:32 AM, Ilias Apalodimas wrote: >>> U-Boot Driver Model is supposed to remove devices with either >>> DM_REMOVE_ACTIVE_DMA or DM_REMOVE_OS_PREPARE flags set, before exiting. >>> Our bootm command does that by explicitly calling calling >>> "dm_remove_devices_flags(DM_REMOVE_ACTIVE_ALL);" and we also disable any >>> USB devices. >>> >>> The EFI equivalent is doing none of those at the moment. As a result >>> probing an fTPM driver now renders it unusable in Linux. During our >>> (*probe) callback we open a session with OP-TEE, which is supposed to >>> close with our (*remove) callback. Since the (*remove) is never called, >>> once we boot into Linux and try to probe the device again we are getting >>> a busy error response. We also never free >>> >>> So let's fix this by mimicking what bootm does and disconnect devices >>> when efi_exit_boot_services() is called. Note that for the OP-TEE case >>> and in particular any subsequent bootloader that wants to use a device >>> (e.g GRUB) will need to call exit_boot_services() in order to close the >>> session. >> >> Hello Ilias, >> >> thanks for the patch. Adding the function calls looks correct to me, > > Well the only doubt I have is what if GRUB has to extend some PCRs before > calling Linux? Any idea if it's currently calling ExitBootSevices? > I was considering if it would be a better idea to call the device unbinding > during > some kind of "exit" from U-boot's EFI code. (i.e before StartImage)
ExitBootServices() is called by the Linux EFI stub in function allocate_new_fdt_and_exit_boot(). If GRUB would call ExitBootServices(), it would not be able to launch the EFI stub via StartImage(). Best regards Heinrich > >> but >> sandbox_spl_defconfig gives me a compile time error with this patch, cf. >> https://gitlab.denx.de/u-boot/custodians/u-boot-efi/-/jobs/166842 >> >> You might want to use Travis CI before resubmitting. > > Ok will do. > > Thanks > /Ilias > >> >> Best regards >> >> Heinrich >> >> >>> >>> Signed-off-by: Ilias Apalodimas <[email protected]> >>> --- >>> lib/efi_loader/efi_boottime.c | 5 +++++ >>> 1 file changed, 5 insertions(+) >>> >>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c >>> index bf78176217c6..25e6cf0fe719 100644 >>> --- a/lib/efi_loader/efi_boottime.c >>> +++ b/lib/efi_loader/efi_boottime.c >>> @@ -18,6 +18,8 @@ >>> #include <pe.h> >>> #include <u-boot/crc.h> >>> #include <watchdog.h> >>> +#include <dm/device.h> >>> +#include <dm/root.h> >>> >>> DECLARE_GLOBAL_DATA_PTR; >>> >>> @@ -1994,7 +1996,10 @@ static efi_status_t EFIAPI >>> efi_exit_boot_services(efi_handle_t image_handle, >>> list_del(&evt->link); >>> } >>> >>> + if IS_ENABLED(CONFIG_USB_DEVICE) >>> + udc_disconnect(); >>> board_quiesce_devices(); >>> + dm_remove_devices_flags(DM_REMOVE_ACTIVE_ALL); >>> >>> /* Patch out unsupported runtime function */ >>> efi_runtime_detach(); >>> >>
