[RFC PATCH 2/2] cmd: replace efi_create_handle/add_protocol with InstallMultipleProtocol
Ilias Apalodimas
ilias.apalodimas at linaro.org
Thu Oct 6 09:38:44 CEST 2022
On Thu, Oct 06, 2022 at 04:26:37AM +0200, Heinrich Schuchardt wrote:
> On 10/6/22 03:53, AKASHI Takahiro wrote:
> > On Wed, Oct 05, 2022 at 06:26:02PM +0300, Ilias Apalodimas wrote:
> > > In general handles should only be deleted if the last remaining protocol
> > > is removed. Instead of explicitly calling
> > > efi_create_handle -> efi_add_protocol -> efi_delete_handle which blindly
> > > removes all protocols from a handle before removing it, use
> > > InstallMultiple/UninstallMultiple which adheres to the EFI spec and only
> > > deletes a handle if there are no additional protocols present
> > >
> > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > > ---
> > > cmd/bootefi.c | 13 ++++++-------
> > > 1 file changed, 6 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> > > index 3041873afbbc..4ab68868cc7e 100644
> > > --- a/cmd/bootefi.c
> > > +++ b/cmd/bootefi.c
> > > @@ -509,12 +509,9 @@ efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size)
> > > * Make sure that device for device_path exist
> > > * in load_image(). Otherwise, shell and grub will fail.
> > > */
> > > - ret = efi_create_handle(&mem_handle);
> > > - if (ret != EFI_SUCCESS)
> > > - goto out;
> > > -
> > > - ret = efi_add_protocol(mem_handle, &efi_guid_device_path,
> > > - file_path);
> > > + ret = efi_install_multiple_protocol_interfaces(&mem_handle,
> > > + &efi_guid_device_path,
> > > + file_path, NULL);
> >
> > nitpick: NULL -> NULL, NULL
> > UEFI spec seems to require "A variable argument list containing pairs of
> > protocol GUIDs and protocol interfaces" even if a protocol interface won't be
> > used with GUID as NULL.
>
> The spec also has:
> "The pairs of arguments are removed in order from the variable argument
> list until a NULL protocol GUID value is found."
>
> This is what a calls inside EDK II looks like:
>
> Status = CoreInstallMultipleProtocolInterfaces (
> &mDecompressHandle,
> &gEfiDecompressProtocolGuid,
> &gEfiDecompress,
> NULL
> );
>
> gBS->UninstallMultipleProtocolInterfaces (
> Controller,
> &gEfiCallerIdGuid,
> AtaBusDriverData,
> NULL
> );
>
> So I am happy with a single NULL here.
>
> >
> > -Takahiro Akashi
> >
> > > if (ret != EFI_SUCCESS)
> > > goto out;
> > > msg_path = file_path;
> > > @@ -542,7 +539,9 @@ efi_status_t efi_run_image(void *source_buffer, efi_uintn_t source_size)
> > > ret = do_bootefi_exec(handle, load_options);
> > >
> > > out:
> > > - efi_delete_handle(mem_handle);
> > > + efi_uninstall_multiple_protocol_interfaces(mem_handle,
> > > + &efi_guid_device_path,
> > > + file_path, NULL);
>
> We have installed a lot more protocols. The binary may have installed
> additional protocols. Consider the case of a boottime driver. To delete
> the handle we would have to remove all installed protocols.
>
> UninstallMultipleProtocolInterfaces() may fail if one of the protocols
> was opened with ByDriver or ByChildcontroller. The return value has to
> be considered before freeing file_path.
Ok I'll fix it in v2
>
> Best regards
>
> Heinrich
>
> > > efi_free_pool(file_path);
> > > return ret;
> > > }
> > > --
> > > 2.34.1
> > >
>
Thanks
/Ilias
More information about the U-Boot
mailing list