Re: [PATCH 2/2] [v2] crypto: hisilicon - allow compile-testing on x86

[Arnd Bergmann]

On Fri, Sep 20, 2019 at 3:26 PM Arnd Bergmann <arnd@xxxxxxxx> wrote:
>
> On Fri, Sep 20, 2019 at 10:34 AM John Garry <john.garry@xxxxxxxxxx> 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