From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:54369) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S5wJA-0001Lu-O3 for qemu-devel@nongnu.org; Fri, 09 Mar 2012 04:40:40 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S5wJ5-0006LD-MB for qemu-devel@nongnu.org; Fri, 09 Mar 2012 04:40:20 -0500 Received: from mx1.redhat.com ([209.132.183.28]:16710) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S5wJ5-0006L2-E2 for qemu-devel@nongnu.org; Fri, 09 Mar 2012 04:40:15 -0500 Message-ID: <4F59D0C7.9040708@redhat.com> Date: Fri, 09 Mar 2012 10:43:35 +0100 From: Kevin Wolf MIME-Version: 1.0 References: <1331269308-22372-1-git-send-email-david@gibson.dropbear.id.au> <1331269308-22372-8-git-send-email-david@gibson.dropbear.id.au> In-Reply-To: <1331269308-22372-8-git-send-email-david@gibson.dropbear.id.au> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 07/13] iommu: Make sglists and dma_bdrv helpers use new universal DMA helpers List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: rth@twiddle.net, eduard.munteanu@linux360.ro, mst@redhat.com, qemu-devel@nongnu.org, agraf@suse.de Am 09.03.2012 06:01, schrieb David Gibson: > dma-helpers.c contains a number of helper functions for doing > scatter/gather DMA, and various block device related DMA. Currently, > these directly access guest memory using cpu_physical_memory_*(), > assuming no IOMMU translation. > > This patch updates this code to use the new universal DMA helper > functions. qemu_sglist_init() now takes a DMAContext * to describe > the DMA address space in which the scatter/gather will take place. > > We minimally update the callers qemu_sglist_init() to pass NULL > (i.e. no translation, same as current behaviour). Some of those > callers should pass something else in some cases to allow proper IOMMU > translation in future, but that will be fixed in later patches. > > Cc: Kevin Wolf > Cc: Michael S. Tsirkin > > Signed-off-by: David Gibson > --- > dma-helpers.c | 26 ++++++++++++++++++-------- > dma.h | 3 ++- > hw/ide/ahci.c | 3 ++- > hw/ide/macio.c | 4 ++-- > hw/pci.h | 2 +- > 5 files changed, 25 insertions(+), 13 deletions(-) > > diff --git a/dma-helpers.c b/dma-helpers.c > index 5f19a85..9dcfb2c 100644 > --- a/dma-helpers.c > +++ b/dma-helpers.c > @@ -11,12 +11,13 @@ > #include "block_int.h" > #include "trace.h" > > -void qemu_sglist_init(QEMUSGList *qsg, int alloc_hint) > +void qemu_sglist_init(QEMUSGList *qsg, int alloc_hint, DMAContext *dma) > { > qsg->sg = g_malloc(alloc_hint * sizeof(ScatterGatherEntry)); > qsg->nsg = 0; > qsg->nalloc = alloc_hint; > qsg->size = 0; > + qsg->dma = dma; > } > > void qemu_sglist_add(QEMUSGList *qsg, dma_addr_t base, dma_addr_t len) > @@ -75,10 +76,9 @@ static void dma_bdrv_unmap(DMAAIOCB *dbs) > int i; > > for (i = 0; i < dbs->iov.niov; ++i) { > - cpu_physical_memory_unmap(dbs->iov.iov[i].iov_base, > - dbs->iov.iov[i].iov_len, > - dbs->dir != DMA_DIRECTION_TO_DEVICE, > - dbs->iov.iov[i].iov_len); > + dma_memory_unmap(dbs->sg->dma, dbs->iov.iov[i].iov_base, > + dbs->iov.iov[i].iov_len, dbs->dir, > + dbs->iov.iov[i].iov_len); > } > qemu_iovec_reset(&dbs->iov); > } > @@ -104,10 +104,20 @@ static void dma_complete(DMAAIOCB *dbs, int ret) > } > } > > +static void dma_bdrv_cancel(void *opaque) > +{ > + DMAAIOCB *dbs = opaque; > + > + bdrv_aio_cancel(dbs->acb); > + dma_bdrv_unmap(dbs); > + qemu_iovec_destroy(&dbs->iov); > + qemu_aio_release(dbs); > +} I'm lacking the context to know when this is actually called, but it looks suspicious. Did you consider that bdrv_aio_cancel() can actually invoke the completion callback? What's the difference between the existing dma_aio_cancel() and the function that you need here? Kevin