[U-Boot] [PATCH 0/3] x86: coreboot: Support running U-Boot in qemu

Currently when U-Boot is loaded by coreboot in qemu, U-Boot does not boot due to several issues. This patch series fix these issues to make coreboot support in U-Boot more robust so that it can run on top of qemu out of the box.
The command to package u-boot into coreboot.rom: ./build/util/cbfstool/cbfstool build/coreboot.rom add-flat-binary \ -f u-boot-dtb.bin -n fallback/payload -c lzma -l 0x1110000 -e 0x1110015
The command to launch qemu: ./qemu-system-i386 -nographic -m 512 -bios coreboot.rom
Bin Meng (3): x86: coreboot: Setup timer base correctly x86: Allow a hardcoded TSC frequency provided by Kconfig x86: coreboot: Wrap cros_ec initialization
arch/x86/Kconfig | 18 ++++++++++++++++++ arch/x86/cpu/coreboot/timestamp.c | 33 ++++++++++++++++++++------------- arch/x86/lib/tsc_timer.c | 8 ++++++-- board/coreboot/coreboot/coreboot.c | 2 ++ include/configs/coreboot.h | 4 +++- 5 files changed, 49 insertions(+), 16 deletions(-)

If coreboot is built with CONFIG_COLLECT_TIMESTAMPS, use the value of base_time in coreboot's timestamp table as our timer base, otherwise TSC counter value will be used.
Note sometimes even coreboot is built with CONFIG_COLLECT_TIMES, the value of base_time in the timestamp table is still zero, so we must exclude this case too (this is currently seen on booting coreboot in qemu).
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
arch/x86/cpu/coreboot/timestamp.c | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-)
diff --git a/arch/x86/cpu/coreboot/timestamp.c b/arch/x86/cpu/coreboot/timestamp.c index bd3558a..e6b8790 100644 --- a/arch/x86/cpu/coreboot/timestamp.c +++ b/arch/x86/cpu/coreboot/timestamp.c @@ -3,18 +3,7 @@ * * Copyright (C) 2011 The ChromiumOS Authors. All rights reserved. * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; version 2 of the License. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA, 02110-1301 USA + * SPDX-License-Identifier: GPL-2.0+ */
#include <common.h> @@ -38,9 +27,27 @@ static struct timestamp_table *ts_table __attribute__((section(".data")));
void timestamp_init(void) { +#ifdef CONFIG_SYS_X86_TSC_TIMER + uint64_t base_time; +#endif + ts_table = lib_sysinfo.tstamp_table; #ifdef CONFIG_SYS_X86_TSC_TIMER - timer_set_base(ts_table->base_time); + /* + * If coreboot is built with CONFIG_COLLECT_TIMESTAMPS, use the value + * of base_time in coreboot's timestamp table as our timer base, + * otherwise TSC counter value will be used. + * + * Note sometimes even coreboot is built with CONFIG_COLLECT_TIMES, + * the value of base_time in the timestamp table is still zero, so + * we must exclude this case too (this is currently seen on booting + * coreboot in qemu) + */ + if (ts_table && ts_table->base_time) + base_time = ts_table->base_time; + else + base_time = rdtsc(); + timer_set_base(base_time); #endif timestamp_add_now(TS_U_BOOT_INITTED); }

On 3 January 2015 at 07:40, Bin Meng bmeng.cn@gmail.com wrote:
If coreboot is built with CONFIG_COLLECT_TIMESTAMPS, use the value of base_time in coreboot's timestamp table as our timer base, otherwise TSC counter value will be used.
Note sometimes even coreboot is built with CONFIG_COLLECT_TIMES, the value of base_time in the timestamp table is still zero, so we must exclude this case too (this is currently seen on booting coreboot in qemu).
Signed-off-by: Bin Meng bmeng.cn@gmail.com
arch/x86/cpu/coreboot/timestamp.c | 33 ++++++++++++++++++++------------- 1 file changed, 20 insertions(+), 13 deletions(-)
Acked-by: Simon Glass sjg@chromium.org

By default U-Boot automatically calibrates TSC running frequency via MSR and PIT. The calibration may not work on every x86 processor, so a new Kconfig option CONFIG_TSC_CALIBRATION_BYPASS is introduced to allow bypassing the calibration and assign a hardcoded TSC frequency CONFIG_TSC_FREQ_IN_MHZ.
Normally the bypass should be turned on in a simulation environment like qemu.
Signed-off-by: Bin Meng bmeng.cn@gmail.com ---
arch/x86/Kconfig | 18 ++++++++++++++++++ arch/x86/lib/tsc_timer.c | 8 ++++++-- 2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index ebf72b3..9c11f0e 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -317,6 +317,24 @@ config FRAMEBUFFER_VESA_MODE
endmenu
+config TSC_CALIBRATION_BYPASS + bool "Bypass TSC calibration" + default n + help + By default U-Boot automatically calibrates TSC running frequency via + MSR and PIT. If the calibration does not work on your board, select + this option and provide a hardcoded TSC running frequency below. + + Normally this option should be turned on in a simulation environment + like qemu. + +config TSC_FREQ_IN_MHZ + int "TSC running frequency in MHz" + depends on TSC_CALIBRATION_BYPASS + default 1000 + help + The running frequency in MHz of TSC + source "arch/x86/cpu/ivybridge/Kconfig"
source "arch/x86/cpu/queensbay/Kconfig" diff --git a/arch/x86/lib/tsc_timer.c b/arch/x86/lib/tsc_timer.c index fb9afed..7f5ba2c 100644 --- a/arch/x86/lib/tsc_timer.c +++ b/arch/x86/lib/tsc_timer.c @@ -78,7 +78,7 @@ static int match_cpu(u8 family, u8 model) * * Returns the calibration value or 0 if MSR calibration failed. */ -static unsigned long try_msr_calibrate_tsc(void) +static unsigned long __maybe_unused try_msr_calibrate_tsc(void) { u32 lo, hi, ratio, freq_id, freq; unsigned long res; @@ -199,7 +199,7 @@ static inline int pit_expect_msb(unsigned char val, u64 *tscp, #define MAX_QUICK_PIT_MS 50 #define MAX_QUICK_PIT_ITERATIONS (MAX_QUICK_PIT_MS * PIT_TICK_RATE / 1000 / 256)
-static unsigned long quick_pit_calibrate(void) +static unsigned long __maybe_unused quick_pit_calibrate(void) { int i; u64 tsc, delta; @@ -306,6 +306,9 @@ unsigned __attribute__((no_instrument_function)) long get_tbclk_mhz(void) if (gd->arch.tsc_mhz) return gd->arch.tsc_mhz;
+#ifdef CONFIG_TSC_CALIBRATION_BYPASS + fast_calibrate = CONFIG_TSC_FREQ_IN_MHZ; +#else fast_calibrate = try_msr_calibrate_tsc(); if (!fast_calibrate) {
@@ -313,6 +316,7 @@ unsigned __attribute__((no_instrument_function)) long get_tbclk_mhz(void) if (!fast_calibrate) panic("TSC frequency is ZERO"); } +#endif
gd->arch.tsc_mhz = fast_calibrate; return fast_calibrate;

Hi Bin,
On 3 January 2015 at 07:40, Bin Meng bmeng.cn@gmail.com wrote:
By default U-Boot automatically calibrates TSC running frequency via MSR and PIT. The calibration may not work on every x86 processor, so a new Kconfig option CONFIG_TSC_CALIBRATION_BYPASS is introduced to allow bypassing the calibration and assign a hardcoded TSC frequency CONFIG_TSC_FREQ_IN_MHZ.
Normally the bypass should be turned on in a simulation environment like qemu.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
Acked-by: Simon Glass sjg@chromium.org
But see my optional thoughts below.
arch/x86/Kconfig | 18 ++++++++++++++++++ arch/x86/lib/tsc_timer.c | 8 ++++++-- 2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index ebf72b3..9c11f0e 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -317,6 +317,24 @@ config FRAMEBUFFER_VESA_MODE
endmenu
+config TSC_CALIBRATION_BYPASS
bool "Bypass TSC calibration"
default n
help
By default U-Boot automatically calibrates TSC running frequency via
MSR and PIT. If the calibration does not work on your board, select
this option and provide a hardcoded TSC running frequency below.
Do you think TSC, MSR and PIT should be spelled out in the help? I worry that people won't make much sense of this. For example, if PIT is Platform Independent Timer we could save 'Platform Independent Timer (PIT)'.
Normally this option should be turned on in a simulation environment
like qemu.
+config TSC_FREQ_IN_MHZ
int "TSC running frequency in MHz"
depends on TSC_CALIBRATION_BYPASS
default 1000
help
The running frequency in MHz of TSC
source "arch/x86/cpu/ivybridge/Kconfig"
source "arch/x86/cpu/queensbay/Kconfig" diff --git a/arch/x86/lib/tsc_timer.c b/arch/x86/lib/tsc_timer.c index fb9afed..7f5ba2c 100644 --- a/arch/x86/lib/tsc_timer.c +++ b/arch/x86/lib/tsc_timer.c @@ -78,7 +78,7 @@ static int match_cpu(u8 family, u8 model)
- Returns the calibration value or 0 if MSR calibration failed.
*/ -static unsigned long try_msr_calibrate_tsc(void) +static unsigned long __maybe_unused try_msr_calibrate_tsc(void) { u32 lo, hi, ratio, freq_id, freq; unsigned long res; @@ -199,7 +199,7 @@ static inline int pit_expect_msb(unsigned char val, u64 *tscp, #define MAX_QUICK_PIT_MS 50 #define MAX_QUICK_PIT_ITERATIONS (MAX_QUICK_PIT_MS * PIT_TICK_RATE / 1000 / 256)
-static unsigned long quick_pit_calibrate(void) +static unsigned long __maybe_unused quick_pit_calibrate(void) { int i; u64 tsc, delta; @@ -306,6 +306,9 @@ unsigned __attribute__((no_instrument_function)) long get_tbclk_mhz(void) if (gd->arch.tsc_mhz) return gd->arch.tsc_mhz;
+#ifdef CONFIG_TSC_CALIBRATION_BYPASS
fast_calibrate = CONFIG_TSC_FREQ_IN_MHZ;
+#else fast_calibrate = try_msr_calibrate_tsc(); if (!fast_calibrate) {
@@ -313,6 +316,7 @@ unsigned __attribute__((no_instrument_function)) long get_tbclk_mhz(void) if (!fast_calibrate) panic("TSC frequency is ZERO"); } +#endif
gd->arch.tsc_mhz = fast_calibrate; return fast_calibrate;
-- 1.8.2.1
Regards, Simon

Hi Simon,
On Sun, Jan 4, 2015 at 10:31 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 3 January 2015 at 07:40, Bin Meng bmeng.cn@gmail.com wrote:
By default U-Boot automatically calibrates TSC running frequency via MSR and PIT. The calibration may not work on every x86 processor, so a new Kconfig option CONFIG_TSC_CALIBRATION_BYPASS is introduced to allow bypassing the calibration and assign a hardcoded TSC frequency CONFIG_TSC_FREQ_IN_MHZ.
Normally the bypass should be turned on in a simulation environment like qemu.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
Acked-by: Simon Glass sjg@chromium.org
But see my optional thoughts below.
arch/x86/Kconfig | 18 ++++++++++++++++++ arch/x86/lib/tsc_timer.c | 8 ++++++-- 2 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index ebf72b3..9c11f0e 100644 --- a/arch/x86/Kconfig +++ b/arch/x86/Kconfig @@ -317,6 +317,24 @@ config FRAMEBUFFER_VESA_MODE
endmenu
+config TSC_CALIBRATION_BYPASS
bool "Bypass TSC calibration"
default n
help
By default U-Boot automatically calibrates TSC running frequency via
MSR and PIT. If the calibration does not work on your board, select
this option and provide a hardcoded TSC running frequency below.
Do you think TSC, MSR and PIT should be spelled out in the help? I worry that people won't make much sense of this. For example, if PIT is Platform Independent Timer we could save 'Platform Independent Timer (PIT)'.
Yes, a good idea. I can respin a v2 patch.
Regards, Bin

cros_ec_board_init() should be called only when CONFIG_CROS_EC is enabled. Also undef CONFIG_CROS_EC in the coreboot configuration.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
---
board/coreboot/coreboot/coreboot.c | 2 ++ include/configs/coreboot.h | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/board/coreboot/coreboot/coreboot.c b/board/coreboot/coreboot/coreboot.c index 154faf6..e076ea6 100644 --- a/board/coreboot/coreboot/coreboot.c +++ b/board/coreboot/coreboot/coreboot.c @@ -10,8 +10,10 @@
int arch_early_init_r(void) { +#ifdef CONFIG_CROS_EC if (cros_ec_board_init()) return -1; +#endif
return 0; } diff --git a/include/configs/coreboot.h b/include/configs/coreboot.h index 990a2d1..e5fe5e0 100644 --- a/include/configs/coreboot.h +++ b/include/configs/coreboot.h @@ -67,9 +67,11 @@
#define CONFIG_BOOTDELAY 2
-#define CONFIG_CROS_EC +#undef CONFIG_CROS_EC +#ifdef CONFIG_CROS_EC #define CONFIG_CROS_EC_LPC #define CONFIG_CMD_CROS_EC +#endif #define CONFIG_ARCH_EARLY_INIT_R
#endif /* __CONFIG_H */

Hi Bin,
On 3 January 2015 at 07:40, Bin Meng bmeng.cn@gmail.com wrote:
cros_ec_board_init() should be called only when CONFIG_CROS_EC is enabled. Also undef CONFIG_CROS_EC in the coreboot configuration.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
board/coreboot/coreboot/coreboot.c | 2 ++ include/configs/coreboot.h | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-)
Can we just remove the node in the device tree? The current 'coreboot' config is designed to run on link (Chromebook Pixel) so it does have an EC. Maybe we should have a separate device tree file for the qemu version?
diff --git a/board/coreboot/coreboot/coreboot.c b/board/coreboot/coreboot/coreboot.c index 154faf6..e076ea6 100644 --- a/board/coreboot/coreboot/coreboot.c +++ b/board/coreboot/coreboot/coreboot.c @@ -10,8 +10,10 @@
int arch_early_init_r(void) { +#ifdef CONFIG_CROS_EC if (cros_ec_board_init()) return -1; +#endif
return 0;
} diff --git a/include/configs/coreboot.h b/include/configs/coreboot.h index 990a2d1..e5fe5e0 100644 --- a/include/configs/coreboot.h +++ b/include/configs/coreboot.h @@ -67,9 +67,11 @@
#define CONFIG_BOOTDELAY 2
-#define CONFIG_CROS_EC +#undef CONFIG_CROS_EC +#ifdef CONFIG_CROS_EC #define CONFIG_CROS_EC_LPC #define CONFIG_CMD_CROS_EC +#endif #define CONFIG_ARCH_EARLY_INIT_R
#endif /* __CONFIG_H */
1.8.2.1
Regards, Simon

Hi Simon,
On Sun, Jan 4, 2015 at 10:33 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 3 January 2015 at 07:40, Bin Meng bmeng.cn@gmail.com wrote:
cros_ec_board_init() should be called only when CONFIG_CROS_EC is enabled. Also undef CONFIG_CROS_EC in the coreboot configuration.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
board/coreboot/coreboot/coreboot.c | 2 ++ include/configs/coreboot.h | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-)
Can we just remove the node in the device tree? The current 'coreboot' config is designed to run on link (Chromebook Pixel) so it does have an EC. Maybe we should have a separate device tree file for the qemu version?
Looks that removing ec node from dts should work with current code logic in cros_ec_init(). Yes, we can have a separate device tree file for maybe a generic board (not naming it as qemu.dts), and make this generic board dts file as the default dts for coreboot board? How about the defines in coreboot.h? Should we make it undefined like I did in this patch?
[snip]
Regards, Bin

Hi Bin,
On 3 January 2015 at 19:58, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sun, Jan 4, 2015 at 10:33 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 3 January 2015 at 07:40, Bin Meng bmeng.cn@gmail.com wrote:
cros_ec_board_init() should be called only when CONFIG_CROS_EC is enabled. Also undef CONFIG_CROS_EC in the coreboot configuration.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
board/coreboot/coreboot/coreboot.c | 2 ++ include/configs/coreboot.h | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-)
Can we just remove the node in the device tree? The current 'coreboot' config is designed to run on link (Chromebook Pixel) so it does have an EC. Maybe we should have a separate device tree file for the qemu version?
Looks that removing ec node from dts should work with current code logic in cros_ec_init(). Yes, we can have a separate device tree file for maybe a generic board (not naming it as qemu.dts), and make this generic board dts file as the default dts for coreboot board? How about the defines in coreboot.h? Should we make it undefined like I did in this patch?
That sounds good, but I would prefer to use the same board config file if possible, perhaps just changing the CONFIG_DEFAULT_DEVICE_TREE?
Regards, Simon

Hi Simon,
On Sun, Jan 4, 2015 at 11:01 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 3 January 2015 at 19:58, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sun, Jan 4, 2015 at 10:33 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 3 January 2015 at 07:40, Bin Meng bmeng.cn@gmail.com wrote:
cros_ec_board_init() should be called only when CONFIG_CROS_EC is enabled. Also undef CONFIG_CROS_EC in the coreboot configuration.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
board/coreboot/coreboot/coreboot.c | 2 ++ include/configs/coreboot.h | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-)
Can we just remove the node in the device tree? The current 'coreboot' config is designed to run on link (Chromebook Pixel) so it does have an EC. Maybe we should have a separate device tree file for the qemu version?
Looks that removing ec node from dts should work with current code logic in cros_ec_init(). Yes, we can have a separate device tree file for maybe a generic board (not naming it as qemu.dts), and make this generic board dts file as the default dts for coreboot board? How about the defines in coreboot.h? Should we make it undefined like I did in this patch?
That sounds good, but I would prefer to use the same board config file if possible, perhaps just changing the CONFIG_DEFAULT_DEVICE_TREE?
Yes, just changing the CONFIG_DEFAULT_DEVICE_TREE to the same board dts file, say for example, I can change CONFIG_DEFAULT_DEVICE_TREE to use crownbay.dts to build a U-Boot to be loaded by coreboot. The two question remain: which board dts file should be used as the default one in coreboot-x86_defconfig file? And how about those CROS_EX defines in coreboot.h? Right now we have SYS_CONFIG_NAME but it is not a visible Kconfig option so we cannot change it. Ideally we should just change CONFIG_DEFAULT_DEVICE_TREE and SYS_CONFIG_NAME to allow the U-Boot for coreboot to run on different boards.
Regards, Bin

Hi Bin,
On 3 January 2015 at 20:12, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sun, Jan 4, 2015 at 11:01 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 3 January 2015 at 19:58, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sun, Jan 4, 2015 at 10:33 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 3 January 2015 at 07:40, Bin Meng bmeng.cn@gmail.com wrote:
cros_ec_board_init() should be called only when CONFIG_CROS_EC is enabled. Also undef CONFIG_CROS_EC in the coreboot configuration.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
board/coreboot/coreboot/coreboot.c | 2 ++ include/configs/coreboot.h | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-)
Can we just remove the node in the device tree? The current 'coreboot' config is designed to run on link (Chromebook Pixel) so it does have an EC. Maybe we should have a separate device tree file for the qemu version?
Looks that removing ec node from dts should work with current code logic in cros_ec_init(). Yes, we can have a separate device tree file for maybe a generic board (not naming it as qemu.dts), and make this generic board dts file as the default dts for coreboot board? How about the defines in coreboot.h? Should we make it undefined like I did in this patch?
That sounds good, but I would prefer to use the same board config file if possible, perhaps just changing the CONFIG_DEFAULT_DEVICE_TREE?
Yes, just changing the CONFIG_DEFAULT_DEVICE_TREE to the same board dts file, say for example, I can change CONFIG_DEFAULT_DEVICE_TREE to use crownbay.dts to build a U-Boot to be loaded by coreboot. The two question remain: which board dts file should be used as the default one in coreboot-x86_defconfig file? And how about those CROS_EX defines in coreboot.h? Right now we have SYS_CONFIG_NAME but it is not a visible Kconfig option so we cannot change it. Ideally we should just change CONFIG_DEFAULT_DEVICE_TREE and SYS_CONFIG_NAME to allow the U-Boot for coreboot to run on different boards.
At present it is link, which is not idea for qemu. We probably need an easier way to select from multiple device trees to use for a board config. But for now perhaps we should have:
- coreboot-link - coreboot-generic (qemu) - chromebook_link - crownbay
The first two can be the same apart from the device tree. From experience once configs split it is really hard to get them joined, so we should keep an eye on this.
If I understand you correctly, I'd prefer to keep the CROS_EC defines in coreboot.h, meaning that the function is available if enabled by device tree.
Regards, Simon

