[U-Boot] [PATCH 0/6] x86: efi: payload: Various fixes

This series includes 3 patches that were previously sent to ML but not applied, along with some new patches with various fixes on efi x86 payload support.
For [01: efi.h: Do not use config options] and [02: x86: Change __kernel_size_t conditionals to use compiler provided defines], the changes are mainly adding some comments to describe the changes in more details.
This series is available at u-boot-x86/efi2-working for testing.
Alexander Graf (1): efi.h: Do not use config options
Bin Meng (5): x86: Change __kernel_size_t conditionals to use compiler provided defines efi: stub: Move the use_uart assignment immediately after exit_boot_services() call x86: efi-x86_payload: Enable PRE_CONSOLE_BUFFER x86: efi: payload: Count in conventional memory above 4GB in DRAM bank cmd: efi: Fix wrong memory descriptor end address
arch/x86/cpu/efi/payload.c | 3 +-- arch/x86/include/asm/posix_types.h | 3 ++- cmd/efi.c | 2 +- configs/efi-x86_payload32_defconfig | 2 ++ configs/efi-x86_payload64_defconfig | 2 ++ include/efi.h | 24 +++++++++++------------- lib/efi/Makefile | 4 ++-- lib/efi/efi_stub.c | 6 +++--- 8 files changed, 24 insertions(+), 22 deletions(-)

From: Alexander Graf agraf@suse.de
Currently efi.h determines a few bits of its environment according to config options. This falls apart with the efi stub support which may result in efi.h getting pulled into the stub as well as real U-Boot code. In that case, one may be 32bit while the other one is 64bit.
This patch changes the conditionals to use compiler provided defines instead. That way we always adhere to the build environment we're in and the definitions adjust automatically.
Signed-off-by: Alexander Graf agraf@suse.de Reviewed-by: Bin Meng bmeng.cn@gmail.com Tested-by: Bin Meng bmeng.cn@gmail.com [bmeng: added some comments to describe the __x86_64__ check] Signed-off-by: Bin Meng bmeng.cn@gmail.com
---
include/efi.h | 24 +++++++++++------------- lib/efi/Makefile | 4 ++-- 2 files changed, 13 insertions(+), 15 deletions(-)
diff --git a/include/efi.h b/include/efi.h index 2448dde..0fe15e6 100644 --- a/include/efi.h +++ b/include/efi.h @@ -19,12 +19,19 @@ #include <linux/string.h> #include <linux/types.h>
-#if CONFIG_EFI_STUB_64BIT || (!defined(CONFIG_EFI_STUB) && defined(__x86_64__)) -/* EFI uses the Microsoft ABI which is not the default for GCC */ +/* + * EFI on x86_64 uses the Microsoft ABI which is not the default for GCC. + * + * There are two scenarios for EFI on x86_64: building a 64-bit EFI stub + * codes (CONFIG_EFI_STUB_64BIT) and building a 64-bit U-Boot (CONFIG_X86_64). + * Either needs to be properly built with the '-m64' compiler flag, and hence + * it is enough to only check the compiler provided define __x86_64__ here. + */ +#ifdef __x86_64__ #define EFIAPI __attribute__((ms_abi)) #else #define EFIAPI asmlinkage -#endif +#endif /* __x86_64__ */
struct efi_device_path;
@@ -32,16 +39,7 @@ typedef struct { u8 b[16]; } efi_guid_t;
-#define EFI_BITS_PER_LONG BITS_PER_LONG - -/* - * With 64-bit EFI stub, EFI_BITS_PER_LONG has to be 64. EFI_STUB is set - * in lib/efi/Makefile, when building the stub. - */ -#if defined(CONFIG_EFI_STUB_64BIT) && defined(EFI_STUB) -#undef EFI_BITS_PER_LONG -#define EFI_BITS_PER_LONG 64 -#endif +#define EFI_BITS_PER_LONG (sizeof(long) * 8)
/* Bit mask for EFI status code with error */ #define EFI_ERROR_MASK (1UL << (EFI_BITS_PER_LONG - 1)) diff --git a/lib/efi/Makefile b/lib/efi/Makefile index f1a3929..a790d2d 100644 --- a/lib/efi/Makefile +++ b/lib/efi/Makefile @@ -7,11 +7,11 @@ obj-$(CONFIG_EFI_STUB) += efi_info.o
CFLAGS_REMOVE_efi_stub.o := -mregparm=3 \ $(if $(CONFIG_EFI_STUB_64BIT),-march=i386 -m32) -CFLAGS_efi_stub.o := -fpic -fshort-wchar -DEFI_STUB \ +CFLAGS_efi_stub.o := -fpic -fshort-wchar \ $(if $(CONFIG_EFI_STUB_64BIT),-m64) CFLAGS_REMOVE_efi.o := -mregparm=3 \ $(if $(CONFIG_EFI_STUB_64BIT),-march=i386 -m32) -CFLAGS_efi.o := -fpic -fshort-wchar -DEFI_STUB \ +CFLAGS_efi.o := -fpic -fshort-wchar \ $(if $(CONFIG_EFI_STUB_64BIT),-m64)
extra-$(CONFIG_EFI_STUB) += efi_stub.o efi.o

