[PATCH] efi_loader: Disable devices before handing over control

[Heinrich Schuchardt]

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 <ilias.apalodimas at linaro.org>
>>> ---
>>>  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();
>>>
>>