On Sun, Jun 21, 2020 at 05:44:36PM +0200, Christian Weisgerber wrote:
> Paul Irofti:
>
> > This also handles negative skew values that my prevoius diff did not.
>
> > --- sys/arch/amd64/amd64/tsc.c
> > +++ sys/arch/amd64/amd64/tsc.c
> > @@ -216,6 +217,8 @@ tsc_get_timecount(struct timecounter *tc)
> > void
> > tsc_timecounter_init(struct cpu_info *ci, uint64_t cpufreq)
> > {
> > + CPU_INFO_ITERATOR cii;
> > +
> > #ifdef TSC_DEBUG
> > printf("%s: TSC skew=%lld observed drift=%lld\n", __func__,
> > (long long)ci->ci_tsc_skew, (long long)tsc_drift_observed);
> > @@ -244,8 +247,16 @@ tsc_timecounter_init(struct cpu_info *ci, uint64_t
> > cpufreq)
> > printf("ERROR: %lld cycle TSC drift observed\n",
> > (long long)tsc_drift_observed);
> > tsc_timecounter.tc_quality = -1000;
> > + tsc_timecounter.tc_user = 0;
> > tsc_is_invariant = 0;
> > }
> > + CPU_INFO_FOREACH(cii, ci) {
> > + if (ci->ci_tsc_skew < -TSC_SKEW_MAX ||
> > + ci->ci_tsc_skew > TSC_SKEW_MAX) {
> > + tsc_timecounter.tc_user = 0;
> > + break;
> > + }
> > + }
> >
> > tc_init(&tsc_timecounter);
> > }
>
> If the output order from TSC_DEBUG in dmesg reflects the actual
> execution order, then the relative call order is this:
>
> cpu0 tsc_timecounter_init
> cpu1 cpu_start_secondary
> cpu1 tsc_timecounter_init
> cpu2 cpu_start_secondary
> cpu2 tsc_timecounter_init
> cpu3 cpu_start_secondary
> cpu3 tsc_timecounter_init
>
> That CPU_INFO_FOREACH() loop would execute in the very first cpu0
> tsc_timecounter_init() call, _before_ the skews of the other CPUs
> are determined in the subsequent cpu_start_secondary() calls.
>
> So, instead, I think the skew check needs to move to the top of
> tsc_timecounter_init, where each secondary CPU checks its own skew
> value and knocks out tsc_timecounter.tc_user if there is a problem.
>
> Unless I'm misunderstanding the whole thing.
I think the diff is fine as the skew is computed during cpu_hatch which
is the first function called after the MP_TRAMPOLINE and before
timecounter_init().
Can't test right now, but if you enable the TSC_DEBUG in cpu.c or if you
put a printf in the CPU_INFO_FOREACH you will probably see the correct
skew values.
If you test before I do and you don't see them, please let me know.
Thanks,
Paul
