[U-Boot] [PATCH v3 0/6] x86: bootstage: Fix bootstage operation on link

Recent bootstage changes have provoked problems with chromebook_link. Bootstage uses the timer before driver model is ready, but link uses driver model for the timer.
This series: - Updates the TSC timer to make the bootstage work before DM is ready - Provides a way to have a separate setting for record count in SPL to save memory - Tidies up a few bootstage options that are no-longer needed
This fixes booting on link which is currently broken.
Changes in v3: - Bracket access to gd->timer with an #ifdef
Changes in v2: - Update to support the early timer - Rebase on top of early timer code and simplify slightly - Add new patch to enable early timer for chromebook_link
Simon Glass (6): board_f: Drop the timer after relocation dm: x86: Allow TSC timer to be used before DM is ready dm: x86: Update timer_get_boot_us to work before DM is ready bootstage: Drop unused options bootstage: Provide a separate record count setting for SPL x86: Enable early timer for chromebook_link
arch/x86/include/asm/global_data.h | 1 + common/Kconfig | 16 ++++++------ common/board_f.c | 3 +++ common/bootstage.c | 6 ++--- configs/chromebook_link64_defconfig | 1 + configs/chromebook_link_defconfig | 1 + configs/sandbox_defconfig | 1 - configs/sandbox_flattree_defconfig | 1 - configs/sandbox_noblk_defconfig | 1 - configs/sandbox_spl_defconfig | 1 - drivers/timer/tsc_timer.c | 49 +++++++++++++++++++++++++++++-------- include/bootstage.h | 6 ----- 12 files changed, 55 insertions(+), 32 deletions(-)

Once U-Boot relocates itself the existing driver-model timer (if any) is no-longer valid until the device is reinitialised. Any use of the device may cause a crash. To handle this, set the timer to NULL after relocation.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v3: - Bracket access to gd->timer with an #ifdef
Changes in v2: None
common/board_f.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/common/board_f.c b/common/board_f.c index 104d144f41..9220815441 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -952,6 +952,9 @@ void board_init_f_r(void) * UART if available. */ gd->flags &= ~GD_FLG_SERIAL_READY; +#ifdef CONFIG_TIMER + gd->timer = NULL; +#endif
/* * U-Boot has been copied into SDRAM, the BSS has been cleared etc.

On Wed, Sep 6, 2017 at 9:49 AM, Simon Glass sjg@chromium.org wrote:
Once U-Boot relocates itself the existing driver-model timer (if any) is no-longer valid until the device is reinitialised. Any use of the device may cause a crash. To handle this, set the timer to NULL after relocation.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v3:
- Bracket access to gd->timer with an #ifdef
Changes in v2: None
common/board_f.c | 3 +++ 1 file changed, 3 insertions(+)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

On Tue, Sep 12, 2017 at 9:30 PM, Bin Meng bmeng.cn@gmail.com wrote:
On Wed, Sep 6, 2017 at 9:49 AM, Simon Glass sjg@chromium.org wrote:
Once U-Boot relocates itself the existing driver-model timer (if any) is no-longer valid until the device is reinitialised. Any use of the device may cause a crash. To handle this, set the timer to NULL after relocation.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v3:
- Bracket access to gd->timer with an #ifdef
Changes in v2: None
common/board_f.c | 3 +++ 1 file changed, 3 insertions(+)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
applied to u-boot-x86, thanks!

With bootstage we need access to the timer before driver model is set up. To handle this, put the required state in global_data and provide a new function to set up the device, separate from the driver's probe() method.
This will be used by the 'early' timer also.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---
Changes in v3: None Changes in v2: - Update to support the early timer
arch/x86/include/asm/global_data.h | 1 + drivers/timer/tsc_timer.c | 30 +++++++++++++++++++++++++----- 2 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/arch/x86/include/asm/global_data.h b/arch/x86/include/asm/global_data.h index 93a80fe2b6..fcb6853a38 100644 --- a/arch/x86/include/asm/global_data.h +++ b/arch/x86/include/asm/global_data.h @@ -77,6 +77,7 @@ struct arch_global_data { uint8_t x86_mask; uint32_t x86_device; uint64_t tsc_base; /* Initial value returned by rdtsc() */ + unsigned long clock_rate; /* Clock rate of timer in Hz */ void *new_fdt; /* Relocated FDT */ uint32_t bist; /* Built-in self test value */ enum pei_boot_mode_t pei_boot_mode; diff --git a/drivers/timer/tsc_timer.c b/drivers/timer/tsc_timer.c index 4d1fc9cd13..0012fecde0 100644 --- a/drivers/timer/tsc_timer.c +++ b/drivers/timer/tsc_timer.c @@ -328,17 +328,17 @@ static int tsc_timer_get_count(struct udevice *dev, u64 *count) return 0; }
-static int tsc_timer_probe(struct udevice *dev) +static void tsc_timer_ensure_setup(void) { - struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev); - + if (gd->arch.tsc_base) + return; gd->arch.tsc_base = rdtsc();
/* * If there is no clock frequency specified in the device tree, * calibrate it by ourselves. */ - if (!uc_priv->clock_rate) { + if (!gd->arch.clock_rate) { unsigned long fast_calibrate;
fast_calibrate = cpu_mhz_from_msr(); @@ -348,12 +348,32 @@ static int tsc_timer_probe(struct udevice *dev) panic("TSC frequency is ZERO"); }
- uc_priv->clock_rate = fast_calibrate * 1000000; + gd->arch.clock_rate = fast_calibrate * 1000000; } +} + +static int tsc_timer_probe(struct udevice *dev) +{ + struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev); + + tsc_timer_ensure_setup(); + uc_priv->clock_rate = gd->arch.clock_rate;
return 0; }
+unsigned long notrace timer_early_get_rate(void) +{ + tsc_timer_ensure_setup(); + + return gd->arch.clock_rate; +} + +u64 notrace timer_early_get_count(void) +{ + return rdtsc() - gd->arch.tsc_base; +} + static const struct timer_ops tsc_timer_ops = { .get_count = tsc_timer_get_count, };

