On Fri, Jun 22, 2012 at 3:29 AM, Benjamin Herrenschmidt <b...@kernel.crashing.org> wrote: > The emulated devices can run simultaneously with the guest, so > we need to be careful with ordering of load and stores done by > them to the guest system memory, which need to be observed in > the right order by the guest operating system. > > This adds a barrier call to the basic DMA read/write ops which > is currently implemented as a smp_mb(), but could be later > improved for more fine grained control of barriers. > > Additionally, a _relaxed() variant of the accessors is provided > to easily convert devices who would be performance sensitive > and negatively impacted by the change. > > Signed-off-by: Benjamin Herrenschmidt <b...@kernel.crashing.org> > --- > dma-helpers.c | 2 ++ > dma.h | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++-- > 2 files changed, 52 insertions(+), 2 deletions(-) > > diff --git a/dma-helpers.c b/dma-helpers.c > index 2e09ceb..35cb500 100644 > --- a/dma-helpers.c > +++ b/dma-helpers.c > @@ -31,6 +31,8 @@ static void do_dma_memory_set(dma_addr_t addr, uint8_t c, > dma_addr_t len) > > int dma_memory_set(DMAContext *dma, dma_addr_t addr, uint8_t c, dma_addr_t > len) > { > + dma_barrier(dma, DMA_DIRECTION_FROM_DEVICE); > + > if (dma_has_iommu(dma)) { > return iommu_dma_memory_set(dma, addr, c, len); > } > diff --git a/dma.h b/dma.h > index f52a656..54cdd24 100644 > --- a/dma.h > +++ b/dma.h > @@ -13,6 +13,7 @@ > #include <stdio.h> > #include "hw/hw.h" > #include "block.h" > +#include "kvm.h" > > typedef struct DMAContext DMAContext; > typedef struct ScatterGatherEntry ScatterGatherEntry; > @@ -65,6 +66,30 @@ struct DMAContext { > DMAUnmapFunc *unmap; > }; > > +static inline void dma_barrier(DMAContext *dma, DMADirection dir) > +{ > + /* > + * This is called before DMA read and write operations > + * unless the _relaxed form is used and is responsible > + * for providing some sane ordering of accesses vs > + * concurrently running VCPUs. > + * > + * Users of map(), unmap() or lower level st/ld_* > + * operations are responsible for providing their own > + * ordering via barriers. > + * > + * This primitive implementation does a simple smp_mb() > + * before each operation which provides pretty much full > + * ordering. > + * > + * A smarter implementation can be devised if needed to > + * use lighter barriers based on the direction of the > + * transfer, the DMA context, etc... > + */ > + if (kvm_enabled()) > + smp_mb();
Missing braces. Please use checkpatch.pl to avoid issues like this. > +} > + > static inline bool dma_has_iommu(DMAContext *dma) > { > return !!dma; > @@ -88,8 +113,9 @@ static inline bool dma_memory_valid(DMAContext *dma, > > int iommu_dma_memory_rw(DMAContext *dma, dma_addr_t addr, > void *buf, dma_addr_t len, DMADirection dir); > -static inline int dma_memory_rw(DMAContext *dma, dma_addr_t addr, > - void *buf, dma_addr_t len, DMADirection dir) > +static inline int dma_memory_rw_relaxed(DMAContext *dma, dma_addr_t addr, > + void *buf, dma_addr_t len, > + DMADirection dir) > { > if (!dma_has_iommu(dma)) { > /* Fast-path for no IOMMU */ > @@ -101,6 +127,28 @@ static inline int dma_memory_rw(DMAContext *dma, > dma_addr_t addr, > } > } > > +static inline int dma_memory_read_relaxed(DMAContext *dma, dma_addr_t addr, > + void *buf, dma_addr_t len) > +{ > + return dma_memory_rw_relaxed(dma, addr, buf, len, > DMA_DIRECTION_TO_DEVICE); > +} > + > +static inline int dma_memory_write_relaxed(DMAContext *dma, dma_addr_t addr, > + const void *buf, dma_addr_t len) > +{ > + return dma_memory_rw_relaxed(dma, addr, (void *)buf, len, > + DMA_DIRECTION_FROM_DEVICE); > +} > + > +static inline int dma_memory_rw(DMAContext *dma, dma_addr_t addr, > + void *buf, dma_addr_t len, > + DMADirection dir) > +{ > + dma_barrier(dma, dir); > + > + return dma_memory_rw_relaxed(dma, addr, buf, len, dir); > +} > + > static inline int dma_memory_read(DMAContext *dma, dma_addr_t addr, > void *buf, dma_addr_t len) > { > -- > 1.7.9.5 > >