efi_loader: DM_REMOVE_ACTIVE_ALL

efi_loader: DM_REMOVE_ACTIVE_ALL

[Heinrich Schuchardt]

On 10/16/20 8:17 AM, Ilias Apalodimas wrote:
> On Fri, 16 Oct 2020 at 03:23, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:

You suggested to apply the following patch:

>>
>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
>> index bf78176217c6..813f4f79bc5b 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;
>>
>> @@ -1995,6 +1997,7 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle,
>> }

Shouldn't we also call udc_disconnect() in ExitBootServices()?

+if IS_ENABLED(CONFIG_USB_DEVICE)
+	udc_disconnect()

Cf. arch/arm/lib/bootm.c.

Best regards

Heinrich

>>
>> board_quiesce_devices();
>> + dm_remove_devices_flags(DM_REMOVE_ACTIVE_ALL);
>>
>> /* Patch out unsupported runtime function */
>> efi_runtime_detach();
>>
>>
>> What incorrect behavior did you observe?
>> Are there devices creating interrupts that need to be deactivated?
>
> Haven't checked the irq side, but I'd imagine it could be since
> boom/booti do run that.
>
> My problem is on the firmwareTPM driver [1] . During the 'device'
> probe the driver opens a session with op-tee
> which keeps it alive for all the transactions (and saves it on it's
> prv struct). Since the driver has DM_FLAG_OS_PREPARE
> set it expects the remove routine to run before handing over
> everything to the OS. That remove function is what
> eventually closes the session.
> So if the removal doesn't run you get a TEE_ERROR_BUSY back from
> OP-TEE when trying to probe/use the driver from Linux.
>
> [1] drivers/tpm/tpm2_ftpm_tee.c
>
>
> Cheers
> /Ilias

efi_loader: DM_REMOVE_ACTIVE_ALL

[Ilias Apalodimas]

On Fri, 16 Oct 2020 at 13:17, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 10/16/20 8:17 AM, Ilias Apalodimas wrote:
> > On Fri, 16 Oct 2020 at 03:23, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> You suggested to apply the following patch:
>
> >>
> >> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> >> index bf78176217c6..813f4f79bc5b 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;
> >>
> >> @@ -1995,6 +1997,7 @@ static efi_status_t EFIAPI efi_exit_boot_services(efi_handle_t image_handle,
> >> }
>
> Shouldn't we also call udc_disconnect() in ExitBootServices()?
>
> +if IS_ENABLED(CONFIG_USB_DEVICE)
> +       udc_disconnect()
>
> Cf. arch/arm/lib/bootm.c.

Good point, I assumed that disabled all devices without looking into
boom any further.
Let me have a closer look and I'll send something on the ML.

Cheers
/Ilias
>
> Best regards
>
> Heinrich
>
> >>
> >> board_quiesce_devices();
> >> + dm_remove_devices_flags(DM_REMOVE_ACTIVE_ALL);
> >>
> >> /* Patch out unsupported runtime function */
> >> efi_runtime_detach();
> >>
> >>
> >> What incorrect behavior did you observe?
> >> Are there devices creating interrupts that need to be deactivated?
> >
> > Haven't checked the irq side, but I'd imagine it could be since
> > boom/booti do run that.
> >
> > My problem is on the firmwareTPM driver [1] . During the 'device'
> > probe the driver opens a session with op-tee
> > which keeps it alive for all the transactions (and saves it on it's
> > prv struct). Since the driver has DM_FLAG_OS_PREPARE
> > set it expects the remove routine to run before handing over
> > everything to the OS. That remove function is what
> > eventually closes the session.
> > So if the removal doesn't run you get a TEE_ERROR_BUSY back from
> > OP-TEE when trying to probe/use the driver from Linux.
> >
> > [1] drivers/tpm/tpm2_ftpm_tee.c
> >
> >
> > Cheers
> > /Ilias