* [Qemu-devel] [PATCH] Use DMADirection type for dma_bdrv_io
@ 2012-02-20 3:01 David Gibson
2012-02-20 10:50 ` Alexander Graf
0 siblings, 1 reply; 14+ messages in thread
From: David Gibson @ 2012-02-20 3:01 UTC (permalink / raw)
To: kwolf; +Cc: agraf, qemu-devel
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 <david@gibson.dropbear.id.au>
---
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);
dbs->acb = NULL;
dbs->bs = bs;
@@ -182,7 +184,7 @@ BlockDriverAIOCB *dma_bdrv_io(
dbs->sector_num = sector_num;
dbs->sg_cur_index = 0;
dbs->sg_cur_byte = 0;
- dbs->to_dev = to_dev;
+ dbs->dir = dir;
dbs->io_func = io_func;
dbs->bh = NULL;
qemu_iovec_init(&dbs->iov, sg->nsg);
@@ -195,12 +197,14 @@ BlockDriverAIOCB *dma_bdrv_read(BlockDriverState *bs,
QEMUSGList *sg, uint64_t sector,
void (*cb)(void *opaque, int ret), void *opaque)
{
- return dma_bdrv_io(bs, sg, sector, bdrv_aio_readv, cb, opaque, false);
+ return dma_bdrv_io(bs, sg, sector, bdrv_aio_readv, cb, opaque,
+ DMA_DIRECTION_FROM_DEVICE);
}
BlockDriverAIOCB *dma_bdrv_write(BlockDriverState *bs,
QEMUSGList *sg, uint64_t sector,
void (*cb)(void *opaque, int ret), void *opaque)
{
- return dma_bdrv_io(bs, sg, sector, bdrv_aio_writev, cb, opaque, true);
+ return dma_bdrv_io(bs, sg, sector, bdrv_aio_writev, cb, opaque,
+ DMA_DIRECTION_TO_DEVICE);
}
diff --git a/dma.h b/dma.h
index a13209d..0847b42 100644
--- a/dma.h
+++ b/dma.h
@@ -51,7 +51,7 @@ typedef BlockDriverAIOCB *DMAIOFunc(BlockDriverState *bs, int64_t sector_num,
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);
BlockDriverAIOCB *dma_bdrv_read(BlockDriverState *bs,
QEMUSGList *sg, uint64_t sector,
BlockDriverCompletionFunc *cb, void *opaque);
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 56b219b..e173117 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -604,7 +604,8 @@ void ide_dma_cb(void *opaque, int ret)
break;
case IDE_DMA_TRIM:
s->bus->dma->aiocb = dma_bdrv_io(s->bs, &s->sg, sector_num,
- ide_issue_trim, ide_dma_cb, s, true);
+ ide_issue_trim, ide_dma_cb, s,
+ DMA_DIRECTION_TO_DEVICE);
break;
}
return;
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index abbc41b..edcf885 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -149,7 +149,8 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
break;
case IDE_DMA_TRIM:
m->aiocb = dma_bdrv_io(s->bs, &s->sg, sector_num,
- ide_issue_trim, pmac_ide_transfer_cb, s, true);
+ ide_issue_trim, pmac_ide_transfer_cb, s,
+ DMA_DIRECTION_TO_DEVICE);
break;
}
return;
--
1.7.9
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH] Use DMADirection type for dma_bdrv_io
2012-02-20 3:01 [Qemu-devel] [PATCH] Use DMADirection type for dma_bdrv_io David Gibson
@ 2012-02-20 10:50 ` Alexander Graf
2012-02-21 1:14 ` David Gibson
2012-02-21 8:35 ` Paolo Bonzini
0 siblings, 2 replies; 14+ messages in thread
From: Alexander Graf @ 2012-02-20 10:50 UTC (permalink / raw)
To: David Gibson; +Cc: kwolf, qemu-devel
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 <david@gibson.dropbear.id.au>
> ---
> 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.
Alex
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH] Use DMADirection type for dma_bdrv_io
2012-02-20 10:50 ` Alexander Graf
@ 2012-02-21 1:14 ` David Gibson
2012-02-21 8:35 ` Paolo Bonzini
1 sibling, 0 replies; 14+ messages in thread
From: David Gibson @ 2012-02-21 1:14 UTC (permalink / raw)
To: Alexander Graf; +Cc: kwolf, qemu-devel
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 <david@gibson.dropbear.id.au>
> > ---
> > 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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH] Use DMADirection type for dma_bdrv_io
2012-02-20 10:50 ` Alexander Graf
2012-02-21 1:14 ` David Gibson
@ 2012-02-21 8:35 ` Paolo Bonzini
2012-02-21 9:09 ` Kevin Wolf
1 sibling, 1 reply; 14+ messages in thread
From: Paolo Bonzini @ 2012-02-21 8:35 UTC (permalink / raw)
To: Alexander Graf; +Cc: kwolf, qemu-devel, David Gibson
On 02/20/2012 11:50 AM, Alexander Graf wrote:
>> > 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.
Not sure what you mean. :)
Paolo
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH] Use DMADirection type for dma_bdrv_io
2012-02-21 8:35 ` Paolo Bonzini
@ 2012-02-21 9:09 ` Kevin Wolf
2012-02-22 1:11 ` David Gibson
0 siblings, 1 reply; 14+ messages in thread
From: Kevin Wolf @ 2012-02-21 9:09 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, Alexander Graf, David Gibson
Am 21.02.2012 09:35, schrieb Paolo Bonzini:
> On 02/20/2012 11:50 AM, Alexander Graf wrote:
>>>> 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.
>
> Not sure what you mean. :)
trace-events:
dma_bdrv_io(void *dbs, void *bs, int64_t sector_num, bool to_dev)
"dbs=%p bs=%p sector_num=%" PRId64 " to_dev=%d"
to_dev is declared bool here, and it should also be renamed to dir (the
unfortunate thing about DMADirection is that it swaps 0 and 1 compared
to bool to_dev... We need to check carefully that all occurrences have
been caught.)
Kevin
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH] Use DMADirection type for dma_bdrv_io
2012-02-21 9:09 ` Kevin Wolf
@ 2012-02-22 1:11 ` David Gibson
2012-02-22 9:54 ` Andreas Färber
0 siblings, 1 reply; 14+ messages in thread
From: David Gibson @ 2012-02-22 1:11 UTC (permalink / raw)
To: Kevin Wolf; +Cc: Paolo Bonzini, Alexander Graf, qemu-devel
On Tue, Feb 21, 2012 at 10:09:01AM +0100, Kevin Wolf wrote:
> Am 21.02.2012 09:35, schrieb Paolo Bonzini:
> > On 02/20/2012 11:50 AM, Alexander Graf wrote:
> >>>> 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.
> >
> > Not sure what you mean. :)
>
> trace-events:
>
> dma_bdrv_io(void *dbs, void *bs, int64_t sector_num, bool to_dev)
> "dbs=%p bs=%p sector_num=%" PRId64 " to_dev=%d"
Ah, damnit. Didn't find that file. I'll resubmit with trace-events
updated too.
> to_dev is declared bool here, and it should also be renamed to dir (the
> unfortunate thing about DMADirection is that it swaps 0 and 1 compared
> to bool to_dev... We need to check carefully that all occurrences have
> been caught.)
Yeah. And even more unfortunate that afaict there's no way to make
gcc warn on enum<->bool conversions. Still there are only about 4
callers of dma_bdrv_io() - nearly everything uses the
dma_bdrv_{read,write}() wrappers - so I'm confident I got them all.
--
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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH] Use DMADirection type for dma_bdrv_io
2012-02-22 1:11 ` David Gibson
@ 2012-02-22 9:54 ` Andreas Färber
2012-02-22 9:55 ` Hannes Reinecke
0 siblings, 1 reply; 14+ messages in thread
From: Andreas Färber @ 2012-02-22 9:54 UTC (permalink / raw)
To: Kevin Wolf, David Gibson
Cc: Paolo Bonzini, Hannes Reinecke, Alexander Graf, qemu-devel
Am 22.02.2012 02:11, schrieb David Gibson:
> On Tue, Feb 21, 2012 at 10:09:01AM +0100, Kevin Wolf wrote:
>> Am 21.02.2012 09:35, schrieb Paolo Bonzini:
>>> On 02/20/2012 11:50 AM, Alexander Graf wrote:
>>>>>> 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.
>>>
>>> Not sure what you mean. :)
>>
>> trace-events:
>>
>> dma_bdrv_io(void *dbs, void *bs, int64_t sector_num, bool to_dev)
>> "dbs=%p bs=%p sector_num=%" PRId64 " to_dev=%d"
>
> Ah, damnit. Didn't find that file. I'll resubmit with trace-events
> updated too.
>
>> to_dev is declared bool here, and it should also be renamed to dir (the
>> unfortunate thing about DMADirection is that it swaps 0 and 1 compared
>> to bool to_dev... We need to check carefully that all occurrences have
>> been caught.)
>
> Yeah. And even more unfortunate that afaict there's no way to make
> gcc warn on enum<->bool conversions. Still there are only about 4
> callers of dma_bdrv_io() - nearly everything uses the
> dma_bdrv_{read,write}() wrappers - so I'm confident I got them all.
Re all occurrences: megasas might be affected by such a change, too
(patch on the list).
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH] Use DMADirection type for dma_bdrv_io
2012-02-22 9:54 ` Andreas Färber
@ 2012-02-22 9:55 ` Hannes Reinecke
0 siblings, 0 replies; 14+ messages in thread
From: Hannes Reinecke @ 2012-02-22 9:55 UTC (permalink / raw)
To: Andreas Färber
Cc: Kevin Wolf, Paolo Bonzini, qemu-devel, Alexander Graf,
David Gibson
On 02/22/2012 10:54 AM, Andreas Färber wrote:
> Am 22.02.2012 02:11, schrieb David Gibson:
>> On Tue, Feb 21, 2012 at 10:09:01AM +0100, Kevin Wolf wrote:
>>> Am 21.02.2012 09:35, schrieb Paolo Bonzini:
>>>> On 02/20/2012 11:50 AM, Alexander Graf wrote:
>>>>>>> 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.
>>>>
>>>> Not sure what you mean. :)
>>>
>>> trace-events:
>>>
>>> dma_bdrv_io(void *dbs, void *bs, int64_t sector_num, bool to_dev)
>>> "dbs=%p bs=%p sector_num=%" PRId64 " to_dev=%d"
>>
>> Ah, damnit. Didn't find that file. I'll resubmit with trace-events
>> updated too.
>>
>>> to_dev is declared bool here, and it should also be renamed to dir (the
>>> unfortunate thing about DMADirection is that it swaps 0 and 1 compared
>>> to bool to_dev... We need to check carefully that all occurrences have
>>> been caught.)
>>
>> Yeah. And even more unfortunate that afaict there's no way to make
>> gcc warn on enum<->bool conversions. Still there are only about 4
>> callers of dma_bdrv_io() - nearly everything uses the
>> dma_bdrv_{read,write}() wrappers - so I'm confident I got them all.
>
> Re all occurrences: megasas might be affected by such a change, too
> (patch on the list).
>
Nope. megasas is using the scsi interface for reading/writing,
so it should be insulated against this kind of change.
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@suse.de +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Qemu-devel] [PATCH] Use DMADirection type for dma_bdrv_io
@ 2012-03-27 2:42 David Gibson
2012-03-27 7:17 ` Stefan Hajnoczi
2012-03-27 14:30 ` Andreas Färber
0 siblings, 2 replies; 14+ messages in thread
From: David Gibson @ 2012-03-27 2:42 UTC (permalink / raw)
To: aliguori, kwolf; +Cc: David Gibson, qemu-devel, mst
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. This involves removing
the DMADirection definition from the #ifdef it was inside, but since that
only existed to protect the definition of dma_addr_t from places where
config.h is not included, there wasn't any reason for it to be there in
the first place.
Cc: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
---
dma-helpers.c | 20 ++++++++++++--------
dma.h | 12 ++++++------
hw/ide/core.c | 3 ++-
hw/ide/macio.c | 3 ++-
4 files changed, 22 insertions(+), 16 deletions(-)
diff --git a/dma-helpers.c b/dma-helpers.c
index c29ea6d..5f19a85 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 == DMA_DIRECTION_TO_DEVICE));
dbs->acb = NULL;
dbs->bs = bs;
@@ -182,7 +184,7 @@ BlockDriverAIOCB *dma_bdrv_io(
dbs->sector_num = sector_num;
dbs->sg_cur_index = 0;
dbs->sg_cur_byte = 0;
- dbs->to_dev = to_dev;
+ dbs->dir = dir;
dbs->io_func = io_func;
dbs->bh = NULL;
qemu_iovec_init(&dbs->iov, sg->nsg);
@@ -195,14 +197,16 @@ BlockDriverAIOCB *dma_bdrv_read(BlockDriverState *bs,
QEMUSGList *sg, uint64_t sector,
void (*cb)(void *opaque, int ret), void *opaque)
{
- return dma_bdrv_io(bs, sg, sector, bdrv_aio_readv, cb, opaque, false);
+ return dma_bdrv_io(bs, sg, sector, bdrv_aio_readv, cb, opaque,
+ DMA_DIRECTION_FROM_DEVICE);
}
BlockDriverAIOCB *dma_bdrv_write(BlockDriverState *bs,
QEMUSGList *sg, uint64_t sector,
void (*cb)(void *opaque, int ret), void *opaque)
{
- return dma_bdrv_io(bs, sg, sector, bdrv_aio_writev, cb, opaque, true);
+ return dma_bdrv_io(bs, sg, sector, bdrv_aio_writev, cb, opaque,
+ DMA_DIRECTION_TO_DEVICE);
}
diff --git a/dma.h b/dma.h
index 20e86d2..05ac325 100644
--- a/dma.h
+++ b/dma.h
@@ -17,6 +17,11 @@
typedef struct ScatterGatherEntry ScatterGatherEntry;
+typedef enum {
+ DMA_DIRECTION_TO_DEVICE = 0,
+ DMA_DIRECTION_FROM_DEVICE = 1,
+} DMADirection;
+
struct QEMUSGList {
ScatterGatherEntry *sg;
int nsg;
@@ -29,11 +34,6 @@ typedef target_phys_addr_t dma_addr_t;
#define DMA_ADDR_FMT TARGET_FMT_plx
-typedef enum {
- DMA_DIRECTION_TO_DEVICE = 0,
- DMA_DIRECTION_FROM_DEVICE = 1,
-} DMADirection;
-
struct ScatterGatherEntry {
dma_addr_t base;
dma_addr_t len;
@@ -51,7 +51,7 @@ typedef BlockDriverAIOCB *DMAIOFunc(BlockDriverState *bs, int64_t sector_num,
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);
BlockDriverAIOCB *dma_bdrv_read(BlockDriverState *bs,
QEMUSGList *sg, uint64_t sector,
BlockDriverCompletionFunc *cb, void *opaque);
diff --git a/hw/ide/core.c b/hw/ide/core.c
index 4d568ac..43da841 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -604,7 +604,8 @@ void ide_dma_cb(void *opaque, int ret)
break;
case IDE_DMA_TRIM:
s->bus->dma->aiocb = dma_bdrv_io(s->bs, &s->sg, sector_num,
- ide_issue_trim, ide_dma_cb, s, true);
+ ide_issue_trim, ide_dma_cb, s,
+ DMA_DIRECTION_TO_DEVICE);
break;
}
return;
diff --git a/hw/ide/macio.c b/hw/ide/macio.c
index a4df244..7b38d9e 100644
--- a/hw/ide/macio.c
+++ b/hw/ide/macio.c
@@ -149,7 +149,8 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
break;
case IDE_DMA_TRIM:
m->aiocb = dma_bdrv_io(s->bs, &s->sg, sector_num,
- ide_issue_trim, pmac_ide_transfer_cb, s, true);
+ ide_issue_trim, pmac_ide_transfer_cb, s,
+ DMA_DIRECTION_TO_DEVICE);
break;
}
return;
--
1.7.9.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH] Use DMADirection type for dma_bdrv_io
2012-03-27 2:42 David Gibson
@ 2012-03-27 7:17 ` Stefan Hajnoczi
2012-03-27 14:30 ` Andreas Färber
1 sibling, 0 replies; 14+ messages in thread
From: Stefan Hajnoczi @ 2012-03-27 7:17 UTC (permalink / raw)
To: David Gibson; +Cc: kwolf, aliguori, qemu-devel, mst
On Tue, Mar 27, 2012 at 01:42:23PM +1100, 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. This involves removing
> the DMADirection definition from the #ifdef it was inside, but since that
> only existed to protect the definition of dma_addr_t from places where
> config.h is not included, there wasn't any reason for it to be there in
> the first place.
>
> Cc: Kevin Wolf <kwolf@redhat.com>
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> ---
> dma-helpers.c | 20 ++++++++++++--------
> dma.h | 12 ++++++------
> hw/ide/core.c | 3 ++-
> hw/ide/macio.c | 3 ++-
> 4 files changed, 22 insertions(+), 16 deletions(-)
Reviewed-by: Stefan Hajnoczi <stefanha@linux.vnet.ibm.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH] Use DMADirection type for dma_bdrv_io
2012-03-27 2:42 David Gibson
2012-03-27 7:17 ` Stefan Hajnoczi
@ 2012-03-27 14:30 ` Andreas Färber
2012-03-27 14:43 ` Kevin Wolf
1 sibling, 1 reply; 14+ messages in thread
From: Andreas Färber @ 2012-03-27 14:30 UTC (permalink / raw)
To: David Gibson; +Cc: kwolf, aliguori, mst, Alexander Graf, qemu-devel, qemu-ppc
Am 27.03.2012 04:42, schrieb David Gibson:
> 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. This involves removing
> the DMADirection definition from the #ifdef it was inside, but since that
> only existed to protect the definition of dma_addr_t from places where
> config.h is not included, there wasn't any reason for it to be there in
> the first place.
>
> Cc: Kevin Wolf <kwolf@redhat.com>
>
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Including for ppc macio in absence of Alex,
Reviewed-by: Andreas Färber <afaerber@suse.de>
(Expecting this to go through IDE/Kevin's tree.)
Andreas
> ---
> dma-helpers.c | 20 ++++++++++++--------
> dma.h | 12 ++++++------
> hw/ide/core.c | 3 ++-
> hw/ide/macio.c | 3 ++-
> 4 files changed, 22 insertions(+), 16 deletions(-)
>
> diff --git a/dma-helpers.c b/dma-helpers.c
> index c29ea6d..5f19a85 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 == DMA_DIRECTION_TO_DEVICE));
>
> dbs->acb = NULL;
> dbs->bs = bs;
> @@ -182,7 +184,7 @@ BlockDriverAIOCB *dma_bdrv_io(
> dbs->sector_num = sector_num;
> dbs->sg_cur_index = 0;
> dbs->sg_cur_byte = 0;
> - dbs->to_dev = to_dev;
> + dbs->dir = dir;
> dbs->io_func = io_func;
> dbs->bh = NULL;
> qemu_iovec_init(&dbs->iov, sg->nsg);
> @@ -195,14 +197,16 @@ BlockDriverAIOCB *dma_bdrv_read(BlockDriverState *bs,
> QEMUSGList *sg, uint64_t sector,
> void (*cb)(void *opaque, int ret), void *opaque)
> {
> - return dma_bdrv_io(bs, sg, sector, bdrv_aio_readv, cb, opaque, false);
> + return dma_bdrv_io(bs, sg, sector, bdrv_aio_readv, cb, opaque,
> + DMA_DIRECTION_FROM_DEVICE);
> }
>
> BlockDriverAIOCB *dma_bdrv_write(BlockDriverState *bs,
> QEMUSGList *sg, uint64_t sector,
> void (*cb)(void *opaque, int ret), void *opaque)
> {
> - return dma_bdrv_io(bs, sg, sector, bdrv_aio_writev, cb, opaque, true);
> + return dma_bdrv_io(bs, sg, sector, bdrv_aio_writev, cb, opaque,
> + DMA_DIRECTION_TO_DEVICE);
> }
>
>
> diff --git a/dma.h b/dma.h
> index 20e86d2..05ac325 100644
> --- a/dma.h
> +++ b/dma.h
> @@ -17,6 +17,11 @@
>
> typedef struct ScatterGatherEntry ScatterGatherEntry;
>
> +typedef enum {
> + DMA_DIRECTION_TO_DEVICE = 0,
> + DMA_DIRECTION_FROM_DEVICE = 1,
> +} DMADirection;
> +
> struct QEMUSGList {
> ScatterGatherEntry *sg;
> int nsg;
> @@ -29,11 +34,6 @@ typedef target_phys_addr_t dma_addr_t;
>
> #define DMA_ADDR_FMT TARGET_FMT_plx
>
> -typedef enum {
> - DMA_DIRECTION_TO_DEVICE = 0,
> - DMA_DIRECTION_FROM_DEVICE = 1,
> -} DMADirection;
> -
> struct ScatterGatherEntry {
> dma_addr_t base;
> dma_addr_t len;
> @@ -51,7 +51,7 @@ typedef BlockDriverAIOCB *DMAIOFunc(BlockDriverState *bs, int64_t sector_num,
> 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);
> BlockDriverAIOCB *dma_bdrv_read(BlockDriverState *bs,
> QEMUSGList *sg, uint64_t sector,
> BlockDriverCompletionFunc *cb, void *opaque);
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index 4d568ac..43da841 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -604,7 +604,8 @@ void ide_dma_cb(void *opaque, int ret)
> break;
> case IDE_DMA_TRIM:
> s->bus->dma->aiocb = dma_bdrv_io(s->bs, &s->sg, sector_num,
> - ide_issue_trim, ide_dma_cb, s, true);
> + ide_issue_trim, ide_dma_cb, s,
> + DMA_DIRECTION_TO_DEVICE);
> break;
> }
> return;
> diff --git a/hw/ide/macio.c b/hw/ide/macio.c
> index a4df244..7b38d9e 100644
> --- a/hw/ide/macio.c
> +++ b/hw/ide/macio.c
> @@ -149,7 +149,8 @@ static void pmac_ide_transfer_cb(void *opaque, int ret)
> break;
> case IDE_DMA_TRIM:
> m->aiocb = dma_bdrv_io(s->bs, &s->sg, sector_num,
> - ide_issue_trim, pmac_ide_transfer_cb, s, true);
> + ide_issue_trim, pmac_ide_transfer_cb, s,
> + DMA_DIRECTION_TO_DEVICE);
> break;
> }
> return;
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH] Use DMADirection type for dma_bdrv_io
2012-03-27 14:30 ` Andreas Färber
@ 2012-03-27 14:43 ` Kevin Wolf
2012-03-27 15:17 ` Andreas Färber
0 siblings, 1 reply; 14+ messages in thread
From: Kevin Wolf @ 2012-03-27 14:43 UTC (permalink / raw)
To: Andreas Färber
Cc: aliguori, mst, Alexander Graf, qemu-devel, qemu-ppc, David Gibson
Am 27.03.2012 16:30, schrieb Andreas Färber:
> Am 27.03.2012 04:42, schrieb David Gibson:
>> 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. This involves removing
>> the DMADirection definition from the #ifdef it was inside, but since that
>> only existed to protect the definition of dma_addr_t from places where
>> config.h is not included, there wasn't any reason for it to be there in
>> the first place.
>>
>> Cc: Kevin Wolf <kwolf@redhat.com>
>>
>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>
> Including for ppc macio in absence of Alex,
>
> Reviewed-by: Andreas Färber <afaerber@suse.de>
>
> (Expecting this to go through IDE/Kevin's tree.)
So this isn't part of another series any more?
Kevin
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH] Use DMADirection type for dma_bdrv_io
2012-03-27 14:43 ` Kevin Wolf
@ 2012-03-27 15:17 ` Andreas Färber
2012-03-27 15:23 ` Kevin Wolf
0 siblings, 1 reply; 14+ messages in thread
From: Andreas Färber @ 2012-03-27 15:17 UTC (permalink / raw)
To: Kevin Wolf
Cc: aliguori, mst, Alexander Graf, qemu-devel, qemu-ppc, David Gibson
Am 27.03.2012 16:43, schrieb Kevin Wolf:
> Am 27.03.2012 16:30, schrieb Andreas Färber:
>> Am 27.03.2012 04:42, schrieb David Gibson:
>>> 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. This involves removing
>>> the DMADirection definition from the #ifdef it was inside, but since that
>>> only existed to protect the definition of dma_addr_t from places where
>>> config.h is not included, there wasn't any reason for it to be there in
>>> the first place.
>>>
>>> Cc: Kevin Wolf <kwolf@redhat.com>
>>>
>>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>>
>> Including for ppc macio in absence of Alex,
>>
>> Reviewed-by: Andreas Färber <afaerber@suse.de>
>>
>> (Expecting this to go through IDE/Kevin's tree.)
>
> So this isn't part of another series any more?
AFAICS it's a single patch on qemu-devel, split out of one of the
"Various fixes David has piled up" series on Alex' suggestion.
Andreas
--
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Qemu-devel] [PATCH] Use DMADirection type for dma_bdrv_io
2012-03-27 15:17 ` Andreas Färber
@ 2012-03-27 15:23 ` Kevin Wolf
0 siblings, 0 replies; 14+ messages in thread
From: Kevin Wolf @ 2012-03-27 15:23 UTC (permalink / raw)
To: Andreas Färber
Cc: aliguori, mst, Alexander Graf, qemu-devel, qemu-ppc, David Gibson
Am 27.03.2012 17:17, schrieb Andreas Färber:
> Am 27.03.2012 16:43, schrieb Kevin Wolf:
>> Am 27.03.2012 16:30, schrieb Andreas Färber:
>>> Am 27.03.2012 04:42, schrieb David Gibson:
>>>> 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. This involves removing
>>>> the DMADirection definition from the #ifdef it was inside, but since that
>>>> only existed to protect the definition of dma_addr_t from places where
>>>> config.h is not included, there wasn't any reason for it to be there in
>>>> the first place.
>>>>
>>>> Cc: Kevin Wolf <kwolf@redhat.com>
>>>>
>>>> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
>>>
>>> Including for ppc macio in absence of Alex,
>>>
>>> Reviewed-by: Andreas Färber <afaerber@suse.de>
>>>
>>> (Expecting this to go through IDE/Kevin's tree.)
>>
>> So this isn't part of another series any more?
>
> AFAICS it's a single patch on qemu-devel, split out of one of the
> "Various fixes David has piled up" series on Alex' suggestion.
Okay. I don't have the time to check the details myself, but two
Reviewed-bys are enough, so I just applied it to the block branch.
Kevin
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2012-03-27 15:20 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-20 3:01 [Qemu-devel] [PATCH] Use DMADirection type for dma_bdrv_io David Gibson
2012-02-20 10:50 ` Alexander Graf
2012-02-21 1:14 ` David Gibson
2012-02-21 8:35 ` Paolo Bonzini
2012-02-21 9:09 ` Kevin Wolf
2012-02-22 1:11 ` David Gibson
2012-02-22 9:54 ` Andreas Färber
2012-02-22 9:55 ` Hannes Reinecke
-- strict thread matches above, loose matches on Subject: below --
2012-03-27 2:42 David Gibson
2012-03-27 7:17 ` Stefan Hajnoczi
2012-03-27 14:30 ` Andreas Färber
2012-03-27 14:43 ` Kevin Wolf
2012-03-27 15:17 ` Andreas Färber
2012-03-27 15:23 ` Kevin Wolf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).