On 21/09/2019 11:26, Zhou Wang wrote:
On 2019/9/20 22:16, John Garry wrote:
On 20/09/2019 14:36, Arnd Bergmann wrote:
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.
Hi Arnd,
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.
So this is proprietary IP integrated with a proprietary ARMv8 implementation,
so there could be a tight coupling, the like of which Will mentioned in that thread,
but I'm doubtful.
I'm looking at the electronically translated documentation on this HW, and it reads
"The Mailbox operation performed by the CPU cannot be interleaved", and then tells
that software should lock against concurrent accesses or alternatively use a 128-bit
access. So it seems that the 128b op used is only to guarantee software is atomic.
Wang Zhou can confirm my understanding
Just to add a few more details here:
We have to do a 128bit atomic write here to trigger a mailbox. The reason is
that one QM hardware entity in one accelerator servers QM mailbox MMIO interfaces in
related PF and VFs.
A mutex can not lock different processing flows in different functions.
This means that we can have userspace drivers and the kernel driver
simultaneously accessing these mailboxes.
As Arnd mentioned, v8.4 extends the support for 16 bytes atomic stp to some kinds of
normal memory, but for device memory, it is still implementation defined. For this
SoC(Kunpeng920) which has QM/ZIP, if the address is 128bit aligned, stp will be atomic.
The offset of QM mailbox is 128bit aligned, so it is safe here.
The strange thing (to me) about this hw is that we cannot interleave
mailbox accesses on different functions - with 2x 64b accesses, for
example. This is the reason that we require 128b accesses.
The upshot is that we can't switch to generic code unfortunately.
But I am not sure if the change is right, as I mentioned originally,
above ***.
I'll leave to Arnd's better judgment :)
Thanks,
John
Best,
Zhou
If true, I see that we seem to be already guaranteeing mutual exclusion in qm_mb(),
in taking a mutex.
Thanks,
John
Arnd
.
.
.
