linux-kernel - Re: [PATCH 2/2] [v2] crypto: hisilicon

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:   Fri, 20 Sep 2019 15:36:37 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     John Garry <john.garry@...wei.com>
Cc:     Herbert Xu <herbert@...dor.apana.org.au>,
        "David S. Miller" <davem@...emloft.net>,
        Zhou Wang <wangzhou1@...ilicon.com>,
        Jonathan Cameron <Jonathan.Cameron@...wei.com>,
        Kenneth Lee <liguozhu@...ilicon.com>,
        Mao Wenan <maowenan@...wei.com>,
        Hao Fang <fanghao11@...wei.com>,
        Shiju Jose <shiju.jose@...wei.com>,
        "open list:HARDWARE RANDOM NUMBER GENERATOR CORE" 
        <linux-crypto@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Will Deacon <will@...nel.org>
Subject: Re: [PATCH 2/2] [v2] crypto: hisilicon - allow compile-testing on x86

On Fri, Sep 20, 2019 at 3:26 PM Arnd Bergmann <arnd@...db.de> wrote:
>
> On Fri, Sep 20, 2019 at 10:34 AM John Garry <john.garry@...wei.com> wrote:
>
> > > +     if (!IS_ENABLED(CONFIG_ARM64)) {
> > > +             memcpy_toio(fun_base, src, 16);
> > > +             wmb();
> > > +             return;
> > > +     }
> > > +
> > >       asm volatile("ldp %0, %1, %3\n"
> > >                    "stp %0, %1, %2\n"
> > >                    "dsb sy\n"
> > >
> >
> > As I understand, this operation needs to be done atomically. So - even
> > though your change is just for compile testing - the memcpy_to_io() may
> > not do the same thing on other archs, right?
> >
> > I just wonder if it's right to make that change, or at least warn the
> > imaginary user of possible malfunction for !arm64.
>
> It's probably not necessary here. From what I can tell from the documentation,
> this is only safe on ARMv8.4 or higher anyway, earlier ARMv8.x implementations
> don't guarantee that an stp arrives on the bus in one piece either.
>
> Usually, hardware like this has no hard requirement on an atomic store,
> it just needs the individual bits to arrive in a particular order, and then
> triggers the update on the last bit that gets stored. If that is the case here
> as well, it might actually be better to use two writeq_relaxed() and
> a barrier. This would also solve the endianess issue.

See also https://lkml.org/lkml/2018/1/26/554 for a previous attempt
to introduce 128-bit MMIO accessors, this got rejected since they
are not atomic even on ARMv8.4.

    Arnd