x86: efi: Fixes and enhancements to application and payload support

Since commit f3b5056c4e72 ("efi_loader: split README.efi into two separate documents"), the original README.efi was renamed to README.u-boot_on_efi, but x86 doc still refers to the old one.

Signed-off-by Bin Meng <[email protected]>

---

 doc/README.x86 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

doc/README.x86 +0 -0

index 78664c3..865a965 100644

doc/README.x86 +0 -0

 64-bit Support
 --------------
 

Quoted 18 lines · Bin Meng

Since commit f3b5056c4e72 ("efi_loader: split README.efi into two separate documents"), the original README.efi was renamed to README.u-boot_on_efi, but x86 doc still refers to the old one.

Signed-off-by Bin Meng <[email protected]>

---

 doc/README.x86 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

doc/README.x86 +0 -0

index 78664c3..865a965 100644
@@ -1139,7 +1139,7 @@ application, with CONFIG_EFI_APP. The CONFIG_EFI_LOADER option, where U-Booot
 provides an EFI environment to the kernel (i.e. replaces UEFI completely but
 provides the same EFI run-time services) is not currently supported on x86.
 

doc/README.x86 application, with CONFIG_EFI_APP. The CONFIG_EFI_LOADER option, where U-Booot · line 1142

Quoted 2 lines

doc/README.x86 +1 -1

-See README.efi for details of EFI support in U-Boot.
+See  for details of EFI support in U-Boot.

Thanks for updating the references.

The chapter "EFI Support" refers both to running U-Boot as an EFI payload as well as using U-Boot as an EFI platform. So both doc/README.u-boot_on_efi and doc/README.uefi should be referenced.

But I would prefer if you could also move the sentences concerning EFI that has spilled over into "64-bit Support" into the "EFI Support" chapter.

Hi Heinrich,

Quoted 24 lines · Heinrich Schuchardt

Quoted 15 lines · Bin Meng

Since commit f3b5056c4e72 ("efi_loader: split README.efi into two separate documents"), the original README.efi was renamed to README.u-boot_on_efi, but x86 doc still refers to the old one.

Signed-off-by Bin Meng <[email protected]>

---

 doc/README.x86 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

doc/README.x86 +0 -0

index 78664c3..865a965 100644

doc/README.x86 application, with CONFIG_EFI_APP. The CONFIG_EFI_LOADER option, where U-Booot · line 1142

Quoted 5 lines

doc/README.x86 +1 -1

@@ -1139,7 +1139,7 @@ application, with CONFIG_EFI_APP. The CONFIG_EFI_LOADER option, where U-Booot
 provides an EFI environment to the kernel (i.e. replaces UEFI completely but
 provides the same EFI run-time services) is not currently supported on x86.
 
-See README.efi for details of EFI support in U-Boot.
+See  for details of EFI support in U-Boot.

Thanks for updating the references.

Quoted 4 lines

The chapter "EFI Support" refers both to running U-Boot as an EFI payload as well as using U-Boot as an EFI platform. So both doc/README.u-boot_on_efi and doc/README.uefi should be referenced.

