On Thu, Dec 24, 2020 at 12:42:23AM +0000, James Cook wrote:
> On Wed, Dec 23, 2020 at 08:43:10PM +0000, James Cook wrote:
> > On Wed, Dec 23, 2020 at 11:47:05PM +1100, Jonathan Gray wrote:
> > > On Wed, Dec 23, 2020 at 12:31:10PM +1100, Jonathan Gray wrote:
> > > > On Tue, Dec 22, 2020 at 06:30:48PM +0000, James Cook wrote:
> > > > > > + case 0xa6: /* Coffeelake mobile */
> > > > >
> > > > > The laptop's CPU is an i7-10710U, which I think is in the Comet Lake
> > > > > series, not Coffee Lake.
> > > >
> > > > Yes 0xa6 is comet lake.
> > > >
> > > > But we should really do what FreeBSD and Linux do and fallback to
> > > > cpuid 0x16 as Intel keeps creating new skylake variants.
> > > >
> > > > The frequency from cpuid 0x15 is Hz, from 0x16 it is MHz.
> > > >
> > > > Untested as I don't have any >= skylake machines.
> > > > If you can add a printf to check the value is sane that would
> > > > be helpful.
> > >
> > > As noticed by tb@ the last diff wasn't quite right:
> > >
> > > Index: sys/arch/amd64/amd64/tsc.c
> > > ===================================================================
> > > RCS file: /cvs/src/sys/arch/amd64/amd64/tsc.c,v
> > > retrieving revision 1.21
> > > diff -u -p -r1.21 tsc.c
> > > --- sys/arch/amd64/amd64/tsc.c 6 Sep 2020 20:50:00 -0000 1.21
> > > +++ sys/arch/amd64/amd64/tsc.c 23 Dec 2020 12:25:32 -0000
> > > @@ -66,14 +66,16 @@ tsc_freq_cpuid(struct cpu_info *ci)
> > > eax = ebx = khz = dummy = 0;
> > > CPUID(0x15, eax, ebx, khz, dummy);
> > > khz /= 1000;
> > > - if (khz == 0) {
> > > + /*
> > > + * Fallback to 'Processor Base Frequency' from cpuid 0x16 when
> > > + * 'nominal frequency of the core crystal clock' from cpuid 0x15
> > > + * is 0 on >= Skylake
> > > + */
> > > + if (khz == 0 && cpuid_level >= 0x16) {
> > > + CPUID(0x16, khz, dummy, dummy, dummy);
> > > + khz = khz * 1000 * eax / ebx;
> > > + } else if (khz == 0) {
> > > switch (ci->ci_model) {
> > > - case 0x4e: /* Skylake mobile */
> > > - case 0x5e: /* Skylake desktop */
> > > - case 0x8e: /* Kabylake mobile */
> > > - case 0x9e: /* Kabylake desktop */
> > > - khz = 24000; /* 24.0 MHz */
> > > - break;
> > > case 0x5f: /* Atom Denverton */
> > > khz = 25000; /* 25.0 MHz */
> > > break;
> >
> > The patch works (I tested bsd.rd; sleep and date both behave right).
> >
> > Based on added printfs, it ends up with a khz of 23880, computed as
> > 1600 * 1000 * 2 / 134.
>
> I noticed something strange about the hw.setperf and hw.cpuspeed
> sysctls. I don't know if they're related to the original bug or its
> fix.
>
> My hw.cpuspeed sysctl starts out at 16264, which seems way too high.
> This page
>
>
> https://ark.intel.com/content/www/us/en/ark/products/196448/intel-core-i7-10710u-processor-12m-cache-up-to-4-70-ghz.html
>
> claims a "Max Turbo Frequency" of 4.70 GHz.
>
> hw.setperf seems to start out at 1320, as indicated by sysctl output
> when I change it. Of course, if I lower it, I can't bring it back to
> 1320. If I set it to 100, hw.cpuspeed is a more plausible 1601.
The max + 1 state is the turbo state.
As for why cpuspeed is so high, have a look at
sys/arch/amd64/amd64/identcpu.c cpu_freq() can you confirm if
cpu_freq_ctr() returns a non zero value? If so there is something wrong
with the performance counter method of getting cpuspeed.
>
> This is mostly measured with bsd.sp with the above change plus all my
> printfs. I also briefly tried with bsd.mp and confirmed hw.setperf
> seems to start at 1320 there. I haven't tried testing the actual
> performance, but none of this seems to cause time distortion, at least.
>
> Manually transcribed session with bsd.sp, the above patch, and my
> printfs:
>
> nomad# sysctl hw.cpuspeed
> hw.cpuspeed=16264
> nomad# sysctl hw.setperf=0
> hw.setperf: 1320 -> 0
> nomad# sysctl hw.cpuspeed
> hw.cpuspeed=400
> nomad# sysctl hw.setperf=100
> hw.setperf: 0 -> 100
> nomad# sysctl hw.cpuspeed
> 1601
> nomad# sysctl hw.setperf=1320
> ssyctl: hw.setperf: Invalid argument
>
> --
> James
>
>
