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
