[RFC PATCH 1/2] efi_loader: define internal implementations of install/uninstallmultiple
AKASHI Takahiro
takahiro.akashi at linaro.org
Thu Oct 6 03:49:09 CEST 2022
Hi Ilias,
On Wed, Oct 05, 2022 at 06:26:01PM +0300, Ilias Apalodimas wrote:
> A following patch is cleaning up the core EFI code trying to remove
> sequences of efi_create_handle, efi_add_protocol.
>
> Although this works fine there's a problem with the latter since it is
> usually combined with efi_delete_handle() which blindly removes all
> protocols on a handle and deletes the handle. We should try to adhere to
> the EFI spec which only deletes a handle if the last instance of a protocol
> has been removed. So let's fix this by replacing all callsites of
> efi_create_handle(), efi_add_protocol() , efi_delete_handle() with
> Install/UninstallMultipleProtocol.
>
> In order to do that redefine functions that can be used by the U-Boot
> proper internally and add '_ext' variants that will be used from the
> EFI API
Our practice so far is to use '_int' postfix for internal interfaces
and not use any frill for external interfaces.
-Takahiro Akashi
> Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> ---
> include/efi.h | 2 +
> include/efi_loader.h | 6 +-
> lib/efi_loader/efi_boottime.c | 180 ++++++++++++++++++++++++-------
> lib/efi_loader/efi_capsule.c | 15 +--
> lib/efi_loader/efi_console.c | 14 +--
> lib/efi_loader/efi_disk.c | 10 +-
> lib/efi_loader/efi_load_initrd.c | 15 ++-
> lib/efi_loader/efi_root_node.c | 44 ++++----
> 8 files changed, 197 insertions(+), 89 deletions(-)
>
> diff --git a/include/efi.h b/include/efi.h
> index 6159f34ad2be..42f4e58a917e 100644
> --- a/include/efi.h
> +++ b/include/efi.h
> @@ -37,12 +37,14 @@
> #define EFIAPI __attribute__((ms_abi))
> #define efi_va_list __builtin_ms_va_list
> #define efi_va_start __builtin_ms_va_start
> +#define efi_va_copy __builtin_ms_va_copy
> #define efi_va_arg __builtin_va_arg
> #define efi_va_end __builtin_ms_va_end
> #else
> #define EFIAPI asmlinkage
> #define efi_va_list va_list
> #define efi_va_start va_start
> +#define efi_va_copy va_copy
> #define efi_va_arg va_arg
> #define efi_va_end va_end
> #endif /* __x86_64__ */
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index ad01395b39c3..2b294d64efd0 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -655,8 +655,10 @@ efi_status_t efi_remove_protocol(const efi_handle_t handle,
> /* Delete all protocols from a handle */
> efi_status_t efi_remove_all_protocols(const efi_handle_t handle);
> /* Install multiple protocol interfaces */
> -efi_status_t EFIAPI efi_install_multiple_protocol_interfaces
> - (efi_handle_t *handle, ...);
> +efi_status_t EFIAPI
> +efi_install_multiple_protocol_interfaces(efi_handle_t *handle, ...);
> +efi_status_t EFIAPI
> +efi_uninstall_multiple_protocol_interfaces(efi_handle_t handle, ...);
> /* Get handles that support a given protocol */
> efi_status_t EFIAPI efi_locate_handle_buffer(
> enum efi_locate_search_type search_type,
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index 1bfd094e89f8..aeb8b27dc676 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -2590,35 +2590,31 @@ found:
> }
>
> /**
> - * efi_install_multiple_protocol_interfaces() - Install multiple protocol
> + * __efi_install_multiple_protocol_interfaces() - Install multiple protocol
> * interfaces
> * @handle: handle on which the protocol interfaces shall be installed
> - * @...: NULL terminated argument list with pairs of protocol GUIDS and
> - * interfaces
> - *
> - * This function implements the MultipleProtocolInterfaces service.
> + * @argptr: va_list of args
> *
> - * See the Unified Extensible Firmware Interface (UEFI) specification for
> - * details.
> + * Core functionality of efi_install_multiple_protocol_interfaces
> + * Must not be called directly
> *
> * Return: status code
> */
> -efi_status_t EFIAPI efi_install_multiple_protocol_interfaces
> - (efi_handle_t *handle, ...)
> +static efi_status_t EFIAPI
> +__efi_install_multiple_protocol_interfaces(efi_handle_t *handle,
> + efi_va_list argptr)
> {
> - EFI_ENTRY("%p", handle);
> -
> - efi_va_list argptr;
> const efi_guid_t *protocol;
> void *protocol_interface;
> efi_handle_t old_handle;
> efi_status_t r = EFI_SUCCESS;
> int i = 0;
> + efi_va_list argptr_copy;
>
> if (!handle)
> - return EFI_EXIT(EFI_INVALID_PARAMETER);
> + return EFI_INVALID_PARAMETER;
>
> - efi_va_start(argptr, handle);
> + efi_va_copy(argptr_copy, argptr);
> for (;;) {
> protocol = efi_va_arg(argptr, efi_guid_t*);
> if (!protocol)
> @@ -2646,52 +2642,103 @@ efi_status_t EFIAPI efi_install_multiple_protocol_interfaces
> break;
> i++;
> }
> - efi_va_end(argptr);
> - if (r == EFI_SUCCESS)
> - return EFI_EXIT(r);
> + if (r == EFI_SUCCESS) {
> + efi_va_end(argptr_copy);
> + return r;
> + }
>
> /* If an error occurred undo all changes. */
> - efi_va_start(argptr, handle);
> for (; i; --i) {
> - protocol = efi_va_arg(argptr, efi_guid_t*);
> - protocol_interface = efi_va_arg(argptr, void*);
> + protocol = efi_va_arg(argptr_copy, efi_guid_t*);
> + protocol_interface = efi_va_arg(argptr_copy, void*);
> EFI_CALL(efi_uninstall_protocol_interface(*handle, protocol,
> protocol_interface));
> }
> - efi_va_end(argptr);
> + efi_va_end(argptr_copy);
> +
> + return r;
>
> - return EFI_EXIT(r);
> }
>
> /**
> - * efi_uninstall_multiple_protocol_interfaces() - uninstall multiple protocol
> - * interfaces
> - * @handle: handle from which the protocol interfaces shall be removed
> + * efi_install_multiple_protocol_interfaces() - Install multiple protocol
> + * interfaces
> + * @handle: handle on which the protocol interfaces shall be installed
> * @...: NULL terminated argument list with pairs of protocol GUIDS and
> * interfaces
> *
> - * This function implements the UninstallMultipleProtocolInterfaces service.
> + *
> + * This is the function for internal usage in U-Boot. For the API function
> + * implementing the InstallMultipleProtocol service see
> + * efi_install_multiple_protocol_interfaces_ext()
> + *
> + * Return: status code
> + */
> +efi_status_t EFIAPI
> +efi_install_multiple_protocol_interfaces(efi_handle_t *handle, ...)
> +{
> + efi_status_t r = EFI_SUCCESS;
> + efi_va_list argptr;
> +
> + efi_va_start(argptr, handle);
> + r = __efi_install_multiple_protocol_interfaces(handle, argptr);
> + efi_va_end(argptr);
> + return r;
> +}
> +
> +/**
> + * efi_install_multiple_protocol_interfaces_ext() - Install multiple protocol
> + * interfaces
> + * @handle: handle on which the protocol interfaces shall be installed
> + * @...: NULL terminated argument list with pairs of protocol GUIDS and
> + * interfaces
> + *
> + * This function implements the MultipleProtocolInterfaces service.
> *
> * See the Unified Extensible Firmware Interface (UEFI) specification for
> * details.
> *
> * Return: status code
> */
> -static efi_status_t EFIAPI efi_uninstall_multiple_protocol_interfaces(
> - efi_handle_t handle, ...)
> +static efi_status_t EFIAPI
> +efi_install_multiple_protocol_interfaces_ext(efi_handle_t *handle, ...)
> {
> EFI_ENTRY("%p", handle);
> -
> + efi_status_t r = EFI_SUCCESS;
> efi_va_list argptr;
> +
> + efi_va_start(argptr, handle);
> + r = __efi_install_multiple_protocol_interfaces(handle, argptr);
> + efi_va_end(argptr);
> + return EFI_EXIT(r);
> +}
> +
> +/**
> + * __efi_uninstall_multiple_protocol_interfaces() - wrapper for uninstall
> + * multiple protocol
> + * interfaces
> + * @handle: handle from which the protocol interfaces shall be removed
> + * @argptr: va_list of args
> + *
> + * Core functionality of efi_uninstall_multiple_protocol_interfaces
> + * Must not be called directly
> + *
> + * Return: status code
> + */
> +static efi_status_t EFIAPI
> +__efi_uninstall_multiple_protocol_interfaces(efi_handle_t handle,
> + efi_va_list argptr)
> +{
> const efi_guid_t *protocol;
> void *protocol_interface;
> efi_status_t r = EFI_SUCCESS;
> size_t i = 0;
> + efi_va_list argptr_copy;
>
> if (!handle)
> - return EFI_EXIT(EFI_INVALID_PARAMETER);
> + return EFI_INVALID_PARAMETER;
>
> - efi_va_start(argptr, handle);
> + efi_va_copy(argptr_copy, argptr);
> for (;;) {
> protocol = efi_va_arg(argptr, efi_guid_t*);
> if (!protocol)
> @@ -2703,29 +2750,82 @@ static efi_status_t EFIAPI efi_uninstall_multiple_protocol_interfaces(
> break;
> i++;
> }
> - efi_va_end(argptr);
> if (r == EFI_SUCCESS) {
> /* If the last protocol has been removed, delete the handle. */
> if (list_empty(&handle->protocols)) {
> list_del(&handle->link);
> free(handle);
> }
> - return EFI_EXIT(r);
> + efi_va_end(argptr_copy);
> + return r;
> }
>
> /* If an error occurred undo all changes. */
> - efi_va_start(argptr, handle);
> for (; i; --i) {
> - protocol = efi_va_arg(argptr, efi_guid_t*);
> - protocol_interface = efi_va_arg(argptr, void*);
> + protocol = efi_va_arg(argptr_copy, efi_guid_t*);
> + protocol_interface = efi_va_arg(argptr_copy, void*);
> EFI_CALL(efi_install_protocol_interface(&handle, protocol,
> EFI_NATIVE_INTERFACE,
> protocol_interface));
> }
> - efi_va_end(argptr);
> + efi_va_end(argptr_copy);
>
> /* In case of an error always return EFI_INVALID_PARAMETER */
> - return EFI_EXIT(EFI_INVALID_PARAMETER);
> + return EFI_INVALID_PARAMETER;
> +}
> +
> +/**
> + * efi_uninstall_multiple_protocol_interfaces() - uninstall multiple protocol
> + * interfaces
> + * @handle: handle from which the protocol interfaces shall be removed
> + * @...: NULL terminated argument list with pairs of protocol GUIDS and
> + * interfaces
> + *
> + * This function implements the UninstallMultipleProtocolInterfaces service.
> + *
> + * This is the function for internal usage in U-Boot. For the API function
> + * implementing the UninstallMultipleProtocolInterfaces service see
> + * efi_uninstall_multiple_protocol_interfaces_ext()
> + *
> + * Return: status code
> + */
> +efi_status_t EFIAPI
> +efi_uninstall_multiple_protocol_interfaces(efi_handle_t handle, ...)
> +{
> + efi_status_t r = EFI_SUCCESS;
> + efi_va_list argptr;
> +
> + efi_va_start(argptr, handle);
> + r = __efi_uninstall_multiple_protocol_interfaces(handle, argptr);
> + efi_va_end(argptr);
> + return r;
> +}
> +
> +/**
> + * efi_uninstall_multiple_protocol_interfaces_ext() - uninstall multiple protocol
> + * interfaces
> + * @handle: handle from which the protocol interfaces shall be removed
> + * @...: NULL terminated argument list with pairs of protocol GUIDS and
> + * interfaces
> + *
> + * This function implements the UninstallMultipleProtocolInterfaces service.
> + *
> + * See the Unified Extensible Firmware Interface (UEFI) specification for
> + * details.
> + *
> + * Return: status code
> + */
> +static efi_status_t EFIAPI
> +efi_uninstall_multiple_protocol_interfaces_ext(efi_handle_t handle, ...)
> +{
> + EFI_ENTRY("%p", handle);
> + efi_status_t r = EFI_SUCCESS;
> + efi_va_list argptr;
> +
> + efi_va_start(argptr, handle);
> + r = __efi_uninstall_multiple_protocol_interfaces(handle, argptr);
> + efi_va_end(argptr);
> + return EFI_EXIT(r);
> }
>
> /**
> @@ -3785,9 +3885,9 @@ static struct efi_boot_services efi_boot_services = {
> .locate_handle_buffer = efi_locate_handle_buffer,
> .locate_protocol = efi_locate_protocol,
> .install_multiple_protocol_interfaces =
> - efi_install_multiple_protocol_interfaces,
> + efi_install_multiple_protocol_interfaces_ext,
> .uninstall_multiple_protocol_interfaces =
> - efi_uninstall_multiple_protocol_interfaces,
> + efi_uninstall_multiple_protocol_interfaces_ext,
> .calculate_crc32 = efi_calculate_crc32,
> .copy_mem = efi_copy_mem,
> .set_mem = efi_set_mem,
> diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> index a6b98f066a0b..b6bd2d6af882 100644
> --- a/lib/efi_loader/efi_capsule.c
> +++ b/lib/efi_loader/efi_capsule.c
> @@ -636,17 +636,18 @@ efi_status_t __weak efi_load_capsule_drivers(void)
>
> if (IS_ENABLED(CONFIG_EFI_CAPSULE_FIRMWARE_FIT)) {
> handle = NULL;
> - ret = EFI_CALL(efi_install_multiple_protocol_interfaces(
> - &handle, &efi_guid_firmware_management_protocol,
> - &efi_fmp_fit, NULL));
> + ret = efi_install_multiple_protocol_interfaces(&handle,
> + &efi_guid_firmware_management_protocol,
> + &efi_fmp_fit,
> + NULL);
> }
>
> if (IS_ENABLED(CONFIG_EFI_CAPSULE_FIRMWARE_RAW)) {
> handle = NULL;
> - ret = EFI_CALL(efi_install_multiple_protocol_interfaces(
> - &handle,
> - &efi_guid_firmware_management_protocol,
> - &efi_fmp_raw, NULL));
> + ret = efi_install_multiple_protocol_interfaces(&handle,
> + &efi_guid_firmware_management_protocol,
> + &efi_fmp_raw,
> + NULL);
> }
>
> return ret;
> diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
> index cf9fbd9cb54d..3354b217a9a4 100644
> --- a/lib/efi_loader/efi_console.c
> +++ b/lib/efi_loader/efi_console.c
> @@ -1278,12 +1278,14 @@ efi_status_t efi_console_register(void)
> struct efi_device_path *dp;
>
> /* Install protocols on root node */
> - r = EFI_CALL(efi_install_multiple_protocol_interfaces
> - (&efi_root,
> - &efi_guid_text_output_protocol, &efi_con_out,
> - &efi_guid_text_input_protocol, &efi_con_in,
> - &efi_guid_text_input_ex_protocol, &efi_con_in_ex,
> - NULL));
> + r = efi_install_multiple_protocol_interfaces(&efi_root,
> + &efi_guid_text_output_protocol,
> + &efi_con_out,
> + &efi_guid_text_input_protocol,
> + &efi_con_in,
> + &efi_guid_text_input_ex_protocol,
> + &efi_con_in_ex,
> + NULL);
>
> /* Create console node and install device path protocols */
> if (CONFIG_IS_ENABLED(DM_SERIAL)) {
> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> index 39ea1a68a683..fb96b0508528 100644
> --- a/lib/efi_loader/efi_disk.c
> +++ b/lib/efi_loader/efi_disk.c
> @@ -449,10 +449,12 @@ static efi_status_t efi_disk_add_dev(
> * in this case.
> */
> handle = &diskobj->header;
> - ret = EFI_CALL(efi_install_multiple_protocol_interfaces(
> - &handle, &efi_guid_device_path, diskobj->dp,
> - &efi_block_io_guid, &diskobj->ops,
> - guid, NULL, NULL));
> + ret = efi_install_multiple_protocol_interfaces(&handle,
> + &efi_guid_device_path,
> + diskobj->dp,
> + &efi_block_io_guid,
> + &diskobj->ops, guid,
> + NULL, NULL);
> if (ret != EFI_SUCCESS)
> goto error;
>
> diff --git a/lib/efi_loader/efi_load_initrd.c b/lib/efi_loader/efi_load_initrd.c
> index 3d6044f76047..87fde3f88c2b 100644
> --- a/lib/efi_loader/efi_load_initrd.c
> +++ b/lib/efi_loader/efi_load_initrd.c
> @@ -208,14 +208,13 @@ efi_status_t efi_initrd_register(void)
> if (ret != EFI_SUCCESS)
> return ret;
>
> - ret = EFI_CALL(efi_install_multiple_protocol_interfaces
> - (&efi_initrd_handle,
> - /* initramfs */
> - &efi_guid_device_path, &dp_lf2_handle,
> - /* LOAD_FILE2 */
> - &efi_guid_load_file2_protocol,
> - (void *)&efi_lf2_protocol,
> - NULL));
> + ret = efi_install_multiple_protocol_interfaces(&efi_initrd_handle,
> + /* initramfs */
> + &efi_guid_device_path, &dp_lf2_handle,
> + /* LOAD_FILE2 */
> + &efi_guid_load_file2_protocol,
> + (void *)&efi_lf2_protocol,
> + NULL);
>
> return ret;
> }
> diff --git a/lib/efi_loader/efi_root_node.c b/lib/efi_loader/efi_root_node.c
> index 739c6867f412..b4696d54c33d 100644
> --- a/lib/efi_loader/efi_root_node.c
> +++ b/lib/efi_loader/efi_root_node.c
> @@ -49,38 +49,38 @@ efi_status_t efi_root_node_register(void)
> dp->end.length = sizeof(struct efi_device_path);
>
> /* Create root node and install protocols */
> - ret = EFI_CALL(efi_install_multiple_protocol_interfaces
> - (&efi_root,
> - /* Device path protocol */
> - &efi_guid_device_path, dp,
> + ret = efi_install_multiple_protocol_interfaces
> + (&efi_root,
> + /* Device path protocol */
> + &efi_guid_device_path, dp,
> #if CONFIG_IS_ENABLED(EFI_DEVICE_PATH_TO_TEXT)
> - /* Device path to text protocol */
> - &efi_guid_device_path_to_text_protocol,
> - (void *)&efi_device_path_to_text,
> + /* Device path to text protocol */
> + &efi_guid_device_path_to_text_protocol,
> + (void *)&efi_device_path_to_text,
> #endif
> #ifdef CONFIG_EFI_DEVICE_PATH_UTIL
> - /* Device path utilities protocol */
> - &efi_guid_device_path_utilities_protocol,
> - (void *)&efi_device_path_utilities,
> + /* Device path utilities protocol */
> + &efi_guid_device_path_utilities_protocol,
> + (void *)&efi_device_path_utilities,
> #endif
> #ifdef CONFIG_EFI_DT_FIXUP
> - /* Device-tree fix-up protocol */
> - &efi_guid_dt_fixup_protocol,
> - (void *)&efi_dt_fixup_prot,
> + /* Device-tree fix-up protocol */
> + &efi_guid_dt_fixup_protocol,
> + (void *)&efi_dt_fixup_prot,
> #endif
> #if CONFIG_IS_ENABLED(EFI_UNICODE_COLLATION_PROTOCOL2)
> - &efi_guid_unicode_collation_protocol2,
> - (void *)&efi_unicode_collation_protocol2,
> + &efi_guid_unicode_collation_protocol2,
> + (void *)&efi_unicode_collation_protocol2,
> #endif
> #if CONFIG_IS_ENABLED(EFI_LOADER_HII)
> - /* HII string protocol */
> - &efi_guid_hii_string_protocol,
> - (void *)&efi_hii_string,
> - /* HII database protocol */
> - &efi_guid_hii_database_protocol,
> - (void *)&efi_hii_database,
> + /* HII string protocol */
> + &efi_guid_hii_string_protocol,
> + (void *)&efi_hii_string,
> + /* HII database protocol */
> + &efi_guid_hii_database_protocol,
> + (void *)&efi_hii_database,
> #endif
> - NULL));
> + NULL);
> efi_root->type = EFI_OBJECT_TYPE_U_BOOT_FIRMWARE;
> return ret;
> }
> --
> 2.34.1
>
More information about the U-Boot
mailing list