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;
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
From:   Andy Lutomirski <luto@amacapital.net>
Mime-Version: 1.0 (1.0)
Subject: Re: I915 CI-run with kfence enabled, issues found
Date:   Mon, 29 Mar 2021 16:26:41 -0700
Message-Id: <2F989294-F0D4-4F1C-86A6-E657F60EF2A8@amacapital.net>
References: <CANpmjNO+_4C0dYs6K8Ofy-xVSYxO8OtXSRbW6vCXBYdjJSjqbQ@mail.gmail.com>
Cc:     Dave Hansen <dave.hansen@intel.com>,
        "Sarvela, Tomi P" <tomi.p.sarvela@intel.com>,
        kasan-dev <kasan-dev@googlegroups.com>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Andy Lutomirski <luto@kernel.org>,
        Peter Zijlstra <peterz@infradead.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
        the arch/x86 maintainers <x86@kernel.org>,
        "H. Peter Anvin" <hpa@zytor.com>,
        LKML <linux-kernel@vger.kernel.org>
In-Reply-To: <CANpmjNO+_4C0dYs6K8Ofy-xVSYxO8OtXSRbW6vCXBYdjJSjqbQ@mail.gmail.com>
To:     Marco Elver <elver@google.com>
Precedence: bulk


> On Mar 29, 2021, at 2:55 PM, Marco Elver <elver@google.com> wrote:
>=20
> =EF=BB=BFOn Mon, 29 Mar 2021 at 23:47, Andy Lutomirski <luto@amacapital.ne=
t> wrote:
>>=20
>>=20
>>>> On Mar 29, 2021, at 2:34 PM, Marco Elver <elver@google.com> wrote:
>>>=20
>>> =EF=BB=BFOn Mon, 29 Mar 2021 at 23:03, Dave Hansen <dave.hansen@intel.co=
m> wrote:
>>>>> On 3/29/21 10:45 AM, Marco Elver wrote:
>>>>>> On Mon, 29 Mar 2021 at 19:32, Dave Hansen <dave.hansen@intel.com> wro=
te:
>>>>> Doing it to all CPUs is too expensive, and we can tolerate this being
>>>>> approximate (nothing bad will happen, KFENCE might just miss a bug and=

>>>>> that's ok).
>>>> ...
>>>>>> BTW, the preempt checks in flush_tlb_one_kernel() are dependent on KP=
TI
>>>>>> being enabled.  That's probably why you don't see this everywhere.  W=
e
>>>>>> should probably have unconditional preempt checks in there.
>>>>>=20
>>>>> In which case I'll add a preempt_disable/enable() pair to
>>>>> kfence_protect_page() in arch/x86/include/asm/kfence.h.
>>>>=20
>>>> That sounds sane to me.  I'd just plead that the special situation (not=

>>>> needing deterministic TLB flushes) is obvious.  We don't want any folks=

>>>> copying this code.
>>>>=20
>>>> BTW, I know you want to avoid the cost of IPIs, but have you considered=

>>>> any other low-cost ways to get quicker TLB flushes?  For instance, you
>>>> could loop over all CPUs and set cpu_tlbstate.invalidate_other=3D1.  Th=
at
>>>> would induce a context switch at the next context switch without needin=
g
>>>> an IPI.
>>>=20
>>> This is interesting. And it seems like it would work well for our
>>> usecase. Ideally we should only flush entries related to the page we
>>> changed. But it seems invalidate_other would flush the entire TLB.
>>>=20
>>> With PTI, flush_tlb_one_kernel() already does that for the current
>>> CPU, but now we'd flush entire TLBs for all CPUs and even if PTI is
>>> off.
>>>=20
>>> Do you have an intuition for how much this would affect large
>>> multi-socket systems? I currently can't quite say, and would err on
>>> the side of caution.
>>=20
>> Flushing the kernel TLB for all addresses
>> Is rather pricy. ISTR 600 cycles on Skylake, not to mention the cost of l=
osing the TLB.  How common is this?
>=20
> AFAIK, invalidate_other resets the asid, so it's not explicit and
> perhaps cheaper?
>=20
> In any case, if we were to do this, it'd be based on the sample
> interval of KFENCE, which can be as low as 1ms. But this is a
> production debugging feature, so the target machines are not test
> machines. For those production deployments we'd be looking at every
> ~500ms. But I know of other deployments that use <100ms.
>=20
> Doesn't sound like much, but as you say, I also worry a bit about
> losing the TLB across >100 CPUs even if it's every 500ms.

On non-PTI, the only way to zap kernel mappings is to do a global flush, eit=
her via INVPCID (expensive) or CR4 (extra expensive). In PTI mode, it=E2=80=99=
s plausible that the implicit flush is good enough, and I=E2=80=99d be happy=
 to review the patch, but it=E2=80=99s a PTI only thing.  Much less expensiv=
e in PTI mode, too, because it only needs to flush kernel mappings.

If this is best-effort, it might be better to have some work in the exit to u=
sermode path or a thread or similar that periodically does targeting single-=
page zaps.