Re: [Qemu-devel] [PATCH] Use DMADirection type for dma_bdrv_io

[David Gibson]

On Mon, Feb 20, 2012 at 11:50:40AM +0100, Alexander Graf wrote:
> 
> On 20.02.2012, at 04:01, David Gibson wrote:
> 
> > Currently dma_bdrv_io() takes a 'to_dev' boolean parameter to
> > determine the direction of DMA it is emulating.  We already have a
> > DMADirection enum designed specifically to encode DMA directions.
> > This patch uses it for dma_bdrv_io() as well.
> > 
> > Signed-off-by: David Gibson <address@hidden>
> > ---
> > dma-helpers.c  |   20 ++++++++++++--------
> > dma.h          |    2 +-
> > hw/ide/core.c  |    3 ++-
> > hw/ide/macio.c |    3 ++-
> > 4 files changed, 17 insertions(+), 11 deletions(-)
> > 
> > diff --git a/dma-helpers.c b/dma-helpers.c
> > index f08cdb5..d4af367 100644
> > --- a/dma-helpers.c
> > +++ b/dma-helpers.c
> > @@ -42,7 +42,7 @@ typedef struct {
> >     BlockDriverAIOCB *acb;
> >     QEMUSGList *sg;
> >     uint64_t sector_num;
> > -    bool to_dev;
> > +    DMADirection dir;
> >     bool in_cancel;
> >     int sg_cur_index;
> >     dma_addr_t sg_cur_byte;
> > @@ -76,7 +76,8 @@ static void dma_bdrv_unmap(DMAAIOCB *dbs)
> > 
> >     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->to_dev,
> > +                                  dbs->iov.iov[i].iov_len,
> > +                                  dbs->dir != DMA_DIRECTION_TO_DEVICE,
> >                                   dbs->iov.iov[i].iov_len);
> >     }
> >     qemu_iovec_reset(&dbs->iov);
> > @@ -123,7 +124,8 @@ static void dma_bdrv_cb(void *opaque, int ret)
> >     while (dbs->sg_cur_index < dbs->sg->nsg) {
> >         cur_addr = dbs->sg->sg[dbs->sg_cur_index].base + dbs->sg_cur_byte;
> >         cur_len = dbs->sg->sg[dbs->sg_cur_index].len - dbs->sg_cur_byte;
> > -        mem = cpu_physical_memory_map(cur_addr, &cur_len, !dbs->to_dev);
> > +        mem = cpu_physical_memory_map(cur_addr, &cur_len,
> > +                                      dbs->dir != DMA_DIRECTION_TO_DEVICE);
> >         if (!mem)
> >             break;
> >         qemu_iovec_add(&dbs->iov, mem, cur_len);
> > @@ -170,11 +172,11 @@ static AIOPool dma_aio_pool = {
> > BlockDriverAIOCB *dma_bdrv_io(
> >     BlockDriverState *bs, QEMUSGList *sg, uint64_t sector_num,
> >     DMAIOFunc *io_func, BlockDriverCompletionFunc *cb,
> > -    void *opaque, bool to_dev)
> > +    void *opaque, DMADirection dir)
> > {
> >     DMAAIOCB *dbs = qemu_aio_get(&dma_aio_pool, bs, cb, opaque);
> > 
> > -    trace_dma_bdrv_io(dbs, bs, sector_num, to_dev);
> > +    trace_dma_bdrv_io(dbs, bs, sector_num, dir);
> 
> Was the trace wrong before or is it now? I don't see its definition changed 
> anywhere.

So, there's no explicit prototype for the trace function anywhere I
could see.  As best as I can tell, it is autogenerated from the
surrounding function definition, and so needs to change when it does.

But I'm certainly no expert on the tracing code.

-- 
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