On 22 June 2018 at 02:38, Bin Meng bmeng.cn@gmail.com wrote:
From: Alexander Graf agraf@suse.de
Currently efi.h determines a few bits of its environment according to config options. This falls apart with the efi stub support which may result in efi.h getting pulled into the stub as well as real U-Boot code. In that case, one may be 32bit while the other one is 64bit.
This patch changes the conditionals to use compiler provided defines instead. That way we always adhere to the build environment we're in and the definitions adjust automatically.
Signed-off-by: Alexander Graf agraf@suse.de Reviewed-by: Bin Meng bmeng.cn@gmail.com Tested-by: Bin Meng bmeng.cn@gmail.com [bmeng: added some comments to describe the __x86_64__ check] Signed-off-by: Bin Meng bmeng.cn@gmail.com
include/efi.h | 24 +++++++++++------------- lib/efi/Makefile | 4 ++-- 2 files changed, 13 insertions(+), 15 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

On Sat, Jun 23, 2018 at 10:21 PM, Simon Glass sjg@chromium.org wrote:
On 22 June 2018 at 02:38, Bin Meng bmeng.cn@gmail.com wrote:
From: Alexander Graf agraf@suse.de
Currently efi.h determines a few bits of its environment according to config options. This falls apart with the efi stub support which may result in efi.h getting pulled into the stub as well as real U-Boot code. In that case, one may be 32bit while the other one is 64bit.
This patch changes the conditionals to use compiler provided defines instead. That way we always adhere to the build environment we're in and the definitions adjust automatically.
Signed-off-by: Alexander Graf agraf@suse.de Reviewed-by: Bin Meng bmeng.cn@gmail.com Tested-by: Bin Meng bmeng.cn@gmail.com [bmeng: added some comments to describe the __x86_64__ check] Signed-off-by: Bin Meng bmeng.cn@gmail.com
include/efi.h | 24 +++++++++++------------- lib/efi/Makefile | 4 ++-- 2 files changed, 13 insertions(+), 15 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
applied to u-boot-x86, thanks!

Since commit bb0bb91cf0aa ("efi_stub: Use efi_uintn_t"), EFI x86 64-bit payload does not work anymore. The call to GetMemoryMap() in efi_stub.c fails with return code EFI_INVALID_PARAMETER. Since the payload itself is still 32-bit U-Boot, efi_uintn_t gets wrongly interpreted as int, but it should actually be long in a 64-bit EFI environment.
This changes the x86 __kernel_size_t conditionals to use compiler provided defines instead. That way we always adhere to the build environment we're in and the definitions adjust automatically.
Fixes: bb0bb91cf0aa ("efi_stub: Use efi_uintn_t") Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
arch/x86/include/asm/posix_types.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/posix_types.h b/arch/x86/include/asm/posix_types.h index 717f6cb..dbcea7f 100644 --- a/arch/x86/include/asm/posix_types.h +++ b/arch/x86/include/asm/posix_types.h @@ -16,7 +16,8 @@ typedef int __kernel_pid_t; typedef unsigned short __kernel_ipc_pid_t; typedef unsigned short __kernel_uid_t; typedef unsigned short __kernel_gid_t; -#if CONFIG_IS_ENABLED(X86_64) +/* checking against __x86_64__ covers both 64-bit EFI stub and 64-bit U-Boot */ +#if defined(__x86_64__) typedef unsigned long __kernel_size_t; typedef long __kernel_ssize_t; #else

On 22 June 2018 at 02:38, Bin Meng bmeng.cn@gmail.com wrote:
Since commit bb0bb91cf0aa ("efi_stub: Use efi_uintn_t"), EFI x86 64-bit payload does not work anymore. The call to GetMemoryMap() in efi_stub.c fails with return code EFI_INVALID_PARAMETER. Since the payload itself is still 32-bit U-Boot, efi_uintn_t gets wrongly interpreted as int, but it should actually be long in a 64-bit EFI environment.
This changes the x86 __kernel_size_t conditionals to use compiler provided defines instead. That way we always adhere to the build environment we're in and the definitions adjust automatically.
Fixes: bb0bb91cf0aa ("efi_stub: Use efi_uintn_t") Signed-off-by: Bin Meng bmeng.cn@gmail.com
arch/x86/include/asm/posix_types.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org

