[PATCH v3 1/1] efi_driver: don't bind internal block devices
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 with device path protocol.
In the Supported() method of our EFI_DRIVER_BINDING_PROTOCOL return EFI_UNSUPPORTED when dealing with an U-Boot internal device.
Reported-by: Etienne Carriere etienne.carriere@linaro.org Fixes: commit 05ef48a2484b ("efi_driver: EFI block driver") Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com Reviewed-by: Etienne Carriere etienne.carriere@linaro.org Tested-by: Etienne Carriere etienne.carriere@linaro.org Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org --- v3: return EFI_UNSUPPORTED changes Fixes: line
v2: add code comment --- lib/efi_driver/efi_uclass.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c index b01ce89c84..1d7a0f57e3 100644 --- a/lib/efi_driver/efi_uclass.c +++ b/lib/efi_driver/efi_uclass.c @@ -71,6 +71,15 @@ static efi_status_t EFIAPI efi_uc_supported( EFI_ENTRY("%p, %p, %ls", this, controller_handle, efi_dp_str(remaining_device_path));
+ /* + * U-Boot internal devices install protocols interfaces without calling + * ConnectController(). Hence we should not bind an extra driver. + */ + if (controller_handle->dev) { + ret = EFI_UNSUPPORTED; + goto out; + } + ret = EFI_CALL(systab.boottime->open_protocol( controller_handle, bp->ops->protocol, &interface, this->driver_binding_handle,
On Fri, Sep 09, 2022 at 04:11:18PM +0200, Heinrich Schuchardt 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 with device path protocol.
In the Supported() method of our EFI_DRIVER_BINDING_PROTOCOL return EFI_UNSUPPORTED when dealing with an U-Boot internal device.
Yes, but see the below.
Reported-by: Etienne Carriere etienne.carriere@linaro.org Fixes: commit 05ef48a2484b ("efi_driver: EFI block driver") Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com Reviewed-by: Etienne Carriere etienne.carriere@linaro.org Tested-by: Etienne Carriere etienne.carriere@linaro.org Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org
v3: return EFI_UNSUPPORTED changes Fixes: line
v2: add code comment
lib/efi_driver/efi_uclass.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c index b01ce89c84..1d7a0f57e3 100644 --- a/lib/efi_driver/efi_uclass.c +++ b/lib/efi_driver/efi_uclass.c @@ -71,6 +71,15 @@ static efi_status_t EFIAPI efi_uc_supported( EFI_ENTRY("%p, %p, %ls", this, controller_handle, efi_dp_str(remaining_device_path));
- /*
* U-Boot internal devices install protocols interfaces without calling* ConnectController(). Hence we should not bind an extra driver.*/- if (controller_handle->dev) {
This check is not good enough to distinguish the two cases, ordinary block devices and EFI-app-based devices. Remember that "dev" field is also set by start() for the latter. (We expect EFI_ALREADY_STARTED in this case.)
I think that you should look at dev's uclass (and/or blk_desc's if_type) for now. Logically, I believe, controller_handle's device path could be used to determine if the handle is to be managed by this driver.
UEFI spec says, "Drivers will typically use the device path attached to ControllerHandle and/or the services from the bus I/O abstraction attached to ControllerHandle to determine if the driver supports ControllerHandle."
-Takahiro Akashi
ret = EFI_UNSUPPORTED;goto out;- }
- ret = EFI_CALL(systab.boottime->open_protocol( controller_handle, bp->ops->protocol, &interface, this->driver_binding_handle,
-- 2.37.2
On 9/12/22 03:55, AKASHI Takahiro wrote:
On Fri, Sep 09, 2022 at 04:11:18PM +0200, Heinrich Schuchardt 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 with device path protocol.
In the Supported() method of our EFI_DRIVER_BINDING_PROTOCOL return EFI_UNSUPPORTED when dealing with an U-Boot internal device.
Yes, but see the below.
Reported-by: Etienne Carriere etienne.carriere@linaro.org Fixes: commit 05ef48a2484b ("efi_driver: EFI block driver") Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com Reviewed-by: Etienne Carriere etienne.carriere@linaro.org Tested-by: Etienne Carriere etienne.carriere@linaro.org Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org
v3: return EFI_UNSUPPORTED changes Fixes: line
v2: add code comment
lib/efi_driver/efi_uclass.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c index b01ce89c84..1d7a0f57e3 100644 --- a/lib/efi_driver/efi_uclass.c +++ b/lib/efi_driver/efi_uclass.c @@ -71,6 +71,15 @@ static efi_status_t EFIAPI efi_uc_supported( EFI_ENTRY("%p, %p, %ls", this, controller_handle, efi_dp_str(remaining_device_path));
- /*
* U-Boot internal devices install protocols interfaces without calling* ConnectController(). Hence we should not bind an extra driver.*/- if (controller_handle->dev) {
This check is not good enough to distinguish the two cases, ordinary block devices and EFI-app-based devices. Remember that "dev" field is also set by start() for the latter. (We expect EFI_ALREADY_STARTED in this case.)
I think that you should look at dev's uclass (and/or blk_desc's if_type) for now. Logically, I believe, controller_handle's device path could be used to determine if the handle is to be managed by this driver.
UEFI spec says, "Drivers will typically use the device path attached to ControllerHandle and/or the services from the bus I/O abstraction attached to ControllerHandle to determine if the driver supports ControllerHandle."
-Takahiro Akashi
Yes, the same you could say about your suggestion to always return EFI_UNSUPPORTED in the supported() method if field dev is populated.
Overall some rework will have to be done in the DM-UEFI integration in the next cycle:
* Implement a stop() method for our UEFI block driver. * Let efi_disk_probe() use ConnectController() to install all protocol interfaces but device-tree and block IO protocol. * Let efi_disk_remove() use UninstallMultipleProtocolInterfaces() to remove all protocols. * Let efi_disk_remove() return an error code if the handle is not removed by Uninstall MultipleProtocolInterfaces(). * Move the generation of device-path nodes to the respective uclasses to let the device-tree in DM and UEFI match.
Best regards
Heinrich
ret = EFI_UNSUPPORTED;goto out;- }
- ret = EFI_CALL(systab.boottime->open_protocol( controller_handle, bp->ops->protocol, &interface, this->driver_binding_handle,
-- 2.37.2
participants (2)
-
AKASHI Takahiro -
Heinrich Schuchardt