Re: [PATCH 06/35] x86: Create a 32/64-bit selection for the app

From: Heinrich Schuchardt <xypron.glpk@gmx.de>
To: Simon Glass <sjg@chromium.org>,
	U-Boot Mailing List <u-boot@lists.denx.de>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>,
	Bin Meng <bmeng.cn@gmail.com>, Tom Rini <trini@konsulko.com>,
	Christian Melki <christian.melki@t2data.com>
Subject: Re: [PATCH 06/35] x86: Create a 32/64-bit selection for the app
Date: Wed, 8 Sep 2021 19:35:22 +0200	[thread overview]
Message-ID: <e1b6ea28-8743-1cf7-230d-c465c1b37392@gmx.de> (raw)
In-Reply-To: <20210908073355.6.Id32ab4eff4b2a6b82fed22813a01fc2a08c6e0a5@changeid>

On 9/8/21 3:33 PM, Simon Glass wrote:
> Most EFI implementations use 64-bit. In order to spupose a 64-bit app,

%s/spupose/support

> update the Kconfig to add an option for 32/64 bit. Update the prompt for

This commit message is unclear. We already can compile U-Boot as an
64bit EFI application. "In order to support" would imply that we don't.

> the existing option so it is clear it relates to the stub. Move both
> up to just under the choice that controls them, since this looks better
> and the menu.
>
> Use CONFIG_EFI_APP in the Makefile instead of CONFIG_TARGET_EFI_APP,
> since the latter is specific to a single target and we will have two.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>   arch/x86/cpu/intel_common/Makefile |  2 +-
>   lib/efi/Kconfig                    | 34 +++++++++++++++++++++++-------
>   2 files changed, 27 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/cpu/intel_common/Makefile b/arch/x86/cpu/intel_common/Makefile
> index 8b9a810f66d..1dc17b45879 100644
> --- a/arch/x86/cpu/intel_common/Makefile
> +++ b/arch/x86/cpu/intel_common/Makefile
> @@ -27,7 +27,7 @@ obj-y += fast_spi.o
>   obj-y += lpc.o
>   obj-y += lpss.o
>   obj-$(CONFIG_$(SPL_)INTEL_GENERIC_WIFI) += generic_wifi.o
> -ifndef CONFIG_TARGET_EFI_APP
> +ifndef CONFIG_EFI_APP
>   obj-$(CONFIG_$(SPL_TPL_)X86_32BIT_INIT) += microcode.o
>   ifndef CONFIG_$(SPL_)X86_64
>   obj-y += microcode.o
> diff --git a/lib/efi/Kconfig b/lib/efi/Kconfig
> index 93b85644920..e818cef0d15 100644
> --- a/lib/efi/Kconfig
> +++ b/lib/efi/Kconfig
> @@ -26,18 +26,26 @@ config EFI_STUB
>
>   endchoice
>
> -config EFI_RAM_SIZE
> -	hex "Amount of EFI RAM for U-Boot"
> +choice
> +	prompt "EFI app 32/64-bit selection"
>   	depends on EFI_APP
> -	default 0x2000000
>   	help
> -	  Set the amount of EFI RAM which is claimed by U-Boot for its own
> -	  use. U-Boot allocates this from EFI on start-up (along with a few
> -	  other smaller amounts) and it can never be increased after that.
> -	  It is used as the RAM size in with U-Boot.
> +	  EFI does not support mixing 32-bit and 64-bit modes. This is a
> +	  significant problem because it means that you must build a stub with
> +	  the correct type for EFI to load it correctly. If you are using
> +	  32-bit EFI, select 32-bit here, else select 64-bit. Failure to do
> +	  this may produce no error message - it just won't start!
> +
> +config EFI_APP_32BIT
> +	bool "Produce an app for running with 32-bit EFI"
> +
> +config EFI_APP_64BIT
> +	bool "Produce an app for running with 64-bit EFI"
> +
> +endchoice
>
>   choice
> -	prompt "EFI 32/64-bit selection"
> +	prompt "EFI stub 32/64-bit selection"
>   	depends on EFI_STUB
>   	help
>   	  EFI does not support mixing 32-bit and 64-bit modes. This is a
> @@ -53,3 +61,13 @@ config EFI_STUB_64BIT
>   	bool "Produce a stub for running with 64-bit EFI"
>
>   endchoice
> +
> +config EFI_RAM_SIZE
> +	hex "Amount of EFI RAM for U-Boot"
> +	depends on EFI_APP
> +	default 0x2000000

32 MiB is quite small for loading a kernel and an initrd.

Best regards

Heinrich

> +	help
> +	  Set the amount of EFI RAM which is claimed by U-Boot for its own
> +	  use. U-Boot allocates this from EFI on start-up (along with a few
> +	  other smaller amounts) and it can never be increased after that.
> +	  It is used as the RAM size in with U-Boot.
>