On Thursday, February 23, 2017 11:04:58 PM Jia-Shiun Li wrote:
> On Thu, Feb 23, 2017 at 6:08 PM, Konstantin Belousov <kostik...@gmail.com>
> wrote:
>
> >
> > This is a useful analysis.
> >
> > Yes, I think that there is an init ordering issue. Note that
> > cpu_disable_c2_sleep is only changed in tc_windup() when timecounter
> > is changed. If existing and already engadged timecounter suddenly gets
> > TC_FLAG_C2STOP set, tc_windup() ignores the flag. And with the early
> > AP startup, tsc seems to be set as timecounter too early.
> >
> > Just moving order of init_TSC_tc() would not help, since tsc checks smp
> > consistency, which requires started APs. Try this for now, but might
> > be John has better idea how to handle the issue. You might need to add
> > some extern declarations for the patch to compile.
> >
> > diff --git a/sys/x86/x86/tsc.c b/sys/x86/x86/tsc.c
> > index 3f36fbd9f8a..f8e33069c70 100644
> > --- a/sys/x86/x86/tsc.c
> > +++ b/sys/x86/x86/tsc.c
> > @@ -545,6 +545,8 @@ init_TSC_tc(void)
> > if (cpu_deepest_sleep >= 2 && cpu_vendor_id == CPU_VENDOR_INTEL &&
> > (amd_pminfo & AMDPM_TSC_INVARIANT) == 0) {
> > tsc_timecounter.tc_flags |= TC_FLAGS_C2STOP;
> > + if (timecounter == &tsc_timecounter)
> > + cpu_disable_c2_sleep++;
> > if (bootverbose)
> > printf("TSC timecounter disables C2 and C3.\n");
> > }
> >
>
>
> This does not work.
>
> I added a printf before the outer if clause, and it says
>
> init_TSC_tc:546: deepest 00000000 vendor 00008086 amd_pminfo 00000000
>
> full boot dmesg attached. Looks init_TSC_tc() is called too early before
> acpi_cpu_attach() initializing cpu_deepest_sleep. Maybe it should be put
> after
> driver initialization, since it depends on probed ACPI C states?
We don't actually need cpu_deepest_sleep. We could just set C2STOP always.
It doesn't hurt to have the flag set if the system only supports C1 except
that you get the printf in a verbose boot.
Try this slight variation of Konstantin's patch. If this works we can remove
cpu_deepest_sleep entirely as a followup since it will no longer be used:
Index: tsc.c
===================================================================
--- tsc.c (revision 314113)
+++ tsc.c (working copy)
@@ -542,9 +542,11 @@ init_TSC_tc(void)
* result incorrect runtimes for kernel idle threads (but not
* for any non-idle threads).
*/
- if (cpu_deepest_sleep >= 2 && cpu_vendor_id == CPU_VENDOR_INTEL &&
+ if (cpu_vendor_id == CPU_VENDOR_INTEL &&
(amd_pminfo & AMDPM_TSC_INVARIANT) == 0) {
tsc_timecounter.tc_flags |= TC_FLAGS_C2STOP;
+ if (timecounter == &tsc_timecounter)
+ cpu_disable_c2_sleep++;
if (bootverbose)
printf("TSC timecounter disables C2 and C3.\n");
}
--
John Baldwin
_______________________________________________
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"