OK, will do in v2. One question: the doc says: CONFIG_EFI_LOADER option is not currently supported on x86. Is this still the case, or outdated? (I haven't tried the EFI loader on x86 or ARM yet)

Quoted 4 lines

But I would prefer if you could also move the sentences concerning EFI that has spilled over into "64-bit Support" into the "EFI Support" chapter.

Sure.

Regards,
Bin

The pinctrl_ich6 driver is currently unconditionally built for all x86 boards. Let's use a Kconfig option to control the build.

Signed-off-by Bin Meng <[email protected]>

---

 arch/x86/Kconfig               | 6 ++++++
 arch/x86/cpu/baytrail/Kconfig  | 1 +
 arch/x86/cpu/ivybridge/Kconfig | 1 +
 arch/x86/lib/Makefile          | 2 +-
 4 files changed, 9 insertions(+), 1 deletion(-)

Quoted 6 lines · Bin Meng

The pinctrl_ich6 driver is currently unconditionally built for all x86 boards. Let's use a Kconfig option to control the build.

Signed-off-by Bin Meng <[email protected]>

---

Quoted 5 lines

 arch/x86/Kconfig               | 6 ++++++
 arch/x86/cpu/baytrail/Kconfig  | 1 +
 arch/x86/cpu/ivybridge/Kconfig | 1 +
 arch/x86/lib/Makefile          | 2 +-
 4 files changed, 9 insertions(+), 1 deletion(-)

Reviewed-by Simon Glass <[email protected]>

The EFI application does not boot currently. It's due to the call to syscon_get_by_driver_data() in cpu_init_r() maps to nowhere as CONFIG_SYSCON is not included in the configuration.

EFI application is built as a shared library, so GCC won't complain during the build process if some symbols are not found. GCC will simply put these symbols into the .plt section and expect dynamic loader to fix these up.

While we are here, remove some commands and drivers that are not needed at present.

Signed-off-by Bin Meng <[email protected]>

---

 configs/efi-x86_defconfig | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

Quoted 12 lines · Bin Meng

The EFI application does not boot currently. It's due to the call to syscon_get_by_driver_data() in cpu_init_r() maps to nowhere as CONFIG_SYSCON is not included in the configuration.

EFI application is built as a shared library, so GCC won't complain during the build process if some symbols are not found. GCC will simply put these symbols into the .plt section and expect dynamic loader to fix these up.

While we are here, remove some commands and drivers that are not needed at present.

Quoted 6 lines

Signed-off-by Bin Meng <[email protected]>

---

 configs/efi-x86_defconfig | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

Reviewed-by Simon Glass <[email protected]>

The EFI application does not boot currently. It's due to the call to syscon_get_by_driver_data() in cpu_init_r() maps to nowhere as CONFIG_SYSCON is not included in the configuration.

EFI application is built as a shared library, so GCC won't complain during the build process if some symbols are not found. GCC will simply put these symbols into the .plt section and expect dynamic loader to fix these up.

While we are here, remove some commands and drivers that are not needed at present.

Signed-off-by Bin Meng <[email protected]>

---

 configs/efi-x86_defconfig | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

configs/efi-x86_defconfig +2 -6

index 4cf5554..741a169 100644
@@ -11,12 +11,8 @@ CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_LAST_STAGE_INIT=y
 CONFIG_HUSH_PARSER=y
 # CONFIG_CMD_BOOTM is not set
-CONFIG_CMD_GPIO=y
 CONFIG_CMD_PART=y
-CONFIG_CMD_SF=y
 # CONFIG_CMD_SF_TEST is not set
-CONFIG_CMD_SPI=y
-CONFIG_CMD_USB=y
 # CONFIG_CMD_NET is not set
 CONFIG_CMD_TIME=y
 CONFIG_CMD_EXT2=y

@@ -28,12 +24,12 @@ CONFIG_MAC_PARTITION=y
 CONFIG_ISO_PARTITION=y
 CONFIG_EFI_PARTITION=y
 CONFIG_OF_EMBED=y
+CONFIG_REGMAP=y
+CONFIG_SYSCON=y
 # CONFIG_DM_ETH is not set
 CONFIG_DEBUG_EFI_CONSOLE=y
 CONFIG_DEBUG_UART_BASE=0
 CONFIG_DEBUG_UART_CLOCK=0
-CONFIG_SPI=y
-CONFIG_ICH_SPI=y
 # CONFIG_REGEX is not set
 CONFIG_EFI=y
 # CONFIG_EFI_LOADER is not set

Quoted 11 lines · Bin Meng

The EFI application does not boot currently. It's due to the call to syscon_get_by_driver_data() in cpu_init_r() maps to nowhere as CONFIG_SYSCON is not included in the configuration.

EFI application is built as a shared library, so GCC won't complain during the build process if some symbols are not found. GCC will simply put these symbols into the .plt section and expect dynamic loader to fix these up.

While we are here, remove some commands and drivers that are not needed at present.

This patch is not applicable to master:

Applying: x86: efi: app: Fix broken EFI application
error: patch failed: configs/efi-x86_defconfig:28
error: configs/efi-x86_defconfig: patch does not apply

Please, rebase.

Attempting to use a toolchain that is preconfigured to generate code for the 32-bit architecture (i386), for example, the i386-linux-gcc toolchain on kernel.org, to compile the 64-bit EFI payload does not build. This updates the makefile fragments to ensure '-m64' is passed to toolchain when building the 64-bit EFI payload stub codes.

Signed-off-by Bin Meng <[email protected]>

Quoted 6 lines · Bin Meng

Attempting to use a toolchain that is preconfigured to generate code for the 32-bit architecture (i386), for example, the i386-linux-gcc toolchain on kernel.org, to compile the 64-bit EFI payload does not build. This updates the makefile fragments to ensure '-m64' is passed to toolchain when building the 64-bit EFI payload stub codes.

Quoted 1 line

Signed-off-by Bin Meng <[email protected]>

Is there any sane reason to keep the split between EFI and U-Boot long size alive? The x86_64 U-Boot port is getting along reasonably well from what I can tell and 32bit UEFI implementations on 64-bit hosts are dying out.

So can't we just remove all of that cruft altogether instead and just have x86_64 U-Boot with 64bit EFI stub and i386 U-Boot with 32bit EFI stub as only combinations?

That would dramatically simplify the code.

Quoted 9 lines · Alexander Graf

Quoted 6 lines · Bin Meng

Attempting to use a toolchain that is preconfigured to generate code for the 32-bit architecture (i386), for example, the i386-linux-gcc toolchain on kernel.org, to compile the 64-bit EFI payload does not build. This updates the makefile fragments to ensure '-m64' is passed to toolchain when building the 64-bit EFI payload stub codes.

Quoted 7 lines

Quoted 1 line

Signed-off-by Bin Meng <[email protected]>

Is there any sane reason to keep the split between EFI and U-Boot long size alive? The x86_64 U-Boot port is getting along reasonably well from what I can tell and 32bit UEFI implementations on 64-bit hosts are dying out.

I think you are mixing two type of things. U-Boot building as 32-bit or 64-bit is one thing. How to build U-Boot is another thing. This patch was to address the build.

Quoted 4 lines

So can't we just remove all of that cruft altogether instead and just have x86_64 U-Boot with 64bit EFI stub and i386 U-Boot with 32bit EFI stub as only combinations?

Some day once U-Boot x86_64 support is mature, we can consider that. Even if we only support 64-bit U-Boot as the 64-bit EFI payload, that does not mean i386-linux-gcc cannot be used. '-m64' is required to pass to such toolchain to cross-compile 64-bit codes correctly.

Quoted 27 lines · Bin Meng

Quoted 9 lines · Alexander Graf

Quoted 6 lines · Bin Meng

Attempting to use a toolchain that is preconfigured to generate code for the 32-bit architecture (i386), for example, the i386-linux-gcc toolchain on kernel.org, to compile the 64-bit EFI payload does not build. This updates the makefile fragments to ensure '-m64' is passed to toolchain when building the 64-bit EFI payload stub codes.

Quoted 7 lines

Quoted 1 line

Signed-off-by Bin Meng <[email protected]>

Is there any sane reason to keep the split between EFI and U-Boot long size alive? The x86_64 U-Boot port is getting along reasonably well from what I can tell and 32bit UEFI implementations on 64-bit hosts are dying out.

I think you are mixing two type of things. U-Boot building as 32-bit or 64-bit is one thing. How to build U-Boot is another thing. This patch was to address the build.

Quoted 4 lines

So can't we just remove all of that cruft altogether instead and just have x86_64 U-Boot with 64bit EFI stub and i386 U-Boot with 32bit EFI stub as only combinations?

Quoted 4 lines

Some day once U-Boot x86_64 support is mature, we can consider that. Even if we only support 64-bit U-Boot as the 64-bit EFI payload, that does not mean i386-linux-gcc cannot be used. '-m64' is required to pass to such toolchain to cross-compile 64-bit codes correctly.

Yes, but then all of this would be unconditional for the full code base and not depend on CONFIG_EFI_STUB_64BIT at all :).

I'm just wary that the efi payload code is quite hard to follow with all the combinations of 32/64 payload/u-boot bitnesses.

Attempting to use a toolchain that is preconfigured to generate code for the 32-bit architecture (i386), for example, the i386-linux-gcc toolchain on kernel.org, to compile the 64-bit EFI payload does not build. This updates the makefile fragments to ensure '-m64' is passed to toolchain when building the 64-bit EFI payload stub codes.

Signed-off-by Bin Meng <[email protected]>

---

 arch/x86/cpu/Makefile | 3 ++-
 arch/x86/lib/Makefile | 4 ++--
 lib/efi/Makefile      | 6 ++++--
 3 files changed, 8 insertions(+), 5 deletions(-)

Quoted 6 lines · Bin Meng

Attempting to use a toolchain that is preconfigured to generate code for the 32-bit architecture (i386), for example, the i386-linux-gcc toolchain on kernel.org, to compile the 64-bit EFI payload does not build. This updates the makefile fragments to ensure '-m64' is passed to toolchain when building the 64-bit EFI payload stub codes.

Quoted 8 lines

Signed-off-by Bin Meng <[email protected]>

---

 arch/x86/cpu/Makefile | 3 ++-
 arch/x86/lib/Makefile | 4 ++--
 lib/efi/Makefile      | 6 ++++--
 3 files changed, 8 insertions(+), 5 deletions(-)

Reviewed-by Simon Glass <[email protected]>

Nice to get this working with an i386 compiler!

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.

Fixes bb0bb91cf0aa ("efi_stub: Use efi_uintn_t")

Signed-off-by Bin Meng <[email protected]>

---

 include/efi_api.h | 4 ++++
 1 file changed, 4 insertions(+)

include/efi_api.h +0 -0

index 64c27e4..d1158de 100644

include/efi_api.h +1 -0

+#endif
 typedef uint16_t *efi_string_t;
 
 #define EVT_TIMER				0x80000000

Quoted 4 lines · Bin Meng

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

Above you say 64-bit payload and now you say 32-bit?

Why don't you compile U-Boot as 64-bit? How do you want to load a 64bit Linux EFI stub from an 32-bit EFI implementation in U-Boot?

Quoted 16 lines

, efi_uintn_t gets wrongly
interpreted as int, but it should actually be long in a 64-bit EFI environment.

Fixes bb0bb91cf0aa ("efi_stub: Use efi_uintn_t")

Signed-off-by Bin Meng <[email protected]>

---

 include/efi_api.h | 4 ++++
 1 file changed, 4 insertions(+)

include/efi_api.h +0 -0

index 64c27e4..d1158de 100644