On Sat, Jun 23, 2018 at 10:22 PM, Simon Glass sjg@chromium.org wrote:
On 22 June 2018 at 02:38, Bin Meng bmeng.cn@gmail.com wrote:
Since commit bb0bb91cf0aa ("efi_stub: Use efi_uintn_t"), EFI x86 64-bit payload does not work anymore. The call to GetMemoryMap() in efi_stub.c fails with return code EFI_INVALID_PARAMETER. Since the payload itself is still 32-bit U-Boot, efi_uintn_t gets wrongly interpreted as int, but it should actually be long in a 64-bit EFI environment.
This changes the x86 __kernel_size_t conditionals to use compiler provided defines instead. That way we always adhere to the build environment we're in and the definitions adjust automatically.
Fixes: bb0bb91cf0aa ("efi_stub: Use efi_uintn_t") Signed-off-by: Bin Meng bmeng.cn@gmail.com
arch/x86/include/asm/posix_types.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org
applied to u-boot-x86, thanks!

The use_uart assignment should follow immediately after the call to exit_boot_services(), in case we want some debug output after that.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
lib/efi/efi_stub.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/efi/efi_stub.c b/lib/efi/efi_stub.c index 262fc56..1b495ec 100644 --- a/lib/efi/efi_stub.c +++ b/lib/efi/efi_stub.c @@ -361,14 +361,14 @@ efi_status_t EFIAPI efi_main(efi_handle_t image, } }
+ /* The EFI UART won't work now, switch to a debug one */ + use_uart = true; + map.version = version; map.desc_size = desc_size; add_entry_addr(priv, EFIET_MEMORY_MAP, &map, sizeof(map), desc, size); add_entry_addr(priv, EFIET_END, NULL, 0, 0, 0);
- /* The EFI UART won't work now, switch to a debug one */ - use_uart = true; - memcpy((void *)CONFIG_SYS_TEXT_BASE, _binary_u_boot_bin_start, (ulong)_binary_u_boot_bin_end - (ulong)_binary_u_boot_bin_start);

On 22 June 2018 at 02:38, Bin Meng bmeng.cn@gmail.com wrote:
The use_uart assignment should follow immediately after the call to exit_boot_services(), in case we want some debug output after that.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
lib/efi/efi_stub.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

On Sat, Jun 23, 2018 at 3:28 AM, Simon Glass sjg@chromium.org wrote:
On 22 June 2018 at 02:38, Bin Meng bmeng.cn@gmail.com wrote:
The use_uart assignment should follow immediately after the call to exit_boot_services(), in case we want some debug output after that.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
lib/efi/efi_stub.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
applied to u-boot-x86, thanks!

Enable PRE_CONSOLE_BUFFER so that the full boot output can be viewed on the video console for the EFI payload.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
configs/efi-x86_payload32_defconfig | 2 ++ configs/efi-x86_payload64_defconfig | 2 ++ 2 files changed, 4 insertions(+)
diff --git a/configs/efi-x86_payload32_defconfig b/configs/efi-x86_payload32_defconfig index 7f0cab0..5b6f125 100644 --- a/configs/efi-x86_payload32_defconfig +++ b/configs/efi-x86_payload32_defconfig @@ -6,6 +6,8 @@ CONFIG_FIT=y CONFIG_FIT_SIGNATURE=y CONFIG_USE_BOOTARGS=y CONFIG_BOOTARGS="root=/dev/sdb3 init=/sbin/init rootwait ro" +CONFIG_PRE_CONSOLE_BUFFER=y +CONFIG_PRE_CON_BUF_ADDR=0x100000 CONFIG_SYS_CONSOLE_INFO_QUIET=y CONFIG_DISPLAY_BOARDINFO_LATE=y CONFIG_LAST_STAGE_INIT=y diff --git a/configs/efi-x86_payload64_defconfig b/configs/efi-x86_payload64_defconfig index 8d7f3f0..71fdb5c 100644 --- a/configs/efi-x86_payload64_defconfig +++ b/configs/efi-x86_payload64_defconfig @@ -6,6 +6,8 @@ CONFIG_FIT=y CONFIG_FIT_SIGNATURE=y CONFIG_USE_BOOTARGS=y CONFIG_BOOTARGS="root=/dev/sdb3 init=/sbin/init rootwait ro" +CONFIG_PRE_CONSOLE_BUFFER=y +CONFIG_PRE_CON_BUF_ADDR=0x100000 CONFIG_SYS_CONSOLE_INFO_QUIET=y CONFIG_DISPLAY_BOARDINFO_LATE=y CONFIG_LAST_STAGE_INIT=y