Hi Simon,
On Sun, Jan 4, 2015 at 11:19 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 3 January 2015 at 20:12, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sun, Jan 4, 2015 at 11:01 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 3 January 2015 at 19:58, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sun, Jan 4, 2015 at 10:33 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 3 January 2015 at 07:40, Bin Meng bmeng.cn@gmail.com wrote:
cros_ec_board_init() should be called only when CONFIG_CROS_EC is enabled. Also undef CONFIG_CROS_EC in the coreboot configuration.
Signed-off-by: Bin Meng bmeng.cn@gmail.com
board/coreboot/coreboot/coreboot.c | 2 ++ include/configs/coreboot.h | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-)
Can we just remove the node in the device tree? The current 'coreboot' config is designed to run on link (Chromebook Pixel) so it does have an EC. Maybe we should have a separate device tree file for the qemu version?
Looks that removing ec node from dts should work with current code logic in cros_ec_init(). Yes, we can have a separate device tree file for maybe a generic board (not naming it as qemu.dts), and make this generic board dts file as the default dts for coreboot board? How about the defines in coreboot.h? Should we make it undefined like I did in this patch?
That sounds good, but I would prefer to use the same board config file if possible, perhaps just changing the CONFIG_DEFAULT_DEVICE_TREE?
Yes, just changing the CONFIG_DEFAULT_DEVICE_TREE to the same board dts file, say for example, I can change CONFIG_DEFAULT_DEVICE_TREE to use crownbay.dts to build a U-Boot to be loaded by coreboot. The two question remain: which board dts file should be used as the default one in coreboot-x86_defconfig file? And how about those CROS_EX defines in coreboot.h? Right now we have SYS_CONFIG_NAME but it is not a visible Kconfig option so we cannot change it. Ideally we should just change CONFIG_DEFAULT_DEVICE_TREE and SYS_CONFIG_NAME to allow the U-Boot for coreboot to run on different boards.
At present it is link, which is not idea for qemu. We probably need an easier way to select from multiple device trees to use for a board config. But for now perhaps we should have:
- coreboot-link
- coreboot-generic (qemu)
- chromebook_link
- crownbay
I mean if we are building U-Boot for coreboot, we just need change two things in Kconfig: CONFIG_DEFAULT_DEVICE_TREE and SYS_CONFIG_NAME. We should use the same dts file (arch/x86/dts/xxx.dts) for the board we want U-Boot to run on, as dts file is a file to describe the hardware information on a board. Similarly, we should use the same board configuration file (include/configs/xxx.h) as defined by SYS_CONFIG_NAME. Note since U-Boot for coreboot does not need run from reset vector, we should make CONFIG_X86_RESET_VECTOR a Kconfig option too so that we can switch it on/off. This coreboot build process should be properly documented in README.x86.
The first two can be the same apart from the device tree. From experience once configs split it is really hard to get them joined, so we should keep an eye on this.
The question is: with above build procedure, which board should we put in coreboot-x86_defconfig as the default dts file name shown in Kconfig?
If I understand you correctly, I'd prefer to keep the CROS_EC defines in coreboot.h, meaning that the function is available if enabled by device tree.
With above procedure, I think we need use link.h directly for chromebook_link to run U-Boot with coreboot. The coreboot.h should be removed.
Regards, Bin

