Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200509113717.9084-1-pmenzel@molgen.mpg.de> <87eerr3ppb.fsf@nanos.tec.linutronix.de>
 <edc5af47-27e6-753f-c095-bd3087942690@molgen.mpg.de>
In-Reply-To: <edc5af47-27e6-753f-c095-bd3087942690@molgen.mpg.de>
From:   Radoslaw Biernacki <biernacki@google.com>
Date:   Mon, 11 May 2020 10:45:28 +0200
Message-ID: <CAM4=Rn+7RGHEGa7u33zUA0b-cBehadw4NKN75JtjKjOhxm2Fxg@mail.gmail.com>
Subject: Re: [PATCH] x86/tsc: Use hard-coded crystal clock for Skylake mobile
To:     Paul Menzel <pmenzel@molgen.mpg.de>
Cc:     Thomas Gleixner <tglx@linutronix.de>, x86@kernel.org,
        Ross Zwisler <zwisler@google.com>,
        Daniel Drake <drake@endlessm.com>,
        Andy Lutomirski <luto@kernel.org>,
        Borislav Petkov <bp@alien8.de>,
        "H . Peter Anvin" <hpa@zytor.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Len Brown <len.brown@intel.com>,
        Linux Upstreaming Team <linux@endlessm.com>,
        "Rafael J . Wysocki" <rafael.j.wysocki@intel.com>,
        Thorsten Leemhuis <regressions@leemhuis.info>,
        LKML <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, May 11, 2020 at 9:38 AM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>
> Dear Thomas,
>
>
> Thank you for the quick reply.
>
> Am 11.05.20 um 09:17 schrieb Thomas Gleixner:
>
> > Paul Menzel <pmenzel@molgen.mpg.de> writes:
> >
> > please send patches to LKML and not offlist.
>
> Sorry about that. From `MAINTAINERS` I thought x86@kernel.org is wanted.
> Other subsystems list LKML explicitly there.
>
> >> From: Radoslaw Biernacki <biernacki@google.com>
> >>
> >> @@ -636,10 +636,24 @@ unsigned long native_calibrate_tsc(void)
> >>       * Denverton SoCs don't report crystal clock, and also don't supp=
ort
> >>       * CPUID.0x16 for the calculation below, so hardcode the 25MHz cr=
ystal
> >>       * clock.
> >> +     * Also estimation code is not reliable and gives 1.5%  differenc=
e for
> >> +     * tsc/clock ratio on Skylake mobile. Therefore below is a hardco=
ded
> >> +     * crystal frequency for Skylake which was removed by upstream co=
mmit
> >> +     * "x86/tsc: Use CPUID.0x16 to calculate missing crystal frequenc=
y"
> >> +     * This is temporary workaround for bugs:
> >> +     * b/148108096, b/154283905, b/146787525, b/153400677, b/14817892=
9
> >> +     * chromium/1031054
> >>       */
> >> -    if (crystal_khz =3D=3D 0 &&
> >> -                    boot_cpu_data.x86_model =3D=3D INTEL_FAM6_ATOM_GO=
LDMONT_D)
> >> -            crystal_khz =3D 25000;
> >> +    if (crystal_khz =3D=3D 0) {
> >> +            switch (boot_cpu_data.x86_model) {
> >> +            case INTEL_FAM6_SKYLAKE_MOBILE:
> >> +                    crystal_khz =3D 24000;    /* 24.0 MHz */
> >> +                    break;
> >> +            case INTEL_FAM6_ATOM_GOLDMONT_X:
> >> +                    crystal_khz =3D 25000;    /* 25.0 MHz */
> >> +                    break;
> >> +            }
> >
> > Aside of being a workaround for Google issues which are probably caused=
 > by broken BIOSes
>
> Even if it was caused by broken firmware, wouldn=E2=80=99t Linux=E2=80=99=
 no regression
> policy still consider this a regression as user should be able to the
> Linux kernel =E2=80=9Cno matter what=E2=80=9D?
>
> > that patch is broken.
> >
> >     INTEL_FAM6_ATOM_GOLDMONT_D !=3D INTEL_FAM6_ATOM_GOLDMONT_X
>
> Good catch. The commit didn=E2=80=99t apply cleanly to the master branch,=
 and I
> missed this.
>
> I=E2=80=99ll wait for Radoslaw to comment before proceeding further with =
this.

We found that regression only on specific SKU which was used in one
model of ChromeBook.
What's interesting is that some other SKU is fine.

The consequences of this are rather not trivial,
so this was considered a quickfix and temporary till we develop
something better.
In contrast to ChromeOs, I know that there is no way of finding if
there are in fact regressions on generic kernel in the field (this is
SKU dependent),
but we also think that this problem should be addressed in a better
way (if possible).

We plan to work on this.
Any help is welcome.

>
>
> Kind regards,
>
> Paul