On 22 June 2018 at 02:38, Bin Meng bmeng.cn@gmail.com wrote:
Enable PRE_CONSOLE_BUFFER so that the full boot output can be viewed on the video console for the EFI payload.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
configs/efi-x86_payload32_defconfig | 2 ++ configs/efi-x86_payload64_defconfig | 2 ++ 2 files changed, 4 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org

On Sat, Jun 23, 2018 at 3:28 AM, Simon Glass sjg@chromium.org wrote:
On 22 June 2018 at 02:38, Bin Meng bmeng.cn@gmail.com wrote:
Enable PRE_CONSOLE_BUFFER so that the full boot output can be viewed on the video console for the EFI payload.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
configs/efi-x86_payload32_defconfig | 2 ++ configs/efi-x86_payload64_defconfig | 2 ++ 2 files changed, 4 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
applied to u-boot-x86, thanks!

At present in dram_init_banksize() it ignores conventional memory below 4GB. This leads to wrong DRAM size is printed during boot. Remove such limitation.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
arch/x86/cpu/efi/payload.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/x86/cpu/efi/payload.c b/arch/x86/cpu/efi/payload.c index e3f0f82..4649bfe 100644 --- a/arch/x86/cpu/efi/payload.c +++ b/arch/x86/cpu/efi/payload.c @@ -109,11 +109,10 @@ int dram_init_banksize(void) desc < end && num_banks < CONFIG_NR_DRAM_BANKS; desc = efi_get_next_mem_desc(map, desc)) { /* - * We only use conventional memory below 4GB, and ignore + * We only use conventional memory and ignore * anything less than 1MB. */ if (desc->type != EFI_CONVENTIONAL_MEMORY || - desc->physical_start >= 1ULL << 32 || (desc->num_pages << EFI_PAGE_SHIFT) < 1 << 20) continue; gd->bd->bi_dram[num_banks].start = desc->physical_start;

On 22 June 2018 at 02:38, Bin Meng bmeng.cn@gmail.com wrote:
At present in dram_init_banksize() it ignores conventional memory below 4GB. This leads to wrong DRAM size is printed during boot. Remove such limitation.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
arch/x86/cpu/efi/payload.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org

On Sat, Jun 23, 2018 at 3:28 AM, Simon Glass sjg@chromium.org wrote:
On 22 June 2018 at 02:38, Bin Meng bmeng.cn@gmail.com wrote:
At present in dram_init_banksize() it ignores conventional memory below 4GB. This leads to wrong DRAM size is printed during boot.
This should be "above" 4GB.
Remove such limitation.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
arch/x86/cpu/efi/payload.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Fixed the commit message, and
applied to u-boot-x86, thanks!

Each entry of the EFI memory descriptors occupies map->desc_size, not sizeof(struct efi_mem_desc).
Signed-off-by: Bin Meng bmeng.cn@gmail.com
---
cmd/efi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cmd/efi.c b/cmd/efi.c index 2511c6c..6c1eb88 100644 --- a/cmd/efi.c +++ b/cmd/efi.c @@ -83,7 +83,7 @@ void *efi_build_mem_table(struct efi_entry_memmap *map, int size, bool skip_bs) prev = NULL; addr = 0; dest = base; - end = base + count; + end = (struct efi_mem_desc *)((ulong)base + count * map->desc_size); for (desc = base; desc < end; desc = efi_get_next_mem_desc(map, desc)) { bool merge = true; int type = desc->type;

On 22 June 2018 at 02:38, Bin Meng bmeng.cn@gmail.com wrote:
Each entry of the EFI memory descriptors occupies map->desc_size, not sizeof(struct efi_mem_desc).
Signed-off-by: Bin Meng bmeng.cn@gmail.com
cmd/efi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org

On Sat, Jun 23, 2018 at 3:28 AM, Simon Glass sjg@chromium.org wrote:
On 22 June 2018 at 02:38, Bin Meng bmeng.cn@gmail.com wrote:
Each entry of the EFI memory descriptors occupies map->desc_size, not sizeof(struct efi_mem_desc).
Signed-off-by: Bin Meng bmeng.cn@gmail.com
cmd/efi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org
applied to u-boot-x86, thanks!

Hi Simon,
On Fri, Jun 22, 2018 at 4:38 PM, Bin Meng bmeng.cn@gmail.com wrote:
This series includes 3 patches that were previously sent to ML but not applied, along with some new patches with various fixes on efi x86 payload support.
I see your have reviewed the other 3 patches. Thanks a lot!
For [01: efi.h: Do not use config options] and [02: x86: Change __kernel_size_t conditionals to use compiler provided defines], the changes are mainly adding some comments to describe the changes in more details.
For patch [01] & [02], does the added comments look good to you?
This series is available at u-boot-x86/efi2-working for testing.
Regards, Bin
participants (2)
-
Bin Meng
-
Simon Glass