Received-SPF: pass (google.com: best guess record for domain of linux-crypto-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190919140650.1289963-2-arnd@arndb.de> <20190919140917.1290556-1-arnd@arndb.de>
 <f801a4c1-8fa6-8c14-120c-49c24ec84449@huawei.com>
In-Reply-To: <f801a4c1-8fa6-8c14-120c-49c24ec84449@huawei.com>
From:   Arnd Bergmann <arnd@arndb.de>
Date:   Fri, 20 Sep 2019 15:26:52 +0200
Message-ID: <CAK8P3a3jCv--VHu9r4ZTnLXXGaCjdJ6royP5LFk_9RCTTRsRBA@mail.gmail.com>
Subject: Re: [PATCH 2/2] [v2] crypto: hisilicon - allow compile-testing on x86
To:     John Garry <john.garry@huawei.com>
Cc:     Herbert Xu <herbert@gondor.apana.org.au>,
        "David S. Miller" <davem@davemloft.net>,
        Zhou Wang <wangzhou1@hisilicon.com>,
        Jonathan Cameron <Jonathan.Cameron@huawei.com>,
        Kenneth Lee <liguozhu@hisilicon.com>,
        Mao Wenan <maowenan@huawei.com>,
        Hao Fang <fanghao11@huawei.com>,
        Shiju Jose <shiju.jose@huawei.com>,
        "open list:HARDWARE RANDOM NUMBER GENERATOR CORE" 
        <linux-crypto@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-crypto-owner@vger.kernel.org
Precedence: bulk

On Fri, Sep 20, 2019 at 10:34 AM John Garry <john.garry@huawei.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.

     Arnd