diff mbox series

[1/1] efi_driver: don't bind internal block devices

Message ID 20220909065758.64817-1-xypron.glpk@gmx.de
State Superseded, archived
Delegated to: Heinrich Schuchardt
Headers show
Series [1/1] efi_driver: don't bind internal block devices | expand

Commit Message

Heinrich Schuchardt Sept. 9, 2022, 6:57 a.m. UTC 
UEFI block devices can either mirror U-Boot's internal devices or be
provided by an EFI application like iPXE.

When ConnectController() is invoked for the EFI_BLOCK_IO_PROTOCOL
interface for such an application provided device we create a virtual
U-Boot block device of type "efi_blk".

Currently we do not call ConnectController() when handles for U-Boot's
internal block devices are created. If an EFI application calls
ConnectController() for a handle relating to an internal block device,
we erroneously create an extra "efi_blk" block device.

E.g. the UEFI shell has a command 'connect -r' which calls
ConnectController() for all handles.

In the Supported() method of our EFI_DRIVER_BINDING_PROTOCOL return
EFI_ALREADY_STARTED when dealing with an U-Boot internal device.

Reported-by: Etienne Carriere <etienne.carriere@linaro.org>
Fixes: b406eb04c360 ("efi_loader: disk: a helper function to delete efi_disk objects")
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
 lib/efi_driver/efi_uclass.c | 5 +++++
 1 file changed, 5 insertions(+)

--
2.30.2

Comments

Etienne Carriere Sept. 9, 2022, 9:01 a.m. UTC | #1 
Hello Heinrich,