On Wed, Sep 6, 2017 at 9:49 AM, Simon Glass sjg@chromium.org wrote:
With bootstage we need access to the timer before driver model is set up. To handle this, put the required state in global_data and provide a new function to set up the device, separate from the driver's probe() method.
This will be used by the 'early' timer also.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com
Changes in v3: None Changes in v2:
- Update to support the early timer
arch/x86/include/asm/global_data.h | 1 + drivers/timer/tsc_timer.c | 30 +++++++++++++++++++++++++----- 2 files changed, 26 insertions(+), 5 deletions(-)
applied to u-boot-x86, thanks!

Use the new separate init function so that we can make use of the timer before driver model is started up.
At some point we should consider adding the microsecond timer to the timer uclass interface since it would reduce the amount of plumbing here slightly.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v3: None Changes in v2: - Rebase on top of early timer code and simplify slightly
drivers/timer/tsc_timer.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/timer/tsc_timer.c b/drivers/timer/tsc_timer.c index 0012fecde0..db49de87a2 100644 --- a/drivers/timer/tsc_timer.c +++ b/drivers/timer/tsc_timer.c @@ -295,11 +295,6 @@ ulong notrace timer_get_us(void) return get_ticks() / get_tbclk_mhz(); }
-ulong timer_get_boot_us(void) -{ - return timer_get_us(); -} - void __udelay(unsigned long usec) { u64 now = get_ticks(); @@ -374,6 +369,20 @@ u64 notrace timer_early_get_count(void) return rdtsc() - gd->arch.tsc_base; }
+ulong timer_get_boot_us(void) +{ + if (!gd->timer) { + u64 now_tick; + + tsc_timer_ensure_setup(); + now_tick = rdtsc() - gd->arch.tsc_base; + + return now_tick / (gd->arch.clock_rate / 1000000); + } + + return timer_get_us(); +} + static const struct timer_ops tsc_timer_ops = { .get_count = tsc_timer_get_count, };

