* [Qemu-devel] [PATCH 0/2] dma-helpers: explicitly pass alignment into dma-helpers @ 2016-10-09 16:43 Mark Cave-Ayland 2016-10-09 16:43 ` [Qemu-devel] [PATCH 1/2] " Mark Cave-Ayland 2016-10-09 16:43 ` [Qemu-devel] [PATCH 2/2] macio: switch over to new byte-aligned DMA helpers Mark Cave-Ayland 0 siblings, 2 replies; 13+ messages in thread From: Mark Cave-Ayland @ 2016-10-09 16:43 UTC (permalink / raw) To: keith.busch, kwolf, mreitz, jsnow, pbonzini, qemu-devel, qemu-block This is a follow-up to the thread at https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg01396.html which introduces an explicit alignment to the DMA helpers to facilitate conversion of the macio controller over to use the now byte-aligned DMA helpers. Patch 1 introduces an alignment parameter as suggested by Paolo above, whilst patch 2 performs the conversion for the macio controller. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Mark Cave-Ayland (2): dma-helpers: explicitly pass alignment into dma-helpers macio: switch over to new byte-aligned DMA helpers dma-helpers.c | 20 ++--- hw/block/nvme.c | 6 +- hw/ide/ahci.c | 2 + hw/ide/core.c | 6 +- hw/ide/macio.c | 213 +++++++------------------------------------------- hw/scsi/scsi-disk.c | 2 + include/sysemu/dma.h | 6 +- 7 files changed, 53 insertions(+), 202 deletions(-) -- 1.7.10.4 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 1/2] dma-helpers: explicitly pass alignment into dma-helpers 2016-10-09 16:43 [Qemu-devel] [PATCH 0/2] dma-helpers: explicitly pass alignment into dma-helpers Mark Cave-Ayland @ 2016-10-09 16:43 ` Mark Cave-Ayland 2016-10-10 16:34 ` Eric Blake 2016-10-09 16:43 ` [Qemu-devel] [PATCH 2/2] macio: switch over to new byte-aligned DMA helpers Mark Cave-Ayland 1 sibling, 1 reply; 13+ messages in thread From: Mark Cave-Ayland @ 2016-10-09 16:43 UTC (permalink / raw) To: keith.busch, kwolf, mreitz, jsnow, pbonzini, qemu-devel, qemu-block The hard-coded default alignment is BDRV_SECTOR_SIZE, however this is not necessarily the case for all platforms. Use this as the default alignment for all current callers. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> --- dma-helpers.c | 20 +++++++++++--------- hw/block/nvme.c | 6 ++++-- hw/ide/ahci.c | 2 ++ hw/ide/core.c | 6 +++--- hw/scsi/scsi-disk.c | 2 ++ include/sysemu/dma.h | 6 +++--- 6 files changed, 25 insertions(+), 17 deletions(-) diff --git a/dma-helpers.c b/dma-helpers.c index 9defc10..3caa2ab 100644 --- a/dma-helpers.c +++ b/dma-helpers.c @@ -73,6 +73,7 @@ typedef struct { AioContext *ctx; BlockAIOCB *acb; QEMUSGList *sg; + uint32_t align; uint64_t offset; DMADirection dir; int sg_cur_index; @@ -160,8 +161,8 @@ static void dma_blk_cb(void *opaque, int ret) return; } - if (dbs->iov.size & ~BDRV_SECTOR_MASK) { - qemu_iovec_discard_back(&dbs->iov, dbs->iov.size & ~BDRV_SECTOR_MASK); + if (dbs->iov.size & (dbs->align - 1)) { + qemu_iovec_discard_back(&dbs->iov, dbs->iov.size & (dbs->align - 1)); } dbs->acb = dbs->io_func(dbs->offset, &dbs->iov, @@ -199,7 +200,7 @@ static const AIOCBInfo dma_aiocb_info = { }; BlockAIOCB *dma_blk_io(AioContext *ctx, - QEMUSGList *sg, uint64_t offset, + QEMUSGList *sg, uint64_t offset, uint32_t align, DMAIOFunc *io_func, void *io_func_opaque, BlockCompletionFunc *cb, void *opaque, DMADirection dir) @@ -212,6 +213,7 @@ BlockAIOCB *dma_blk_io(AioContext *ctx, dbs->sg = sg; dbs->ctx = ctx; dbs->offset = offset; + dbs->align = align; dbs->sg_cur_index = 0; dbs->sg_cur_byte = 0; dbs->dir = dir; @@ -234,11 +236,11 @@ BlockAIOCB *dma_blk_read_io_func(int64_t offset, QEMUIOVector *iov, } BlockAIOCB *dma_blk_read(BlockBackend *blk, - QEMUSGList *sg, uint64_t offset, + QEMUSGList *sg, uint64_t offset, uint32_t align, void (*cb)(void *opaque, int ret), void *opaque) { - return dma_blk_io(blk_get_aio_context(blk), - sg, offset, dma_blk_read_io_func, blk, cb, opaque, + return dma_blk_io(blk_get_aio_context(blk), sg, offset, align, + dma_blk_read_io_func, blk, cb, opaque, DMA_DIRECTION_FROM_DEVICE); } @@ -252,11 +254,11 @@ BlockAIOCB *dma_blk_write_io_func(int64_t offset, QEMUIOVector *iov, } BlockAIOCB *dma_blk_write(BlockBackend *blk, - QEMUSGList *sg, uint64_t offset, + QEMUSGList *sg, uint64_t offset, uint32_t align, void (*cb)(void *opaque, int ret), void *opaque) { - return dma_blk_io(blk_get_aio_context(blk), - sg, offset, dma_blk_write_io_func, blk, cb, opaque, + return dma_blk_io(blk_get_aio_context(blk), sg, offset, align, + dma_blk_write_io_func, blk, cb, opaque, DMA_DIRECTION_TO_DEVICE); } diff --git a/hw/block/nvme.c b/hw/block/nvme.c index cef3bb4..b380142 100644 --- a/hw/block/nvme.c +++ b/hw/block/nvme.c @@ -258,8 +258,10 @@ static uint16_t nvme_rw(NvmeCtrl *n, NvmeNamespace *ns, NvmeCmd *cmd, req->has_sg = true; dma_acct_start(n->conf.blk, &req->acct, &req->qsg, acct); req->aiocb = is_write ? - dma_blk_write(n->conf.blk, &req->qsg, data_offset, nvme_rw_cb, req) : - dma_blk_read(n->conf.blk, &req->qsg, data_offset, nvme_rw_cb, req); + dma_blk_write(n->conf.blk, &req->qsg, data_offset, BDRV_SECTOR_SIZE, + nvme_rw_cb, req) : + dma_blk_read(n->conf.blk, &req->qsg, data_offset, BDRV_SECTOR_SIZE, + nvme_rw_cb, req); return NVME_NO_COMPLETE; } diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c index 63ead21..3c19bda 100644 --- a/hw/ide/ahci.c +++ b/hw/ide/ahci.c @@ -1009,6 +1009,7 @@ static void execute_ncq_command(NCQTransferState *ncq_tfs) &ncq_tfs->sglist, BLOCK_ACCT_READ); ncq_tfs->aiocb = dma_blk_read(ide_state->blk, &ncq_tfs->sglist, ncq_tfs->lba << BDRV_SECTOR_BITS, + BDRV_SECTOR_SIZE, ncq_cb, ncq_tfs); break; case WRITE_FPDMA_QUEUED: @@ -1022,6 +1023,7 @@ static void execute_ncq_command(NCQTransferState *ncq_tfs) &ncq_tfs->sglist, BLOCK_ACCT_WRITE); ncq_tfs->aiocb = dma_blk_write(ide_state->blk, &ncq_tfs->sglist, ncq_tfs->lba << BDRV_SECTOR_BITS, + BDRV_SECTOR_SIZE, ncq_cb, ncq_tfs); break; default: diff --git a/hw/ide/core.c b/hw/ide/core.c index 7291677..43709e5 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -882,15 +882,15 @@ static void ide_dma_cb(void *opaque, int ret) switch (s->dma_cmd) { case IDE_DMA_READ: s->bus->dma->aiocb = dma_blk_read(s->blk, &s->sg, offset, - ide_dma_cb, s); + BDRV_SECTOR_SIZE, ide_dma_cb, s); break; case IDE_DMA_WRITE: s->bus->dma->aiocb = dma_blk_write(s->blk, &s->sg, offset, - ide_dma_cb, s); + BDRV_SECTOR_SIZE, ide_dma_cb, s); break; case IDE_DMA_TRIM: s->bus->dma->aiocb = dma_blk_io(blk_get_aio_context(s->blk), - &s->sg, offset, + &s->sg, offset, BDRV_SECTOR_SIZE, ide_issue_trim, s->blk, ide_dma_cb, s, DMA_DIRECTION_TO_DEVICE); break; diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c index 88beaf4..a963191 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -341,6 +341,7 @@ static void scsi_do_read(SCSIDiskReq *r, int ret) r->req.resid -= r->req.sg->size; r->req.aiocb = dma_blk_io(blk_get_aio_context(s->qdev.conf.blk), r->req.sg, r->sector << BDRV_SECTOR_BITS, + BDRV_SECTOR_SIZE, sdc->dma_readv, r, scsi_dma_complete, r, DMA_DIRECTION_FROM_DEVICE); } else { @@ -539,6 +540,7 @@ static void scsi_write_data(SCSIRequest *req) r->req.resid -= r->req.sg->size; r->req.aiocb = dma_blk_io(blk_get_aio_context(s->qdev.conf.blk), r->req.sg, r->sector << BDRV_SECTOR_BITS, + BDRV_SECTOR_SIZE, sdc->dma_writev, r, scsi_dma_complete, r, DMA_DIRECTION_TO_DEVICE); } else { diff --git a/include/sysemu/dma.h b/include/sysemu/dma.h index 34c8eaf..c228c66 100644 --- a/include/sysemu/dma.h +++ b/include/sysemu/dma.h @@ -199,14 +199,14 @@ typedef BlockAIOCB *DMAIOFunc(int64_t offset, QEMUIOVector *iov, void *opaque); BlockAIOCB *dma_blk_io(AioContext *ctx, - QEMUSGList *sg, uint64_t offset, + QEMUSGList *sg, uint64_t offset, uint32_t align, DMAIOFunc *io_func, void *io_func_opaque, BlockCompletionFunc *cb, void *opaque, DMADirection dir); BlockAIOCB *dma_blk_read(BlockBackend *blk, - QEMUSGList *sg, uint64_t offset, + QEMUSGList *sg, uint64_t offset, uint32_t align, BlockCompletionFunc *cb, void *opaque); BlockAIOCB *dma_blk_write(BlockBackend *blk, - QEMUSGList *sg, uint64_t offset, + QEMUSGList *sg, uint64_t offset, uint32_t align, BlockCompletionFunc *cb, void *opaque); uint64_t dma_buf_read(uint8_t *ptr, int32_t len, QEMUSGList *sg); uint64_t dma_buf_write(uint8_t *ptr, int32_t len, QEMUSGList *sg); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] dma-helpers: explicitly pass alignment into dma-helpers 2016-10-09 16:43 ` [Qemu-devel] [PATCH 1/2] " Mark Cave-Ayland @ 2016-10-10 16:34 ` Eric Blake 2016-10-10 19:23 ` Mark Cave-Ayland 0 siblings, 1 reply; 13+ messages in thread From: Eric Blake @ 2016-10-10 16:34 UTC (permalink / raw) To: Mark Cave-Ayland, keith.busch, kwolf, mreitz, jsnow, pbonzini, qemu-devel, qemu-block [-- Attachment #1: Type: text/plain, Size: 1104 bytes --] On 10/09/2016 11:43 AM, Mark Cave-Ayland wrote: > The hard-coded default alignment is BDRV_SECTOR_SIZE, however this is not > necessarily the case for all platforms. Use this as the default alignment for > all current callers. > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > --- > @@ -160,8 +161,8 @@ static void dma_blk_cb(void *opaque, int ret) > return; > } > > - if (dbs->iov.size & ~BDRV_SECTOR_MASK) { > - qemu_iovec_discard_back(&dbs->iov, dbs->iov.size & ~BDRV_SECTOR_MASK); > + if (dbs->iov.size & (dbs->align - 1)) { > + qemu_iovec_discard_back(&dbs->iov, dbs->iov.size & (dbs->align - 1)); Would it be any smarter to use osdep.h's QEMU_IS_ALIGNED(dbs->iov.size, dbs->align) and QEMU_ALIGN_DOWN(dbs->iov.size, dbs->align)? Semantically it is the same, but the macros make it obvious what the bit-twiddling is doing. Unless you think that needs a tweak, Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 604 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] dma-helpers: explicitly pass alignment into dma-helpers 2016-10-10 16:34 ` Eric Blake @ 2016-10-10 19:23 ` Mark Cave-Ayland 2016-10-11 15:47 ` John Snow 0 siblings, 1 reply; 13+ messages in thread From: Mark Cave-Ayland @ 2016-10-10 19:23 UTC (permalink / raw) To: Eric Blake, keith.busch, kwolf, mreitz, jsnow, pbonzini, qemu-devel, qemu-block On 10/10/16 17:34, Eric Blake wrote: > On 10/09/2016 11:43 AM, Mark Cave-Ayland wrote: >> The hard-coded default alignment is BDRV_SECTOR_SIZE, however this is not >> necessarily the case for all platforms. Use this as the default alignment for >> all current callers. >> >> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> >> --- > >> @@ -160,8 +161,8 @@ static void dma_blk_cb(void *opaque, int ret) >> return; >> } >> >> - if (dbs->iov.size & ~BDRV_SECTOR_MASK) { >> - qemu_iovec_discard_back(&dbs->iov, dbs->iov.size & ~BDRV_SECTOR_MASK); >> + if (dbs->iov.size & (dbs->align - 1)) { >> + qemu_iovec_discard_back(&dbs->iov, dbs->iov.size & (dbs->align - 1)); > > Would it be any smarter to use osdep.h's QEMU_IS_ALIGNED(dbs->iov.size, > dbs->align) and QEMU_ALIGN_DOWN(dbs->iov.size, dbs->align)? > Semantically it is the same, but the macros make it obvious what the > bit-twiddling is doing. > > Unless you think that needs a tweak, > Reviewed-by: Eric Blake <eblake@redhat.com> I can't say I feel too strongly about it since there are plenty of other examples of this style in the codebase, so I'm happy to go with whatever John/Paolo are most happy with. ATB, Mark. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] dma-helpers: explicitly pass alignment into dma-helpers 2016-10-10 19:23 ` Mark Cave-Ayland @ 2016-10-11 15:47 ` John Snow 2016-10-12 10:22 ` Kevin Wolf 0 siblings, 1 reply; 13+ messages in thread From: John Snow @ 2016-10-11 15:47 UTC (permalink / raw) To: Mark Cave-Ayland, Eric Blake, keith.busch, kwolf, mreitz, pbonzini, qemu-devel, qemu-block On 10/10/2016 03:23 PM, Mark Cave-Ayland wrote: > On 10/10/16 17:34, Eric Blake wrote: > >> On 10/09/2016 11:43 AM, Mark Cave-Ayland wrote: >>> The hard-coded default alignment is BDRV_SECTOR_SIZE, however this is not >>> necessarily the case for all platforms. Use this as the default alignment for >>> all current callers. >>> >>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> >>> --- >> >>> @@ -160,8 +161,8 @@ static void dma_blk_cb(void *opaque, int ret) >>> return; >>> } >>> >>> - if (dbs->iov.size & ~BDRV_SECTOR_MASK) { >>> - qemu_iovec_discard_back(&dbs->iov, dbs->iov.size & ~BDRV_SECTOR_MASK); >>> + if (dbs->iov.size & (dbs->align - 1)) { >>> + qemu_iovec_discard_back(&dbs->iov, dbs->iov.size & (dbs->align - 1)); >> >> Would it be any smarter to use osdep.h's QEMU_IS_ALIGNED(dbs->iov.size, >> dbs->align) and QEMU_ALIGN_DOWN(dbs->iov.size, dbs->align)? >> Semantically it is the same, but the macros make it obvious what the >> bit-twiddling is doing. >> >> Unless you think that needs a tweak, >> Reviewed-by: Eric Blake <eblake@redhat.com> > > I can't say I feel too strongly about it since there are plenty of other > examples of this style in the codebase, so I'm happy to go with whatever > John/Paolo are most happy with. > > > ATB, > > Mark. > I can't pretend I am consistent, but when in doubt use the macro. Not worth a respin IMO, but I think this falls out of my jurisdiction :) Acked-by: John Snow <jsnow@redhat.com> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] dma-helpers: explicitly pass alignment into dma-helpers 2016-10-11 15:47 ` John Snow @ 2016-10-12 10:22 ` Kevin Wolf 2016-10-12 16:04 ` John Snow 0 siblings, 1 reply; 13+ messages in thread From: Kevin Wolf @ 2016-10-12 10:22 UTC (permalink / raw) To: John Snow Cc: Mark Cave-Ayland, Eric Blake, keith.busch, mreitz, pbonzini, qemu-devel, qemu-block Am 11.10.2016 um 17:47 hat John Snow geschrieben: > On 10/10/2016 03:23 PM, Mark Cave-Ayland wrote: > >On 10/10/16 17:34, Eric Blake wrote: > > > >>On 10/09/2016 11:43 AM, Mark Cave-Ayland wrote: > >>>The hard-coded default alignment is BDRV_SECTOR_SIZE, however this is not > >>>necessarily the case for all platforms. Use this as the default alignment for > >>>all current callers. > >>> > >>>Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > >>>--- > >> > >>>@@ -160,8 +161,8 @@ static void dma_blk_cb(void *opaque, int ret) > >>> return; > >>> } > >>> > >>>- if (dbs->iov.size & ~BDRV_SECTOR_MASK) { > >>>- qemu_iovec_discard_back(&dbs->iov, dbs->iov.size & ~BDRV_SECTOR_MASK); > >>>+ if (dbs->iov.size & (dbs->align - 1)) { > >>>+ qemu_iovec_discard_back(&dbs->iov, dbs->iov.size & (dbs->align - 1)); > >> > >>Would it be any smarter to use osdep.h's QEMU_IS_ALIGNED(dbs->iov.size, > >>dbs->align) and QEMU_ALIGN_DOWN(dbs->iov.size, dbs->align)? > >>Semantically it is the same, but the macros make it obvious what the > >>bit-twiddling is doing. > >> > >>Unless you think that needs a tweak, > >>Reviewed-by: Eric Blake <eblake@redhat.com> > > > >I can't say I feel too strongly about it since there are plenty of other > >examples of this style in the codebase, so I'm happy to go with whatever > >John/Paolo are most happy with. > > > > > >ATB, > > > >Mark. > > > > I can't pretend I am consistent, but when in doubt use the macro. > Not worth a respin IMO, but I think this falls out of my > jurisdiction :) > > Acked-by: John Snow <jsnow@redhat.com> dma-helpers.c is officially unmaintained, and as the other patch is clearly IDE, I think the series should go through your tree. Kevin ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] dma-helpers: explicitly pass alignment into dma-helpers 2016-10-12 10:22 ` Kevin Wolf @ 2016-10-12 16:04 ` John Snow 0 siblings, 0 replies; 13+ messages in thread From: John Snow @ 2016-10-12 16:04 UTC (permalink / raw) To: Kevin Wolf Cc: Mark Cave-Ayland, Eric Blake, keith.busch, mreitz, pbonzini, qemu-devel, qemu-block On 10/12/2016 06:22 AM, Kevin Wolf wrote: > Am 11.10.2016 um 17:47 hat John Snow geschrieben: >> On 10/10/2016 03:23 PM, Mark Cave-Ayland wrote: >>> On 10/10/16 17:34, Eric Blake wrote: >>> >>>> On 10/09/2016 11:43 AM, Mark Cave-Ayland wrote: >>>>> The hard-coded default alignment is BDRV_SECTOR_SIZE, however this is not >>>>> necessarily the case for all platforms. Use this as the default alignment for >>>>> all current callers. >>>>> >>>>> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> >>>>> --- >>>> >>>>> @@ -160,8 +161,8 @@ static void dma_blk_cb(void *opaque, int ret) >>>>> return; >>>>> } >>>>> >>>>> - if (dbs->iov.size & ~BDRV_SECTOR_MASK) { >>>>> - qemu_iovec_discard_back(&dbs->iov, dbs->iov.size & ~BDRV_SECTOR_MASK); >>>>> + if (dbs->iov.size & (dbs->align - 1)) { >>>>> + qemu_iovec_discard_back(&dbs->iov, dbs->iov.size & (dbs->align - 1)); >>>> >>>> Would it be any smarter to use osdep.h's QEMU_IS_ALIGNED(dbs->iov.size, >>>> dbs->align) and QEMU_ALIGN_DOWN(dbs->iov.size, dbs->align)? >>>> Semantically it is the same, but the macros make it obvious what the >>>> bit-twiddling is doing. >>>> >>>> Unless you think that needs a tweak, >>>> Reviewed-by: Eric Blake <eblake@redhat.com> >>> >>> I can't say I feel too strongly about it since there are plenty of other >>> examples of this style in the codebase, so I'm happy to go with whatever >>> John/Paolo are most happy with. >>> >>> >>> ATB, >>> >>> Mark. >>> >> >> I can't pretend I am consistent, but when in doubt use the macro. >> Not worth a respin IMO, but I think this falls out of my >> jurisdiction :) >> >> Acked-by: John Snow <jsnow@redhat.com> > > dma-helpers.c is officially unmaintained, and as the other patch is > clearly IDE, I think the series should go through your tree. > > Kevin > Oh! I was under the impression that Paolo had the dma-helpers. My mistake. I will test and stage this. --js ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 2/2] macio: switch over to new byte-aligned DMA helpers 2016-10-09 16:43 [Qemu-devel] [PATCH 0/2] dma-helpers: explicitly pass alignment into dma-helpers Mark Cave-Ayland 2016-10-09 16:43 ` [Qemu-devel] [PATCH 1/2] " Mark Cave-Ayland @ 2016-10-09 16:43 ` Mark Cave-Ayland 2016-10-10 16:50 ` Eric Blake 2016-10-11 16:58 ` John Snow 1 sibling, 2 replies; 13+ messages in thread From: Mark Cave-Ayland @ 2016-10-09 16:43 UTC (permalink / raw) To: keith.busch, kwolf, mreitz, jsnow, pbonzini, qemu-devel, qemu-block Now that the DMA helpers are byte-aligned they can be called directly from the macio routines rather than emulating byte-aligned accesses via multiple block-level accesses. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> --- hw/ide/macio.c | 213 ++++++++------------------------------------------------ 1 file changed, 28 insertions(+), 185 deletions(-) diff --git a/hw/ide/macio.c b/hw/ide/macio.c index 76f97c2..9742c00 100644 --- a/hw/ide/macio.c +++ b/hw/ide/macio.c @@ -52,187 +52,6 @@ static const int debug_macio = 0; #define MACIO_PAGE_SIZE 4096 -/* - * Unaligned DMA read/write access functions required for OS X/Darwin which - * don't perform DMA transactions on sector boundaries. These functions are - * modelled on bdrv_co_preadv()/bdrv_co_pwritev() and so should be easy to - * remove if the unaligned block APIs are ever exposed. - */ - -static void pmac_dma_read(BlockBackend *blk, - int64_t offset, unsigned int bytes, - void (*cb)(void *opaque, int ret), void *opaque) -{ - DBDMA_io *io = opaque; - MACIOIDEState *m = io->opaque; - IDEState *s = idebus_active_if(&m->bus); - dma_addr_t dma_addr; - int64_t sector_num; - int nsector; - uint64_t align = BDRV_SECTOR_SIZE; - size_t head_bytes, tail_bytes; - - qemu_iovec_destroy(&io->iov); - qemu_iovec_init(&io->iov, io->len / MACIO_PAGE_SIZE + 1); - - sector_num = (offset >> 9); - nsector = (io->len >> 9); - - MACIO_DPRINTF("--- DMA read transfer (0x%" HWADDR_PRIx ",0x%x): " - "sector_num: %" PRId64 ", nsector: %d\n", io->addr, io->len, - sector_num, nsector); - - dma_addr = io->addr; - io->dir = DMA_DIRECTION_FROM_DEVICE; - io->dma_len = io->len; - io->dma_mem = dma_memory_map(&address_space_memory, dma_addr, &io->dma_len, - io->dir); - - if (offset & (align - 1)) { - head_bytes = offset & (align - 1); - - MACIO_DPRINTF("--- DMA unaligned head: sector %" PRId64 ", " - "discarding %zu bytes\n", sector_num, head_bytes); - - qemu_iovec_add(&io->iov, &io->head_remainder, head_bytes); - - bytes += offset & (align - 1); - offset = offset & ~(align - 1); - } - - qemu_iovec_add(&io->iov, io->dma_mem, io->len); - - if ((offset + bytes) & (align - 1)) { - tail_bytes = (offset + bytes) & (align - 1); - - MACIO_DPRINTF("--- DMA unaligned tail: sector %" PRId64 ", " - "discarding bytes %zu\n", sector_num, tail_bytes); - - qemu_iovec_add(&io->iov, &io->tail_remainder, align - tail_bytes); - bytes = ROUND_UP(bytes, align); - } - - s->io_buffer_size -= io->len; - s->io_buffer_index += io->len; - - io->len = 0; - - MACIO_DPRINTF("--- Block read transfer - sector_num: %" PRIx64 " " - "nsector: %x\n", (offset >> 9), (bytes >> 9)); - - s->bus->dma->aiocb = blk_aio_preadv(blk, offset, &io->iov, 0, cb, io); -} - -static void pmac_dma_write(BlockBackend *blk, - int64_t offset, int bytes, - void (*cb)(void *opaque, int ret), void *opaque) -{ - DBDMA_io *io = opaque; - MACIOIDEState *m = io->opaque; - IDEState *s = idebus_active_if(&m->bus); - dma_addr_t dma_addr; - int64_t sector_num; - int nsector; - uint64_t align = BDRV_SECTOR_SIZE; - size_t head_bytes, tail_bytes; - bool unaligned_head = false, unaligned_tail = false; - - qemu_iovec_destroy(&io->iov); - qemu_iovec_init(&io->iov, io->len / MACIO_PAGE_SIZE + 1); - - sector_num = (offset >> 9); - nsector = (io->len >> 9); - - MACIO_DPRINTF("--- DMA write transfer (0x%" HWADDR_PRIx ",0x%x): " - "sector_num: %" PRId64 ", nsector: %d\n", io->addr, io->len, - sector_num, nsector); - - dma_addr = io->addr; - io->dir = DMA_DIRECTION_TO_DEVICE; - io->dma_len = io->len; - io->dma_mem = dma_memory_map(&address_space_memory, dma_addr, &io->dma_len, - io->dir); - - if (offset & (align - 1)) { - head_bytes = offset & (align - 1); - sector_num = ((offset & ~(align - 1)) >> 9); - - MACIO_DPRINTF("--- DMA unaligned head: pre-reading head sector %" - PRId64 "\n", sector_num); - - blk_pread(s->blk, (sector_num << 9), &io->head_remainder, align); - - qemu_iovec_add(&io->iov, &io->head_remainder, head_bytes); - qemu_iovec_add(&io->iov, io->dma_mem, io->len); - - bytes += offset & (align - 1); - offset = offset & ~(align - 1); - - unaligned_head = true; - } - - if ((offset + bytes) & (align - 1)) { - tail_bytes = (offset + bytes) & (align - 1); - sector_num = (((offset + bytes) & ~(align - 1)) >> 9); - - MACIO_DPRINTF("--- DMA unaligned tail: pre-reading tail sector %" - PRId64 "\n", sector_num); - - blk_pread(s->blk, (sector_num << 9), &io->tail_remainder, align); - - if (!unaligned_head) { - qemu_iovec_add(&io->iov, io->dma_mem, io->len); - } - - qemu_iovec_add(&io->iov, &io->tail_remainder + tail_bytes, - align - tail_bytes); - - bytes = ROUND_UP(bytes, align); - - unaligned_tail = true; - } - - if (!unaligned_head && !unaligned_tail) { - qemu_iovec_add(&io->iov, io->dma_mem, io->len); - } - - s->io_buffer_size -= io->len; - s->io_buffer_index += io->len; - - io->len = 0; - - MACIO_DPRINTF("--- Block write transfer - sector_num: %" PRIx64 " " - "nsector: %x\n", (offset >> 9), (bytes >> 9)); - - s->bus->dma->aiocb = blk_aio_pwritev(blk, offset, &io->iov, 0, cb, io); -} - -static void pmac_dma_trim(BlockBackend *blk, - int64_t offset, int bytes, - void (*cb)(void *opaque, int ret), void *opaque) -{ - DBDMA_io *io = opaque; - MACIOIDEState *m = io->opaque; - IDEState *s = idebus_active_if(&m->bus); - dma_addr_t dma_addr; - - qemu_iovec_destroy(&io->iov); - qemu_iovec_init(&io->iov, io->len / MACIO_PAGE_SIZE + 1); - - dma_addr = io->addr; - io->dir = DMA_DIRECTION_TO_DEVICE; - io->dma_len = io->len; - io->dma_mem = dma_memory_map(&address_space_memory, dma_addr, &io->dma_len, - io->dir); - - qemu_iovec_add(&io->iov, io->dma_mem, io->len); - s->io_buffer_size -= io->len; - s->io_buffer_index += io->len; - io->len = 0; - - s->bus->dma->aiocb = ide_issue_trim(offset, &io->iov, cb, io, blk); -} - static void pmac_ide_atapi_transfer_cb(void *opaque, int ret) { DBDMA_io *io = opaque; @@ -244,6 +63,7 @@ static void pmac_ide_atapi_transfer_cb(void *opaque, int ret) if (ret < 0) { MACIO_DPRINTF("DMA error: %d\n", ret); + qemu_sglist_destroy(&s->sg); ide_atapi_io_error(s, ret); goto done; } @@ -258,6 +78,7 @@ static void pmac_ide_atapi_transfer_cb(void *opaque, int ret) if (s->io_buffer_size <= 0) { MACIO_DPRINTF("End of IDE transfer\n"); + qemu_sglist_destroy(&s->sg); ide_atapi_cmd_ok(s); m->dma_active = false; goto done; @@ -282,7 +103,15 @@ static void pmac_ide_atapi_transfer_cb(void *opaque, int ret) /* Calculate current offset */ offset = ((int64_t)s->lba << 11) + s->io_buffer_index; - pmac_dma_read(s->blk, offset, io->len, pmac_ide_atapi_transfer_cb, io); + qemu_sglist_init(&s->sg, DEVICE(m), io->len / MACIO_PAGE_SIZE + 1, + &address_space_memory); + qemu_sglist_add(&s->sg, io->addr, io->len); + s->io_buffer_size -= io->len; + s->io_buffer_index += io->len; + io->len = 0; + + s->bus->dma->aiocb = dma_blk_read(s->blk, &s->sg, offset, 0x1, + pmac_ide_atapi_transfer_cb, io); return; done: @@ -310,6 +139,7 @@ static void pmac_ide_transfer_cb(void *opaque, int ret) if (ret < 0) { MACIO_DPRINTF("DMA error: %d\n", ret); + qemu_sglist_destroy(&s->sg); ide_dma_error(s); goto done; } @@ -324,6 +154,7 @@ static void pmac_ide_transfer_cb(void *opaque, int ret) if (s->io_buffer_size <= 0) { MACIO_DPRINTF("End of IDE transfer\n"); + qemu_sglist_destroy(&s->sg); s->status = READY_STAT | SEEK_STAT; ide_set_irq(s->bus); m->dma_active = false; @@ -338,15 +169,27 @@ static void pmac_ide_transfer_cb(void *opaque, int ret) /* Calculate number of sectors */ offset = (ide_get_sector(s) << 9) + s->io_buffer_index; + qemu_sglist_init(&s->sg, DEVICE(m), io->len / MACIO_PAGE_SIZE + 1, + &address_space_memory); + qemu_sglist_add(&s->sg, io->addr, io->len); + s->io_buffer_size -= io->len; + s->io_buffer_index += io->len; + io->len = 0; + switch (s->dma_cmd) { case IDE_DMA_READ: - pmac_dma_read(s->blk, offset, io->len, pmac_ide_transfer_cb, io); + s->bus->dma->aiocb = dma_blk_read(s->blk, &s->sg, offset, 0x1, + pmac_ide_atapi_transfer_cb, io); break; case IDE_DMA_WRITE: - pmac_dma_write(s->blk, offset, io->len, pmac_ide_transfer_cb, io); + s->bus->dma->aiocb = dma_blk_write(s->blk, &s->sg, offset, 0x1, + pmac_ide_transfer_cb, io); break; case IDE_DMA_TRIM: - pmac_dma_trim(s->blk, offset, io->len, pmac_ide_transfer_cb, io); + s->bus->dma->aiocb = dma_blk_io(blk_get_aio_context(s->blk), &s->sg, + offset, 0x1, ide_issue_trim, s->blk, + pmac_ide_transfer_cb, io, + DMA_DIRECTION_TO_DEVICE); break; default: abort(); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] macio: switch over to new byte-aligned DMA helpers 2016-10-09 16:43 ` [Qemu-devel] [PATCH 2/2] macio: switch over to new byte-aligned DMA helpers Mark Cave-Ayland @ 2016-10-10 16:50 ` Eric Blake 2016-10-11 16:58 ` John Snow 1 sibling, 0 replies; 13+ messages in thread From: Eric Blake @ 2016-10-10 16:50 UTC (permalink / raw) To: Mark Cave-Ayland, keith.busch, kwolf, mreitz, jsnow, pbonzini, qemu-devel, qemu-block [-- Attachment #1: Type: text/plain, Size: 609 bytes --] On 10/09/2016 11:43 AM, Mark Cave-Ayland wrote: > Now that the DMA helpers are byte-aligned they can be called directly from > the macio routines rather than emulating byte-aligned accesses via multiple > block-level accesses. > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > --- > hw/ide/macio.c | 213 ++++++++------------------------------------------------ > 1 file changed, 28 insertions(+), 185 deletions(-) > Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 604 bytes --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] macio: switch over to new byte-aligned DMA helpers 2016-10-09 16:43 ` [Qemu-devel] [PATCH 2/2] macio: switch over to new byte-aligned DMA helpers Mark Cave-Ayland 2016-10-10 16:50 ` Eric Blake @ 2016-10-11 16:58 ` John Snow 1 sibling, 0 replies; 13+ messages in thread From: John Snow @ 2016-10-11 16:58 UTC (permalink / raw) To: Mark Cave-Ayland, keith.busch, kwolf, mreitz, pbonzini, qemu-devel, qemu-block On 10/09/2016 12:43 PM, Mark Cave-Ayland wrote: > Now that the DMA helpers are byte-aligned they can be called directly from > the macio routines rather than emulating byte-aligned accesses via multiple > block-level accesses. > _cool_ > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > --- > hw/ide/macio.c | 213 ++++++++------------------------------------------------ > 1 file changed, 28 insertions(+), 185 deletions(-) > > diff --git a/hw/ide/macio.c b/hw/ide/macio.c > index 76f97c2..9742c00 100644 > --- a/hw/ide/macio.c > +++ b/hw/ide/macio.c > @@ -52,187 +52,6 @@ static const int debug_macio = 0; > > #define MACIO_PAGE_SIZE 4096 > > -/* > - * Unaligned DMA read/write access functions required for OS X/Darwin which > - * don't perform DMA transactions on sector boundaries. These functions are > - * modelled on bdrv_co_preadv()/bdrv_co_pwritev() and so should be easy to > - * remove if the unaligned block APIs are ever exposed. > - */ > - > -static void pmac_dma_read(BlockBackend *blk, > - int64_t offset, unsigned int bytes, > - void (*cb)(void *opaque, int ret), void *opaque) > -{ > - DBDMA_io *io = opaque; > - MACIOIDEState *m = io->opaque; > - IDEState *s = idebus_active_if(&m->bus); > - dma_addr_t dma_addr; > - int64_t sector_num; > - int nsector; > - uint64_t align = BDRV_SECTOR_SIZE; > - size_t head_bytes, tail_bytes; > - > - qemu_iovec_destroy(&io->iov); > - qemu_iovec_init(&io->iov, io->len / MACIO_PAGE_SIZE + 1); > - > - sector_num = (offset >> 9); > - nsector = (io->len >> 9); > - > - MACIO_DPRINTF("--- DMA read transfer (0x%" HWADDR_PRIx ",0x%x): " > - "sector_num: %" PRId64 ", nsector: %d\n", io->addr, io->len, > - sector_num, nsector); > - > - dma_addr = io->addr; > - io->dir = DMA_DIRECTION_FROM_DEVICE; > - io->dma_len = io->len; > - io->dma_mem = dma_memory_map(&address_space_memory, dma_addr, &io->dma_len, > - io->dir); > - > - if (offset & (align - 1)) { > - head_bytes = offset & (align - 1); > - > - MACIO_DPRINTF("--- DMA unaligned head: sector %" PRId64 ", " > - "discarding %zu bytes\n", sector_num, head_bytes); > - > - qemu_iovec_add(&io->iov, &io->head_remainder, head_bytes); > - > - bytes += offset & (align - 1); > - offset = offset & ~(align - 1); > - } > - > - qemu_iovec_add(&io->iov, io->dma_mem, io->len); > - > - if ((offset + bytes) & (align - 1)) { > - tail_bytes = (offset + bytes) & (align - 1); > - > - MACIO_DPRINTF("--- DMA unaligned tail: sector %" PRId64 ", " > - "discarding bytes %zu\n", sector_num, tail_bytes); > - > - qemu_iovec_add(&io->iov, &io->tail_remainder, align - tail_bytes); > - bytes = ROUND_UP(bytes, align); > - } > - > - s->io_buffer_size -= io->len; > - s->io_buffer_index += io->len; > - > - io->len = 0; > - > - MACIO_DPRINTF("--- Block read transfer - sector_num: %" PRIx64 " " > - "nsector: %x\n", (offset >> 9), (bytes >> 9)); > - > - s->bus->dma->aiocb = blk_aio_preadv(blk, offset, &io->iov, 0, cb, io); > -} > - > -static void pmac_dma_write(BlockBackend *blk, > - int64_t offset, int bytes, > - void (*cb)(void *opaque, int ret), void *opaque) > -{ > - DBDMA_io *io = opaque; > - MACIOIDEState *m = io->opaque; > - IDEState *s = idebus_active_if(&m->bus); > - dma_addr_t dma_addr; > - int64_t sector_num; > - int nsector; > - uint64_t align = BDRV_SECTOR_SIZE; > - size_t head_bytes, tail_bytes; > - bool unaligned_head = false, unaligned_tail = false; > - > - qemu_iovec_destroy(&io->iov); > - qemu_iovec_init(&io->iov, io->len / MACIO_PAGE_SIZE + 1); > - > - sector_num = (offset >> 9); > - nsector = (io->len >> 9); > - > - MACIO_DPRINTF("--- DMA write transfer (0x%" HWADDR_PRIx ",0x%x): " > - "sector_num: %" PRId64 ", nsector: %d\n", io->addr, io->len, > - sector_num, nsector); > - > - dma_addr = io->addr; > - io->dir = DMA_DIRECTION_TO_DEVICE; > - io->dma_len = io->len; > - io->dma_mem = dma_memory_map(&address_space_memory, dma_addr, &io->dma_len, > - io->dir); > - > - if (offset & (align - 1)) { > - head_bytes = offset & (align - 1); > - sector_num = ((offset & ~(align - 1)) >> 9); > - > - MACIO_DPRINTF("--- DMA unaligned head: pre-reading head sector %" > - PRId64 "\n", sector_num); > - > - blk_pread(s->blk, (sector_num << 9), &io->head_remainder, align); > - > - qemu_iovec_add(&io->iov, &io->head_remainder, head_bytes); > - qemu_iovec_add(&io->iov, io->dma_mem, io->len); > - > - bytes += offset & (align - 1); > - offset = offset & ~(align - 1); > - > - unaligned_head = true; > - } > - > - if ((offset + bytes) & (align - 1)) { > - tail_bytes = (offset + bytes) & (align - 1); > - sector_num = (((offset + bytes) & ~(align - 1)) >> 9); > - > - MACIO_DPRINTF("--- DMA unaligned tail: pre-reading tail sector %" > - PRId64 "\n", sector_num); > - > - blk_pread(s->blk, (sector_num << 9), &io->tail_remainder, align); > - > - if (!unaligned_head) { > - qemu_iovec_add(&io->iov, io->dma_mem, io->len); > - } > - > - qemu_iovec_add(&io->iov, &io->tail_remainder + tail_bytes, > - align - tail_bytes); > - > - bytes = ROUND_UP(bytes, align); > - > - unaligned_tail = true; > - } > - > - if (!unaligned_head && !unaligned_tail) { > - qemu_iovec_add(&io->iov, io->dma_mem, io->len); > - } > - > - s->io_buffer_size -= io->len; > - s->io_buffer_index += io->len; > - > - io->len = 0; > - > - MACIO_DPRINTF("--- Block write transfer - sector_num: %" PRIx64 " " > - "nsector: %x\n", (offset >> 9), (bytes >> 9)); > - > - s->bus->dma->aiocb = blk_aio_pwritev(blk, offset, &io->iov, 0, cb, io); > -} > - > -static void pmac_dma_trim(BlockBackend *blk, > - int64_t offset, int bytes, > - void (*cb)(void *opaque, int ret), void *opaque) > -{ > - DBDMA_io *io = opaque; > - MACIOIDEState *m = io->opaque; > - IDEState *s = idebus_active_if(&m->bus); > - dma_addr_t dma_addr; > - > - qemu_iovec_destroy(&io->iov); > - qemu_iovec_init(&io->iov, io->len / MACIO_PAGE_SIZE + 1); > - > - dma_addr = io->addr; > - io->dir = DMA_DIRECTION_TO_DEVICE; > - io->dma_len = io->len; > - io->dma_mem = dma_memory_map(&address_space_memory, dma_addr, &io->dma_len, > - io->dir); > - > - qemu_iovec_add(&io->iov, io->dma_mem, io->len); > - s->io_buffer_size -= io->len; > - s->io_buffer_index += io->len; > - io->len = 0; > - > - s->bus->dma->aiocb = ide_issue_trim(offset, &io->iov, cb, io, blk); > -} > - > static void pmac_ide_atapi_transfer_cb(void *opaque, int ret) > { > DBDMA_io *io = opaque; > @@ -244,6 +63,7 @@ static void pmac_ide_atapi_transfer_cb(void *opaque, int ret) > > if (ret < 0) { > MACIO_DPRINTF("DMA error: %d\n", ret); > + qemu_sglist_destroy(&s->sg); > ide_atapi_io_error(s, ret); > goto done; > } > @@ -258,6 +78,7 @@ static void pmac_ide_atapi_transfer_cb(void *opaque, int ret) > > if (s->io_buffer_size <= 0) { > MACIO_DPRINTF("End of IDE transfer\n"); > + qemu_sglist_destroy(&s->sg); > ide_atapi_cmd_ok(s); > m->dma_active = false; > goto done; > @@ -282,7 +103,15 @@ static void pmac_ide_atapi_transfer_cb(void *opaque, int ret) > /* Calculate current offset */ > offset = ((int64_t)s->lba << 11) + s->io_buffer_index; > > - pmac_dma_read(s->blk, offset, io->len, pmac_ide_atapi_transfer_cb, io); > + qemu_sglist_init(&s->sg, DEVICE(m), io->len / MACIO_PAGE_SIZE + 1, > + &address_space_memory); > + qemu_sglist_add(&s->sg, io->addr, io->len); > + s->io_buffer_size -= io->len; > + s->io_buffer_index += io->len; > + io->len = 0; > + > + s->bus->dma->aiocb = dma_blk_read(s->blk, &s->sg, offset, 0x1, > + pmac_ide_atapi_transfer_cb, io); > return; > > done: > @@ -310,6 +139,7 @@ static void pmac_ide_transfer_cb(void *opaque, int ret) > > if (ret < 0) { > MACIO_DPRINTF("DMA error: %d\n", ret); > + qemu_sglist_destroy(&s->sg); > ide_dma_error(s); > goto done; > } > @@ -324,6 +154,7 @@ static void pmac_ide_transfer_cb(void *opaque, int ret) > > if (s->io_buffer_size <= 0) { > MACIO_DPRINTF("End of IDE transfer\n"); > + qemu_sglist_destroy(&s->sg); > s->status = READY_STAT | SEEK_STAT; > ide_set_irq(s->bus); > m->dma_active = false; > @@ -338,15 +169,27 @@ static void pmac_ide_transfer_cb(void *opaque, int ret) > /* Calculate number of sectors */ > offset = (ide_get_sector(s) << 9) + s->io_buffer_index; > > + qemu_sglist_init(&s->sg, DEVICE(m), io->len / MACIO_PAGE_SIZE + 1, > + &address_space_memory); > + qemu_sglist_add(&s->sg, io->addr, io->len); > + s->io_buffer_size -= io->len; > + s->io_buffer_index += io->len; > + io->len = 0; > + > switch (s->dma_cmd) { > case IDE_DMA_READ: > - pmac_dma_read(s->blk, offset, io->len, pmac_ide_transfer_cb, io); > + s->bus->dma->aiocb = dma_blk_read(s->blk, &s->sg, offset, 0x1, > + pmac_ide_atapi_transfer_cb, io); > break; > case IDE_DMA_WRITE: > - pmac_dma_write(s->blk, offset, io->len, pmac_ide_transfer_cb, io); > + s->bus->dma->aiocb = dma_blk_write(s->blk, &s->sg, offset, 0x1, > + pmac_ide_transfer_cb, io); > break; > case IDE_DMA_TRIM: > - pmac_dma_trim(s->blk, offset, io->len, pmac_ide_transfer_cb, io); > + s->bus->dma->aiocb = dma_blk_io(blk_get_aio_context(s->blk), &s->sg, > + offset, 0x1, ide_issue_trim, s->blk, > + pmac_ide_transfer_cb, io, > + DMA_DIRECTION_TO_DEVICE); > break; > default: > abort(); > Looks good, and I assume you've tested it well. I might be pushing it, but any plans in your future for MacIO qtests? Reviewed-by: John Snow <jsnow@redhat.com> ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 0/2] macio: switch over to new byte-aligned DMA helpers @ 2016-05-27 8:48 Mark Cave-Ayland 2016-05-27 8:48 ` [Qemu-devel] [PATCH 2/2] " Mark Cave-Ayland 0 siblings, 1 reply; 13+ messages in thread From: Mark Cave-Ayland @ 2016-05-27 8:48 UTC (permalink / raw) To: qemu-devel, qemu-ppc, pbonzini, aurelien Here is a tidied up version of my patch to convert the macio controller over to using the new byte-aligned DMA helpers. The first patch is just a hack and temporarily disables unaligned iovec truncation in the DMA helper (as discussed in the recent thread) until Paolo or someone else can devise a proper solution. Without this, the subsequent switch over to the DMA helpers will appear to work during a Darwin PPC install but the resulting image is corrupt and will fail to boot. The second patch is the real one and switches the macio controller over to use the new byte-aligned DMA helpers. Here I see a speed-up of around 2.5x-3x for a typical Darwin PPC installation compared to the previous code. Aurelien, I'd be grateful if you could test the TRIM path as I know this is something you've had issues with before and I couldn't quite figure out how to reproduce your TRIM tests from before. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Mark Cave-Ayland (2): dma-helpers.c: [HACK] disable iovec truncation to nearest sector size macio: switch over to new byte-aligned DMA helpers dma-helpers.c | 2 + hw/ide/macio.c | 213 ++++++++------------------------------------------------ 2 files changed, 30 insertions(+), 185 deletions(-) -- 1.7.10.4 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Qemu-devel] [PATCH 2/2] macio: switch over to new byte-aligned DMA helpers 2016-05-27 8:48 [Qemu-devel] [PATCH 0/2] " Mark Cave-Ayland @ 2016-05-27 8:48 ` Mark Cave-Ayland 2016-05-27 15:02 ` John Snow 0 siblings, 1 reply; 13+ messages in thread From: Mark Cave-Ayland @ 2016-05-27 8:48 UTC (permalink / raw) To: qemu-devel, qemu-ppc, pbonzini, aurelien Now that the DMA helpers are byte-aligned they can be called directly from the macio routines rather than emulating byte-aligned accesses via multiple block-level accesses. Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> --- hw/ide/macio.c | 213 ++++++++------------------------------------------------ 1 file changed, 28 insertions(+), 185 deletions(-) diff --git a/hw/ide/macio.c b/hw/ide/macio.c index 42ad68a..e4e567e 100644 --- a/hw/ide/macio.c +++ b/hw/ide/macio.c @@ -52,187 +52,6 @@ static const int debug_macio = 0; #define MACIO_PAGE_SIZE 4096 -/* - * Unaligned DMA read/write access functions required for OS X/Darwin which - * don't perform DMA transactions on sector boundaries. These functions are - * modelled on bdrv_co_preadv()/bdrv_co_pwritev() and so should be easy to - * remove if the unaligned block APIs are ever exposed. - */ - -static void pmac_dma_read(BlockBackend *blk, - int64_t offset, unsigned int bytes, - void (*cb)(void *opaque, int ret), void *opaque) -{ - DBDMA_io *io = opaque; - MACIOIDEState *m = io->opaque; - IDEState *s = idebus_active_if(&m->bus); - dma_addr_t dma_addr, dma_len; - void *mem; - int64_t sector_num; - int nsector; - uint64_t align = BDRV_SECTOR_SIZE; - size_t head_bytes, tail_bytes; - - qemu_iovec_destroy(&io->iov); - qemu_iovec_init(&io->iov, io->len / MACIO_PAGE_SIZE + 1); - - sector_num = (offset >> 9); - nsector = (io->len >> 9); - - MACIO_DPRINTF("--- DMA read transfer (0x%" HWADDR_PRIx ",0x%x): " - "sector_num: %" PRId64 ", nsector: %d\n", io->addr, io->len, - sector_num, nsector); - - dma_addr = io->addr; - dma_len = io->len; - mem = dma_memory_map(&address_space_memory, dma_addr, &dma_len, - DMA_DIRECTION_FROM_DEVICE); - - if (offset & (align - 1)) { - head_bytes = offset & (align - 1); - - MACIO_DPRINTF("--- DMA unaligned head: sector %" PRId64 ", " - "discarding %zu bytes\n", sector_num, head_bytes); - - qemu_iovec_add(&io->iov, &io->head_remainder, head_bytes); - - bytes += offset & (align - 1); - offset = offset & ~(align - 1); - } - - qemu_iovec_add(&io->iov, mem, io->len); - - if ((offset + bytes) & (align - 1)) { - tail_bytes = (offset + bytes) & (align - 1); - - MACIO_DPRINTF("--- DMA unaligned tail: sector %" PRId64 ", " - "discarding bytes %zu\n", sector_num, tail_bytes); - - qemu_iovec_add(&io->iov, &io->tail_remainder, align - tail_bytes); - bytes = ROUND_UP(bytes, align); - } - - s->io_buffer_size -= io->len; - s->io_buffer_index += io->len; - - io->len = 0; - - MACIO_DPRINTF("--- Block read transfer - sector_num: %" PRIx64 " " - "nsector: %x\n", (offset >> 9), (bytes >> 9)); - - s->bus->dma->aiocb = blk_aio_preadv(blk, offset, &io->iov, 0, cb, io); -} - -static void pmac_dma_write(BlockBackend *blk, - int64_t offset, int bytes, - void (*cb)(void *opaque, int ret), void *opaque) -{ - DBDMA_io *io = opaque; - MACIOIDEState *m = io->opaque; - IDEState *s = idebus_active_if(&m->bus); - dma_addr_t dma_addr, dma_len; - void *mem; - int64_t sector_num; - int nsector; - uint64_t align = BDRV_SECTOR_SIZE; - size_t head_bytes, tail_bytes; - bool unaligned_head = false, unaligned_tail = false; - - qemu_iovec_destroy(&io->iov); - qemu_iovec_init(&io->iov, io->len / MACIO_PAGE_SIZE + 1); - - sector_num = (offset >> 9); - nsector = (io->len >> 9); - - MACIO_DPRINTF("--- DMA write transfer (0x%" HWADDR_PRIx ",0x%x): " - "sector_num: %" PRId64 ", nsector: %d\n", io->addr, io->len, - sector_num, nsector); - - dma_addr = io->addr; - dma_len = io->len; - mem = dma_memory_map(&address_space_memory, dma_addr, &dma_len, - DMA_DIRECTION_TO_DEVICE); - - if (offset & (align - 1)) { - head_bytes = offset & (align - 1); - sector_num = ((offset & ~(align - 1)) >> 9); - - MACIO_DPRINTF("--- DMA unaligned head: pre-reading head sector %" - PRId64 "\n", sector_num); - - blk_pread(s->blk, (sector_num << 9), &io->head_remainder, align); - - qemu_iovec_add(&io->iov, &io->head_remainder, head_bytes); - qemu_iovec_add(&io->iov, mem, io->len); - - bytes += offset & (align - 1); - offset = offset & ~(align - 1); - - unaligned_head = true; - } - - if ((offset + bytes) & (align - 1)) { - tail_bytes = (offset + bytes) & (align - 1); - sector_num = (((offset + bytes) & ~(align - 1)) >> 9); - - MACIO_DPRINTF("--- DMA unaligned tail: pre-reading tail sector %" - PRId64 "\n", sector_num); - - blk_pread(s->blk, (sector_num << 9), &io->tail_remainder, align); - - if (!unaligned_head) { - qemu_iovec_add(&io->iov, mem, io->len); - } - - qemu_iovec_add(&io->iov, &io->tail_remainder + tail_bytes, - align - tail_bytes); - - bytes = ROUND_UP(bytes, align); - - unaligned_tail = true; - } - - if (!unaligned_head && !unaligned_tail) { - qemu_iovec_add(&io->iov, mem, io->len); - } - - s->io_buffer_size -= io->len; - s->io_buffer_index += io->len; - - io->len = 0; - - MACIO_DPRINTF("--- Block write transfer - sector_num: %" PRIx64 " " - "nsector: %x\n", (offset >> 9), (bytes >> 9)); - - s->bus->dma->aiocb = blk_aio_pwritev(blk, offset, &io->iov, 0, cb, io); -} - -static void pmac_dma_trim(BlockBackend *blk, - int64_t offset, int bytes, - void (*cb)(void *opaque, int ret), void *opaque) -{ - DBDMA_io *io = opaque; - MACIOIDEState *m = io->opaque; - IDEState *s = idebus_active_if(&m->bus); - dma_addr_t dma_addr, dma_len; - void *mem; - - qemu_iovec_destroy(&io->iov); - qemu_iovec_init(&io->iov, io->len / MACIO_PAGE_SIZE + 1); - - dma_addr = io->addr; - dma_len = io->len; - mem = dma_memory_map(&address_space_memory, dma_addr, &dma_len, - DMA_DIRECTION_TO_DEVICE); - - qemu_iovec_add(&io->iov, mem, io->len); - s->io_buffer_size -= io->len; - s->io_buffer_index += io->len; - io->len = 0; - - s->bus->dma->aiocb = ide_issue_trim(offset, &io->iov, cb, io, blk); -} - static void pmac_ide_atapi_transfer_cb(void *opaque, int ret) { DBDMA_io *io = opaque; @@ -244,6 +63,7 @@ static void pmac_ide_atapi_transfer_cb(void *opaque, int ret) if (ret < 0) { MACIO_DPRINTF("DMA error: %d\n", ret); + qemu_sglist_destroy(&s->sg); ide_atapi_io_error(s, ret); goto done; } @@ -258,6 +78,7 @@ static void pmac_ide_atapi_transfer_cb(void *opaque, int ret) if (s->io_buffer_size <= 0) { MACIO_DPRINTF("End of IDE transfer\n"); + qemu_sglist_destroy(&s->sg); ide_atapi_cmd_ok(s); m->dma_active = false; goto done; @@ -280,7 +101,15 @@ static void pmac_ide_atapi_transfer_cb(void *opaque, int ret) /* Calculate current offset */ offset = ((int64_t)s->lba << 11) + s->io_buffer_index; - pmac_dma_read(s->blk, offset, io->len, pmac_ide_atapi_transfer_cb, io); + qemu_sglist_init(&s->sg, DEVICE(m), io->len / MACIO_PAGE_SIZE + 1, + &address_space_memory); + qemu_sglist_add(&s->sg, io->addr, io->len); + s->io_buffer_size -= io->len; + s->io_buffer_index += io->len; + io->len = 0; + + s->bus->dma->aiocb = dma_blk_read(s->blk, &s->sg, offset, + pmac_ide_atapi_transfer_cb, io); return; done: @@ -305,6 +134,7 @@ static void pmac_ide_transfer_cb(void *opaque, int ret) if (ret < 0) { MACIO_DPRINTF("DMA error: %d\n", ret); + qemu_sglist_destroy(&s->sg); ide_dma_error(s); goto done; } @@ -319,6 +149,7 @@ static void pmac_ide_transfer_cb(void *opaque, int ret) if (s->io_buffer_size <= 0) { MACIO_DPRINTF("End of IDE transfer\n"); + qemu_sglist_destroy(&s->sg); s->status = READY_STAT | SEEK_STAT; ide_set_irq(s->bus); m->dma_active = false; @@ -333,15 +164,27 @@ static void pmac_ide_transfer_cb(void *opaque, int ret) /* Calculate number of sectors */ offset = (ide_get_sector(s) << 9) + s->io_buffer_index; + qemu_sglist_init(&s->sg, DEVICE(m), io->len / MACIO_PAGE_SIZE + 1, + &address_space_memory); + qemu_sglist_add(&s->sg, io->addr, io->len); + s->io_buffer_size -= io->len; + s->io_buffer_index += io->len; + io->len = 0; + switch (s->dma_cmd) { case IDE_DMA_READ: - pmac_dma_read(s->blk, offset, io->len, pmac_ide_transfer_cb, io); + s->bus->dma->aiocb = dma_blk_read(s->blk, &s->sg, offset, + pmac_ide_atapi_transfer_cb, io); break; case IDE_DMA_WRITE: - pmac_dma_write(s->blk, offset, io->len, pmac_ide_transfer_cb, io); + s->bus->dma->aiocb = dma_blk_write(s->blk, &s->sg, offset, + pmac_ide_transfer_cb, io); break; case IDE_DMA_TRIM: - pmac_dma_trim(s->blk, offset, io->len, pmac_ide_transfer_cb, io); + s->bus->dma->aiocb = dma_blk_io(blk_get_aio_context(s->blk), &s->sg, + offset, ide_issue_trim, s->blk, + pmac_ide_transfer_cb, io, + DMA_DIRECTION_TO_DEVICE); break; default: abort(); -- 1.7.10.4 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] macio: switch over to new byte-aligned DMA helpers 2016-05-27 8:48 ` [Qemu-devel] [PATCH 2/2] " Mark Cave-Ayland @ 2016-05-27 15:02 ` John Snow 2016-05-27 15:22 ` Mark Cave-Ayland 0 siblings, 1 reply; 13+ messages in thread From: John Snow @ 2016-05-27 15:02 UTC (permalink / raw) To: Mark Cave-Ayland, qemu-devel, qemu-ppc, pbonzini, aurelien On 05/27/2016 04:48 AM, Mark Cave-Ayland wrote: > Now that the DMA helpers are byte-aligned they can be called directly from > the macio routines rather than emulating byte-aligned accesses via multiple > block-level accesses. > > Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> > --- > hw/ide/macio.c | 213 ++++++++------------------------------------------------ ^ _cool_ ^ I assume you'll actually not be needing me to deal with this until you resolve the HACK in the pre-requisite patch, yes? > 1 file changed, 28 insertions(+), 185 deletions(-) > > diff --git a/hw/ide/macio.c b/hw/ide/macio.c > index 42ad68a..e4e567e 100644 > --- a/hw/ide/macio.c > +++ b/hw/ide/macio.c > @@ -52,187 +52,6 @@ static const int debug_macio = 0; > > #define MACIO_PAGE_SIZE 4096 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] macio: switch over to new byte-aligned DMA helpers 2016-05-27 15:02 ` John Snow @ 2016-05-27 15:22 ` Mark Cave-Ayland 0 siblings, 0 replies; 13+ messages in thread From: Mark Cave-Ayland @ 2016-05-27 15:22 UTC (permalink / raw) To: John Snow, qemu-devel, qemu-ppc, pbonzini, aurelien On 27/05/16 16:02, John Snow wrote: > On 05/27/2016 04:48 AM, Mark Cave-Ayland wrote: >> Now that the DMA helpers are byte-aligned they can be called directly from >> the macio routines rather than emulating byte-aligned accesses via multiple >> block-level accesses. >> >> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> >> --- >> hw/ide/macio.c | 213 ++++++++------------------------------------------------ > > ^ _cool_ ^ > > I assume you'll actually not be needing me to deal with this until you > resolve the HACK in the pre-requisite patch, yes? Yes, that's correct. The reason I posted it as-is was to provide Paolo with a test case and for Aurelien to verify the TRIM behaviour. Once the hack is no longer required, I'll resubmit it properly. ATB, Mark. ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2016-10-12 16:04 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-10-09 16:43 [Qemu-devel] [PATCH 0/2] dma-helpers: explicitly pass alignment into dma-helpers Mark Cave-Ayland 2016-10-09 16:43 ` [Qemu-devel] [PATCH 1/2] " Mark Cave-Ayland 2016-10-10 16:34 ` Eric Blake 2016-10-10 19:23 ` Mark Cave-Ayland 2016-10-11 15:47 ` John Snow 2016-10-12 10:22 ` Kevin Wolf 2016-10-12 16:04 ` John Snow 2016-10-09 16:43 ` [Qemu-devel] [PATCH 2/2] macio: switch over to new byte-aligned DMA helpers Mark Cave-Ayland 2016-10-10 16:50 ` Eric Blake 2016-10-11 16:58 ` John Snow -- strict thread matches above, loose matches on Subject: below -- 2016-05-27 8:48 [Qemu-devel] [PATCH 0/2] " Mark Cave-Ayland 2016-05-27 8:48 ` [Qemu-devel] [PATCH 2/2] " Mark Cave-Ayland 2016-05-27 15:02 ` John Snow 2016-05-27 15:22 ` Mark Cave-Ayland
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.