Re: [PATCH] arm64: io: Relax implicit barriers in default I/O accessors

[Dave Martin <Dave.Martin@arm.com>]

On Mon, Aug 05, 2019 at 12:35:04PM +0100, Will Deacon wrote:
> On Mon, Aug 05, 2019 at 11:39:05AM +0100, Catalin Marinas wrote:
> > On Mon, Jul 29, 2019 at 06:05:18PM +0100, Will Deacon wrote:
> > > As a concrete example, consider the following:
> > > 
> > > 	memcpy(dma_buffer, data, bufsz);
> > > 	writel(DMA_START, dev->ctrl_reg);
> > > 
> > > A DMB ST instruction between the final write to the DMA buffer and the
> > > write to the control register will ensure that the writes to the DMA
> > > buffer are observed before the write to the control register by all
> > > observers. Put another way, if an observer can see the write to the
> > > control register, it can also see the writes to memory.
> > 
> > I think one of the counter arguments here were that a device does not
> > "observe" the write to the control register as that's not a master
> > access (by the device). Do you mean that if another CPU (not the device)
> > can observe the writel(), it would have also observed the write to the
> > DMA buffer (assuming the DMB)? Since the device is also an observer of
> > the DMA buffer accesses, the multi-copy atomicity ensures that the
> > device is also seeing the buffer updates following a DMB.
> 
> Yes, that's right.
> 
> > > This has always
> > > been the case and is not sufficient to provide the ordering required by
> > > Linux, since there is no guarantee that the master interface of the
> > > DMA-capable device has observed either of the accesses. However, in an
> > > other-multi-copy atomic world, we can infer two things:
> > > 
> > >   1. A write arriving at an endpoint shared between multiple CPUs is
> > >      visible to all CPUs
> > > 
> > >   2. A write that is visible to all CPUs is also visible to all other
> > >      observers in the shareability domain
> > > 
> > > Pieced together, this allows us to use DMB OSHST for our default I/O
> > > write accessors and DMB OSHLD for our default I/O read accessors (the
> > > outer-shareability is for handling non-cacheable mappings) for shared
> > > devices. Memory-mapped, DMA-capable peripherals that are private to a
> > > CPU (i.e. inaccessible to other CPUs) still require the DSB, however
> > > these are few and far between and typically require special treatment
> > > anyway which is outside of the scope of the portable driver API (e.g.
> > > GIC, page-table walker, SPE profiler).

[...]

I think there may be something missing from the argument:

One supposes some kind of causal dependency between the writel() and
any dependent read done by "the device".  This is entirely up to the
device implementation, but sanity seems to require that this depencency
is at least as strong as an address dependency, so that the device
mustn't speculatively read dma_buffer in advance of receiving the
DMA_START command etc.  If not, you probably have a badly-designed or
broken device and you deserve to have to carry ugly workarounds in
your drivers.

The multi-copy-atomicity requirement effectively rules out some
bus topologies: if a device masters directly onto a bus that doesn't
share a common ancestor and shareability domain with the bus its slave
interface is connected to, there would be no single place to resolve
the DMB (unless some explicit logic were added to handle that somehow).

It's reasonable to assume that the hardware is somewhat sane for the
default I/O accessors: for weird hardware, drivers would have to work
around it explicitly with extra synchronisation but we wouldn't expect
this to be common.

The per-CPU device case is one where there may be an explicitly weird
topology, but this only applies to a few specific devices and we can
work around those as appropriate.

Does that makes sense?  This might be "obvious", so I'm not sure we
need to write anything.  Just checking my understanding.

Cheers
---Dave

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