include/efi_api.h enum efi_timer_delay { · line 34

Quoted 7 lines

include/efi_api.h +3 -0

@@ -28,7 +28,11 @@ enum efi_timer_delay {
      EFI_TIMER_RELATIVE = 2
 };
 
+#if defined(CONFIG_EFI_STUB_64BIT) && defined(EFI_STUB)
+#define efi_uintn_t unsigned long
+#else
 #define efi_uintn_t size_t

NAK

This change will create a lot of build warnings if EFI_STUB and EFI_LOADER are both configured.

Could you, please, explain under which compiler settings size_t and unsigned long have a different number of bits?

Obviously we have the EFI API exposed by U-Boot which has the bitness of U-Boot.

If you want to consume an EFI API of another bitness I suggest that you create separate interface definitions.

Hi Heinrich,

Quoted 11 lines · Heinrich Schuchardt

Quoted 4 lines · Bin Meng

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

Above you say 64-bit payload and now you say 32-bit?

Why don't you compile U-Boot as 64-bit? How do you want to load a 64bit Linux EFI stub from an 32-bit EFI implementation in U-Boot?

U-Boot itself as the EFI pyaload is 32-bit. The EFI stub is 64-bit as it has to be loaded from the 64-bit EFI BIOS. Note in case you
misunderstand: the generated u-boot-payload.efi is 64-bit stub codes (for 64-bit EFI BIOS) or 32-bit stub codes (for 32-bit EFI BIOS) plus 32-bit U-Boot payload. The payload is always 32-bit as of today as U-Boot on x86 is mainly on 32-bit. 64-bit support, as you see from README.x86, is far from mature yet.

Quoted 26 lines

Quoted 16 lines

, efi_uintn_t gets wrongly
interpreted as int, but it should actually be long in a 64-bit EFI environment.

Fixes bb0bb91cf0aa ("efi_stub: Use efi_uintn_t")

Signed-off-by Bin Meng <[email protected]>

---

 include/efi_api.h | 4 ++++
 1 file changed, 4 insertions(+)

include/efi_api.h +0 -0

index 64c27e4..d1158de 100644

include/efi_api.h enum efi_timer_delay { · line 34

Quoted 7 lines

include/efi_api.h +3 -0

@@ -28,7 +28,11 @@ enum efi_timer_delay {
      EFI_TIMER_RELATIVE = 2
 };
 
+#if defined(CONFIG_EFI_STUB_64BIT) && defined(EFI_STUB)
+#define efi_uintn_t unsigned long
+#else
 #define efi_uintn_t size_t

NAK

Quoted 3 lines

This change will create a lot of build warnings if EFI_STUB and EFI_LOADER are both configured.

I don't see any build warnings when building efi-x86_payload32 or efi-x86_payload64. I see both EFI_STUB and EFI_LOADER are enabled with these two targets. AFAIK, only x86 supports EFI_STUB currently. I don't know where you see a lot of build warnings.

Quoted 4 lines

Could you, please, explain under which compiler settings size_t and unsigned long have a different number of bits?

As mentioned above, the EFI stub codes (efi_stub.c) are built for 64-bit, which expects 'efi_uintn_t' to be 64-bit. However, it is defined as size_t, which in U-Boot 32, will be 'unsigned int' (see arch/x86/include/asm/posix_types.h) - the bug here!

Quoted 6 lines

Obviously we have the EFI API exposed by U-Boot which has the bitness of U-Boot.

If you want to consume an EFI API of another bitness I suggest that you create separate interface definitions.

Regards,
Bin

Quoted 50 lines · Bin Meng

Hi Heinrich,

Quoted 11 lines · Heinrich Schuchardt

Quoted 4 lines · Bin Meng

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

Above you say 64-bit payload and now you say 32-bit?

Why don't you compile U-Boot as 64-bit? How do you want to load a 64bit Linux EFI stub from an 32-bit EFI implementation in U-Boot?

U-Boot itself as the EFI pyaload is 32-bit. The EFI stub is 64-bit as it has to be loaded from the 64-bit EFI BIOS. Note in case you
misunderstand: the generated u-boot-payload.efi is 64-bit stub codes (for 64-bit EFI BIOS) or 32-bit stub codes (for 32-bit EFI BIOS) plus 32-bit U-Boot payload. The payload is always 32-bit as of today as U-Boot on x86 is mainly on 32-bit. 64-bit support, as you see from README.x86, is far from mature yet.

Quoted 26 lines

Quoted 16 lines

, efi_uintn_t gets wrongly
interpreted as int, but it should actually be long in a 64-bit EFI environment.

Fixes bb0bb91cf0aa ("efi_stub: Use efi_uintn_t")

Signed-off-by Bin Meng <[email protected]>

---

 include/efi_api.h | 4 ++++
 1 file changed, 4 insertions(+)

include/efi_api.h +0 -0

index 64c27e4..d1158de 100644

include/efi_api.h enum efi_timer_delay { · line 34

Quoted 7 lines

include/efi_api.h +3 -0

@@ -28,7 +28,11 @@ enum efi_timer_delay {
      EFI_TIMER_RELATIVE = 2
 };
 
+#if defined(CONFIG_EFI_STUB_64BIT) && defined(EFI_STUB)
+#define efi_uintn_t unsigned long
+#else
 #define efi_uintn_t size_t

NAK

Quoted 8 lines

Quoted 3 lines

This change will create a lot of build warnings if EFI_STUB and EFI_LOADER are both configured.

I don't see any build warnings when building efi-x86_payload32 or efi-x86_payload64. I see both EFI_STUB and EFI_LOADER are enabled with these two targets. AFAIK, only x86 supports EFI_STUB currently. I don't know where you see a lot of build warnings.

Currently you cannot build with EFI_LOADER=Y on 32 bit with a 64bit stub. See lib/efi_loader/Kconfig. The problem is with the build scripts for the stub using the same CONFIG variables as those used for other binaries.

To emulate what would happen with your change once we can build with EFI_LOADER=y and 64bit stub I made the following change:

And then I tried to build:
make qemu-x86_defconfig
make -j6

It gives me a bunch of errors like below.

Hi Heinrich,

Quoted 74 lines · Heinrich Schuchardt

Quoted 50 lines · Bin Meng

Hi Heinrich,

Quoted 11 lines · Heinrich Schuchardt

Quoted 4 lines · Bin Meng

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

Above you say 64-bit payload and now you say 32-bit?

Why don't you compile U-Boot as 64-bit? How do you want to load a 64bit Linux EFI stub from an 32-bit EFI implementation in U-Boot?

U-Boot itself as the EFI pyaload is 32-bit. The EFI stub is 64-bit as it has to be loaded from the 64-bit EFI BIOS. Note in case you
misunderstand: the generated u-boot-payload.efi is 64-bit stub codes (for 64-bit EFI BIOS) or 32-bit stub codes (for 32-bit EFI BIOS) plus 32-bit U-Boot payload. The payload is always 32-bit as of today as U-Boot on x86 is mainly on 32-bit. 64-bit support, as you see from README.x86, is far from mature yet.

Quoted 26 lines

Quoted 16 lines

, efi_uintn_t gets wrongly
interpreted as int, but it should actually be long in a 64-bit EFI environment.

Fixes bb0bb91cf0aa ("efi_stub: Use efi_uintn_t")

Signed-off-by Bin Meng <[email protected]>

---

 include/efi_api.h | 4 ++++
 1 file changed, 4 insertions(+)

include/efi_api.h +0 -0

index 64c27e4..d1158de 100644

include/efi_api.h enum efi_timer_delay { · line 34

Quoted 7 lines

include/efi_api.h +3 -0

@@ -28,7 +28,11 @@ enum efi_timer_delay {
      EFI_TIMER_RELATIVE = 2
 };
 
+#if defined(CONFIG_EFI_STUB_64BIT) && defined(EFI_STUB)
+#define efi_uintn_t unsigned long
+#else
 #define efi_uintn_t size_t

NAK

Quoted 8 lines

Quoted 3 lines

This change will create a lot of build warnings if EFI_STUB and EFI_LOADER are both configured.

I don't see any build warnings when building efi-x86_payload32 or efi-x86_payload64. I see both EFI_STUB and EFI_LOADER are enabled with these two targets. AFAIK, only x86 supports EFI_STUB currently. I don't know where you see a lot of build warnings.

Currently you cannot build with EFI_LOADER=Y on 32 bit with a 64bit stub. See lib/efi_loader/Kconfig. The problem is with the build scripts for the stub using the same CONFIG variables as those used for other binaries.

To emulate what would happen with your change once we can build with EFI_LOADER=y and 64bit stub I made the following change:

include/efi_api.h +0 -0

@@ -28,7 +28,8 @@ enum efi_timer_delay {
        EFI_TIMER_RELATIVE = 2
 };
 

include/efi_api.h enum efi_timer_delay { · line 33

Quoted 4 lines

include/efi_api.h +1 -1

-#define efi_uintn_t size_t
+#define efi_uintn_t unsigned long
 typedef uint16_t *efi_string_t;
 

I am not sure why do you unconditionally change efi_uintn_t? My patch has

#if defined(CONFIG_EFI_STUB_64BIT) && defined(EFI_STUB)

to guard the change.

Quoted 4 lines

And then I tried to build:
make qemu-x86_defconfig
make -j6

I know. But my patch does not produce any build warnings on efi-x86_payload32_defconfig and efi-x86_payload64_defconfig, on u-boot-x86/efi-working branch.

Quoted 2 lines

It gives me a bunch of errors like below.

[snip]

Regards,
Bin

Quoted 84 lines · Bin Meng

Hi Heinrich,

Quoted 71 lines · Heinrich Schuchardt

Quoted 50 lines · Bin Meng

Hi Heinrich,

Quoted 11 lines · Heinrich Schuchardt

Quoted 4 lines · Bin Meng

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

Above you say 64-bit payload and now you say 32-bit?

Why don't you compile U-Boot as 64-bit? How do you want to load a 64bit Linux EFI stub from an 32-bit EFI implementation in U-Boot?

U-Boot itself as the EFI pyaload is 32-bit. The EFI stub is 64-bit as it has to be loaded from the 64-bit EFI BIOS. Note in case you
misunderstand: the generated u-boot-payload.efi is 64-bit stub codes (for 64-bit EFI BIOS) or 32-bit stub codes (for 32-bit EFI BIOS) plus 32-bit U-Boot payload. The payload is always 32-bit as of today as U-Boot on x86 is mainly on 32-bit. 64-bit support, as you see from README.x86, is far from mature yet.

Quoted 26 lines

Quoted 16 lines

, efi_uintn_t gets wrongly
interpreted as int, but it should actually be long in a 64-bit EFI environment.

Fixes bb0bb91cf0aa ("efi_stub: Use efi_uintn_t")

Signed-off-by Bin Meng <[email protected]>

---

 include/efi_api.h | 4 ++++
 1 file changed, 4 insertions(+)

include/efi_api.h +0 -0

index 64c27e4..d1158de 100644

include/efi_api.h enum efi_timer_delay { · line 34

Quoted 7 lines

include/efi_api.h +3 -0

@@ -28,7 +28,11 @@ enum efi_timer_delay {
      EFI_TIMER_RELATIVE = 2
 };
 
+#if defined(CONFIG_EFI_STUB_64BIT) && defined(EFI_STUB)
+#define efi_uintn_t unsigned long
+#else
 #define efi_uintn_t size_t

NAK

Quoted 8 lines

Quoted 3 lines

This change will create a lot of build warnings if EFI_STUB and EFI_LOADER are both configured.

I don't see any build warnings when building efi-x86_payload32 or efi-x86_payload64. I see both EFI_STUB and EFI_LOADER are enabled with these two targets. AFAIK, only x86 supports EFI_STUB currently. I don't know where you see a lot of build warnings.

Currently you cannot build with EFI_LOADER=Y on 32 bit with a 64bit stub. See lib/efi_loader/Kconfig. The problem is with the build scripts for the stub using the same CONFIG variables as those used for other binaries.

To emulate what would happen with your change once we can build with EFI_LOADER=y and 64bit stub I made the following change:

include/efi_api.h enum efi_timer_delay { · line 33

Quoted 7 lines

include/efi_api.h +1 -1

@@ -28,7 +28,8 @@ enum efi_timer_delay {
        EFI_TIMER_RELATIVE = 2
 };
 
-#define efi_uintn_t size_t
+#define efi_uintn_t unsigned long
 typedef uint16_t *efi_string_t;
 

I am not sure why do you unconditionally change efi_uintn_t? My patch has

Quoted 1 line

#if defined(CONFIG_EFI_STUB_64BIT) && defined(EFI_STUB)

Currently it is not possible to enable CONFIG_EFI_STUB_64BIT and EFI_LOADER on 32bit U-Boot. Once we have removed this restriction in the build system your patch together with EFI_LOADER=y and CONFIG_EFI_STUB_64BIT=y will provoke all those errors. This is why I NAKed you patch.

Hi Heinrich,

Quoted 93 lines · Heinrich Schuchardt

Quoted 77 lines · Bin Meng

Hi Heinrich,

Quoted 74 lines · Heinrich Schuchardt

Quoted 50 lines · Bin Meng

Hi Heinrich,

Quoted 11 lines · Heinrich Schuchardt

Quoted 4 lines · Bin Meng

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

Above you say 64-bit payload and now you say 32-bit?

Why don't you compile U-Boot as 64-bit? How do you want to load a 64bit Linux EFI stub from an 32-bit EFI implementation in U-Boot?

U-Boot itself as the EFI pyaload is 32-bit. The EFI stub is 64-bit as it has to be loaded from the 64-bit EFI BIOS. Note in case you
misunderstand: the generated u-boot-payload.efi is 64-bit stub codes (for 64-bit EFI BIOS) or 32-bit stub codes (for 32-bit EFI BIOS) plus 32-bit U-Boot payload. The payload is always 32-bit as of today as U-Boot on x86 is mainly on 32-bit. 64-bit support, as you see from README.x86, is far from mature yet.

Quoted 26 lines

Quoted 16 lines

, efi_uintn_t gets wrongly
interpreted as int, but it should actually be long in a 64-bit EFI environment.

Fixes bb0bb91cf0aa ("efi_stub: Use efi_uintn_t")

Signed-off-by Bin Meng <[email protected]>

---

 include/efi_api.h | 4 ++++
 1 file changed, 4 insertions(+)

include/efi_api.h +0 -0

index 64c27e4..d1158de 100644

include/efi_api.h enum efi_timer_delay { · line 34

Quoted 7 lines

include/efi_api.h +3 -0

@@ -28,7 +28,11 @@ enum efi_timer_delay {
      EFI_TIMER_RELATIVE = 2
 };
 
+#if defined(CONFIG_EFI_STUB_64BIT) && defined(EFI_STUB)
+#define efi_uintn_t unsigned long
+#else
 #define efi_uintn_t size_t

NAK

Quoted 8 lines

Quoted 3 lines

This change will create a lot of build warnings if EFI_STUB and EFI_LOADER are both configured.

I don't see any build warnings when building efi-x86_payload32 or efi-x86_payload64. I see both EFI_STUB and EFI_LOADER are enabled with these two targets. AFAIK, only x86 supports EFI_STUB currently. I don't know where you see a lot of build warnings.

Currently you cannot build with EFI_LOADER=Y on 32 bit with a 64bit stub. See lib/efi_loader/Kconfig. The problem is with the build scripts for the stub using the same CONFIG variables as those used for other binaries.

To emulate what would happen with your change once we can build with EFI_LOADER=y and 64bit stub I made the following change:

include/efi_api.h +0 -0

@@ -28,7 +28,8 @@ enum efi_timer_delay {
        EFI_TIMER_RELATIVE = 2
 };
 

Quoted 8 lines

include/efi_api.h enum efi_timer_delay { · line 33

Quoted 4 lines

include/efi_api.h +1 -1

-#define efi_uintn_t size_t
+#define efi_uintn_t unsigned long
 typedef uint16_t *efi_string_t;
 

I am not sure why do you unconditionally change efi_uintn_t? My patch has

#if defined(CONFIG_EFI_STUB_64BIT) && defined(EFI_STUB)

Currently it is not possible to enable CONFIG_EFI_STUB_64BIT and EFI_LOADER on 32bit U-Boot. Once we have removed this restriction in the build system your patch together with EFI_LOADER=y and CONFIG_EFI_STUB_64BIT=y will provoke all those errors. This is why I NAKed you patch.

I did the following to try to reproduce what you saw:

lib/efi_loader/Kconfig +0 -4

index d38780b..29e67b6 100644
@@ -3,10 +3,6 @@ config EFI_LOADER
        depends on (ARM || X86) && OF_LIBFDT
        # We do not support bootefi booting ARMv7 in non-secure mode
        depends on !ARMV7_NONSEC
-       # 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

Then do a

$ make efi-x86_payload32_defconfig
$ make

It succeeded without any error

But doing:

$ make efi-x86_payload64_defconfig
$ make

Failed with a different error (no compiler warning were seen)

toolchains/gcc-7.3.0-nolibc/i386-linux/bin/i386-linux-ld.bfd: i386 architecture of input file `lib/efi_loader/helloworld.o' is incompatible with i386:x86-64 output scripts/Makefile.lib:407: recipe for target 'lib/efi_loader/helloworld_efi.so' failed

Switching to x86_64-linux-gcc, exposed the same error:

toolchains/gcc-7.3.0-nolibc/x86_64-linux/bin/x86_64-linux-ld.bfd: i386 architecture of input file `lib/efi_loader/helloworld.o' is incompatible with i386:x86-64 output

Regards,
Bin

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.

Fixes bb0bb91cf0aa ("efi_stub: Use efi_uintn_t")

Signed-off-by Bin Meng <[email protected]>

---

 include/efi_api.h | 4 ++++
 1 file changed, 4 insertions(+)

include/efi_api.h +0 -0

index 64c27e4..d1158de 100644
@@ -28,7 +28,11 @@ enum efi_timer_delay {
        EFI_TIMER_RELATIVE = 2
 };

Hi Alex,

Quoted 21 lines · Bin Meng

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.

Fixes bb0bb91cf0aa ("efi_stub: Use efi_uintn_t")

Signed-off-by Bin Meng <[email protected]>

---

 include/efi_api.h | 4 ++++
 1 file changed, 4 insertions(+)

include/efi_api.h +0 -0

index 64c27e4..d1158de 100644
@@ -28,7 +28,11 @@ enum efi_timer_delay {
        EFI_TIMER_RELATIVE = 2
 };

include/efi_api.h enum efi_timer_delay {

Quoted 10 lines

include/efi_api.h +4 -1

 
+#if defined(CONFIG_EFI_STUB_64BIT) && defined(EFI_STUB)
+#define efi_uintn_t unsigned long
+#else
 #define efi_uintn_t size_t
+#endif
 typedef uint16_t *efi_string_t;
 
 #define EVT_TIMER                              0x80000000
--

With your proposed changes to efi.h in [1], do you think we should also fix this by:

#ifdef __x86_64__
#define efi_uintn_t unsigned long
#else
#define efi_uintn_t size_t

From: Christian Gmeiner <[email protected]>

If we use U-Boot as coreboot payload with a generic dts without any ranges specified we fail in pci pre_probe and our pci bus is not usable.

So convert decode_regions(..) into a void function and do the simple error handling there.

Signed-off-by Christian Gmeiner <[email protected]>

Reviewed-by Bin Meng <[email protected]>

[bmeng: fixed 'u-boot' in the commit message and checkpatch warning]

Signed-off-by Bin Meng <[email protected]>

---

 drivers/pci/pci-uclass.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

Quoted 14 lines · Bin Meng

From: Christian Gmeiner <[email protected]>

If we use U-Boot as coreboot payload with a generic dts without any ranges specified we fail in pci pre_probe and our pci bus is not usable.

So convert decode_regions(..) into a void function and do the simple error handling there.

Signed-off-by Christian Gmeiner <[email protected]>

Reviewed-by Bin Meng <[email protected]>

[bmeng: fixed 'u-boot' in the commit message and checkpatch warning]

Signed-off-by Bin Meng <[email protected]>

Quoted 4 lines

---

 drivers/pci/pci-uclass.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

Reviewed-by Simon Glass <[email protected]>

From: Christian Gmeiner <[email protected]>

If U-Boot gets used as coreboot payload all pci resources got assigned by coreboot. If a dts without any pci ranges gets used the dm is not able to access pci device memory. To get things working make use of a 1:1 mapping for bus <-> phy addresses.

This change makes it possible to get the e1000 U-Boot driver working on a sandybridge device where U-Boot is used as coreboot payload.

Signed-off-by Christian Gmeiner <[email protected]>

Reviewed-by Bin Meng <[email protected]>

[bmeng: fixed 'u-boot' in the commit message]

Signed-off-by Bin Meng <[email protected]>

---

 drivers/pci/pci-uclass.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Quoted 16 lines · Bin Meng

From: Christian Gmeiner <[email protected]>

If U-Boot gets used as coreboot payload all pci resources got assigned by coreboot. If a dts without any pci ranges gets used the dm is not able to access pci device memory. To get things working make use of a 1:1 mapping for bus <-> phy addresses.

This change makes it possible to get the e1000 U-Boot driver working on a sandybridge device where U-Boot is used as coreboot payload.

Signed-off-by Christian Gmeiner <[email protected]>

Reviewed-by Bin Meng <[email protected]>

[bmeng: fixed 'u-boot' in the commit message]

Signed-off-by Bin Meng <[email protected]>

Quoted 4 lines

---

 drivers/pci/pci-uclass.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

Reviewed-by Simon Glass <[email protected]>

At present the EFI application and payload support codes in the x86 directory is distributed in a hybrid way. For example, the Kconfig options for both app and payload are in arch/x86/lib/efi/Kconfig, but the source codes in the same directory get built only for CONFIG_EFI_STUB.

This refactors the codes by consolidating all the EFI support codes into arch/x86/cpu/efi, just like other x86 targets.

Signed-off-by Bin Meng <[email protected]>

---

 arch/x86/Kconfig                              | 3 +--
 arch/x86/cpu/Makefile                         | 2 +-
 arch/x86/{lib => cpu}/efi/Kconfig             | 0
 arch/x86/cpu/efi/Makefile                     | 7 +++++++
 arch/x86/{lib => cpu}/efi/car.S               | 0
 arch/x86/{lib/efi/efi.c => cpu/efi/payload.c} | 0
 arch/x86/lib/Makefile                         | 1 -
 arch/x86/lib/efi/Makefile                     | 7 -------
 8 files changed, 9 insertions(+), 11 deletions(-)
 rename arch/x86/{lib => cpu}/efi/Kconfig (100%)
 rename arch/x86/{lib => cpu}/efi/car.S (100%)
 rename arch/x86/{lib/efi/efi.c => cpu/efi/payload.c} (100%)
 delete mode 100644 arch/x86/lib/efi/Makefile

This adds arch_cpu_init() to the payload codes, in preparation for supporting a generic efi payload.

Signed-off-by Bin Meng <[email protected]>

---

 arch/x86/cpu/efi/payload.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

If GetMemoryMap() fails, we really want to know EFI_BITS_PER_LONG instead of BITS_PER_LONG. A space and LF are added in places where error message is output to improve readability.

Signed-off-by Bin Meng <[email protected]>

---

 lib/efi/efi_stub.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Quoted 4 lines · Bin Meng

If GetMemoryMap() fails, we really want to know EFI_BITS_PER_LONG instead of BITS_PER_LONG. A space and LF are added in places where error message is output to improve readability.

Quoted 5 lines

Signed-off-by Bin Meng <[email protected]>

---

 lib/efi/efi_stub.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

Reviewed-by Simon Glass <[email protected]>

It is possible to create a generic EFI payload for all x86 boards. The payload is configured to include as many generic drivers as possible. All stuff that touches low-level initialization are not allowed as such is the EFI BIOS's responsibility. Platform specific drivers (like gpio, spi, etc) are not included.

Signed-off-by Bin Meng <[email protected]>

---

 arch/x86/dts/Makefile                 |  1 +
 arch/x86/dts/efi-x86_payload.dts      | 36 +++++++++++++++++++++++++++++++++
 board/efi/Kconfig                     |  9 +++++++++
 board/efi/efi-x86_payload/Kconfig     | 38 +++++++++++++++++++++++++++++++++++
 board/efi/efi-x86_payload/MAINTAINERS |  7 +++++++
 board/efi/efi-x86_payload/Makefile    |  5 +++++
 board/efi/efi-x86_payload/start.S     |  8 ++++++++
 configs/efi-x86_payload32_defconfig   | 35 ++++++++++++++++++++++++++++++++
 configs/efi-x86_payload64_defconfig   | 36 +++++++++++++++++++++++++++++++++
 include/configs/efi-x86_payload.h     | 32 +++++++++++++++++++++++++++++
 10 files changed, 207 insertions(+)

 create mode 100644 arch/x86/dts/efi-x86_payload.dts
 create mode 100644 board/efi/efi-x86_payload/Kconfig
 create mode 100644 board/efi/efi-x86_payload/MAINTAINERS
 create mode 100644 board/efi/efi-x86_payload/Makefile
 create mode 100644 board/efi/efi-x86_payload/start.S
 create mode 100644 configs/efi-x86_payload32_defconfig
 create mode 100644 configs/efi-x86_payload64_defconfig
 create mode 100644 include/configs/efi-x86_payload.h

Now that we have generic EFI payload support for all x86 boards, drop the QEMU-specific one.

Signed-off-by Bin Meng <[email protected]>

---

 arch/x86/cpu/qemu/Makefile               |  2 --
 arch/x86/cpu/qemu/qemu.c                 |  4 ---
 board/emulation/qemu-x86/Kconfig         |  6 ++--
 configs/qemu-x86_efi_payload32_defconfig | 47 -------------------------------
 configs/qemu-x86_efi_payload64_defconfig | 48 --------------------------------
 5 files changed, 3 insertions(+), 104 deletions(-)
 delete mode 100644 configs/qemu-x86_efi_payload32_defconfig
 delete mode 100644 configs/qemu-x86_efi_payload64_defconfig

Now that we have generic EFI payload support, drop EFI-specific test logics in BayTrail Kconfig and codes, and all BayTrail boards too.

Signed-off-by Bin Meng <[email protected]>

---

 arch/x86/cpu/baytrail/Kconfig                  | 6 +++---
 arch/x86/cpu/baytrail/valleyview.c             | 3 ---
 board/advantech/som-db5800-som-6867/Kconfig    | 5 ++---
 board/congatec/conga-qeval20-qa3-e3845/Kconfig | 5 ++---
 board/dfi/dfi-bt700/Kconfig                    | 5 ++---
 board/intel/minnowmax/Kconfig                  | 5 ++---
 6 files changed, 11 insertions(+), 18 deletions(-)

If UEFI BIOS has the graphics output protocol (GOP), let's pass its information to U-Boot payload so that U-Boot can utilize it (eg:
an EFI framebuffer driver).

Signed-off-by Bin Meng <[email protected]>

Quoted 5 lines · Bin Meng

If UEFI BIOS has the graphics output protocol (GOP), let's pass its information to U-Boot payload so that U-Boot can utilize it (eg:
an EFI framebuffer driver).

Signed-off-by Bin Meng <[email protected]>

Why can't the FB drive determine all of this on its own and just fail probe if no GOP protocol can be found?

Quoted 7 lines · Alexander Graf

Quoted 4 lines · Bin Meng

If UEFI BIOS has the graphics output protocol (GOP), let's pass its information to U-Boot payload so that U-Boot can utilize it (eg:
an EFI framebuffer driver).

Quoted 5 lines

Quoted 1 line

Signed-off-by Bin Meng <[email protected]>

Why can't the FB drive determine all of this on its own and just fail probe if no GOP protocol can be found?

It cannot. Once U-Boot payload is running, the boot services are gone. There is no way to determine the GOP protocol.

Quoted 14 lines · Bin Meng

Quoted 12 lines · Alexander Graf

Quoted 5 lines · Bin Meng

If UEFI BIOS has the graphics output protocol (GOP), let's pass its information to U-Boot payload so that U-Boot can utilize it (eg:
an EFI framebuffer driver).

Signed-off-by Bin Meng <[email protected]>

Why can't the FB drive determine all of this on its own and just fail probe if no GOP protocol can be found?

Quoted 2 lines

It cannot. Once U-Boot payload is running, the boot services are gone. There is no way to determine the GOP protocol.

Interesting. Is there a particular reason you're not preserving boot services?

It might be quite handy to reuse UEFI block interfaces for example. It might also be useful to preserve (parts of?) the memory map so we don't overwrite random SMM code for example.

Maybe I misunderstand the purpose of the UEFI U-Boot support though ;).

Hi Alex,

Quoted 15 lines · Alexander Graf

Quoted 13 lines · Bin Meng

Quoted 10 lines · Alexander Graf

Quoted 5 lines · Bin Meng

If UEFI BIOS has the graphics output protocol (GOP), let's pass its information to U-Boot payload so that U-Boot can utilize it (eg:
an EFI framebuffer driver).

Signed-off-by Bin Meng <[email protected]>

Why can't the FB drive determine all of this on its own and just fail probe if no GOP protocol can be found?

It cannot. Once U-Boot payload is running, the boot services are gone. There is no way to determine the GOP protocol.

Quoted 3 lines

Interesting. Is there a particular reason you're not preserving boot services?

This is EFI payload support with CONFIG_EFI_STUB. Preserving boot services is EFI application, with CONFIG_EFI_APP. For example, see serial_efi.c which is the serial driver that uses EFI's boot services to output characters on the serial port.

Quoted 6 lines

It might be quite handy to reuse UEFI block interfaces for example. It might also be useful to preserve (parts of?) the memory map so we don't overwrite random SMM code for example.

Maybe I misunderstand the purpose of the UEFI U-Boot support though ;).

Looks there is some misunderstanding around the EFI payload support.

Quoted 25 lines · Bin Meng

Hi Alex,

Quoted 21 lines · Alexander Graf

Quoted 17 lines · Bin Meng

Quoted 4 lines · Alexander Graf

Quoted 1 line · Bin Meng

Quoted 9 lines

Quoted 5 lines

If UEFI BIOS has the graphics output protocol (GOP), let's pass its information to U-Boot payload so that U-Boot can utilize it (eg:
an EFI framebuffer driver).

Signed-off-by Bin Meng <[email protected]>

Why can't the FB drive determine all of this on its own and just fail probe if no GOP protocol can be found?

It cannot. Once U-Boot payload is running, the boot services are gone. There is no way to determine the GOP protocol.

Quoted 7 lines

Quoted 3 lines

Interesting. Is there a particular reason you're not preserving boot services?

This is EFI payload support with CONFIG_EFI_STUB. Preserving boot services is EFI application, with CONFIG_EFI_APP. For example, see serial_efi.c which is the serial driver that uses EFI's boot services to output characters on the serial port.

Oh, I see. That makes sense now.

Do people actually need CONFIG_EFI_STUB then?

Quoted 6 lines

Quoted 6 lines

It might be quite handy to reuse UEFI block interfaces for example. It might also be useful to preserve (parts of?) the memory map so we don't overwrite random SMM code for example.

Quoted 3 lines

Quoted 1 line

Maybe I misunderstand the purpose of the UEFI U-Boot support though ;).

Looks there is some misunderstanding around the EFI payload support.

Yes, I have to admit that I never really cared enough about it to read its documentation though :).

