| Message ID | 20171205031919.26644-1-oohall@gmail.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | chiptod: Keep boot timestamps contiguous | expand |
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@gmail.com> Hell-Yeahed-by: Michael Neuling <mikey@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; > }
* Michael Neuling <mikey@neuling.org> [2017-12-05 14:23:13]: > 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@gmail.com> > > Hell-Yeahed-by: Michael Neuling <mikey@neuling.org> Thanks for the patch. Good usability improvement for people looking at the console. Reviewed-by: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com> > > > --- > > 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; Good idea to sync all TB based on current value of one of them rather than starting from zero. > > + > > /* 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; > > } --Vaidy
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. Nak - what is a petikernel? > > [ 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@gmail.com> > --- > 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; > }
Oliver O'Halloran <oohall@gmail.com> writes: > 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@gmail.com> > --- > 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(-) Merged to master as of 5bb9d6f1e7dc0e38beb1db8f9f032a926310beac Now I'm sure to be confused :)
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; }
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@gmail.com> --- 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(-)