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