Re: [Qemu-devel] [RFC/PATCH] Add a memory barrier to guest memory access functions

[David Gibson]

On Thu, May 17, 2012 at 05:09:22PM -0500, Anthony Liguori wrote:
> On 05/16/2012 09:44 PM, Benjamin Herrenschmidt wrote:
> >On Wed, 2012-05-16 at 21:28 -0500, Anthony Liguori wrote:
> >
> >>>@@ -2794,6 +2795,9 @@ void *qemu_get_ram_ptr(ram_addr_t addr)
> >>>   {
> >>>       RAMBlock *block;
> >>>
> >>>+    /* We ensure ordering for all DMA transactions */
> >>>+    dma_mb();
> >>>+
> >>
> >>I get being conservative, but I don't think this makes a lot of sense.  
> >>There
> >>are cases where the return of this function is cached (like the VGA ram 
> >>area).
> >>I think it would make more sense if you explicitly put a barrier after write
> >>operations.
> >
> >Well, it depends ... sure something that caches the result is akin to
> >map/unmap and responsible for doing its own barriers between accesses,
> >however as a whole, this means that an entire map/unmap section is
> >ordered surrounding accesses which is actually not a bad idea.
> >
> >Anyway, I'll post a different patch that adds the barrier more
> >selectively to:
> >
> >  - cpu_physical_memory_rw  (that's the obvious main one)
> >  - cpu_physical_memory_write_rom (probably overkill but
> >    not a fast path so no big deal)
> >  - ld*_* and st*_* (or do you think these should require
> >    explicit barriers in the callers ?)
> 
> ld/st should not ever be used by device emulation because they use a
> concept of "target endianness" that doesn't exist for devices.

Ah, but there are the explicit endian versions, which were used
routinely until I replaced a lot of them with ld*_dma().

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson