linux-kernel - Re: [PATCH] arch/cacheflush: Introduce flush_all

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 16 Aug 2022 10:42:03 -0700
From:   Dan Williams <dan.j.williams@...el.com>
To:     Davidlohr Bueso <dave@...olabs.net>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Mark Rutland <mark.rutland@....com>,
        Dave Jiang <dave.jiang@...el.com>,
        Jonathan Cameron <Jonathan.Cameron@...wei.com>,
        linux-cxl@...r.kernel.org, nvdimm@...ts.linux.dev,
        bwidawsk@...nel.org, ira.weiny@...el.com, vishal.l.verma@...el.com,
        alison.schofield@...el.com, a.manzanares@...sung.com,
        linux-arch@...r.kernel.org, Arnd Bergmann <arnd@...db.de>,
        linux-arm-kernel@...ts.infradead.org, bp@...en8.de, x86@...nel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] arch/cacheflush: Introduce flush_all_caches()

On Tue, Aug 16, 2022 at 10:30 AM Davidlohr Bueso <dave@...olabs.net> wrote:
>
> On Tue, 16 Aug 2022, Dan Williams wrote:
>
> >Peter Zijlstra wrote:
> >> On Mon, Aug 15, 2022 at 09:07:06AM -0700, Davidlohr Bueso wrote:
> >> > diff --git a/arch/x86/include/asm/cacheflush.h b/arch/x86/include/asm/cacheflush.h
> >> > index b192d917a6d0..ce2ec9556093 100644
> >> > --- a/arch/x86/include/asm/cacheflush.h
> >> > +++ b/arch/x86/include/asm/cacheflush.h
> >> > @@ -10,4 +10,7 @@
> >> >
> >> >  void clflush_cache_range(void *addr, unsigned int size);
> >> >
> >> > +#define flush_all_caches() \
> >> > +  do { wbinvd_on_all_cpus(); } while(0)
> >> > +
> >>
> >> This is horrific... we've done our utmost best to remove all WBINVD
> >> usage and here you're adding it back in the most horrible form possible
> >> ?!?
> >>
> >> Please don't do this, do *NOT* use WBINVD.
> >
> >Unfortunately there are a few good options here, and the changelog did
> >not make clear that this is continuing legacy [1], not adding new wbinvd
> >usage.
>
> While I was hoping that it was obvious from the intel.c changes that this
> was not a new wbinvd, I can certainly improve the changelog with the below.

I also think this cache_flush_region() API wants a prominent comment
clarifying the limited applicability of this API. I.e. that it is not
for general purpose usage, not for VMs, and only for select bare metal
scenarios that instantaneously invalidate wide swaths of memory.
Otherwise, I can now see how this looks like a potentially scary
expansion of the usage of wbinvd.