LSE atomic op ordering is weaker than intended?

7 messages, 2 authors, 2021-03-04 · open the first message on its own page

LSE atomic op ordering is weaker than intended?

From: Hector Martin <hidden>
Date: 2021-03-03 22:44:22

Hi Will and everyone else,

While yak shaving the AIC driver ordering minutiae, I came across this.

atomic_t.txt describes "fully ordered" atomic ops as follows:

 > Fully ordered primitives are ordered against everything prior and
 > everything subsequent. Therefore a fully ordered primitive is like
 > having an smp_mb() before and an smp_mb() after the primitive.

And among those ops are the atomic_fetch_* ops. These are implemented as 
e.g. LDSETAL, with acquire-release semantics.

However, the *AL LSE ops have acquire semantics *for the read* and 
release semantics *for the write*. As independent components of the same 
atomic op, I cannot find anything in the ARM ARM that would imply 
ordering between the Load-Acquire and *prior* memory operations, nor 
ordering between the Store-Release and *subsequent* memory operations.

So it would seem these ops are not in fact fully ordered, but rather, 
only order the read component against prior ops, and the write component 
against subsequent ops.

Put another way: the current implementation means that unqualified ops 
are equal to _acquire + _release semantics as they are described in 
atomic_t.txt, but that is weaker than "fully ordered".

Throwing this litmus test at herd7 seems to confirm this theory:

AArch64 lse-atomic-al-ops-are-not-fully-ordered
""
{
0:X1=x; 0:X3=y;
1:X1=x; 1:X3=y;
}
  P0                   | P1                  ;
  MOV X0, #1           | MOV X0, #1          ;
  LDSETAL X0, X2, [X1] | LDSETAL X0, X2, [X3];
  LDR X4, [X3]         | LDR X4, [X1]        ;
exists (0:X4=0 /\ 1:X4=0)

The positive result goes away adding a DMB ISH (i.e. smp_mb()) after the 
atomic ops, which contradicts the atomic_t.txt claim.

Did I miss something, or is this in fact an issue?

(And while I'm talking to the right people: this issue aside, do atomic 
ops on Normal memory create ordering with Device memory ops, or are 
there no guarantees there due to the fact that Normal memory is mapped 
inner-shareable and the ordering guarantees thus do not extend to 
outer-shareable Device accesses? My currenty understanding is the 
latter, but I find the ARM ARM wording hard to conclusively grok here.)

-- 
Hector Martin (marcan@marcan.st)
Public Key: https://mrcn.st/pub

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: LSE atomic op ordering is weaker than intended?

From: Will Deacon <will@kernel.org>
Date: 2021-03-03 23:02:58

Hi Hector,

On Wed, Mar 03, 2021 at 10:05:19PM +0900, Hector Martin wrote:
While yak shaving the AIC driver ordering minutiae, I came across this.

atomic_t.txt describes "fully ordered" atomic ops as follows:
quoted
Fully ordered primitives are ordered against everything prior and
everything subsequent. Therefore a fully ordered primitive is like
having an smp_mb() before and an smp_mb() after the primitive.
And among those ops are the atomic_fetch_* ops. These are implemented as
e.g. LDSETAL, with acquire-release semantics.

However, the *AL LSE ops have acquire semantics *for the read* and release
semantics *for the write*. As independent components of the same atomic op,
I cannot find anything in the ARM ARM that would imply ordering between the
Load-Acquire and *prior* memory operations, nor ordering between the
Store-Release and *subsequent* memory operations.

So it would seem these ops are not in fact fully ordered, but rather, only
order the read component against prior ops, and the write component against
subsequent ops.

Put another way: the current implementation means that unqualified ops are
equal to _acquire + _release semantics as they are described in
atomic_t.txt, but that is weaker than "fully ordered".

Throwing this litmus test at herd7 seems to confirm this theory:

AArch64 lse-atomic-al-ops-are-not-fully-ordered
""
{
0:X1=x; 0:X3=y;
1:X1=x; 1:X3=y;
}
 P0                   | P1                  ;
 MOV X0, #1           | MOV X0, #1          ;
 LDSETAL X0, X2, [X1] | LDSETAL X0, X2, [X3];
 LDR X4, [X3]         | LDR X4, [X1]        ;
exists (0:X4=0 /\ 1:X4=0)

The positive result goes away adding a DMB ISH (i.e. smp_mb()) after the
atomic ops, which contradicts the atomic_t.txt claim.

Did I miss something, or is this in fact an issue?
Both. The -AL atomics are actually special-cased in the
"barrier-ordered-before" relation in the Arm ARM:

  [RW1 is barrier-ordered-before if]
  * RW1 appears in program order before an atomic instruction with both
    Acquire and Release semantics that appears in program order before
    RW2.

However, that isn't sufficient to order prior accesses with the "load part"
of the RmW and later accesses with the "store part" of the RmW, as you have
observed in your test. I'm aware of some pending proposals in this area of
the architecture, so I'm reluctant to make any changes until that's
bottomed-out, but I'll make a note to chase that up.
(And while I'm talking to the right people: this issue aside, do atomic ops
on Normal memory create ordering with Device memory ops, or are there no
guarantees there due to the fact that Normal memory is mapped
inner-shareable and the ordering guarantees thus do not extend to
outer-shareable Device accesses? My currenty understanding is the latter,
but I find the ARM ARM wording hard to conclusively grok here.)
Outer-shareable is a superset of inner-shareable, but I think this would be
easier with a specific example. I'll go and look at the AIC patch, since
this is all a lot easier to talk about in the context of some real code.

Which is the latest version I should look at?

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: LSE atomic op ordering is weaker than intended?

From: Hector Martin <hidden>
Date: 2021-03-04 00:25:08

On 04/03/2021 00.36, Will Deacon wrote:
quoted
Did I miss something, or is this in fact an issue?
Both. The -AL atomics are actually special-cased in the
"barrier-ordered-before" relation in the Arm ARM:

   [RW1 is barrier-ordered-before if]
   * RW1 appears in program order before an atomic instruction with both
     Acquire and Release semantics that appears in program order before
     RW2.

However, that isn't sufficient to order prior accesses with the "load part"
of the RmW and later accesses with the "store part" of the RmW, as you have
observed in your test. I'm aware of some pending proposals in this area of
the architecture, so I'm reluctant to make any changes until that's
bottomed-out, but I'll make a note to chase that up.
I had actually seen that part of the spec, and looked at it sideways a 
few times, but concluded it wasn't giving me the ordering guarantees I 
was looking for (this was before I wrote the litmus test). You're right, 
it does nonetheless make it stronger than the mere combination of 
_acquire and _release semantics.

Glad to hear this is something being worked on! I've been giving myself 
a crash course in memory model minutiae over the past few weeks :)
quoted
(And while I'm talking to the right people: this issue aside, do atomic ops
on Normal memory create ordering with Device memory ops, or are there no
guarantees there due to the fact that Normal memory is mapped
inner-shareable and the ordering guarantees thus do not extend to
outer-shareable Device accesses? My currenty understanding is the latter,
but I find the ARM ARM wording hard to conclusively grok here.)
Outer-shareable is a superset of inner-shareable, but I think this would be
easier with a specific example. I'll go and look at the AIC patch, since
this is all a lot easier to talk about in the context of some real code.

