[U-Boot] [PATCH] efi_loader: re-organize UEFI-related configuration menu

[Heinrich Schuchardt]

On 6/4/19 5:11 AM, AKASHI Takahiro wrote:
> More and more features are coming into UEFI support, while some people
> are worried about the growing size of the code. Adding a configuration
> option for each feature is a solution, but it will also make
> the configuration complicated and unreadable.
>
> With this patch, this issue will be addressed by
> 1.putting UEFI-related menu to a sub-menu, "UEFI API support"
> in addition,
> 2.moving EFI_LOADER option, along with USE_BOUND_BUFFER, to "Boot Images"
> 3.moving forward *non-functional* configurations, currently
>    PLATFORM_LANG_CODES only, at the beginning
>
> (1) helps improve readability by separating UEFI-specific configs from
> other U-Boot features. For (2), "Boot Images" is a right place for
> EFI_LOADER with architecture-dependent configs.
> Lastly, coming functional features (runtime services, protocols and
> others) will be no doubt added to the last.

>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> ---
>   Kconfig                | 27 ++++++++++++++++++++++++
>   lib/efi_loader/Kconfig | 47 +++++++++++-------------------------------
>   2 files changed, 39 insertions(+), 35 deletions(-)
>
> diff --git a/Kconfig b/Kconfig
> index a02168690f5b..2bb3db6262fe 100644

Most of the stuff under "Boot images" has nothing to do with boot
images. I suggest that whole menu to be dissolved (but not with this patch).

> --- a/Kconfig
> +++ b/Kconfig
> @@ -526,6 +526,33 @@ config ARCH_FIXUP_FDT_MEMORY
>   	  used for booting OS with different memory setup where the part of
>   	  the memory location should be used for different purpose.
>
> +config EFI_LOADER

I find it irritating to have to look into two different places for UEFI
support.

Couldn't we make EFI_LOADER a menuconfig and put all UEFI stuff beneath it?

> +	bool "Support UEFI images"
> +	depends on (ARM || X86 || RISCV || SANDBOX) && OF_LIBFDT
> +	# We need EFI_STUB_64BIT to be set on x86_64 with EFI_STUB
> +	depends on !EFI_STUB || !X86_64 || EFI_STUB_64BIT
> +	# We need EFI_STUB_32BIT to be set on x86_32 with EFI_STUB
> +	depends on !EFI_STUB || !X86 || X86_64 || EFI_STUB_32BIT
> +	default y
> +	select LIB_UUID
> +	select HAVE_BLOCK_DEVICE
> +	select REGEX
> +	imply CFB_CONSOLE_ANSI
> +	help
> +	  Select this option if you want to run UEFI applications (like GNU
> +	  GRUB or iPXE) on top of U-Boot. If this option is enabled, U-Boot
> +	  will expose the UEFI API to a loaded application, enabling it to
> +	  reuse U-Boot's device drivers.
> +
> +config EFI_LOADER_BOUNCE_BUFFER

This is not a boot image property.

> +	bool "EFI Applications use bounce buffers for DMA operations"
> +	depends on EFI_LOADER && ARM64
> +	default n
> +	help
> +	  Some hardware does not support DMA to full 64bit addresses. For this
> +	  hardware we can create a bounce buffer so that payloads don't have to
> +	  worry about platform details.
> +
>   endmenu		# Boot images
>
>   source "api/Kconfig"
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index cd5436c576b1..8b9e85e9a534 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -1,23 +1,16 @@
> -config EFI_LOADER
> -	bool "Support running UEFI applications"
> -	depends on (ARM || X86 || RISCV || SANDBOX) && OF_LIBFDT
> -	# We need EFI_STUB_64BIT to be set on x86_64 with EFI_STUB
> -	depends on !EFI_STUB || !X86_64 || EFI_STUB_64BIT
> -	# We need EFI_STUB_32BIT to be set on x86_32 with EFI_STUB
> -	depends on !EFI_STUB || !X86 || X86_64 || EFI_STUB_32BIT
> -	default y
> -	select LIB_UUID
> -	select HAVE_BLOCK_DEVICE
> -	select REGEX
> -	imply CFB_CONSOLE_ANSI
> -	help
> -	  Select this option if you want to run UEFI applications (like GNU
> -	  GRUB or iPXE) on top of U-Boot. If this option is enabled, U-Boot
> -	  will expose the UEFI API to a loaded application, enabling it to
> -	  reuse U-Boot's device drivers.
> -
>   if EFI_LOADER
>
> +menu "UEFI API support"

I would prefer to keep EFI_LOADER here but as menuconfig. EFI_LOADER is
simply exposing an API. So putting it under "Library routines" seems to
be a valid choice.

> +
> +config EFI_PLATFORM_LANG_CODES

Protocols are more relevant than this nitty gritty. So why not keep it
at the end?

So the change needed reduces to:

-config EFI_LOADER
+menuconfig EFI_LOADER

Best regards

Heinrich


> +	string "Language codes supported by firmware"
> +	default "en-US"
> +	help
> +	  This value is used to initialize the PlatformLangCodes variable. Its
> +	  value is a semicolon (;) separated list of language codes in native
> +	  RFC 4646 format, e.g. "en-US;de-DE". The first language code is used
> +	  to initialize the PlatformLang variable.
> +
>   config EFI_GET_TIME
>   	bool "GetTime() runtime service"
>   	depends on DM_RTC
> @@ -83,22 +76,6 @@ config EFI_UNICODE_COLLATION_PROTOCOL
>
>   endif
>
> -config EFI_LOADER_BOUNCE_BUFFER
> -	bool "EFI Applications use bounce buffers for DMA operations"
> -	depends on ARM64
> -	default n
> -	help
> -	  Some hardware does not support DMA to full 64bit addresses. For this
> -	  hardware we can create a bounce buffer so that payloads don't have to
> -	  worry about platform details.
> -
> -config EFI_PLATFORM_LANG_CODES
> -	string "Language codes supported by firmware"
> -	default "en-US"
> -	help
> -	  This value is used to initialize the PlatformLangCodes variable. Its
> -	  value is a semicolon (;) separated list of language codes in native
> -	  RFC 4646 format, e.g. "en-US;de-DE". The first language code is used
> -	  to initialize the PlatformLang variable.
> +endmenu
>
>   endif
>