[Skiboot] [PATCH] chiptod: Keep boot timestamps contiguous

Michael Neuling mikey at neuling.org
Tue Dec 5 14:23:13 AEDT 2017


On Tue, 2017-12-05 at 14:19 +1100, Oliver O'Halloran wrote:
> Currently we reset the timebase value to (almost) zero when
> synchronising the timebase of each chip to the Chip TOD network which
> results in this:
> 
> [   42.374813167,5] CPU: All 80 processors called in...
> [    2.222791151,5] FLASH: Found system flash: Macronix MXxxL51235F id:0
> [    2.222977933,5] BT: Interface initialized, IO 0x00e4
> 
> This patch modifies the chiptod_init() process to use the current
> timebase value rather than resetting it to zero. This results in the
> timestamps remaining contigious from the start of hostboot until
> the petikernel starts. e.g.
> 
> [   70.188811484,5] CPU: All 144 processors called in...
> [   72.458004252,5] FLASH: Found system flash:  id:0
> [   72.458147358,5] BT: Interface initialized, IO 0x00e4
> 
> Signed-off-by: Oliver O'Halloran <oohall at gmail.com>

Hell-Yeahed-by: Michael Neuling <mikey at neuling.org>

> ---
> Tested on a Witherspoon and a P8 FSP box. Seems to work ok in both
> cases.
> ---
>  core/init.c  |  8 +++++---
>  hw/chiptod.c | 15 +++++++++++----
>  2 files changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/core/init.c b/core/init.c
> index 89a2758128e2..d008e2dc40f2 100644
> --- a/core/init.c
> +++ b/core/init.c
> @@ -966,9 +966,11 @@ void __noreturn __nomcount main_cpu_entry(const void
> *fdt)
>  	cpu_set_sreset_enable(true);
>  
>  	/*
> -	 * Synchronize time bases. Thi resets all the TB values to a small
> -	 * value (so they appear to go backward at this point), and
> synchronize
> -	 * all core timebases to the global ChipTOD network
> +	 * Synchronize time bases. Prior to chiptod_init() the timebase
> +	 * is free-running at a frequency based on the core clock rather
> +	 * than being synchronised to the ChipTOD network. This means
> +	 * that the timestamps in early boot might be a little off compared
> +	 * to wall clock time.
>  	 */
>  	chiptod_init();
>  
> diff --git a/hw/chiptod.c b/hw/chiptod.c
> index b9e47741bf21..3a0940d2b8bc 100644
> --- a/hw/chiptod.c
> +++ b/hw/chiptod.c
> @@ -120,9 +120,6 @@
>  #define LOCAL_CORE_FIR		0x0104000C
>  #define LFIR_SWITCH_COMPLETE	PPC_BIT(18)
>  
> -/* Magic TB value. One step cycle ahead of sync */
> -#define INIT_TB	0x000000000001ff0
> -
>  /* Number of iterations for the various timeouts */
>  #define TIMEOUT_LOOPS		20000000
>  
> @@ -775,6 +772,7 @@ static void chiptod_reset_tod_errors(void)
>  
>  static void chiptod_sync_master(void *data)
>  {
> +	uint64_t initial_tb_value;
>  	bool *result = data;
>  
>  	prlog(PR_DEBUG, "Master sync on CPU PIR 0x%04x...\n",
> @@ -824,8 +822,17 @@ static void chiptod_sync_master(void *data)
>  		goto error;
>  	}
>  
> +	/*
> +	 * Load the master's current timebase value into the Chip TOD
> +	 * network. This is so we have sane timestamps across the whole
> +	 * IPL process. The Chip TOD documentation says that the loaded
> +	 * value needs to be one STEP before a SYNC. In other words,
> +	 * set the low bits to 0x1ff0.
> +	 */
> +	initial_tb_value = (mftb() & ~0x1fff) | 0x1ff0;
> +
>  	/* Chip TOD load initial value */
> -	if (xscom_writeme(TOD_CHIPTOD_LOAD_TB, INIT_TB)) {
> +	if (xscom_writeme(TOD_CHIPTOD_LOAD_TB, initial_tb_value)) {
>  		prerror("XSCOM error setting init TB\n");
>  		goto error;
>  	}


More information about the Skiboot mailing list