From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60417) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YzsOh-0005mA-Fy for qemu-devel@nongnu.org; Tue, 02 Jun 2015 16:02:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YzsOc-0007oV-Jq for qemu-devel@nongnu.org; Tue, 02 Jun 2015 16:02:51 -0400 Message-ID: <556E0BE0.3080904@redhat.com> Date: Tue, 02 Jun 2015 16:02:40 -0400 From: John Snow MIME-Version: 1.0 References: <1433102732-24034-1-git-send-email-mark.cave-ayland@ilande.co.uk> <1433102732-24034-2-git-send-email-mark.cave-ayland@ilande.co.uk> In-Reply-To: <1433102732-24034-2-git-send-email-mark.cave-ayland@ilande.co.uk> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 1/4] macio: switch pmac_dma_read() over to new offset/len implementation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Mark Cave-Ayland , agraf@suse.de, qemu-devel@nongnu.org, qemu-ppc@nongnu.org On 05/31/2015 04:05 PM, Mark Cave-Ayland wrote: > For better handling of unaligned block device accesses. >=20 > Signed-off-by: Mark Cave-Ayland > --- > hw/ide/macio.c | 102 ++++++++++++++++++++++--------------------------= -------- > 1 file changed, 40 insertions(+), 62 deletions(-) >=20 > diff --git a/hw/ide/macio.c b/hw/ide/macio.c > index 585a27b..f1ac001 100644 > --- a/hw/ide/macio.c > +++ b/hw/ide/macio.c > @@ -52,7 +52,7 @@ static const int debug_macio =3D 0; > #define MACIO_PAGE_SIZE 4096 > =20 > static void pmac_dma_read(BlockBackend *blk, > - int64_t sector_num, int nb_sectors, > + int64_t offset, unsigned int bytes, > void (*cb)(void *opaque, int ret), void *opa= que) > { > DBDMA_io *io =3D opaque; > @@ -60,76 +60,48 @@ static void pmac_dma_read(BlockBackend *blk, > IDEState *s =3D idebus_active_if(&m->bus); > dma_addr_t dma_addr, dma_len; > void *mem; > - int nsector, remainder; > + int64_t sector_num; > + int nsector; > + uint64_t align =3D BDRV_SECTOR_SIZE; > + size_t head_bytes, tail_bytes; > =20 > qemu_iovec_destroy(&io->iov); > qemu_iovec_init(&io->iov, io->len / MACIO_PAGE_SIZE + 1); > =20 > - if (io->remainder_len > 0) { > - /* Return remainder of request */ > - int transfer =3D MIN(io->remainder_len, io->len); > + sector_num =3D (offset >> 9); > + nsector =3D (io->len >> 9); > =20 > - MACIO_DPRINTF("--- DMA read pop - bounce addr: %p addr: %" > - HWADDR_PRIx " remainder_len: %x\n", > - &io->remainder + (0x200 - transfer), io->addr, > - io->remainder_len); > + MACIO_DPRINTF("--- DMA read transfer (0x%" HWADDR_PRIx ",0x%x): " > + "sector_num: %ld, nsector: %d\n", io->addr, io->len, > + sector_num, nsector); > =20 > - cpu_physical_memory_write(io->addr, > - &io->remainder + (0x200 - transfer), > - transfer); > + dma_addr =3D io->addr; > + dma_len =3D io->len; > + mem =3D dma_memory_map(&address_space_memory, dma_addr, &dma_len, > + DMA_DIRECTION_FROM_DEVICE); > =20 > - io->remainder_len -=3D transfer; > - io->len -=3D transfer; > - io->addr +=3D transfer; > + if (offset & (align - 1)) { > + head_bytes =3D offset & (align - 1); > =20 > - s->io_buffer_index +=3D transfer; > - s->io_buffer_size -=3D transfer; > + MACIO_DPRINTF("--- DMA unaligned head: sector %ld, " > + "discarding %ld bytes\n", sector_num, head_bytes= ); > =20 > - if (io->remainder_len !=3D 0) { > - /* Still waiting for remainder */ > - return; > - } > + qemu_iovec_add(&io->iov, &io->remainder, head_bytes); > =20 > - if (io->len =3D=3D 0) { > - MACIO_DPRINTF("--- finished all read processing; go and fi= nish\n"); > - cb(opaque, 0); > - return; > - } > + bytes +=3D offset & (align - 1); > + offset =3D offset & ~(align - 1); > } > =20 > - if (s->drive_kind =3D=3D IDE_CD) { > - sector_num =3D (int64_t)(s->lba << 2) + (s->io_buffer_index >>= 9); > - } else { > - sector_num =3D ide_get_sector(s) + (s->io_buffer_index >> 9); > - } > + qemu_iovec_add(&io->iov, mem, io->len); > =20 > - nsector =3D ((io->len + 0x1ff) >> 9); > - remainder =3D (nsector << 9) - io->len; > + if ((offset + bytes) & (align - 1)) { > + tail_bytes =3D (offset + bytes) & (align - 1); > =20 > - MACIO_DPRINTF("--- DMA read transfer - addr: %" HWADDR_PRIx " len:= %x\n", > - io->addr, io->len); > - > - dma_addr =3D io->addr; > - dma_len =3D io->len; > - mem =3D dma_memory_map(&address_space_memory, dma_addr, &dma_len, > - DMA_DIRECTION_FROM_DEVICE); > + MACIO_DPRINTF("--- DMA unaligned tail: sector %ld, " > + "discarding bytes %ld\n", sector_num, tail_bytes= ); > =20 > - if (!remainder) { > - MACIO_DPRINTF("--- DMA read aligned - addr: %" HWADDR_PRIx > - " len: %x\n", io->addr, io->len); > - qemu_iovec_add(&io->iov, mem, io->len); > - } else { > - MACIO_DPRINTF("--- DMA read unaligned - addr: %" HWADDR_PRIx > - " len: %x\n", io->addr, io->len); > - qemu_iovec_add(&io->iov, mem, io->len); > - > - MACIO_DPRINTF("--- DMA read push - bounce addr: %p " > - "remainder_len: %x\n", > - &io->remainder + 0x200 - remainder, remainder); > - qemu_iovec_add(&io->iov, &io->remainder + 0x200 - remainder, > - remainder); > - > - io->remainder_len =3D remainder; > + qemu_iovec_add(&io->iov, &io->remainder, align - tail_bytes); > + bytes =3D ROUND_UP(bytes, align); > } > =20 > s->io_buffer_size -=3D io->len; > @@ -137,11 +109,11 @@ static void pmac_dma_read(BlockBackend *blk, > =20 > io->len =3D 0; > =20 > - MACIO_DPRINTF("--- Block read transfer - sector_num: %"PRIx64" = " > - "nsector: %x\n", > - sector_num, nsector); > + MACIO_DPRINTF("--- Block read transfer - sector_num: %" PRIx64 " = " > + "nsector: %x\n", (offset >> 9), (bytes >> 9)); > =20 > - m->aiocb =3D blk_aio_readv(blk, sector_num, &io->iov, nsector, cb,= io); > + m->aiocb =3D blk_aio_readv(blk, (offset >> 9), &io->iov, (bytes >>= 9), > + cb, io); > } > =20 > static void pmac_dma_write(BlockBackend *blk, > @@ -246,6 +218,7 @@ static void pmac_ide_atapi_transfer_cb(void *opaque= , int ret) > IDEState *s =3D idebus_active_if(&m->bus); > int64_t sector_num; > int nsector, remainder; > + int64_t offset; > =20 > MACIO_DPRINTF("\ns is %p\n", s); > MACIO_DPRINTF("io_buffer_index: %x\n", s->io_buffer_index); > @@ -294,10 +267,13 @@ static void pmac_ide_atapi_transfer_cb(void *opaq= ue, int ret) > nsector =3D (io->len + 0x1ff) >> 9; > remainder =3D io->len & 0x1ff; > =20 > + /* Calculate current offset */ > + offset =3D (int64_t)(s->lba << 11) + s->io_buffer_index; > + Bike shedding: Worth an ATAPI_BLOCK_SIZE_BITS define or similar? I guess we don't really currently try to avoid magic constants for 512, 2048, etc etc anywhere else, so ... nevermind. Just a project for a different day. Forget I said anything. > MACIO_DPRINTF("nsector: %d remainder: %x\n", nsector, remainder)= ; > MACIO_DPRINTF("sector: %"PRIx64" %zx\n", sector_num, io->iov.siz= e / 512); > =20 > - pmac_dma_read(s->blk, sector_num, nsector, pmac_ide_atapi_transfer= _cb, io); > + pmac_dma_read(s->blk, offset, io->len, pmac_ide_atapi_transfer_cb,= io); > return; > =20 > done: > @@ -315,6 +291,7 @@ static void pmac_ide_transfer_cb(void *opaque, int = ret) > IDEState *s =3D idebus_active_if(&m->bus); > int64_t sector_num; > int nsector, remainder; > + int64_t offset; > =20 > MACIO_DPRINTF("pmac_ide_transfer_cb\n"); > =20 > @@ -349,6 +326,7 @@ static void pmac_ide_transfer_cb(void *opaque, int = ret) > =20 > /* Calculate number of sectors */ > sector_num =3D ide_get_sector(s) + (s->io_buffer_index >> 9); > + offset =3D (ide_get_sector(s) << 9) + s->io_buffer_index; > nsector =3D (io->len + 0x1ff) >> 9; > remainder =3D io->len & 0x1ff; > =20 > @@ -359,7 +337,7 @@ static void pmac_ide_transfer_cb(void *opaque, int = ret) > =20 > switch (s->dma_cmd) { > case IDE_DMA_READ: > - pmac_dma_read(s->blk, sector_num, nsector, pmac_ide_transfer_c= b, io); > + pmac_dma_read(s->blk, offset, io->len, pmac_ide_transfer_cb, i= o); > break; > case IDE_DMA_WRITE: > pmac_dma_write(s->blk, sector_num, nsector, pmac_ide_transfer_= cb, io); >=20