Hi Alex,

Quoted 22 lines · Alexander Graf

Quoted 21 lines · Bin Meng

Hi Alex,

Quoted 18 lines · Alexander Graf

Quoted 7 lines · Bin Meng

Quoted 6 lines · Alexander Graf

Quoted 4 lines · Bin Meng

If UEFI BIOS has the graphics output protocol (GOP), let's pass its information to U-Boot payload so that U-Boot can utilize it (eg:
an EFI framebuffer driver).

Quoted 6 lines

Quoted 4 lines

Quoted 1 line

Signed-off-by Bin Meng <[email protected]>

Why can't the FB drive determine all of this on its own and just fail probe if no GOP protocol can be found?

It cannot. Once U-Boot payload is running, the boot services are gone. There is no way to determine the GOP protocol.

Interesting. Is there a particular reason you're not preserving boot services?

Quoted 8 lines

Quoted 4 lines

This is EFI payload support with CONFIG_EFI_STUB. Preserving boot services is EFI application, with CONFIG_EFI_APP. For example, see serial_efi.c which is the serial driver that uses EFI's boot services to output characters on the serial port.

Oh, I see. That makes sense now.

Do people actually need CONFIG_EFI_STUB then?

I think you wanted to say: Do people actually need CONFIG_EFI_APP then? CONFIG_EFI_STUB is more useful than CONFIG_EFI_APP. The application support is very limited.