Hi Bin,
On 3 January 2015 at 20:41, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sun, Jan 4, 2015 at 11:19 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 3 January 2015 at 20:12, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sun, Jan 4, 2015 at 11:01 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 3 January 2015 at 19:58, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sun, Jan 4, 2015 at 10:33 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 3 January 2015 at 07:40, Bin Meng bmeng.cn@gmail.com wrote: > cros_ec_board_init() should be called only when CONFIG_CROS_EC is > enabled. Also undef CONFIG_CROS_EC in the coreboot configuration. > > Signed-off-by: Bin Meng bmeng.cn@gmail.com > > --- > > board/coreboot/coreboot/coreboot.c | 2 ++ > include/configs/coreboot.h | 4 +++- > 2 files changed, 5 insertions(+), 1 deletion(-)
Can we just remove the node in the device tree? The current 'coreboot' config is designed to run on link (Chromebook Pixel) so it does have an EC. Maybe we should have a separate device tree file for the qemu version?
Looks that removing ec node from dts should work with current code logic in cros_ec_init(). Yes, we can have a separate device tree file for maybe a generic board (not naming it as qemu.dts), and make this generic board dts file as the default dts for coreboot board? How about the defines in coreboot.h? Should we make it undefined like I did in this patch?
That sounds good, but I would prefer to use the same board config file if possible, perhaps just changing the CONFIG_DEFAULT_DEVICE_TREE?
Yes, just changing the CONFIG_DEFAULT_DEVICE_TREE to the same board dts file, say for example, I can change CONFIG_DEFAULT_DEVICE_TREE to use crownbay.dts to build a U-Boot to be loaded by coreboot. The two question remain: which board dts file should be used as the default one in coreboot-x86_defconfig file? And how about those CROS_EX defines in coreboot.h? Right now we have SYS_CONFIG_NAME but it is not a visible Kconfig option so we cannot change it. Ideally we should just change CONFIG_DEFAULT_DEVICE_TREE and SYS_CONFIG_NAME to allow the U-Boot for coreboot to run on different boards.
At present it is link, which is not idea for qemu. We probably need an easier way to select from multiple device trees to use for a board config. But for now perhaps we should have:
- coreboot-link
- coreboot-generic (qemu)
- chromebook_link
- crownbay
I mean if we are building U-Boot for coreboot, we just need change two things in Kconfig: CONFIG_DEFAULT_DEVICE_TREE and SYS_CONFIG_NAME. We should use the same dts file (arch/x86/dts/xxx.dts) for the board we want U-Boot to run on, as dts file is a file to describe the hardware information on a board. Similarly, we should use the same board configuration file (include/configs/xxx.h) as defined by SYS_CONFIG_NAME. Note since U-Boot for coreboot does not need run from reset vector, we should make CONFIG_X86_RESET_VECTOR a Kconfig option too so that we can switch it on/off. This coreboot build process should be properly documented in README.x86.
OK, that sounds good.
The first two can be the same apart from the device tree. From experience once configs split it is really hard to get them joined, so we should keep an eye on this.
The question is: with above build procedure, which board should we put in coreboot-x86_defconfig as the default dts file name shown in Kconfig?
I'd say link for now since it is the only real hardware tested. But perhaps when qemu is better supported we could switch to that if you prefer.
If I understand you correctly, I'd prefer to keep the CROS_EC defines in coreboot.h, meaning that the function is available if enabled by device tree.
With above procedure, I think we need use link.h directly for chromebook_link to run U-Boot with coreboot. The coreboot.h should be removed.
OK. As you say we need to move CONFIG_X86_RESET_VECTOR to Kconfig.
Regards, Simon