Which is the latest version I should look at?
I'm just about to send a v3 tomorrow, so I'll CC you on that patch 
(don't bother with v2, this part of the code is changing a lot). That 
said, it's basically the following two sequences:

A:

// ...stuff that needs to be ordered prior to the atomic here
ret = atomic_fetch_or_release(flags...)
if (condition on ret and unrelated stuff) {
	writel(reg_send, ...) // includes pre-barrier
}

B:

writel_relaxed(reg_ack, ...)
dma_wmb() // need a post-barrier
atomic_fetch_andnot_acquire(flags...)
// ...stuff that needs to be ordered after the atomic here

My current understanding is that I cannot drop the dma_wmb() in B and 
use _relaxed in A() and instead use full-ordered atomic ops, because the 
atomic ops, operating on normal IS memory, would not make any statements 
regarding ordering with device OS memory. I need the I/O writes to be 
ordered with regard to the atomics.

-- 
Hector Martin (marcan@marcan.st)
Public Key: https://mrcn.st/pub

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: LSE atomic op ordering is weaker than intended?

From: Will Deacon <will@kernel.org>
Date: 2021-03-04 00:34:06

On Thu, Mar 04, 2021 at 03:04:20AM +0900, Hector Martin wrote:
On 04/03/2021 00.36, Will Deacon wrote:
quoted
quoted
Did I miss something, or is this in fact an issue?
Both. The -AL atomics are actually special-cased in the
"barrier-ordered-before" relation in the Arm ARM:

   [RW1 is barrier-ordered-before if]
   * RW1 appears in program order before an atomic instruction with both
     Acquire and Release semantics that appears in program order before
     RW2.

However, that isn't sufficient to order prior accesses with the "load part"
of the RmW and later accesses with the "store part" of the RmW, as you have
observed in your test. I'm aware of some pending proposals in this area of
the architecture, so I'm reluctant to make any changes until that's
bottomed-out, but I'll make a note to chase that up.
I had actually seen that part of the spec, and looked at it sideways a few
times, but concluded it wasn't giving me the ordering guarantees I was
looking for (this was before I wrote the litmus test). You're right, it does
nonetheless make it stronger than the mere combination of _acquire and
_release semantics.

Glad to hear this is something being worked on! I've been giving myself a
crash course in memory model minutiae over the past few weeks :)
quoted
quoted
(And while I'm talking to the right people: this issue aside, do atomic ops
on Normal memory create ordering with Device memory ops, or are there no
guarantees there due to the fact that Normal memory is mapped
inner-shareable and the ordering guarantees thus do not extend to
outer-shareable Device accesses? My currenty understanding is the latter,
but I find the ARM ARM wording hard to conclusively grok here.)
Outer-shareable is a superset of inner-shareable, but I think this would be
easier with a specific example. I'll go and look at the AIC patch, since
this is all a lot easier to talk about in the context of some real code.

Which is the latest version I should look at?
I'm just about to send a v3 tomorrow, so I'll CC you on that patch (don't
bother with v2, this part of the code is changing a lot). That said, it's
basically the following two sequences:

A:

// ...stuff that needs to be ordered prior to the atomic here
ret = atomic_fetch_or_release(flags...)
if (condition on ret and unrelated stuff) {
	writel(reg_send, ...) // includes pre-barrier
Looks you have a control dependency here, so I think that can be
writel_relaxed() [with a comment!].
}

B:

writel_relaxed(reg_ack, ...)
Interesting, is there no MMIO read on the IRQ path?
dma_wmb() // need a post-barrier
atomic_fetch_andnot_acquire(flags...)
// ...stuff that needs to be ordered after the atomic here
This looks pretty dodgy to me as a later read could drift up before teh
writel_relaxed().
My current understanding is that I cannot drop the dma_wmb() in B and use
_relaxed in A() and instead use full-ordered atomic ops, because the atomic
ops, operating on normal IS memory, would not make any statements regarding
ordering with device OS memory. I need the I/O writes to be ordered with
regard to the atomics.
If this is all to do with IPIs between CPUs and the "stuff that needs to be
ordered <before/after> the atomic" is all normal, cacheable memory then I
dont't think you need to worry about the outer-shareable domain at all. The
only reason we have that in dma_*mb() is because the coherent DMA buffers
might be non-cacheable to deal with non-coherent DMA.