As the README.u-boot_on_efi says:

Running U-Boot on EFI is useful in several situations:

- You have EFI running on a board but U-Boot does not natively support it fully yet. You can boot into U-Boot from EFI and use that until U-Boot is fully ported

- You need to use an EFI implementation (e.g. UEFI) because your vendor requires it in order to provide support

- You plan to use coreboot to boot into U-Boot but coreboot support does not currently exist for your platform. In the meantime you can use U-Boot on EFI and then move to U-Boot on coreboot when ready

- You use EFI but want to experiment with a simpler alternative like U-Boot

Right now, I have one Intel SkyLake board, and before I get a native U-Boot up and running as the 1st stage bootloader on this board, I can run U-Boot as the EFI payload to experiment something, which is very handy.

Quoted 63 lines · Bin Meng

Hi Alex,

Quoted 36 lines · Alexander Graf

Quoted 25 lines · Bin Meng

Hi Alex,

Quoted 21 lines · Alexander Graf

Quoted 17 lines · Bin Meng

Quoted 4 lines · Alexander Graf

Quoted 1 line · Bin Meng

Quoted 9 lines

Quoted 5 lines

If UEFI BIOS has the graphics output protocol (GOP), let's pass its information to U-Boot payload so that U-Boot can utilize it (eg:
an EFI framebuffer driver).