On Wed, Sep 6, 2017 at 9:49 AM, Simon Glass sjg@chromium.org wrote:
Use the new separate init function so that we can make use of the timer before driver model is started up.
At some point we should consider adding the microsecond timer to the timer uclass interface since it would reduce the amount of plumbing here slightly.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v3: None Changes in v2:
- Rebase on top of early timer code and simplify slightly
drivers/timer/tsc_timer.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-)
Drop this patch as it is not needed. thanks!
Regards, Bin

The CONFIG_BOOTSTAGE_USER_COUNT option is no-longer needed since we can now support any number of user IDs. Also BOOTSTAGE_ID_COUNT is not needed now.
Drop these unused options.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---
Changes in v3: None Changes in v2: None
common/Kconfig | 9 --------- common/bootstage.c | 2 +- configs/sandbox_defconfig | 1 - configs/sandbox_flattree_defconfig | 1 - configs/sandbox_noblk_defconfig | 1 - configs/sandbox_spl_defconfig | 1 - include/bootstage.h | 6 ------ 7 files changed, 1 insertion(+), 20 deletions(-)
diff --git a/common/Kconfig b/common/Kconfig index 4d8cae9610..abd4146922 100644 --- a/common/Kconfig +++ b/common/Kconfig @@ -46,15 +46,6 @@ config BOOTSTAGE_REPORT 29,916,167 26,005,792 bootm_start 30,361,327 445,160 start_kernel
-config BOOTSTAGE_USER_COUNT - int "Number of boot ID numbers available for user use" - default 20 - help - This is the number of available user bootstage records. - Each time you call bootstage_mark(BOOTSTAGE_ID_ALLOC, ...) - a new ID will be allocated from this stash. If you exceed - the limit, recording will stop. - config BOOTSTAGE_RECORD_COUNT int "Number of boot stage records to store" default 30 diff --git a/common/bootstage.c b/common/bootstage.c index 61479d7f07..efc99fc681 100644 --- a/common/bootstage.c +++ b/common/bootstage.c @@ -456,7 +456,7 @@ int bootstage_unstash(const void *base, int size)
if (data->rec_count + hdr->count > RECORD_COUNT) { debug("%s: Bootstage has %d records, we have space for %d\n" - "- please increase CONFIG_BOOTSTAGE_USER_COUNT\n", + "- please increase CONFIG_BOOTSTAGE_RECORD_COUNT\n", __func__, hdr->count, RECORD_COUNT - data->rec_count); return -ENOSPC; } diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index 4c4e4809be..5d8440fbcc 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -7,7 +7,6 @@ CONFIG_FIT_SIGNATURE=y CONFIG_FIT_VERBOSE=y CONFIG_BOOTSTAGE=y CONFIG_BOOTSTAGE_REPORT=y -CONFIG_BOOTSTAGE_USER_COUNT=32 CONFIG_BOOTSTAGE_FDT=y CONFIG_BOOTSTAGE_STASH=y CONFIG_BOOTSTAGE_STASH_ADDR=0x0 diff --git a/configs/sandbox_flattree_defconfig b/configs/sandbox_flattree_defconfig index 325f4ff57f..50ea35e64b 100644 --- a/configs/sandbox_flattree_defconfig +++ b/configs/sandbox_flattree_defconfig @@ -7,7 +7,6 @@ CONFIG_FIT_SIGNATURE=y CONFIG_FIT_VERBOSE=y CONFIG_BOOTSTAGE=y CONFIG_BOOTSTAGE_REPORT=y -CONFIG_BOOTSTAGE_USER_COUNT=32 CONFIG_BOOTSTAGE_FDT=y CONFIG_BOOTSTAGE_STASH=y CONFIG_BOOTSTAGE_STASH_ADDR=0x0 diff --git a/configs/sandbox_noblk_defconfig b/configs/sandbox_noblk_defconfig index e152917fe4..693c14bc60 100644 --- a/configs/sandbox_noblk_defconfig +++ b/configs/sandbox_noblk_defconfig @@ -6,7 +6,6 @@ CONFIG_FIT_SIGNATURE=y CONFIG_FIT_VERBOSE=y CONFIG_BOOTSTAGE=y CONFIG_BOOTSTAGE_REPORT=y -CONFIG_BOOTSTAGE_USER_COUNT=32 CONFIG_BOOTSTAGE_FDT=y CONFIG_BOOTSTAGE_STASH=y CONFIG_BOOTSTAGE_STASH_ADDR=0x0 diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig index be4a85da33..db9129456b 100644 --- a/configs/sandbox_spl_defconfig +++ b/configs/sandbox_spl_defconfig @@ -13,7 +13,6 @@ CONFIG_FIT_VERBOSE=y CONFIG_SPL_LOAD_FIT=y CONFIG_BOOTSTAGE=y CONFIG_BOOTSTAGE_REPORT=y -CONFIG_BOOTSTAGE_USER_COUNT=32 CONFIG_BOOTSTAGE_FDT=y CONFIG_BOOTSTAGE_STASH=y CONFIG_BOOTSTAGE_STASH_ADDR=0x0 diff --git a/include/bootstage.h b/include/bootstage.h index c5d93f57fd..7a524782ba 100644 --- a/include/bootstage.h +++ b/include/bootstage.h @@ -12,11 +12,6 @@ #ifndef _BOOTSTAGE_H #define _BOOTSTAGE_H
-/* Define this for host tools */ -#ifndef CONFIG_BOOTSTAGE_USER_COUNT -#define CONFIG_BOOTSTAGE_USER_COUNT 20 -#endif - /* Flags for each bootstage record */ enum bootstage_flags { BOOTSTAGEF_ERROR = 1 << 0, /* Error record */ @@ -208,7 +203,6 @@ enum bootstage_id {
/* a few spare for the user, from here */ BOOTSTAGE_ID_USER, - BOOTSTAGE_ID_COUNT = BOOTSTAGE_ID_USER + CONFIG_BOOTSTAGE_USER_COUNT, BOOTSTAGE_ID_ALLOC, };

