[PATCH] efi_loader: Disable devices before handing over control

[PATCH] efi_loader: Disable devices before handing over control

[Ilias Apalodimas]

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.

Signed-off-by: Ilias Apalodimas <ilias.apalodimas@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();
-- 
2.28.0

[PATCH] efi_loader: Disable devices before handing over control

[Heinrich Schuchardt]

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, 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.

Best regards

Heinrich


>
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@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();
>

[PATCH] efi_loader: Disable devices before handing over control

[Ilias Apalodimas]

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)

> 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@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();
> >
> 

[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@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();
>>>
>>

[PATCH] efi_loader: Disable devices before handing over control

[Ilias Apalodimas]

Hi Heinrich,


On Wed, 21 Oct 2020 at 15:35, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> 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().

Yea that's my point. So with the current patch, you won't be able to
access the fTPM driver from GRUB
(or any other EFI application) until the Linux EFI stub calls exit
boot services. Maybe calling those 2 functions in
StartImage is a better idea?


/Ilias
>
> 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@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();
> >>>
> >>
>

[PATCH] efi_loader: Disable devices before handing over control

[Ilias Apalodimas]

Hi Heinrich

On Wed, 21 Oct 2020 at 15:42, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Heinrich,
>
>
> On Wed, 21 Oct 2020 at 15:35, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >
> > 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().
>
> Yea that's my point. So with the current patch, you won't be able to
> access the fTPM driver from GRUB
> (or any other EFI application) until the Linux EFI stub calls exit
> boot services. Maybe calling those 2 functions in
> StartImage is a better idea?

So as it seems (over irc discussion), GRUB expects an EFI protocol to
use the fTPM device.
So removing the device on ExitBootServices is fine, since GRUB never
access the TPM directly.
U-Boot still doesn't have support for EFI_TCG2_PROTOCOL , but if we
add that GRUB will be able
to access the device via the EFI protocol.


I'll send a v2 fixing the compilation error shortly

Cheers
/Ilias
>
>
> /Ilias
> >
> > 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@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();
> > >>>
> > >>
> >

[PATCH] efi_loader: Disable devices before handing over control

[Mark Kettenis]

> From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Date: Wed, 21 Oct 2020 15:42:02 +0300
> 
> Hi Heinrich,
> 
> 
> On Wed, 21 Oct 2020 at 15:35, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> >
> > 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().
> 
> Yea that's my point. So with the current patch, you won't be able to
> access the fTPM driver from GRUB
> (or any other EFI application) until the Linux EFI stub calls exit
> boot services. Maybe calling those 2 functions in
> StartImage is a better idea?

Shouldn't an EFI application (such as GRUB) be using EFI protocols to
access the TPM?

[PATCH] efi_loader: Disable devices before handing over control

[Ilias Apalodimas]

Hi Mark, 

On Wed, Oct 21, 2020 at 03:21:51PM +0200, Mark Kettenis wrote:
> > From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> > Date: Wed, 21 Oct 2020 15:42:02 +0300
> > 
> > Hi Heinrich,
> > 
> > 
> > On Wed, 21 Oct 2020 at 15:35, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> > >
> > > 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().
> > 
> > Yea that's my point. So with the current patch, you won't be able to
> > access the fTPM driver from GRUB
> > (or any other EFI application) until the Linux EFI stub calls exit
> > boot services. Maybe calling those 2 functions in
> > StartImage is a better idea?
> 
> Shouldn't an EFI application (such as GRUB) be using EFI protocols to
> access the TPM?

Yea I already responded to myself on the previous mail. We don't have support
for the EFI protocol in U-Boot (yet), but cleaning up in exit boot services makes
more sense. So I'll fix the compilation error and send a V2 with the DM removal 
as-is. GRUB will be able to access the TPM once we add the EFI protocols in U-boot.

Cheers
/Ilias