[PATCH 0/6] x86: efi: Add a 64-bit coreboot payload

[PATCH 0/6] x86: efi: Add a 64-bit coreboot payload

[Simon Glass]

This series creates a new 64-bit 'coreboot64' build which can be launched
from coreboot. It uses SPL to effect the jump to 64-bit mode.

This was done in an attempt to get the Ubuntu FirmwareTestSuite[1]
working. Unfortunately the latest version shows a splash screen and hangs,
perhaps due to some missing EFI support.

[1] https://wiki.ubuntu.com/FirmwareTestSuite


Simon Glass (6):
  x86: Allow building an SPL image for coreboot
  x86: Move work-around out of cpu_jump_to_64bit_uboot()
  x86: Update SPL for coreboot
  x86: cbfs: Drop unwanted declaration
  x86: coreboot: Allow building an SPL image
  x86: Add a 64-bit coreboot build

 Makefile                             |  6 ++++
 arch/x86/cpu/Makefile                |  4 ++-
 arch/x86/cpu/coreboot/Kconfig        |  1 +
 arch/x86/cpu/coreboot/Makefile       |  8 ++++-
 arch/x86/cpu/coreboot/coreboot.c     |  3 +-
 arch/x86/cpu/coreboot/coreboot_spl.c | 12 +++++++
 arch/x86/cpu/i386/cpu.c              | 10 ------
 arch/x86/cpu/intel_common/Makefile   |  2 ++
 arch/x86/cpu/x86_64/cpu.c            |  2 ++
 arch/x86/dts/coreboot-u-boot.dtsi    | 18 +++++++++++
 arch/x86/lib/spl.c                   | 23 ++++++++++---
 board/coreboot/coreboot/MAINTAINERS  |  7 ++++
 configs/coreboot64_defconfig         | 48 ++++++++++++++++++++++++++++
 doc/board/coreboot/coreboot.rst      | 10 ++++++
 include/cbfs.h                       |  2 +-
 15 files changed, 138 insertions(+), 18 deletions(-)
 create mode 100644 arch/x86/cpu/coreboot/coreboot_spl.c
 create mode 100644 arch/x86/dts/coreboot-u-boot.dtsi
 create mode 100644 configs/coreboot64_defconfig

-- 
2.26.0.292.g33ef6b2f38-goog

[PATCH 1/6] x86: Allow building an SPL image for coreboot

[Simon Glass]

Coreboot runs in 32-bit mode and cannot run a 64-bit U-Boot. To get around
this we can build a combined image with 32-bit SPL and 64-bit U-Boot. Add
a build rule and binman definition for this.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 Makefile                          |  6 ++++++
 arch/x86/cpu/coreboot/Kconfig     |  1 +
 arch/x86/dts/coreboot-u-boot.dtsi | 18 ++++++++++++++++++
 3 files changed, 25 insertions(+)
 create mode 100644 arch/x86/dts/coreboot-u-boot.dtsi

diff --git a/Makefile b/Makefile
index 0db44d42494..5249f1e9610 100644
--- a/Makefile
+++ b/Makefile
@@ -884,6 +884,9 @@ ALL-$(CONFIG_EFI_STUB) += u-boot-payload.efi
 ifneq ($(BUILD_ROM)$(CONFIG_BUILD_ROM),)
 ALL-$(CONFIG_X86_RESET_VECTOR) += u-boot.rom
 endif
+ifeq ($(CONFIG_SYS_COREBOOT)$(CONFIG_SPL),yy)
+ALL-$(CONFIG_BINMAN) += u-boot-x86-with-spl.bin
+endif
 
 # Build a combined spl + u-boot image for sunxi
 ifeq ($(CONFIG_ARCH_SUNXI)$(CONFIG_SPL),yy)
@@ -1563,6 +1566,9 @@ u-boot-sunxi-with-spl.bin: spl/sunxi-spl.bin u-boot.itb FORCE
 endif
 endif
 
+u-boot-x86-with-spl.bin: spl/u-boot-spl.bin u-boot.bin FORCE
+	$(call if_changed,binman)
+
 ifneq ($(CONFIG_TEGRA),)
 ifneq ($(CONFIG_BINMAN),)
 # Makes u-boot-dtb-tegra.bin u-boot-tegra.bin u-boot-nodtb-tegra.bin
diff --git a/arch/x86/cpu/coreboot/Kconfig b/arch/x86/cpu/coreboot/Kconfig
index c8e6a889d02..497d6284ac1 100644
--- a/arch/x86/cpu/coreboot/Kconfig
+++ b/arch/x86/cpu/coreboot/Kconfig
@@ -25,5 +25,6 @@ config SYS_COREBOOT
 	imply FS_CBFS
 	imply CBMEM_CONSOLE
 	imply X86_TSC_READ_BASE
+	select BINMAN if X86_64
 
 endif
diff --git a/arch/x86/dts/coreboot-u-boot.dtsi b/arch/x86/dts/coreboot-u-boot.dtsi
new file mode 100644
index 00000000000..38efc48d836
--- /dev/null
+++ b/arch/x86/dts/coreboot-u-boot.dtsi
@@ -0,0 +1,18 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2020 Google LLC
+ * Written by Simon Glass <sjg@chromium.org>
+ */
+
+#include <config.h>
+
+/ {
+	binman {
+		filename = "u-boot-x86-with-spl.bin";
+		u-boot-spl {
+		};
+		u-boot {
+			offset = <0x10000>;
+		};
+	};
+};
-- 
2.26.0.292.g33ef6b2f38-goog

[PATCH 2/6] x86: Move work-around out of cpu_jump_to_64bit_uboot()

[Simon Glass]

At present this function copies U-Boot from the last 1MB of ROM. This is
not the right way to do it. Instead, the binman symbol should provide the
location.

But in any case the code should live in the caller,
spl_board_load_image(), so that the 64-bit jump function can be used
elsewhere. Move it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/x86/cpu/i386/cpu.c | 10 ----------
 arch/x86/lib/spl.c      | 13 +++++++++++++
 2 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/arch/x86/cpu/i386/cpu.c b/arch/x86/cpu/i386/cpu.c
index c8da7f10e9b..45416a9be5b 100644
--- a/arch/x86/cpu/i386/cpu.c
+++ b/arch/x86/cpu/i386/cpu.c
@@ -611,16 +611,6 @@ int cpu_jump_to_64bit_uboot(ulong target)
 
 	func = (func_t)ptr;
 
-	/*
-	 * Copy U-Boot from ROM
-	 * TODO(sjg at chromium.org): Figure out a way to get the text base
-	 * correctly here, and in the device-tree binman definition.
-	 *
-	 * Also consider using FIT so we get the correct image length and
-	 * parameters.
-	 */
-	memcpy((char *)target, (char *)0xfff00000, 0x100000);
-
 	/* Jump to U-Boot */
 	func((ulong)pgtable, 0, (ulong)target);
 
diff --git a/arch/x86/lib/spl.c b/arch/x86/lib/spl.c
index 90baec2a175..95a89c072d5 100644
--- a/arch/x86/lib/spl.c
+++ b/arch/x86/lib/spl.c
@@ -207,6 +207,19 @@ static int spl_board_load_image(struct spl_image_info *spl_image,
 	spl_image->os = IH_OS_U_BOOT;
 	spl_image->name = "U-Boot";
 
+	if (!IS_ENABLED(CONFIG_SYS_COREBOOT)) {
+		/*
+		 * Copy U-Boot from ROM
+		 * TODO(sjg at chromium.org): Figure out a way to get the text base
+		 * correctly here, and in the device-tree binman definition.
+		 *
+		 * Also consider using FIT so we get the correct image length
+		 * and parameters.
+		 */
+		memcpy((char *)spl_image->load_addr, (char *)0xfff00000,
+		       0x100000);
+	}
+
 	debug("Loading to %lx\n", spl_image->load_addr);
 
 	return 0;
-- 
2.26.0.292.g33ef6b2f38-goog

[PATCH 3/6] x86: Update SPL for coreboot

[Simon Glass]

At present SPL only works on bare-metal builds. With a few tweaks it can
be used for coreboot also.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/x86/lib/spl.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/x86/lib/spl.c b/arch/x86/lib/spl.c
index 95a89c072d5..212b4d596d2 100644
--- a/arch/x86/lib/spl.c
+++ b/arch/x86/lib/spl.c
@@ -63,7 +63,7 @@ static int x86_spl_init(void)
 	 * is not needed. We could make this a CONFIG option or perhaps
 	 * place it immediately below CONFIG_SYS_TEXT_BASE.
 	 */
-	char *ptr = (char *)0x110000;
+	__maybe_unused char *ptr = (char *)0x110000;
 #else
 	struct udevice *punit;
 #endif
@@ -111,7 +111,8 @@ static int x86_spl_init(void)
 			      __func__, ret);
 	}
 
-#ifndef CONFIG_TPL
+#ifndef CONFIG_SYS_COREBOOT
+# ifndef CONFIG_TPL
 	memset(&__bss_start, 0, (ulong)&__bss_end - (ulong)&__bss_start);
 
 	/* TODO(sjg at chromium.org): Consider calling cpu_init_r() here */
@@ -140,7 +141,7 @@ static int x86_spl_init(void)
 		return ret;
 	}
 	mtrr_commit(true);
-#else
+# else
 	ret = syscon_get_by_driver_data(X86_SYSCON_PUNIT, &punit);
 	if (ret)
 		debug("Could not find PUNIT (err=%d)\n", ret);