On Sat, Jan 3, 2015 at 12:40 PM, Bin Meng bmeng.cn@gmail.com wrote:
Currently when U-Boot is loaded by coreboot in qemu, U-Boot does not boot due to several issues. This patch series fix these issues to make coreboot support in U-Boot more robust so that it can run on top of qemu out of the box.
The command to package u-boot into coreboot.rom: ./build/util/cbfstool/cbfstool build/coreboot.rom add-flat-binary \ -f u-boot-dtb.bin -n fallback/payload -c lzma -l 0x1110000 -e 0x1110015
It'd be awesome to have it documented somewhere :)

Hi Otavio,
On Sun, Jan 4, 2015 at 9:06 AM, Otavio Salvador otavio@ossystems.com.br wrote:
On Sat, Jan 3, 2015 at 12:40 PM, Bin Meng bmeng.cn@gmail.com wrote:
Currently when U-Boot is loaded by coreboot in qemu, U-Boot does not boot due to several issues. This patch series fix these issues to make coreboot support in U-Boot more robust so that it can run on top of qemu out of the box.
The command to package u-boot into coreboot.rom: ./build/util/cbfstool/cbfstool build/coreboot.rom add-flat-binary \ -f u-boot-dtb.bin -n fallback/payload -c lzma -l 0x1110000 -e 0x1110015
It'd be awesome to have it documented somewhere :)
Yes, I can document it in README.x86.
Regards, Bin

Hi Bin,
On 3 January 2015 at 18:59, Bin Meng bmeng.cn@gmail.com wrote:
Hi Otavio,
On Sun, Jan 4, 2015 at 9:06 AM, Otavio Salvador otavio@ossystems.com.br wrote:
On Sat, Jan 3, 2015 at 12:40 PM, Bin Meng bmeng.cn@gmail.com wrote:
Currently when U-Boot is loaded by coreboot in qemu, U-Boot does not boot due to several issues. This patch series fix these issues to make coreboot support in U-Boot more robust so that it can run on top of qemu out of the box.
The command to package u-boot into coreboot.rom: ./build/util/cbfstool/cbfstool build/coreboot.rom add-flat-binary \ -f u-boot-dtb.bin -n fallback/payload -c lzma -l 0x1110000 -e 0x1110015
It'd be awesome to have it documented somewhere :)
Yes, I can document it in README.x86.
Thanks. Also does qemu run U-Boot bare? I assume not. What is the main use case for Coreboot + U-Boot under qemu?
Regards, Simon

Hi Simon,
On Sun, Jan 4, 2015 at 10:28 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 3 January 2015 at 18:59, Bin Meng bmeng.cn@gmail.com wrote:
Hi Otavio,
On Sun, Jan 4, 2015 at 9:06 AM, Otavio Salvador otavio@ossystems.com.br wrote:
On Sat, Jan 3, 2015 at 12:40 PM, Bin Meng bmeng.cn@gmail.com wrote:
Currently when U-Boot is loaded by coreboot in qemu, U-Boot does not boot due to several issues. This patch series fix these issues to make coreboot support in U-Boot more robust so that it can run on top of qemu out of the box.
The command to package u-boot into coreboot.rom: ./build/util/cbfstool/cbfstool build/coreboot.rom add-flat-binary \ -f u-boot-dtb.bin -n fallback/payload -c lzma -l 0x1110000 -e 0x1110015
It'd be awesome to have it documented somewhere :)
Yes, I can document it in README.x86.
Thanks. Also does qemu run U-Boot bare? I assume not. What is the main use case for Coreboot + U-Boot under qemu?
So far only coreboot can run bare on qemu. It is a specific board (Emulation board) for qemu in coreboot Kconfig. We can support qemu running U-Boot bare as well. It is just a matter of time of doing that. If you think this might be helpful, I can spend some time to support qemu bare in U-Boot. As for the main use case, I believe this provides us a way of testing U-Boot coreboot support without a real hardware, just like the purpose of any emulation/simulation tool we have in the market.
Regards, Bin