But hack something together, and I'll look at your v3 to see what's going
on.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: LSE atomic op ordering is weaker than intended?

From: Hector Martin <hidden>
Date: 2021-03-04 00:51:26

On 04/03/2021 03.40, Will Deacon wrote:
quoted
// ...stuff that needs to be ordered prior to the atomic here
ret = atomic_fetch_or_release(flags...)
if (condition on ret and unrelated stuff) {
	writel(reg_send, ...) // includes pre-barrier
Looks you have a control dependency here, so I think that can be
writel_relaxed() [with a comment!].
Wouldn't the control dependency be strictly on the *read* portion of the 
atomic, while I'm looking to order on the write part?

Though, actually, I omitted something; the unrelated stuff includes an 
atomic_read_acquire that will create order against the write-release 
portion of the atomic (not per the Linux cross-platform docs, but yes 
per ARMv8 B2.3.3) and *that* should be a suitable control dependency if 
I'm not mistaken.
quoted
writel_relaxed(reg_ack, ...)
Interesting, is there no MMIO read on the IRQ path?
There's a prior read, but that doesn't help here; the important part is 
that this writel (ACKing the hardware IPI) cannot move after the atomic 
op after it (figuring out which software IPIs it represents), because if 
it does we might ACK a subsequent IPI we're not about to handle.
quoted
dma_wmb() // need a post-barrier
atomic_fetch_andnot_acquire(flags...)
// ...stuff that needs to be ordered after the atomic here
This looks pretty dodgy to me as a later read could drift up before teh
writel_relaxed().
That's fine, as long as those reads are ordered after the atomic fetch 
portion itself (which they should be since it's an _acquire).

AIUI the worst case ordering this can result in is:

- read portion of atomic
- the entire rest of the code (if it does no writes)
- writel_relaxed()
- write portion of atomic

That's fine: the result of the atomic read gives us license to run the 
rest of the code; the writel doesn't need to have happened. The 
important part is that the write portion of the atomic can't happen 
before the writel.
If this is all to do with IPIs between CPUs and the "stuff that needs to be
ordered <before/after> the atomic" is all normal, cacheable memory then I
dont't think you need to worry about the outer-shareable domain at all. The
only reason we have that in dma_*mb() is because the coherent DMA buffers
might be non-cacheable to deal with non-coherent DMA.
What worried me is whether the order created by atomics on normal memory 
might not extend to MMIO ops on device memory, which are treated as 
outer-shareable (per D5.5.1).

I don't fully understand how the entire B2.3 section on ordering 
interacts across memory regions having different attributes. That 
section talks about a Common Shareability Domain, and also explicitly 
mentions only Normal memory locations, so I'm having a lot of trouble 
piecing together how MMIO ops are treated within the entire memory 
ordering framework (and all the litmus test stuff seems to assume 
everythign is IS...).

All the actual memory involved in everything but the writel() stuff is 
all normal cacheable memory; what I need to ensure is that I can 
transitively create memory ordering constraints between normal memory 
accesses through MMIO writes.

That is, that I can create two ordered sequences on two PEs:

(normal access 1; mmio write 1)
(mmio write 2; normal access 2)

Such that I can assert that at least one of the following orderings are 
true:

(normal access 1; normal access 2)
(as observed by the PEs)

or:

(mmio write 2; mmio write 1)
(as observed by the MMIO device)

But never this combination:

(normal access 2; normal access 1)
(mmio write 1; mmio write 2)
But hack something together, and I'll look at your v3 to see what's going
on.
Please do, hopefully the full code makes more sense.

-- 
Hector Martin (marcan@marcan.st)
Public Key: https://mrcn.st/pub

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: LSE atomic op ordering is weaker than intended?

From: Will Deacon <will@kernel.org>
Date: 2021-03-04 01:48:06

On Thu, Mar 04, 2021 at 04:37:31AM +0900, Hector Martin wrote:
On 04/03/2021 03.40, Will Deacon wrote:
quoted
quoted
// ...stuff that needs to be ordered prior to the atomic here
ret = atomic_fetch_or_release(flags...)
if (condition on ret and unrelated stuff) {
	writel(reg_send, ...) // includes pre-barrier
Looks you have a control dependency here, so I think that can be
writel_relaxed() [with a comment!].
Wouldn't the control dependency be strictly on the *read* portion of the
atomic, while I'm looking to order on the write part?
Ah, sorry, I didn't realise you were ordering against the write. I'll wait
for the v3, but just one thing that might help:
That is, that I can create two ordered sequences on two PEs:

(normal access 1; mmio write 1)
(mmio write 2; normal access 2)

Such that I can assert that at least one of the following orderings are
true:

(normal access 1; normal access 2)
(as observed by the PEs)

or:

(mmio write 2; mmio write 1)
(as observed by the MMIO device)
One thing to bear in mind here is that the MMIO device cannot "observe"
anything in the architectural sense because it is a slave interface. In
order to observe a memory access, you must emit a read or a write
transaction, and it's this notion of observation which the shareability
domains are built around.

So for this example, we can talk about the CPUs (in the inner-shareable
domain) observing the MMIO writes and inner-shareable barriers are
sufficient for that. The device mapping of the MMIO registers will then
ensure that they arrive at the endpoint in that order too.
But never this combination:

(normal access 2; normal access 1)
(mmio write 1; mmio write 2)
quoted
But hack something together, and I'll look at your v3 to see what's going
on.
Please do, hopefully the full code makes more sense.
Hopefully, as I don't grok how this deals with spurious interrupts if it
only does MMIO writes.

Will

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: LSE atomic op ordering is weaker than intended?

From: Hector Martin <hidden>
Date: 2021-03-04 08:20:47

On 04/03/2021 06.38, Will Deacon wrote:
One thing to bear in mind here is that the MMIO device cannot "observe"
anything in the architectural sense because it is a slave interface. In
order to observe a memory access, you must emit a read or a write
transaction, and it's this notion of observation which the shareability
domains are built around.

So for this example, we can talk about the CPUs (in the inner-shareable
domain) observing the MMIO writes and inner-shareable barriers are
sufficient for that. The device mapping of the MMIO registers will then
ensure that they arrive at the endpoint in that order too.
Ah! That makes sense now. So as long as the CPUs agree about the MMIO 
ordering, the endpoint will see that ordering too.

In that case I should be able to get away with simple SMP/atomic 
barriers (or nothing where the control dependency implies order).
Hopefully, as I don't grok how this deals with spurious interrupts if it
only does MMIO writes.
It's an implementation of a virtual (software) interrupt controller 
multiplexing several IPIs over one, hence the atomics stand in for what 
would be MMIO on a real controller. The actual hardware IPI underlying 
it all does use a single MMIO read to fetch/mask the event at the 
controller, but then the tricky ordering is between ACKing that IPI 
itself (which is a write) and the virtual stuff on top.

Spurious hardware IPIs are possible in this model, and are taken care of 
by the atomic flags being the source of truth for what is actually 
pending downstream; what I need to make sure to avoid is the opposite 
case where a virtual IPI ends up pending and unmasked, but the hardware 
IPI is not correctly raised due to a race.

For additional fun: this SoC supports a completely separate "fast IPI" 
mechanism built on IMP-DEF system registers without any MMIO, which 
requires ordering against not loads and stores, but rather sysregs. I 
have no idea if this stuff is formally defined in the architecture in 
any strict sense (especially since this is IMP-DEF), but I'm probably 
going to have to run some litmus-style experiments to see how the CPU 
behaves in practice. Right now we don't use/support this mechanism; that 
will come later. Still just has one IPI per CPU though, so it won't let 
us get rid of the virtual stuff on top.

-- 
Hector Martin (marcan@marcan.st)
Public Key: https://mrcn.st/pub

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help