@@ -148,6 +149,7 @@ static int x86_spl_init(void)
 	ret = set_max_freq();
 	if (ret)
 		debug("Failed to set CPU frequency (err=%d)\n", ret);
+# endif
 #endif
 
 	return 0;
@@ -162,7 +164,7 @@ void board_init_f(ulong flags)
 		debug("Error %d\n", ret);
 		panic("x86_spl_init fail");
 	}
-#ifdef CONFIG_TPL
+#if IS_ENABLED(CONFIG_TPL) || IS_ENABLED(CONFIG_SYS_COREBOOT)
 	gd->bd = malloc(sizeof(*gd->bd));
 	if (!gd->bd) {
 		printf("Out of memory for bd_info size %x\n", sizeof(*gd->bd));
-- 
2.26.0.292.g33ef6b2f38-goog

[PATCH 4/6] x86: cbfs: Drop unwanted declaration

[Simon Glass]

The intention here is add a forward declaration, not actually declare a
variable. Fix it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 include/cbfs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/cbfs.h b/include/cbfs.h
index f3bc8ca24af..d915f9426d8 100644
--- a/include/cbfs.h
+++ b/include/cbfs.h
@@ -135,7 +135,7 @@ void file_cbfs_get_next(const struct cbfs_cachenode **file);
  */
 const struct cbfs_cachenode *file_cbfs_find(const char *name);
 
-struct cbfs_priv *priv;
+struct cbfs_priv;
 
 /**
  * cbfs_find_file() - Find a file in a given CBFS
-- 
2.26.0.292.g33ef6b2f38-goog

[PATCH 5/6] x86: coreboot: Allow building an SPL image

[Simon Glass]

Make a few adjustments to allow us to build an SPL image for coreboot.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

 arch/x86/cpu/Makefile                |  4 +++-
 arch/x86/cpu/coreboot/Makefile       |  8 +++++++-
 arch/x86/cpu/coreboot/coreboot.c     |  3 ++-
 arch/x86/cpu/coreboot/coreboot_spl.c | 12 ++++++++++++
 arch/x86/cpu/intel_common/Makefile   |  2 ++
 arch/x86/cpu/x86_64/cpu.c            |  2 ++
 6 files changed, 28 insertions(+), 3 deletions(-)
 create mode 100644 arch/x86/cpu/coreboot/coreboot_spl.c

diff --git a/arch/x86/cpu/Makefile b/arch/x86/cpu/Makefile
index 307267a8fb3..ee0499f5d79 100644
--- a/arch/x86/cpu/Makefile
+++ b/arch/x86/cpu/Makefile
@@ -54,9 +54,11 @@ obj-$(CONFIG_INTEL_QUARK) += quark/
 obj-$(CONFIG_INTEL_QUEENSBAY) += queensbay/
 obj-$(CONFIG_INTEL_TANGIER) += tangier/
 obj-$(CONFIG_APIC) += lapic.o ioapic.o
-obj-$(CONFIG_$(SPL_TPL_)X86_32BIT_INIT) += irq.o
 obj-$(CONFIG_$(SPL_TPL_)ACPI_GPE) += acpi_gpe.o
 obj-$(CONFIG_QFW) += qfw_cpu.o
+ifndef CONFIG_SYS_COREBOOT
+obj-$(CONFIG_$(SPL_TPL_)X86_32BIT_INIT) += irq.o
+endif
 ifndef CONFIG_$(SPL_)X86_64
 obj-$(CONFIG_SMP) += mp_init.o
 endif
diff --git a/arch/x86/cpu/coreboot/Makefile b/arch/x86/cpu/coreboot/Makefile
index 35b15bb1da8..605f90304e3 100644
--- a/arch/x86/cpu/coreboot/Makefile
+++ b/arch/x86/cpu/coreboot/Makefile
@@ -11,8 +11,14 @@
 # (C) Copyright 2002
 # Daniel Engstr?m, Omicron Ceti AB, daniel at omicron.se.
 
+ifndef CONFIG_SPL
 obj-y += car.o
+endif
+ifdef CONFIG_SPL_BUILD
+obj-y += coreboot_spl.o
+else
+obj-y += sdram.o
+endif
 obj-y += coreboot.o
 obj-y += tables.o
-obj-y += sdram.o
 obj-y += timestamp.o
diff --git a/arch/x86/cpu/coreboot/coreboot.c b/arch/x86/cpu/coreboot/coreboot.c
index 0c4c6348d1d..624caf67a6a 100644
--- a/arch/x86/cpu/coreboot/coreboot.c
+++ b/arch/x86/cpu/coreboot/coreboot.c
@@ -27,7 +27,8 @@ int arch_cpu_init(void)
 
 	timestamp_init();
 
-	return x86_cpu_init_f();
+	return IS_ENABLED(CONFIG_X86_RUN_64BIT) ? x86_cpu_reinit_f() :
+		 x86_cpu_init_f();
 }
 
 int checkcpu(void)
diff --git a/arch/x86/cpu/coreboot/coreboot_spl.c b/arch/x86/cpu/coreboot/coreboot_spl.c
new file mode 100644
index 00000000000..36661871e92
--- /dev/null
+++ b/arch/x86/cpu/coreboot/coreboot_spl.c
@@ -0,0 +1,12 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2020 Google LLC
+ */
+
+#include <common.h>
+#include <init.h>
+
+int dram_init(void)
+{
+	return 0;
+}
diff --git a/arch/x86/cpu/intel_common/Makefile b/arch/x86/cpu/intel_common/Makefile
index 1736bd2b530..374803b8760 100644
--- a/arch/x86/cpu/intel_common/Makefile
+++ b/arch/x86/cpu/intel_common/Makefile
@@ -32,6 +32,8 @@ obj-$(CONFIG_HAVE_P2SB) += p2sb.o
 
 ifdef CONFIG_SPL
 ifndef CONFIG_SPL_BUILD
+ifndef CONFIG_SYS_COREBOOT
 obj-y += cpu_from_spl.o
 endif
 endif
+endif
diff --git a/arch/x86/cpu/x86_64/cpu.c b/arch/x86/cpu/x86_64/cpu.c
index 90925e46ea2..4b64339f259 100644
--- a/arch/x86/cpu/x86_64/cpu.c
+++ b/arch/x86/cpu/x86_64/cpu.c
@@ -53,6 +53,7 @@ int misc_init_r(void)
 	return 0;
 }
 
