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