On Wed, Sep 6, 2017 at 9:49 AM, Simon Glass sjg@chromium.org wrote:
The CONFIG_BOOTSTAGE_USER_COUNT option is no-longer needed since we can now support any number of user IDs. Also BOOTSTAGE_ID_COUNT is not needed now.
Drop these unused options.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com
Changes in v3: None Changes in v2: None
common/Kconfig | 9 --------- common/bootstage.c | 2 +- configs/sandbox_defconfig | 1 - configs/sandbox_flattree_defconfig | 1 - configs/sandbox_noblk_defconfig | 1 - configs/sandbox_spl_defconfig | 1 - include/bootstage.h | 6 ------ 7 files changed, 1 insertion(+), 20 deletions(-)
applied to u-boot-x86, thanks!

With SPL we often have limited memory and do not need very many bootstage records. Add a separate Kconfig option for SPL.
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v3: None Changes in v2: None
common/Kconfig | 7 +++++++ common/bootstage.c | 6 +++--- 2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/common/Kconfig b/common/Kconfig index abd4146922..540cc9999b 100644 --- a/common/Kconfig +++ b/common/Kconfig @@ -53,6 +53,13 @@ config BOOTSTAGE_RECORD_COUNT This is the size of the bootstage record list and is the maximum number of bootstage records that can be recorded.
+config SPL_BOOTSTAGE_RECORD_COUNT + int "Number of boot stage records to store for SPL" + default 5 + help + This is the size of the bootstage record list and is the maximum + number of bootstage records that can be recorded. + config BOOTSTAGE_FDT bool "Store boot timing information in the OS device tree" depends on BOOTSTAGE diff --git a/common/bootstage.c b/common/bootstage.c index efc99fc681..b866e66979 100644 --- a/common/bootstage.c +++ b/common/bootstage.c @@ -18,7 +18,7 @@ DECLARE_GLOBAL_DATA_PTR;
enum { - RECORD_COUNT = CONFIG_BOOTSTAGE_RECORD_COUNT, + RECORD_COUNT = CONFIG_VAL(BOOTSTAGE_RECORD_COUNT), };
struct bootstage_record { @@ -327,7 +327,7 @@ void bootstage_report(void) } if (data->rec_count > RECORD_COUNT) printf("Overflowed internal boot id table by %d entries\n" - "- please increase CONFIG_BOOTSTAGE_RECORD_COUNT\n", + "Please increase CONFIG_(SPL_)BOOTSTAGE_RECORD_COUNT\n", data->rec_count - RECORD_COUNT);
puts("\nAccumulated time:\n"); @@ -456,7 +456,7 @@ int bootstage_unstash(const void *base, int size)
if (data->rec_count + hdr->count > RECORD_COUNT) { debug("%s: Bootstage has %d records, we have space for %d\n" - "- please increase CONFIG_BOOTSTAGE_RECORD_COUNT\n", + "Please increase CONFIG_(SPL_)BOOTSTAGE_RECORD_COUNT\n", __func__, hdr->count, RECORD_COUNT - data->rec_count); return -ENOSPC; }