+#ifndef CONFIG_SYS_COREBOOT
 int checkcpu(void)
 {
 	return 0;
@@ -62,6 +63,7 @@ int print_cpuinfo(void)
 {
 	return 0;
 }
+#endif
 
 int x86_cpu_reinit_f(void)
 {
-- 
2.26.0.292.g33ef6b2f38-goog

[PATCH 6/6] x86: Add a 64-bit coreboot build

[Simon Glass]

Add a build for running 64-bit U-Boot from coreboot (which is 32-bit).
This uses binman to create an image with a 32-bit SPL and a 64-bit U-Boot.

Coreboot boots into SPL and then SPL boots into U-Boot.

This allows running 64-bit EFI images on x86.
Signed-off-by: Simon Glass <sjg@chromium.org>
---

 board/coreboot/coreboot/MAINTAINERS |  7 +++++
 configs/coreboot64_defconfig        | 48 +++++++++++++++++++++++++++++
 doc/board/coreboot/coreboot.rst     | 10 ++++++
 3 files changed, 65 insertions(+)
 create mode 100644 configs/coreboot64_defconfig

diff --git a/board/coreboot/coreboot/MAINTAINERS b/board/coreboot/coreboot/MAINTAINERS
index 188906b0803..a05673bb0be 100644
--- a/board/coreboot/coreboot/MAINTAINERS
+++ b/board/coreboot/coreboot/MAINTAINERS
@@ -4,3 +4,10 @@ S:	Maintained
 F:	board/coreboot/coreboot/
 F:	include/configs/chromebook_link.h
 F:	configs/coreboot_defconfig
+
+COREBOOT64 BOARD
+M:	Simon Glass <sjg@chromium.org>
+S:	Maintained
+F:	board/coreboot/coreboot/
+F:	include/configs/chromebook_link.h
+F:	configs/coreboot64_defconfig
diff --git a/configs/coreboot64_defconfig b/configs/coreboot64_defconfig
new file mode 100644
index 00000000000..80353b8eb36
--- /dev/null
+++ b/configs/coreboot64_defconfig
@@ -0,0 +1,48 @@
+CONFIG_X86=y
+CONFIG_SYS_TEXT_BASE=0x1120000
+CONFIG_ENV_SIZE=0x1000
+CONFIG_NR_DRAM_BANKS=8
+CONFIG_PRE_CON_BUF_ADDR=0x100000
+CONFIG_X86_RUN_64BIT=y
+CONFIG_VENDOR_COREBOOT=y
+CONFIG_TARGET_COREBOOT=y
+CONFIG_SPL_TEXT_BASE=0x1110000
+CONFIG_FIT=y
+CONFIG_FIT_SIGNATURE=y
+CONFIG_SHOW_BOOT_PROGRESS=y
+CONFIG_USE_BOOTARGS=y
+CONFIG_BOOTARGS="root=/dev/sdb3 init=/sbin/init rootwait ro"
+CONFIG_PRE_CONSOLE_BUFFER=y
+CONFIG_SYS_CONSOLE_INFO_QUIET=y
+CONFIG_DISPLAY_BOARDINFO_LATE=y
+CONFIG_LAST_STAGE_INIT=y
+CONFIG_HUSH_PARSER=y
+CONFIG_CMD_IDE=y
+CONFIG_CMD_MMC=y
+CONFIG_CMD_PART=y
+CONFIG_CMD_USB=y
+# CONFIG_CMD_SETEXPR is not set
+CONFIG_CMD_DHCP=y
+# CONFIG_CMD_NFS is not set
+CONFIG_CMD_PING=y
+CONFIG_CMD_TIME=y
+CONFIG_CMD_SOUND=y
+CONFIG_CMD_EXT2=y
+CONFIG_CMD_EXT4=y
+CONFIG_CMD_EXT4_WRITE=y
+CONFIG_CMD_FAT=y
+CONFIG_CMD_FS_GENERIC=y
+CONFIG_MAC_PARTITION=y
+# CONFIG_SPL_MAC_PARTITION is not set
+# CONFIG_SPL_DOS_PARTITION is not set
+CONFIG_ISO_PARTITION=y
+CONFIG_EFI_PARTITION=y
+# CONFIG_SPL_EFI_PARTITION is not set
+CONFIG_DEFAULT_DEVICE_TREE="coreboot"
+CONFIG_SYS_RELOC_GD_ENV_ADDR=y
+CONFIG_REGMAP=y
+CONFIG_SYSCON=y
+# CONFIG_PCI_PNP is not set
+CONFIG_SOUND=y
+CONFIG_SOUND_I8254=y
+CONFIG_CONSOLE_SCROLL_LINES=5
diff --git a/doc/board/coreboot/coreboot.rst b/doc/board/coreboot/coreboot.rst
index fd974229eb4..9c44c025a48 100644
--- a/doc/board/coreboot/coreboot.rst
+++ b/doc/board/coreboot/coreboot.rst
@@ -40,3 +40,13 @@ To enable video you must enable these options in coreboot:
 At present it seems that for Minnowboard Max, coreboot does not pass through
 the video information correctly (it always says the resolution is 0x0). This
 works correctly for link though.
+
+64-bit U-Boot
+-------------
+
+In addition to the 32-bit 'coreboot' build there is a 'coreboot64' build. This
+produces an image which can be booted from coreboot (32-bit). Internally it
+works by using a 32-bit SPL binary to switch to 64-bit for running U-Boot. It
+can be useful for running UEFI applications, for example.
+
+This has only been lightly tested.
-- 
2.26.0.292.g33ef6b2f38-goog

[PATCH 0/6] x86: efi: Add a 64-bit coreboot payload

[Heinrich Schuchardt]

On 4/6/20 1:22 AM, Simon Glass wrote:
> This series creates a new 64-bit 'coreboot64' build which can be launched
> from coreboot. It uses SPL to effect the jump to 64-bit mode.
>
> This was done in an attempt to get the Ubuntu FirmwareTestSuite[1]
> working. Unfortunately the latest version shows a splash screen and hangs,
> perhaps due to some missing EFI support.

fwts exists as a snap package that can be run from the operating system.
I had not problem with running it on ARM64 systems. Cf.
https://snapcraft.io/fwts.

Do you relate to to the live image that gave you problems?

Best regards

Heinrich

>
> [1] https://wiki.ubuntu.com/FirmwareTestSuite
>
>
> Simon Glass (6):
>   x86: Allow building an SPL image for coreboot
>   x86: Move work-around out of cpu_jump_to_64bit_uboot()
>   x86: Update SPL for coreboot
>   x86: cbfs: Drop unwanted declaration
>   x86: coreboot: Allow building an SPL image
>   x86: Add a 64-bit coreboot build
>
>  Makefile                             |  6 ++++
>  arch/x86/cpu/Makefile                |  4 ++-
>  arch/x86/cpu/coreboot/Kconfig        |  1 +
>  arch/x86/cpu/coreboot/Makefile       |  8 ++++-
>  arch/x86/cpu/coreboot/coreboot.c     |  3 +-
>  arch/x86/cpu/coreboot/coreboot_spl.c | 12 +++++++
>  arch/x86/cpu/i386/cpu.c              | 10 ------
>  arch/x86/cpu/intel_common/Makefile   |  2 ++
>  arch/x86/cpu/x86_64/cpu.c            |  2 ++
>  arch/x86/dts/coreboot-u-boot.dtsi    | 18 +++++++++++
>  arch/x86/lib/spl.c                   | 23 ++++++++++---
>  board/coreboot/coreboot/MAINTAINERS  |  7 ++++
>  configs/coreboot64_defconfig         | 48 ++++++++++++++++++++++++++++
>  doc/board/coreboot/coreboot.rst      | 10 ++++++
>  include/cbfs.h                       |  2 +-
>  15 files changed, 138 insertions(+), 18 deletions(-)
>  create mode 100644 arch/x86/cpu/coreboot/coreboot_spl.c
>  create mode 100644 arch/x86/dts/coreboot-u-boot.dtsi
>  create mode 100644 configs/coreboot64_defconfig
>

[PATCH 0/6] x86: efi: Add a 64-bit coreboot payload

[Simon Glass]

Hi Heinrich,

On Sun, 5 Apr 2020 at 23:25, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 4/6/20 1:22 AM, Simon Glass wrote:
> > This series creates a new 64-bit 'coreboot64' build which can be launched
> > from coreboot. It uses SPL to effect the jump to 64-bit mode.
> >
> > This was done in an attempt to get the Ubuntu FirmwareTestSuite[1]
> > working. Unfortunately the latest version shows a splash screen and hangs,
> > perhaps due to some missing EFI support.
>
> fwts exists as a snap package that can be run from the operating system.
> I had not problem with running it on ARM64 systems. Cf.
> https://snapcraft.io/fwts.
>
> Do you relate to to the live image that gave you problems?

Yes that's right.

Regards,
Simon

>
> Best regards
>
> Heinrich
>
> >
> > [1] https://wiki.ubuntu.com/FirmwareTestSuite
> >
> >
> > Simon Glass (6):
> >   x86: Allow building an SPL image for coreboot
> >   x86: Move work-around out of cpu_jump_to_64bit_uboot()
> >   x86: Update SPL for coreboot
> >   x86: cbfs: Drop unwanted declaration
> >   x86: coreboot: Allow building an SPL image
> >   x86: Add a 64-bit coreboot build
> >
> >  Makefile                             |  6 ++++
> >  arch/x86/cpu/Makefile                |  4 ++-
> >  arch/x86/cpu/coreboot/Kconfig        |  1 +
> >  arch/x86/cpu/coreboot/Makefile       |  8 ++++-
> >  arch/x86/cpu/coreboot/coreboot.c     |  3 +-
> >  arch/x86/cpu/coreboot/coreboot_spl.c | 12 +++++++
> >  arch/x86/cpu/i386/cpu.c              | 10 ------
> >  arch/x86/cpu/intel_common/Makefile   |  2 ++
> >  arch/x86/cpu/x86_64/cpu.c            |  2 ++
> >  arch/x86/dts/coreboot-u-boot.dtsi    | 18 +++++++++++
> >  arch/x86/lib/spl.c                   | 23 ++++++++++---
> >  board/coreboot/coreboot/MAINTAINERS  |  7 ++++
> >  configs/coreboot64_defconfig         | 48 ++++++++++++++++++++++++++++
> >  doc/board/coreboot/coreboot.rst      | 10 ++++++
> >  include/cbfs.h                       |  2 +-
> >  15 files changed, 138 insertions(+), 18 deletions(-)
> >  create mode 100644 arch/x86/cpu/coreboot/coreboot_spl.c
> >  create mode 100644 arch/x86/dts/coreboot-u-boot.dtsi
> >  create mode 100644 configs/coreboot64_defconfig
> >
>

[PATCH 0/6] x86: efi: Add a 64-bit coreboot payload

[Andy Shevchenko]

On Sun, Apr 05, 2020 at 05:22:34PM -0600, Simon Glass wrote:
> This series creates a new 64-bit 'coreboot64' build which can be launched
> from coreboot. It uses SPL to effect the jump to 64-bit mode.
> 
> This was done in an attempt to get the Ubuntu FirmwareTestSuite[1]
> working. Unfortunately the latest version shows a splash screen and hangs,
> perhaps due to some missing EFI support.

On the first glance it should not break anyhow existing x86 systems.
LGTM.

> [1] https://wiki.ubuntu.com/FirmwareTestSuite
> 
> 
> Simon Glass (6):
>   x86: Allow building an SPL image for coreboot
>   x86: Move work-around out of cpu_jump_to_64bit_uboot()
>   x86: Update SPL for coreboot
>   x86: cbfs: Drop unwanted declaration
>   x86: coreboot: Allow building an SPL image
>   x86: Add a 64-bit coreboot build
> 
>  Makefile                             |  6 ++++
>  arch/x86/cpu/Makefile                |  4 ++-
>  arch/x86/cpu/coreboot/Kconfig        |  1 +
>  arch/x86/cpu/coreboot/Makefile       |  8 ++++-
>  arch/x86/cpu/coreboot/coreboot.c     |  3 +-
>  arch/x86/cpu/coreboot/coreboot_spl.c | 12 +++++++
>  arch/x86/cpu/i386/cpu.c              | 10 ------
>  arch/x86/cpu/intel_common/Makefile   |  2 ++
>  arch/x86/cpu/x86_64/cpu.c            |  2 ++
>  arch/x86/dts/coreboot-u-boot.dtsi    | 18 +++++++++++
>  arch/x86/lib/spl.c                   | 23 ++++++++++---
>  board/coreboot/coreboot/MAINTAINERS  |  7 ++++
>  configs/coreboot64_defconfig         | 48 ++++++++++++++++++++++++++++
>  doc/board/coreboot/coreboot.rst      | 10 ++++++
>  include/cbfs.h                       |  2 +-
>  15 files changed, 138 insertions(+), 18 deletions(-)
>  create mode 100644 arch/x86/cpu/coreboot/coreboot_spl.c
>  create mode 100644 arch/x86/dts/coreboot-u-boot.dtsi
>  create mode 100644 configs/coreboot64_defconfig
> 
> -- 
> 2.26.0.292.g33ef6b2f38-goog
> 

-- 
With Best Regards,
Andy Shevchenko

[PATCH 0/6] x86: efi: Add a 64-bit coreboot payload

[Bin Meng]

Hi Simon,

On Mon, Apr 6, 2020 at 7:22 AM Simon Glass <sjg@chromium.org> wrote:
>
> This series creates a new 64-bit 'coreboot64' build which can be launched
> from coreboot. It uses SPL to effect the jump to 64-bit mode.

Adding 64-bit U-Boot as a coreboot payload is nice.

>
> This was done in an attempt to get the Ubuntu FirmwareTestSuite[1]

Yet I fail to understand why adding 64-bit U-Boot as the coreboot
payload could help running the Ubuntu Firmware Test Suite. Does 32-bit
U-Boot as the coreboot payload not work in this case?

> working. Unfortunately the latest version shows a splash screen and hangs,
> perhaps due to some missing EFI support.
>
> [1] https://wiki.ubuntu.com/FirmwareTestSuite
>
>

Regards,
Bin

[PATCH 1/6] x86: Allow building an SPL image for coreboot

[Bin Meng]

Hi Simon,

On Mon, Apr 6, 2020 at 7:22 AM Simon Glass <sjg@chromium.org> wrote:
>
> Coreboot runs in 32-bit mode and cannot run a 64-bit U-Boot. To get around
> this we can build a combined image with 32-bit SPL and 64-bit U-Boot. Add
> a build rule and binman definition for this.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  Makefile                          |  6 ++++++
>  arch/x86/cpu/coreboot/Kconfig     |  1 +
>  arch/x86/dts/coreboot-u-boot.dtsi | 18 ++++++++++++++++++
>  3 files changed, 25 insertions(+)
>  create mode 100644 arch/x86/dts/coreboot-u-boot.dtsi
>

Can we reuse the arch/x86/dts/u-boot.dtsi to do the same thing? I feel
creating a new rule in /Makefile is not good.

Regards,
Bin

[PATCH 2/6] x86: Move work-around out of cpu_jump_to_64bit_uboot()

[Bin Meng]

Hi Simon,

On Mon, Apr 6, 2020 at 7:22 AM Simon Glass <sjg@chromium.org> wrote:
>
> At present this function copies U-Boot from the last 1MB of ROM. This is
> not the right way to do it. Instead, the binman symbol should provide the
> location.
>
> But in any case the code should live in the caller,
> spl_board_load_image(), so that the 64-bit jump function can be used
> elsewhere. Move it.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  arch/x86/cpu/i386/cpu.c | 10 ----------
>  arch/x86/lib/spl.c      | 13 +++++++++++++
>  2 files changed, 13 insertions(+), 10 deletions(-)
>
> diff --git a/arch/x86/cpu/i386/cpu.c b/arch/x86/cpu/i386/cpu.c
> index c8da7f10e9b..45416a9be5b 100644
> --- a/arch/x86/cpu/i386/cpu.c
> +++ b/arch/x86/cpu/i386/cpu.c
> @@ -611,16 +611,6 @@ int cpu_jump_to_64bit_uboot(ulong target)
>
>         func = (func_t)ptr;
>
> -       /*
> -        * Copy U-Boot from ROM
> -        * TODO(sjg at chromium.org): Figure out a way to get the text base
> -        * correctly here, and in the device-tree binman definition.
> -        *
> -        * Also consider using FIT so we get the correct image length and
> -        * parameters.
> -        */
> -       memcpy((char *)target, (char *)0xfff00000, 0x100000);
> -
>         /* Jump to U-Boot */
>         func((ulong)pgtable, 0, (ulong)target);
>
> diff --git a/arch/x86/lib/spl.c b/arch/x86/lib/spl.c
> index 90baec2a175..95a89c072d5 100644
> --- a/arch/x86/lib/spl.c
> +++ b/arch/x86/lib/spl.c
> @@ -207,6 +207,19 @@ static int spl_board_load_image(struct spl_image_info *spl_image,
>         spl_image->os = IH_OS_U_BOOT;
>         spl_image->name = "U-Boot";
>
> +       if (!IS_ENABLED(CONFIG_SYS_COREBOOT)) {

For the coreboot case, where does it load the image then?

> +               /*
> +                * Copy U-Boot from ROM
> +                * TODO(sjg at chromium.org): Figure out a way to get the text base
> +                * correctly here, and in the device-tree binman definition.
> +                *
> +                * Also consider using FIT so we get the correct image length
> +                * and parameters.
> +                */
> +               memcpy((char *)spl_image->load_addr, (char *)0xfff00000,
> +                      0x100000);
> +       }
> +
>         debug("Loading to %lx\n", spl_image->load_addr);
>
>         return 0;
> --

Regards,
Bin

[PATCH 4/6] x86: cbfs: Drop unwanted declaration

[Bin Meng]

On Mon, Apr 6, 2020 at 7:22 AM Simon Glass <sjg@chromium.org> wrote:
>
> The intention here is add a forward declaration, not actually declare a
> variable. Fix it.
>
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  include/cbfs.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

[PATCH 6/6] x86: Add a 64-bit coreboot build

[Bin Meng]

Hi Simon,

On Mon, Apr 6, 2020 at 7:22 AM Simon Glass <sjg@chromium.org> wrote:
>
> Add a build for running 64-bit U-Boot from coreboot (which is 32-bit).
> This uses binman to create an image with a 32-bit SPL and a 64-bit U-Boot.
>
> Coreboot boots into SPL and then SPL boots into U-Boot.
>
> This allows running 64-bit EFI images on x86.
> Signed-off-by: Simon Glass <sjg@chromium.org>
> ---
>
>  board/coreboot/coreboot/MAINTAINERS |  7 +++++
>  configs/coreboot64_defconfig        | 48 +++++++++++++++++++++++++++++
>  doc/board/coreboot/coreboot.rst     | 10 ++++++
>  3 files changed, 65 insertions(+)
>  create mode 100644 configs/coreboot64_defconfig
>
> diff --git a/board/coreboot/coreboot/MAINTAINERS b/board/coreboot/coreboot/MAINTAINERS
> index 188906b0803..a05673bb0be 100644
> --- a/board/coreboot/coreboot/MAINTAINERS
> +++ b/board/coreboot/coreboot/MAINTAINERS
> @@ -4,3 +4,10 @@ S:     Maintained
>  F:     board/coreboot/coreboot/
>  F:     include/configs/chromebook_link.h
>  F:     configs/coreboot_defconfig
> +
> +COREBOOT64 BOARD
> +M:     Simon Glass <sjg@chromium.org>
> +S:     Maintained
> +F:     board/coreboot/coreboot/
> +F:     include/configs/chromebook_link.h
> +F:     configs/coreboot64_defconfig
> diff --git a/configs/coreboot64_defconfig b/configs/coreboot64_defconfig
> new file mode 100644
> index 00000000000..80353b8eb36
> --- /dev/null
> +++ b/configs/coreboot64_defconfig
> @@ -0,0 +1,48 @@
> +CONFIG_X86=y
> +CONFIG_SYS_TEXT_BASE=0x1120000
> +CONFIG_ENV_SIZE=0x1000
> +CONFIG_NR_DRAM_BANKS=8
> +CONFIG_PRE_CON_BUF_ADDR=0x100000
> +CONFIG_X86_RUN_64BIT=y
> +CONFIG_VENDOR_COREBOOT=y
> +CONFIG_TARGET_COREBOOT=y
> +CONFIG_SPL_TEXT_BASE=0x1110000
> +CONFIG_FIT=y
> +CONFIG_FIT_SIGNATURE=y
> +CONFIG_SHOW_BOOT_PROGRESS=y
> +CONFIG_USE_BOOTARGS=y
> +CONFIG_BOOTARGS="root=/dev/sdb3 init=/sbin/init rootwait ro"
> +CONFIG_PRE_CONSOLE_BUFFER=y
> +CONFIG_SYS_CONSOLE_INFO_QUIET=y
> +CONFIG_DISPLAY_BOARDINFO_LATE=y
> +CONFIG_LAST_STAGE_INIT=y
> +CONFIG_HUSH_PARSER=y
> +CONFIG_CMD_IDE=y
> +CONFIG_CMD_MMC=y
> +CONFIG_CMD_PART=y
> +CONFIG_CMD_USB=y
> +# CONFIG_CMD_SETEXPR is not set
> +CONFIG_CMD_DHCP=y
> +# CONFIG_CMD_NFS is not set
> +CONFIG_CMD_PING=y
> +CONFIG_CMD_TIME=y
> +CONFIG_CMD_SOUND=y
> +CONFIG_CMD_EXT2=y
> +CONFIG_CMD_EXT4=y
> +CONFIG_CMD_EXT4_WRITE=y
> +CONFIG_CMD_FAT=y
> +CONFIG_CMD_FS_GENERIC=y
> +CONFIG_MAC_PARTITION=y
> +# CONFIG_SPL_MAC_PARTITION is not set
> +# CONFIG_SPL_DOS_PARTITION is not set
> +CONFIG_ISO_PARTITION=y
> +CONFIG_EFI_PARTITION=y
> +# CONFIG_SPL_EFI_PARTITION is not set
> +CONFIG_DEFAULT_DEVICE_TREE="coreboot"
> +CONFIG_SYS_RELOC_GD_ENV_ADDR=y
> +CONFIG_REGMAP=y
> +CONFIG_SYSCON=y
> +# CONFIG_PCI_PNP is not set
> +CONFIG_SOUND=y
> +CONFIG_SOUND_I8254=y
> +CONFIG_CONSOLE_SCROLL_LINES=5
> diff --git a/doc/board/coreboot/coreboot.rst b/doc/board/coreboot/coreboot.rst
> index fd974229eb4..9c44c025a48 100644
> --- a/doc/board/coreboot/coreboot.rst
> +++ b/doc/board/coreboot/coreboot.rst
> @@ -40,3 +40,13 @@ To enable video you must enable these options in coreboot:
>  At present it seems that for Minnowboard Max, coreboot does not pass through
>  the video information correctly (it always says the resolution is 0x0). This
>  works correctly for link though.
> +
> +64-bit U-Boot
> +-------------
> +
> +In addition to the 32-bit 'coreboot' build there is a 'coreboot64' build. This
> +produces an image which can be booted from coreboot (32-bit). Internally it
> +works by using a 32-bit SPL binary to switch to 64-bit for running U-Boot. It
> +can be useful for running UEFI applications, for example.
> +
> +This has only been lightly tested.
> --

I tested this with QEMU, but it reboots at U-Boot proper:

$ qemu-system-x86_64 -nographic -m 2G -bios coreboot.rom

Jumping to boot code at 01110000(7ffd5000)

U-Boot SPL 2020.04-00376-g785560e (Apr 23 2020 - 15:50:17 +0800)
CPU: x86_64, vendor AMD, device 663h
Trying to boot from SPI
Jumping to 64-bit U-Boot: Note many features are missing

U-Boot 2020.04-00376-g785560e (Apr 23 2020 - 15:50:17 +0800)

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

coreboot-4.8.1-dirty Wed May 16 19:00:17 UTC 2018 romstage starting...
CBMEM:

Regards,
Bin

Regards,
Bin

[PATCH 4/6] x86: cbfs: Drop unwanted declaration

[Bin Meng]

On Thu, Apr 23, 2020 at 5:19 PM Bin Meng <bmeng.cn@gmail.com> wrote:
>
> On Mon, Apr 6, 2020 at 7:22 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > The intention here is add a forward declaration, not actually declare a
> > variable. Fix it.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >  include/cbfs.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>

This patch is irrelevant to 64-bit coreboot payload support.

applied to u-boot-x86, thanks!

[PATCH 0/6] x86: efi: Add a 64-bit coreboot payload

[Heinrich Schuchardt]

On 4/23/20 10:05 AM, Bin Meng wrote:
> Hi Simon,
>
> On Mon, Apr 6, 2020 at 7:22 AM Simon Glass <sjg@chromium.org> wrote:
>>
>> This series creates a new 64-bit 'coreboot64' build which can be launched
>> from coreboot. It uses SPL to effect the jump to 64-bit mode.
>
> Adding 64-bit U-Boot as a coreboot payload is nice.
>
>>
>> This was done in an attempt to get the Ubuntu FirmwareTestSuite[1]
>
> Yet I fail to understand why adding 64-bit U-Boot as the coreboot
> payload could help running the Ubuntu Firmware Test Suite. Does 32-bit
> U-Boot as the coreboot payload not work in this case?

coreboot_defconfig has CONFIG_EFI_LOADER=y so the firmware test suite
should work fine if you are running a 32 bit OS.

Of cause calling a 32bit UEFI from a 64bit OS is expected fail due to a
different ABI.

Best regards

Heinrich

>
>> working. Unfortunately the latest version shows a splash screen and hangs,
>> perhaps due to some missing EFI support.
>>
>> [1] https://wiki.ubuntu.com/FirmwareTestSuite
>>
>>
>
> Regards,
> Bin
>

[PATCH 0/6] x86: efi: Add a 64-bit coreboot payload

[Andy Shevchenko]

On Fri, Apr 24, 2020 at 08:01:32AM +0200, Heinrich Schuchardt wrote:
> On 4/23/20 10:05 AM, Bin Meng wrote:
> > Hi Simon,
> >
> > On Mon, Apr 6, 2020 at 7:22 AM Simon Glass <sjg@chromium.org> wrote:
> >>
> >> This series creates a new 64-bit 'coreboot64' build which can be launched
> >> from coreboot. It uses SPL to effect the jump to 64-bit mode.
> >
> > Adding 64-bit U-Boot as a coreboot payload is nice.
> >
> >>
> >> This was done in an attempt to get the Ubuntu FirmwareTestSuite[1]
> >
> > Yet I fail to understand why adding 64-bit U-Boot as the coreboot
> > payload could help running the Ubuntu Firmware Test Suite. Does 32-bit
> > U-Boot as the coreboot payload not work in this case?
> 
> coreboot_defconfig has CONFIG_EFI_LOADER=y so the firmware test suite
> should work fine if you are running a 32 bit OS.
> 
> Of cause calling a 32bit UEFI from a 64bit OS is expected fail due to a
> different ABI.

I'm wonder how Linux (64-bit) kernel with EFI stub enables boots on EFI 32-bit environment?

> >> working. Unfortunately the latest version shows a splash screen and hangs,
> >> perhaps due to some missing EFI support.
> >>
> >> [1] https://wiki.ubuntu.com/FirmwareTestSuite

-- 
With Best Regards,
Andy Shevchenko

[PATCH 0/6] x86: efi: Add a 64-bit coreboot payload

[Simon Glass]

Hi Bin,

On Thu, 23 Apr 2020 at 02:05, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Simon,
>
> On Mon, Apr 6, 2020 at 7:22 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > This series creates a new 64-bit 'coreboot64' build which can be launched
> > from coreboot. It uses SPL to effect the jump to 64-bit mode.
>
> Adding 64-bit U-Boot as a coreboot payload is nice.
>
> >
> > This was done in an attempt to get the Ubuntu FirmwareTestSuite[1]
>
> Yet I fail to understand why adding 64-bit U-Boot as the coreboot
> payload could help running the Ubuntu Firmware Test Suite. Does 32-bit
> U-Boot as the coreboot payload not work in this case?

That's right. They only provide a 64-bit EFI image.

We could presumably make 64-bit EFI images work on 32-bit U-Boot, but
it doesn't at present.

>
> > working. Unfortunately the latest version shows a splash screen and hangs,
> > perhaps due to some missing EFI support.
> >
> > [1] https://wiki.ubuntu.com/FirmwareTestSuite

Regards,
Simon

[PATCH 1/6] x86: Allow building an SPL image for coreboot

[Simon Glass]

Hi Bin,

On Thu, 23 Apr 2020 at 03:14, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Simon,
>
> On Mon, Apr 6, 2020 at 7:22 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > Coreboot runs in 32-bit mode and cannot run a 64-bit U-Boot. To get around
> > this we can build a combined image with 32-bit SPL and 64-bit U-Boot. Add
> > a build rule and binman definition for this.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >  Makefile                          |  6 ++++++
> >  arch/x86/cpu/coreboot/Kconfig     |  1 +
> >  arch/x86/dts/coreboot-u-boot.dtsi | 18 ++++++++++++++++++
> >  3 files changed, 25 insertions(+)
> >  create mode 100644 arch/x86/dts/coreboot-u-boot.dtsi
> >
>
> Can we reuse the arch/x86/dts/u-boot.dtsi to do the same thing? I feel
> creating a new rule in /Makefile is not good.

Not easily.

This one creates an output file specifically for coreboot, which is
quite different from the u-boot.rom we normally create.

We could update the main one to use multiple images, and then we could
#ifdef out the u-boot.rom one when building coreboot. Then we could
add this rule in as well. Would that be better?

Regards,
SImon

[PATCH 2/6] x86: Move work-around out of cpu_jump_to_64bit_uboot()

[Simon Glass]

Hi Bin,

On Thu, 23 Apr 2020 at 03:17, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Simon,
>
> On Mon, Apr 6, 2020 at 7:22 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > At present this function copies U-Boot from the last 1MB of ROM. This is
> > not the right way to do it. Instead, the binman symbol should provide the
> > location.
> >
> > But in any case the code should live in the caller,
> > spl_board_load_image(), so that the 64-bit jump function can be used
> > elsewhere. Move it.
> >
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >  arch/x86/cpu/i386/cpu.c | 10 ----------
> >  arch/x86/lib/spl.c      | 13 +++++++++++++
> >  2 files changed, 13 insertions(+), 10 deletions(-)
> >
> > diff --git a/arch/x86/cpu/i386/cpu.c b/arch/x86/cpu/i386/cpu.c
> > index c8da7f10e9b..45416a9be5b 100644
> > --- a/arch/x86/cpu/i386/cpu.c
> > +++ b/arch/x86/cpu/i386/cpu.c
> > @@ -611,16 +611,6 @@ int cpu_jump_to_64bit_uboot(ulong target)
> >
> >         func = (func_t)ptr;
> >
> > -       /*
> > -        * Copy U-Boot from ROM
> > -        * TODO(sjg at chromium.org): Figure out a way to get the text base
> > -        * correctly here, and in the device-tree binman definition.
> > -        *
> > -        * Also consider using FIT so we get the correct image length and
> > -        * parameters.
> > -        */
> > -       memcpy((char *)target, (char *)0xfff00000, 0x100000);
> > -
> >         /* Jump to U-Boot */
> >         func((ulong)pgtable, 0, (ulong)target);
> >
> > diff --git a/arch/x86/lib/spl.c b/arch/x86/lib/spl.c
> > index 90baec2a175..95a89c072d5 100644
> > --- a/arch/x86/lib/spl.c
> > +++ b/arch/x86/lib/spl.c
> > @@ -207,6 +207,19 @@ static int spl_board_load_image(struct spl_image_info *spl_image,
> >         spl_image->os = IH_OS_U_BOOT;
> >         spl_image->name = "U-Boot";
> >
> > +       if (!IS_ENABLED(CONFIG_SYS_COREBOOT)) {
>
> For the coreboot case, where does it load the image then?

It is already loaded. The combined SPL/U-Boot image is what is loaded.
Then we boot into SPL and jump to U_Boot. This is how tegra works,
since it needs two different toolchains.
>
> > +               /*
> > +                * Copy U-Boot from ROM
> > +                * TODO(sjg at chromium.org): Figure out a way to get the text base
> > +                * correctly here, and in the device-tree binman definition.
> > +                *
> > +                * Also consider using FIT so we get the correct image length
> > +                * and parameters.
> > +                */
> > +               memcpy((char *)spl_image->load_addr, (char *)0xfff00000,
> > +                      0x100000);
> > +       }
> > +
> >         debug("Loading to %lx\n", spl_image->load_addr);
> >
> >         return 0;
> > --
>
> Regards,
> Bin

Regards,
Simon

[PATCH 6/6] x86: Add a 64-bit coreboot build

[Simon Glass]

Hi Bin,

On Thu, 23 Apr 2020 at 03:29, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Simon,
>
> On Mon, Apr 6, 2020 at 7:22 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > Add a build for running 64-bit U-Boot from coreboot (which is 32-bit).
> > This uses binman to create an image with a 32-bit SPL and a 64-bit U-Boot.
> >
> > Coreboot boots into SPL and then SPL boots into U-Boot.
> >
> > This allows running 64-bit EFI images on x86.
> > Signed-off-by: Simon Glass <sjg@chromium.org>
> > ---
> >
> >  board/coreboot/coreboot/MAINTAINERS |  7 +++++
> >  configs/coreboot64_defconfig        | 48 +++++++++++++++++++++++++++++
> >  doc/board/coreboot/coreboot.rst     | 10 ++++++
> >  3 files changed, 65 insertions(+)
> >  create mode 100644 configs/coreboot64_defconfig
> >
> > diff --git a/board/coreboot/coreboot/MAINTAINERS b/board/coreboot/coreboot/MAINTAINERS
> > index 188906b0803..a05673bb0be 100644
> > --- a/board/coreboot/coreboot/MAINTAINERS
> > +++ b/board/coreboot/coreboot/MAINTAINERS
> > @@ -4,3 +4,10 @@ S:     Maintained
> >  F:     board/coreboot/coreboot/
> >  F:     include/configs/chromebook_link.h
> >  F:     configs/coreboot_defconfig
> > +
> > +COREBOOT64 BOARD
> > +M:     Simon Glass <sjg@chromium.org>
> > +S:     Maintained
> > +F:     board/coreboot/coreboot/
> > +F:     include/configs/chromebook_link.h
> > +F:     configs/coreboot64_defconfig
> > diff --git a/configs/coreboot64_defconfig b/configs/coreboot64_defconfig
> > new file mode 100644
> > index 00000000000..80353b8eb36
> > --- /dev/null
> > +++ b/configs/coreboot64_defconfig
> > @@ -0,0 +1,48 @@
> > +CONFIG_X86=y
> > +CONFIG_SYS_TEXT_BASE=0x1120000
> > +CONFIG_ENV_SIZE=0x1000
> > +CONFIG_NR_DRAM_BANKS=8
> > +CONFIG_PRE_CON_BUF_ADDR=0x100000
> > +CONFIG_X86_RUN_64BIT=y
> > +CONFIG_VENDOR_COREBOOT=y
> > +CONFIG_TARGET_COREBOOT=y
> > +CONFIG_SPL_TEXT_BASE=0x1110000
> > +CONFIG_FIT=y
> > +CONFIG_FIT_SIGNATURE=y
> > +CONFIG_SHOW_BOOT_PROGRESS=y
> > +CONFIG_USE_BOOTARGS=y
> > +CONFIG_BOOTARGS="root=/dev/sdb3 init=/sbin/init rootwait ro"
> > +CONFIG_PRE_CONSOLE_BUFFER=y
> > +CONFIG_SYS_CONSOLE_INFO_QUIET=y
> > +CONFIG_DISPLAY_BOARDINFO_LATE=y
> > +CONFIG_LAST_STAGE_INIT=y
> > +CONFIG_HUSH_PARSER=y
> > +CONFIG_CMD_IDE=y
> > +CONFIG_CMD_MMC=y
> > +CONFIG_CMD_PART=y
> > +CONFIG_CMD_USB=y
> > +# CONFIG_CMD_SETEXPR is not set
> > +CONFIG_CMD_DHCP=y
> > +# CONFIG_CMD_NFS is not set
> > +CONFIG_CMD_PING=y
> > +CONFIG_CMD_TIME=y
> > +CONFIG_CMD_SOUND=y
> > +CONFIG_CMD_EXT2=y
> > +CONFIG_CMD_EXT4=y
> > +CONFIG_CMD_EXT4_WRITE=y
> > +CONFIG_CMD_FAT=y
> > +CONFIG_CMD_FS_GENERIC=y
> > +CONFIG_MAC_PARTITION=y
> > +# CONFIG_SPL_MAC_PARTITION is not set
> > +# CONFIG_SPL_DOS_PARTITION is not set
> > +CONFIG_ISO_PARTITION=y
> > +CONFIG_EFI_PARTITION=y
> > +# CONFIG_SPL_EFI_PARTITION is not set
> > +CONFIG_DEFAULT_DEVICE_TREE="coreboot"
> > +CONFIG_SYS_RELOC_GD_ENV_ADDR=y
> > +CONFIG_REGMAP=y
> > +CONFIG_SYSCON=y
> > +# CONFIG_PCI_PNP is not set
> > +CONFIG_SOUND=y
> > +CONFIG_SOUND_I8254=y
> > +CONFIG_CONSOLE_SCROLL_LINES=5
> > diff --git a/doc/board/coreboot/coreboot.rst b/doc/board/coreboot/coreboot.rst
> > index fd974229eb4..9c44c025a48 100644
> > --- a/doc/board/coreboot/coreboot.rst
> > +++ b/doc/board/coreboot/coreboot.rst
> > @@ -40,3 +40,13 @@ To enable video you must enable these options in coreboot:
> >  At present it seems that for Minnowboard Max, coreboot does not pass through
> >  the video information correctly (it always says the resolution is 0x0). This
> >  works correctly for link though.
> > +
> > +64-bit U-Boot
> > +-------------
> > +
> > +In addition to the 32-bit 'coreboot' build there is a 'coreboot64' build. This
> > +produces an image which can be booted from coreboot (32-bit). Internally it
> > +works by using a 32-bit SPL binary to switch to 64-bit for running U-Boot. It
> > +can be useful for running UEFI applications, for example.
> > +
> > +This has only been lightly tested.
> > --
>
> I tested this with QEMU, but it reboots at U-Boot proper:
>
> $ qemu-system-x86_64 -nographic -m 2G -bios coreboot.rom
>
> Jumping to boot code at 01110000(7ffd5000)
>
> U-Boot SPL 2020.04-00376-g785560e (Apr 23 2020 - 15:50:17 +0800)
> CPU: x86_64, vendor AMD, device 663h
> Trying to boot from SPI
> Jumping to 64-bit U-Boot: Note many features are missing
>
> U-Boot 2020.04-00376-g785560e (Apr 23 2020 - 15:50:17 +0800)
>
> CPU: x86_64, vendor <invalid cpu vendor>, device 0h
> DRAM:  2 GiB
>
> coreboot-4.8.1-dirty Wed May 16 19:00:17 UTC 2018 romstage starting...
> CBMEM:
>

I can't repeat this. My full log is below:

$ cbfstool coreboot.rom remove -n fallback/payload
$ cbfstool coreboot.rom add-flat-binary  -n fallback/payload -f
/tmp/b/coreboot64/u-boot-x86-with-spl.bin  -c lzma -l 0x1110000 -e
0x1110000
$ qemu-system-x86_64 --nographic -m 2G -bios coreboot.rom

(process:24926): GLib-WARNING **: 13:17:13.409:
../../../../glib/gmem.c:489: custom memory allocation vtable not
supported


coreboot-4.11-2345-gd6f7ec5f44 Sun Apr 26 08:01:17 UTC 2020 bootblock
starting (log level: 7)...
FMAP: Found "FLASH" version 1.1 at 0x0.
FMAP: base = 0xfff00000 size = 0x100000 #areas = 3
FMAP: area COREBOOT found @ 200 (1048064 bytes)
CBFS: Locating 'fallback/romstage'
CBFS: Found @ offset 80 size 389c
BS: bootblock times (exec / console): total (unknown) / 1 ms


coreboot-4.11-2345-gd6f7ec5f44 Sun Apr 26 08:01:17 UTC 2020 romstage
starting (log level: 7)...
QEMU: firmware config interface detected
Firmware config version id: 1
CBMEM:
IMD: root @ 0x7ffff000 254 entries.
IMD: root @ 0x7fffec00 62 entries.
MTRR Range: Start=fff00000 End=0 (Size 100000)
FMAP: area COREBOOT found @ 200 (1048064 bytes)
CBFS: Locating 'fallback/postcar'
CBFS: Found @ offset fd40 size 3e5c
Decompressing stage fallback/postcar @ 0x7ffd3fc0 (32560 bytes)
Loading module at 0x7ffd4000 with entry 0x7ffd4000. filesize: 0x3c10
memsize: 0x7ef0
Processing 124 relocs. Offset value of 0x7dfd4000
BS: romstage times (exec / console): total (unknown) / 2 ms


coreboot-4.11-2345-gd6f7ec5f44 Sun Apr 26 08:01:17 UTC 2020 postcar
starting (log level: 7)...
FMAP: area COREBOOT found @ 200 (1048064 bytes)
CBFS: Locating 'fallback/ramstage'
CBFS: Found @ offset 3980 size c064
Decompressing stage fallback/ramstage @ 0x7ffaffc0 (140840 bytes)
Loading module at 0x7ffb0000 with entry 0x7ffb0000. filesize: 0x17eb8
memsize: 0x225e8
Processing 1499 relocs. Offset value of 0x7f1b0000
BS: postcar times (exec / console): total (unknown) / 2 ms


coreboot-4.11-2345-gd6f7ec5f44 Sun Apr 26 08:01:17 UTC 2020 ramstage
starting (log level: 7)...
Enumerating buses...
Root Device scanning...
CPU_CLUSTER: 0 enabled
DOMAIN: 0000 enabled
CPU_CLUSTER: 0 scanning...
QEMU: firmware config interface detected
Firmware config version id: 1
QEMU: max_cpus is 1
CPU: APIC: 00 enabled
scan_bus: bus CPU_CLUSTER: 0 finished in 0 msecs
DOMAIN: 0000 scanning...
PCI: pci_scan_bus for bus 00
PCI: 00:00.0 [8086/1237] enabled
PCI: 00:01.0 [8086/7000] enabled
PCI: 00:01.1 [8086/7010] enabled
PCI: 00:01.3 [8086/7113] enabled
PCI: 00:02.0 [1234/1111] enabled
PCI: 00:03.0 [8086/100e] enabled
PCI: 00:01.0 scanning...
scan_bus: bus PCI: 00:01.0 finished in 0 msecs
PCI: 00:01.3 scanning...
scan_bus: bus PCI: 00:01.3 finished in 0 msecs
scan_bus: bus DOMAIN: 0000 finished in 1 msecs
scan_bus: bus Root Device finished in 2 msecs
done
BS: BS_DEV_ENUMERATE run times (exec / console): 1 / 2 ms
found VGA at PCI: 00:02.0
Setting up VGA for PCI: 00:02.0
Setting PCI_BRIDGE_CTL_VGA for bridge DOMAIN: 0000
Setting PCI_BRIDGE_CTL_VGA for bridge Root Device
Allocating resources...
Reading resources...
QEMU: e820/ram: 0x00000000 + 0x80000000
QEMU: reserve ioports 0x0510-0x0511 [firmware-config]
QEMU: reserve ioports 0x5658-0x5658 [vmware-port]
QEMU: reserve ioports 0xae00-0xae0f [pci-hotplug]
QEMU: reserve ioports 0xaf00-0xaf1f [cpu-hotplug]
QEMU: reserve ioports 0xafe0-0xafe3 [piix4-gpe0]
Done reading resources.
Setting resources...
PCI: 00:01.1 20 <- [0x0000005840 - 0x000000584f] size 0x00000010 gran 0x04 io
PCI: 00:02.0 10 <- [0x00fd000000 - 0x00fdffffff] size 0x01000000 gran
0x18 prefmem
PCI: 00:02.0 18 <- [0x00fe070000 - 0x00fe070fff] size 0x00001000 gran 0x0c mem
PCI: 00:02.0 30 <- [0x00fe060000 - 0x00fe06ffff] size 0x00010000 gran 0x10 romem
PCI: 00:03.0 10 <- [0x00fe040000 - 0x00fe05ffff] size 0x00020000 gran 0x11 mem
PCI: 00:03.0 14 <- [0x0000005800 - 0x000000583f] size 0x00000040 gran 0x06 io
PCI: 00:03.0 30 <- [0x00fe000000 - 0x00fe03ffff] size 0x00040000 gran 0x12 romem
Done setting resources.
Done allocating resources.
BS: BS_DEV_RESOURCES run times (exec / console): 2 / 3 ms
Enabling resources...
PCI: 00:00.0 cmd <- 00
PCI: 00:01.0 cmd <- 00
PCI: 00:01.1 cmd <- 01
PCI: 00:01.3 cmd <- 00
PCI: 00:02.0 cmd <- 03
PCI: 00:03.0 cmd <- 03
done.
BS: BS_DEV_ENABLE run times (exec / console): 1 / 0 ms
Initializing devices...
CPU_CLUSTER: 0 init
Initializing CPU #0
CPU: vendor AMD device 663
CPU: family 06, model 06, stepping 03
Setting up local APIC...
 apic_id: 0x00 done.
CPU #0 initialized
CPU_CLUSTER: 0 init finished in 0 msecs
PCI: 00:00.0 init
Assigning IRQ 10 to PCI: 00:01.3
Assigning IRQ 11 to PCI: 00:03.0
PCI: 00:00.0 init finished in 1 msecs
PCI: 00:01.0 init
RTC Init
PCI: 00:01.0 init finished in 0 msecs
PCI: 00:01.1 init
IDE: Primary IDE interface: on
IDE: Secondary IDE interface: on
IDE: Access to legacy IDE ports: off
PCI: 00:01.1 init finished in 0 msecs
PCI: 00:02.0 init
PCI: 00:02.0 init finished in 8 msecs
PCI: 00:03.0 init
PCI: 00:03.0 init finished in 0 msecs
Devices initialized
BS: BS_DEV_INIT run times (exec / console): 11 / 2 ms
Finalize devices...
Devices finalized
Copying Interrupt Routing Table to 0x000f0000... done.
Copying Interrupt Routing Table to 0x7ffa6000... done.
PIRQ table: 128 bytes.
QEMU: found ACPI tables in fw_cfg.
QEMU: loading "etc/acpi/rsdp" to 0x7ff82000 (len 36)
QEMU: loading "etc/acpi/tables" to 0x7ff82040 (len 131072)
QEMU: loaded ACPI tables from fw_cfg.
Looking on 0x7ff82000 for valid checksum
Checksum 1 passed
Checksum 2 passed all OK
ACPI:     * SSDT
Found 1 CPU(s).
ACPI: added table 5/32, length now 56
ACPI tables: 131136 bytes.
smbios_write_tables: 7ff81000
SMBIOS: Unknown CPU
DOMAIN: 0000 (QEMU Northbridge i440fx)
QEMU: found smbios tables in fw_cfg (len 320).
QEMU: coreboot type0 table found at 0x7ff81020.
QEMU: loading smbios tables to 0x7ff81065
SMBIOS tables: 421 bytes.
Writing table forward entry at 0x00000500
Wrote coreboot table at: 0x00000500, 0x10 bytes, checksum fe4
Writing coreboot table at 0x7ffa7000
 0. 0000000000000000-0000000000000fff: CONFIGURATION TABLES
 1. 0000000000001000-000000000009ffff: RAM
 2. 00000000000a0000-00000000000fffff: RESERVED
 3. 0000000000100000-000000007ff80fff: RAM
 4. 000000007ff81000-000000007ffaffff: CONFIGURATION TABLES
 5. 000000007ffb0000-000000007ffd2fff: RAMSTAGE
 6. 000000007ffd3000-000000007fffffff: CONFIGURATION TABLES
 7. 00000000ff800000-00000000ffffffff: RESERVED
FMAP: area COREBOOT found @ 200 (1048064 bytes)
Wrote coreboot table at: 0x7ffa7000, 0x2a0 bytes, checksum df46
coreboot table: 696 bytes.
IMD ROOT    0. 0x7ffff000 0x00001000
IMD SMALL   1. 0x7fffe000 0x00001000
CONSOLE     2. 0x7ffde000 0x00020000
TIME STAMP  3. 0x7ffdd000 0x00000910
ROMSTG STCK 4. 0x7ffdc000 0x00001000
AFTER CAR   5. 0x7ffd3000 0x00009000
RAMSTAGE    6. 0x7ffaf000 0x00024000
COREBOOT    7. 0x7ffa7000 0x00008000
IRQ TABLE   8. 0x7ffa6000 0x00001000
ACPI        9. 0x7ff82000 0x00024000
SMBIOS     10. 0x7ff81000 0x00000800
IMD small region:
  IMD ROOT    0. 0x7fffec00 0x00000400
  FMAP        1. 0x7fffeb40 0x000000b6
BS: BS_WRITE_TABLES run times (exec / console): 26 / 5 ms
FMAP: area COREBOOT found @ 200 (1048064 bytes)
CBFS: Locating 'fallback/payload'
CBFS: Found @ offset 14c00 size 3fa34
Checking segment from ROM address 0xfff14e38
Checking segment from ROM address 0xfff14e54
Loading segment from ROM address 0xfff14e38
  code (compression=1)
  New segment dstaddr 0x01110000 memsize 0x9547b srcaddr 0xfff14e70
filesize 0x3f9fc
Loading Segment: addr: 0x01110000 memsz: 0x000000000009547b filesz:
0x000000000003f9fc
using LZMA
Loading segment from ROM address 0xfff14e54
  Entry Point 0x01110000
BS: BS_PAYLOAD_LOAD run times (exec / console): 102 / 1 ms
Jumping to boot code@0x01110000(0x7ffa7000)

U-Boot SPL 2020.04-00113-g47a225c571e (Apr 26 2020 - 10:29:50 -0600)
CPU: x86_64, vendor AMD, device 663h
Trying to boot from SPI
Jumping to 64-bit U-Boot: Note many features are missing


U-Boot 2020.04-00113-g47a225c571e (Apr 26 2020 - 10:29:50 -0600)

CPU: x86_64, vendor <invalid cpu vendor>, device 0h
DRAM:  2 GiB
MMC:
Video: No video mode configured in coreboot!
Video: No video mode configured in coreboot!
Model: coreboot x86 payload
Net:   e1000: 52:54:00:12:34:56

Warning: e1000#0 using MAC address from ROM
eth0: e1000#0
No working controllers found
Finalizing coreboot
Hit any key to stop autoboot:  0
Error: Invalid Boot Flag (found 0x0000, expected 0xaa55)
## Kernel loading failed ...
zboot - Boot bzImage

Usage:
zboot [addr] [size] [initrd addr] [initrd size]
      addr -        The optional starting address of the bzimage.
                    If not set it defaults to the environment
                    variable "fileaddr".
      size -        The optional size of the bzimage. Defaults to
                    zero.
      initrd addr - The address of the initrd image to use, if any.
      initrd size - The size of the initrd image to use, if any.

=> QEMU: Terminated

Regards,
Simon

[PATCH 2/6] x86: Move work-around out of cpu_jump_to_64bit_uboot()

[Bin Meng]

Hi Simon,

On Mon, Apr 27, 2020 at 3:45 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Bin,
>
> On Thu, 23 Apr 2020 at 03:17, Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Simon,
> >
> > On Mon, Apr 6, 2020 at 7:22 AM Simon Glass <sjg@chromium.org> wrote:
> > >
> > > At present this function copies U-Boot from the last 1MB of ROM. This is
> > > not the right way to do it. Instead, the binman symbol should provide the
> > > location.
> > >
> > > But in any case the code should live in the caller,
> > > spl_board_load_image(), so that the 64-bit jump function can be used
> > > elsewhere. Move it.
> > >
> > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > ---
> > >
> > >  arch/x86/cpu/i386/cpu.c | 10 ----------
> > >  arch/x86/lib/spl.c      | 13 +++++++++++++
> > >  2 files changed, 13 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/arch/x86/cpu/i386/cpu.c b/arch/x86/cpu/i386/cpu.c
> > > index c8da7f10e9b..45416a9be5b 100644
> > > --- a/arch/x86/cpu/i386/cpu.c
> > > +++ b/arch/x86/cpu/i386/cpu.c
> > > @@ -611,16 +611,6 @@ int cpu_jump_to_64bit_uboot(ulong target)
> > >
> > >         func = (func_t)ptr;
> > >
> > > -       /*
> > > -        * Copy U-Boot from ROM
> > > -        * TODO(sjg at chromium.org): Figure out a way to get the text base
> > > -        * correctly here, and in the device-tree binman definition.
> > > -        *
> > > -        * Also consider using FIT so we get the correct image length and
> > > -        * parameters.
> > > -        */
> > > -       memcpy((char *)target, (char *)0xfff00000, 0x100000);
> > > -
> > >         /* Jump to U-Boot */
> > >         func((ulong)pgtable, 0, (ulong)target);
> > >
> > > diff --git a/arch/x86/lib/spl.c b/arch/x86/lib/spl.c
> > > index 90baec2a175..95a89c072d5 100644
> > > --- a/arch/x86/lib/spl.c
> > > +++ b/arch/x86/lib/spl.c
> > > @@ -207,6 +207,19 @@ static int spl_board_load_image(struct spl_image_info *spl_image,
> > >         spl_image->os = IH_OS_U_BOOT;
> > >         spl_image->name = "U-Boot";
> > >
> > > +       if (!IS_ENABLED(CONFIG_SYS_COREBOOT)) {
> >
> > For the coreboot case, where does it load the image then?
>
> It is already loaded. The combined SPL/U-Boot image is what is loaded.
> Then we boot into SPL and jump to U_Boot. This is how tegra works,
> since it needs two different toolchains.

So if I understand this correctly, coreboot loads U-Boot SPL + 64-bit
U-Boot as one payload all together hence there is no need memcpy? Is
adding a coreboot config check here is to speed up the boot time to
avoid the memcpy? If adding memcpy(), does anything break?

> >
> > > +               /*
> > > +                * Copy U-Boot from ROM
> > > +                * TODO(sjg at chromium.org): Figure out a way to get the text base
> > > +                * correctly here, and in the device-tree binman definition.
> > > +                *
> > > +                * Also consider using FIT so we get the correct image length
> > > +                * and parameters.
> > > +                */
> > > +               memcpy((char *)spl_image->load_addr, (char *)0xfff00000,
> > > +                      0x100000);
> > > +       }
> > > +
> > >         debug("Loading to %lx\n", spl_image->load_addr);
> > >
> > >         return 0;
> > > --
> >

Regards,
Bin

[PATCH 2/6] x86: Move work-around out of cpu_jump_to_64bit_uboot()

[Simon Glass]

Hi Bin,

On Sun, 26 Apr 2020 at 19:47, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Simon,
>
> On Mon, Apr 27, 2020 at 3:45 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Bin,
> >
> > On Thu, 23 Apr 2020 at 03:17, Bin Meng <bmeng.cn@gmail.com> wrote:
> > >
> > > Hi Simon,
> > >
> > > On Mon, Apr 6, 2020 at 7:22 AM Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > At present this function copies U-Boot from the last 1MB of ROM. This is
> > > > not the right way to do it. Instead, the binman symbol should provide the
> > > > location.
> > > >
> > > > But in any case the code should live in the caller,
> > > > spl_board_load_image(), so that the 64-bit jump function can be used
> > > > elsewhere. Move it.
> > > >
> > > > Signed-off-by: Simon Glass <sjg@chromium.org>
> > > > ---
> > > >
> > > >  arch/x86/cpu/i386/cpu.c | 10 ----------
> > > >  arch/x86/lib/spl.c      | 13 +++++++++++++
> > > >  2 files changed, 13 insertions(+), 10 deletions(-)
> > > >
> > > > diff --git a/arch/x86/cpu/i386/cpu.c b/arch/x86/cpu/i386/cpu.c
> > > > index c8da7f10e9b..45416a9be5b 100644
> > > > --- a/arch/x86/cpu/i386/cpu.c
> > > > +++ b/arch/x86/cpu/i386/cpu.c
> > > > @@ -611,16 +611,6 @@ int cpu_jump_to_64bit_uboot(ulong target)
> > > >
> > > >         func = (func_t)ptr;
> > > >
> > > > -       /*
> > > > -        * Copy U-Boot from ROM
> > > > -        * TODO(sjg at chromium.org): Figure out a way to get the text base
> > > > -        * correctly here, and in the device-tree binman definition.
> > > > -        *
> > > > -        * Also consider using FIT so we get the correct image length and
> > > > -        * parameters.
> > > > -        */
> > > > -       memcpy((char *)target, (char *)0xfff00000, 0x100000);
> > > > -
> > > >         /* Jump to U-Boot */
> > > >         func((ulong)pgtable, 0, (ulong)target);
> > > >
> > > > diff --git a/arch/x86/lib/spl.c b/arch/x86/lib/spl.c
> > > > index 90baec2a175..95a89c072d5 100644
> > > > --- a/arch/x86/lib/spl.c
> > > > +++ b/arch/x86/lib/spl.c
> > > > @@ -207,6 +207,19 @@ static int spl_board_load_image(struct spl_image_info *spl_image,
> > > >         spl_image->os = IH_OS_U_BOOT;
> > > >         spl_image->name = "U-Boot";
> > > >
> > > > +       if (!IS_ENABLED(CONFIG_SYS_COREBOOT)) {
> > >
> > > For the coreboot case, where does it load the image then?
> >
> > It is already loaded. The combined SPL/U-Boot image is what is loaded.
> > Then we boot into SPL and jump to U_Boot. This is how tegra works,
> > since it needs two different toolchains.
>
> So if I understand this correctly, coreboot loads U-Boot SPL + 64-bit
> U-Boot as one payload all together hence there is no need memcpy? Is
> adding a coreboot config check here is to speed up the boot time to
> avoid the memcpy? If adding memcpy(), does anything break?

In this case yes, since the U-Boot address is wrong. But I suspect it
would be possible to set it correctly so that copying is possible.

But it would be quite strange. The SPL+U-Boot is in a CBFS file and
not really under control of U-Boot. So I think trying to copy it again
would be quite fragile.

Regards,
Simon