Signed-off-by Bin Meng <[email protected]>

Why can't the FB drive determine all of this on its own and just fail probe if no GOP protocol can be found?

It cannot. Once U-Boot payload is running, the boot services are gone. There is no way to determine the GOP protocol.

Quoted 7 lines

Quoted 3 lines

Interesting. Is there a particular reason you're not preserving boot services?

This is EFI payload support with CONFIG_EFI_STUB. Preserving boot services is EFI application, with CONFIG_EFI_APP. For example, see serial_efi.c which is the serial driver that uses EFI's boot services to output characters on the serial port.

Quoted 3 lines

Oh, I see. That makes sense now.

Do people actually need CONFIG_EFI_STUB then?

I think you wanted to say: Do people actually need CONFIG_EFI_APP then? CONFIG_EFI_STUB is more useful than CONFIG_EFI_APP. The application support is very limited.

As the README.u-boot_on_efi says:

Running U-Boot on EFI is useful in several situations:

- You have EFI running on a board but U-Boot does not natively support it fully yet. You can boot into U-Boot from EFI and use that until U-Boot is fully ported

- You need to use an EFI implementation (e.g. UEFI) because your vendor requires it in order to provide support

- You plan to use coreboot to boot into U-Boot but coreboot support does not currently exist for your platform. In the meantime you can use U-Boot on EFI and then move to U-Boot on coreboot when ready