On Fri, 9 Sept 2022 at 08:58, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> UEFI block devices can either mirror U-Boot's internal devices or be
> provided by an EFI application like iPXE.
>
> When ConnectController() is invoked for the EFI_BLOCK_IO_PROTOCOL
> interface for such an application provided device we create a virtual
> U-Boot block device of type "efi_blk".
>
> Currently we do not call ConnectController() when handles for U-Boot's
> internal block devices are created. If an EFI application calls
> ConnectController() for a handle relating to an internal block device,
> we erroneously create an extra "efi_blk" block device.
>
> E.g. the UEFI shell has a command 'connect -r' which calls
> ConnectController() for all handles.
>
> In the Supported() method of our EFI_DRIVER_BINDING_PROTOCOL return
> EFI_ALREADY_STARTED when dealing with an U-Boot internal device.
>
> Reported-by: Etienne Carriere <etienne.carriere@linaro.org>
> Fixes: b406eb04c360 ("efi_loader: disk: a helper function to delete efi_disk objects")
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  lib/efi_driver/efi_uclass.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c
> index b01ce89c84..d348960fc9 100644
> --- a/lib/efi_driver/efi_uclass.c
> +++ b/lib/efi_driver/efi_uclass.c
> @@ -71,6 +71,11 @@ static efi_status_t EFIAPI efi_uc_supported(
>         EFI_ENTRY("%p, %p, %ls", this, controller_handle,
>                   efi_dp_str(remaining_device_path));
>
> +       if (controller_handle->dev) {
> +               ret = EFI_ALREADY_STARTED;
> +               goto out;
> +       }
> +
>         ret = EFI_CALL(systab.boottime->open_protocol(
>                         controller_handle, bp->ops->protocol,
>                         &interface, this->driver_binding_handle,
> --
> 2.30.2
>

Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org>
Tested-by: Etienne Carriere <etienne.carriere@linaro.org>

Thanks for the fix and commitment.
Best regards,
Etienne
Ilias Apalodimas Sept. 9, 2022, 9:15 a.m. UTC | #2 
Hi Heinrich,

On Fri, 9 Sept 2022 at 09:58, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> UEFI block devices can either mirror U-Boot's internal devices or be
> provided by an EFI application like iPXE.
>
> When ConnectController() is invoked for the EFI_BLOCK_IO_PROTOCOL
> interface for such an application provided device we create a virtual
> U-Boot block device of type "efi_blk".
>
> Currently we do not call ConnectController() when handles for U-Boot's
> internal block devices are created. If an EFI application calls
> ConnectController() for a handle relating to an internal block device,
> we erroneously create an extra "efi_blk" block device.
>
> E.g. the UEFI shell has a command 'connect -r' which calls
> ConnectController() for all handles.
>
> In the Supported() method of our EFI_DRIVER_BINDING_PROTOCOL return
> EFI_ALREADY_STARTED when dealing with an U-Boot internal device.
>
> Reported-by: Etienne Carriere <etienne.carriere@linaro.org>
> Fixes: b406eb04c360 ("efi_loader: disk: a helper function to delete efi_disk objects")
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
>  lib/efi_driver/efi_uclass.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c
> index b01ce89c84..d348960fc9 100644
> --- a/lib/efi_driver/efi_uclass.c
> +++ b/lib/efi_driver/efi_uclass.c
> @@ -71,6 +71,11 @@ static efi_status_t EFIAPI efi_uc_supported(
>         EFI_ENTRY("%p, %p, %ls", this, controller_handle,
>                   efi_dp_str(remaining_device_path));
>

Can you add a comment on this while merging?

> +       if (controller_handle->dev) {
> +               ret = EFI_ALREADY_STARTED;
> +               goto out;
> +       }
> +
>         ret = EFI_CALL(systab.boottime->open_protocol(
>                         controller_handle, bp->ops->protocol,
>                         &interface, this->driver_binding_handle,
> --
> 2.30.2
>

with or without that comment
Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Heinrich Schuchardt Sept. 9, 2022, 12:51 p.m. UTC | #3 
On 9/9/22 11:15, Ilias Apalodimas wrote:
> Hi Heinrich,
>
> On Fri, 9 Sept 2022 at 09:58, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>
>> UEFI block devices can either mirror U-Boot's internal devices or be
>> provided by an EFI application like iPXE.
>>
>> When ConnectController() is invoked for the EFI_BLOCK_IO_PROTOCOL
>> interface for such an application provided device we create a virtual
>> U-Boot block device of type "efi_blk".
>>
>> Currently we do not call ConnectController() when handles for U-Boot's
>> internal block devices are created. If an EFI application calls
>> ConnectController() for a handle relating to an internal block device,
>> we erroneously create an extra "efi_blk" block device.
>>
>> E.g. the UEFI shell has a command 'connect -r' which calls
>> ConnectController() for all handles.
>>
>> In the Supported() method of our EFI_DRIVER_BINDING_PROTOCOL return
>> EFI_ALREADY_STARTED when dealing with an U-Boot internal device.
>>
>> Reported-by: Etienne Carriere <etienne.carriere@linaro.org>
>> Fixes: b406eb04c360 ("efi_loader: disk: a helper function to delete efi_disk objects")
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>>   lib/efi_driver/efi_uclass.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c
>> index b01ce89c84..d348960fc9 100644
>> --- a/lib/efi_driver/efi_uclass.c
>> +++ b/lib/efi_driver/efi_uclass.c
>> @@ -71,6 +71,11 @@ static efi_status_t EFIAPI efi_uc_supported(
>>          EFI_ENTRY("%p, %p, %ls", this, controller_handle,
>>                    efi_dp_str(remaining_device_path));
>>
>
> Can you add a comment on this while merging?

Sure

/*
  * U-Boot internal devices install protocols interfaces without
  * calling ConnectController(). Hence we should not bind an
  * extra driver.
  */

>
>> +       if (controller_handle->dev) {
>> +               ret = EFI_ALREADY_STARTED;
>> +               goto out;
>> +       }
>> +
>>          ret = EFI_CALL(systab.boottime->open_protocol(
>>                          controller_handle, bp->ops->protocol,
>>                          &interface, this->driver_binding_handle,
>> --
>> 2.30.2
>>
>
> with or without that comment
> Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

Thanks for reviewing

Best regards

Heinrich
diff mbox series

Patch

diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c
index b01ce89c84..d348960fc9 100644
--- a/lib/efi_driver/efi_uclass.c
+++ b/lib/efi_driver/efi_uclass.c
@@ -71,6 +71,11 @@  static efi_status_t EFIAPI efi_uc_supported(
 	EFI_ENTRY("%p, %p, %ls", this, controller_handle,
 		  efi_dp_str(remaining_device_path));

+	if (controller_handle->dev) {
+		ret = EFI_ALREADY_STARTED;
+		goto out;
+	}
+
 	ret = EFI_CALL(systab.boottime->open_protocol(
 			controller_handle, bp->ops->protocol,
 			&interface, this->driver_binding_handle,