From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:45975) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SV8sz-0003A9-TA for qemu-devel@nongnu.org; Thu, 17 May 2012 18:09:31 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SV8sy-0002cY-1Z for qemu-devel@nongnu.org; Thu, 17 May 2012 18:09:29 -0400 Received: from mail-ob0-f173.google.com ([209.85.214.173]:52763) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SV8sx-0002cR-Sg for qemu-devel@nongnu.org; Thu, 17 May 2012 18:09:27 -0400 Received: by obbwd20 with SMTP id wd20so3667533obb.4 for ; Thu, 17 May 2012 15:09:26 -0700 (PDT) Message-ID: <4FB57712.7030806@codemonkey.ws> Date: Thu, 17 May 2012 17:09:22 -0500 From: Anthony Liguori MIME-Version: 1.0 References: <1336625347-10169-1-git-send-email-benh@kernel.crashing.org> <1336625347-10169-14-git-send-email-benh@kernel.crashing.org> <4FB1A8BF.7060503@codemonkey.ws> <20120515014449.GF30229@truffala.fritz.box> <1337142938.6727.122.camel@pasglop> <4FB4028F.7070003@codemonkey.ws> <1337213257.30558.22.camel@pasglop> <1337214293.30558.25.camel@pasglop> <1337215928.30558.28.camel@pasglop> <4FB46257.3060706@codemonkey.ws> <1337222685.30558.34.camel@pasglop> In-Reply-To: <1337222685.30558.34.camel@pasglop> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC/PATCH] Add a memory barrier to guest memory access functions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Benjamin Herrenschmidt Cc: "Michael S. Tsirkin" , qemu-devel@nongnu.org, David Gibson 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. So no, I don't think you need to put barriers there as they should only be used by VCPU emulation code. > Note that with the above, cpu_physical_memory_map and unmap will > imply a barrier when using bounce buffers, it would make sense to also > provide the same barrier when not. > > That does actually make sense for the same reason explained above, > ie, when those are used for a DMA transfer via async IO, that guarantees > ordering of the "block" vs. surrounding accesses even if accesses within > the actual map/unmap region are not ordered vs. each other. > > Any objection ? I think so. I'd like to see a better comment about barrier usage at the top of the file or something like that too. Regards, Anthony Liguori > > Cheers, > Ben. > > >