On Wed, Sep 6, 2017 at 9:49 AM, Simon Glass sjg@chromium.org wrote:
With SPL we often have limited memory and do not need very many bootstage records. Add a separate Kconfig option for SPL.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v3: None Changes in v2: None
common/Kconfig | 7 +++++++ common/bootstage.c | 6 +++--- 2 files changed, 10 insertions(+), 3 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com

On Tue, Sep 12, 2017 at 9:32 PM, Bin Meng bmeng.cn@gmail.com wrote:
On Wed, Sep 6, 2017 at 9:49 AM, Simon Glass sjg@chromium.org wrote:
With SPL we often have limited memory and do not need very many bootstage records. Add a separate Kconfig option for SPL.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v3: None Changes in v2: None
common/Kconfig | 7 +++++++ common/bootstage.c | 6 +++--- 2 files changed, 10 insertions(+), 3 deletions(-)
Reviewed-by: Bin Meng bmeng.cn@gmail.com
applied to u-boot-x86, thanks!

Enable this option for link so that the timer is available earlier.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com ---
Changes in v3: None Changes in v2: - Add new patch to enable early timer for chromebook_link
configs/chromebook_link64_defconfig | 1 + configs/chromebook_link_defconfig | 1 + 2 files changed, 2 insertions(+)
diff --git a/configs/chromebook_link64_defconfig b/configs/chromebook_link64_defconfig index da247152a8..d7052a645c 100644 --- a/configs/chromebook_link64_defconfig +++ b/configs/chromebook_link64_defconfig @@ -67,6 +67,7 @@ CONFIG_DEBUG_UART_CLOCK=1843200 CONFIG_DEBUG_UART_BOARD_INIT=y CONFIG_SYS_NS16550=y CONFIG_SPL_TIMER=y +CONFIG_TIMER_EARLY=y CONFIG_TPM_TIS_LPC=y CONFIG_USB_STORAGE=y CONFIG_USB_KEYBOARD=y diff --git a/configs/chromebook_link_defconfig b/configs/chromebook_link_defconfig index aaabb22251..f22a03e1cb 100644 --- a/configs/chromebook_link_defconfig +++ b/configs/chromebook_link_defconfig @@ -49,6 +49,7 @@ CONFIG_DEBUG_UART_BASE=0x3f8 CONFIG_DEBUG_UART_CLOCK=1843200 CONFIG_DEBUG_UART_BOARD_INIT=y CONFIG_SYS_NS16550=y +CONFIG_TIMER_EARLY=y CONFIG_TPM_TIS_LPC=y CONFIG_USB_STORAGE=y CONFIG_USB_KEYBOARD=y

On Wed, Sep 6, 2017 at 9:49 AM, Simon Glass sjg@chromium.org wrote:
Enable this option for link so that the timer is available earlier.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Bin Meng bmeng.cn@gmail.com
Changes in v3: None Changes in v2:
- Add new patch to enable early timer for chromebook_link
configs/chromebook_link64_defconfig | 1 + configs/chromebook_link_defconfig | 1 + 2 files changed, 2 insertions(+)
applied to u-boot-x86, thanks!
participants (2)
-
Bin Meng
-
Simon Glass