Quoted 7 lines

- You use EFI but want to experiment with a simpler alternative like U-Boot

Right now, I have one Intel SkyLake board, and before I get a native U-Boot up and running as the 1st stage bootloader on this board, I can run U-Boot as the EFI payload to experiment something, which is very handy.

Oh, I see. So it's mostly used as bringup aid. In that case I agree that the stub bit makes a lot of sense.

The one thing that really bites us with the stub and different bitness is when you want to use efi_api.h again from within U-Boot. The obvious example is a 32bit U-Boot built as 64bit payload, allowing 32bit UEFI applications to run inside.

Do you think we could just agree on efi.h to not consume any stub config options and just determine things from its build environment instead? That way it automatically adapts according to the -m32/-m64 build flag for the stub and we would not need to worry about the stub in generic code.

Something like the patch below? If yes, I'll resend it with proper indentation :).

include/efi.h +3 -7

index 98bddbac1a..5e1e8ac78c 100644
@@ -19,12 +19,12 @@
 #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 */
+#ifdef __x86_64__
 #define EFIAPI __attribute__((ms_abi))
 #else
 #define EFIAPI asmlinkage
-#endif
+#endif /* __x86_64__ */
 
 struct efi_device_path;

@@ -32,16 +32,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

If UEFI BIOS has the graphics output protocol (GOP), let's pass its information to U-Boot payload so that U-Boot can utilize it (eg:
an EFI framebuffer driver).