Hi Bin,
On 3 January 2015 at 19:36, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sun, Jan 4, 2015 at 10:28 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 3 January 2015 at 18:59, Bin Meng bmeng.cn@gmail.com wrote:
Hi Otavio,
On Sun, Jan 4, 2015 at 9:06 AM, Otavio Salvador otavio@ossystems.com.br wrote:
On Sat, Jan 3, 2015 at 12:40 PM, Bin Meng bmeng.cn@gmail.com wrote:
Currently when U-Boot is loaded by coreboot in qemu, U-Boot does not boot due to several issues. This patch series fix these issues to make coreboot support in U-Boot more robust so that it can run on top of qemu out of the box.
The command to package u-boot into coreboot.rom: ./build/util/cbfstool/cbfstool build/coreboot.rom add-flat-binary \ -f u-boot-dtb.bin -n fallback/payload -c lzma -l 0x1110000 -e 0x1110015
It'd be awesome to have it documented somewhere :)
Yes, I can document it in README.x86.
Thanks. Also does qemu run U-Boot bare? I assume not. What is the main use case for Coreboot + U-Boot under qemu?
So far only coreboot can run bare on qemu. It is a specific board (Emulation board) for qemu in coreboot Kconfig. We can support qemu running U-Boot bare as well. It is just a matter of time of doing that. If you think this might be helpful, I can spend some time to support qemu bare in U-Boot. As for the main use case, I believe this provides us a way of testing U-Boot coreboot support without a real hardware, just like the purpose of any emulation/simulation tool we have in the market.
OK thanks for the info. It seems like it might be useful. Presumably it can run without binary blobs which would be good.
Regards, Simon
participants (3)
-
Bin Meng
-
Otavio Salvador
-
Simon Glass