Signed-off-by Bin Meng <[email protected]>

---

 include/efi.h      | 35 +++++++++++++++++++++++++++++++++++
 lib/efi/efi_stub.c | 15 +++++++++++++++
 2 files changed, 50 insertions(+)

This adds a DM video driver for U-Boot as the EFI payload. The driver makes use of all necessary information from the passed EFI GOP info to create a linear framebuffer device, as if it were initialized by U-Boot itself.

Signed-off-by Bin Meng <[email protected]>

This turns on the EFI framebuffer driver support so that a graphics console can be of additional help.

Signed-off-by Bin Meng <[email protected]>

---

 arch/x86/dts/efi-x86_payload.dts  | 4 ++++
 board/efi/efi-x86_payload/Kconfig | 1 +
 2 files changed, 5 insertions(+)

To avoid confusion, let's rename the efi-x86 target to efi-x86_app.

Signed-off-by Bin Meng <[email protected]>

---

 arch/x86/cpu/efi/Makefile                            | 2 +-
 arch/x86/cpu/efi/{efi.c => app.c}                    | 0
 arch/x86/cpu/intel_common/Makefile                   | 2 +-
 arch/x86/dts/Makefile                                | 2 +-
 arch/x86/dts/{efi.dts => efi-x86_app.dts}            | 4 ++--
 board/efi/Kconfig                                    | 6 +++---
 board/efi/{efi-x86 => efi-x86_app}/Kconfig           | 6 +++---
 board/efi/{efi-x86 => efi-x86_app}/MAINTAINERS       | 0
 board/efi/{efi-x86 => efi-x86_app}/Makefile          | 2 +-
 board/efi/{efi-x86/efi.c => efi-x86_app/app.c}       | 0
 configs/{efi-x86_defconfig => efi-x86_app_defconfig} | 4 ++--
 include/configs/{efi-x86.h => efi-x86_app.h}         | 0
 12 files changed, 14 insertions(+), 14 deletions(-)

 rename arch/x86/cpu/efi/{efi.c => app.c} (100%)
 rename arch/x86/dts/{efi.dts => efi-x86_app.dts} (82%)
 rename board/efi/{efi-x86 => efi-x86_app}/Kconfig (64%)
 rename board/efi/{efi-x86 => efi-x86_app}/MAINTAINERS (100%)
 rename board/efi/{efi-x86 => efi-x86_app}/Makefile (82%)
 rename board/efi/{efi-x86/efi.c => efi-x86_app/app.c} (100%)
 rename configs/{efi-x86_defconfig => efi-x86_app_defconfig} (92%)
 rename include/configs/{efi-x86.h => efi-x86_app.h} (100%)

Quoted 18 lines · Bin Meng

To avoid confusion, let's rename the efi-x86 target to efi-x86_app.

Signed-off-by Bin Meng <[email protected]>

---

 arch/x86/cpu/efi/Makefile                            | 2 +-
 arch/x86/cpu/efi/{efi.c => app.c}                    | 0
 arch/x86/cpu/intel_common/Makefile                   | 2 +-
 arch/x86/dts/Makefile                                | 2 +-
 arch/x86/dts/{efi.dts => efi-x86_app.dts}            | 4 ++--
 board/efi/Kconfig                                    | 6 +++---
 board/efi/{efi-x86 => efi-x86_app}/Kconfig           | 6 +++---
 board/efi/{efi-x86 => efi-x86_app}/MAINTAINERS       | 0
 board/efi/{efi-x86 => efi-x86_app}/Makefile          | 2 +-
 board/efi/{efi-x86/efi.c => efi-x86_app/app.c}       | 0
 configs/{efi-x86_defconfig => efi-x86_app_defconfig} | 4 ++--
 include/configs/{efi-x86.h => efi-x86_app.h}         | 0
 12 files changed, 14 insertions(+), 14 deletions(-)

Quoted 9 lines

 rename arch/x86/cpu/efi/{efi.c => app.c} (100%)
 rename arch/x86/dts/{efi.dts => efi-x86_app.dts} (82%)
 rename board/efi/{efi-x86 => efi-x86_app}/Kconfig (64%)
 rename board/efi/{efi-x86 => efi-x86_app}/MAINTAINERS (100%)
 rename board/efi/{efi-x86 => efi-x86_app}/Makefile (82%)
 rename board/efi/{efi-x86/efi.c => efi-x86_app/app.c} (100%)
 rename configs/{efi-x86_defconfig => efi-x86_app_defconfig} (92%)
 rename include/configs/{efi-x86.h => efi-x86_app.h} (100%)

Reviewed-by Simon Glass <[email protected]>

Is a docs update needed too?

Currently when EFI application boots, it says:

CPU: x86_64, vendor <invalid cpu vendor>, device 0h

Fix this by calling x86_cpu_init_f() in arch_cpu_init().

Signed-off-by Bin Meng <[email protected]>

---

 arch/x86/cpu/efi/app.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Unfortunately the EFI x86 application and payload support have been broken for quite a while. This series addresses various bug fixes, plus several enhancements like toolchain compatibility (eg: kernel.org i386-linux-gcc), introduction of generic EFI payload which can run on any x86 board, and a EFI framebuffer driver for the payload.

This series is available at u-boot-x86/efi-working for testing.

Hi Bin,

Quoted 6 lines · Bin Meng

Unfortunately the EFI x86 application and payload support have been broken for quite a while. This series addresses various bug fixes, plus several enhancements like toolchain compatibility (eg: kernel.org i386-linux-gcc), introduction of generic EFI payload which can run on any x86 board, and a EFI framebuffer driver for the payload.

Quoted 1 line

This series is available at u-boot-x86/efi-working for testing.

I wonder if it is possible to put qemu testing of the EFI stuff into travis?

Also, are the problems only with the i386 compiler, or are things truly broken everywhere?

Hi Simon,

Quoted 9 lines · Simon Glass

Hi Bin,

Quoted 6 lines · Bin Meng

Unfortunately the EFI x86 application and payload support have been broken for quite a while. This series addresses various bug fixes, plus several enhancements like toolchain compatibility (eg: kernel.org i386-linux-gcc), introduction of generic EFI payload which can run on any x86 board, and a EFI framebuffer driver for the payload.

Quoted 3 lines

Quoted 1 line

This series is available at u-boot-x86/efi-working for testing.

I wonder if it is possible to put qemu testing of the EFI stuff into travis?

I will take a look.

Quoted 4 lines

Also, are the problems only with the i386 compiler, or are things truly broken everywhere?

Current mainline status of the EFI stuff is:

1. EFI application does not boot
2. EFI 32-bit payload boots fine
3. EFI 64-bit payload does not boot
4. i386-linux-gcc compiler does not build EFI 64-bit payload, but can build 32-bit payload and application
5. x86_64-linux-gcc compiler can build everything