From patchwork Thu Jun 4 21:59:35 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mark Cave-Ayland X-Patchwork-Id: 480914 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id AEE98140218 for ; Fri, 5 Jun 2015 08:02:13 +1000 (AEST) Received: from localhost ([::1]:44275 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z0dDH-0006P2-N0 for incoming@patchwork.ozlabs.org; Thu, 04 Jun 2015 18:02:11 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39160) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z0dBE-0002we-4u for qemu-devel@nongnu.org; Thu, 04 Jun 2015 18:00:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z0dB8-00006y-Ak for qemu-devel@nongnu.org; Thu, 04 Jun 2015 18:00:03 -0400 Received: from s16892447.onlinehome-server.info ([82.165.15.123]:37278) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z0dB8-000061-2h; Thu, 04 Jun 2015 17:59:58 -0400 Received: from 5ec27711.skybroadband.com ([94.194.119.17] helo=kentang.lan) by s16892447.onlinehome-server.info with esmtpsa (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.76) (envelope-from ) id 1Z0dAz-0002zK-Dg; Thu, 04 Jun 2015 22:59:50 +0100 From: Mark Cave-Ayland To: jsnow@redhat.com, agraf@suse.de, qemu-devel@nongnu.org, qemu-ppc@nongnu.org Date: Thu, 4 Jun 2015 22:59:35 +0100 Message-Id: <1433455177-21243-3-git-send-email-mark.cave-ayland@ilande.co.uk> X-Mailer: git-send-email 1.7.10.4 In-Reply-To: <1433455177-21243-1-git-send-email-mark.cave-ayland@ilande.co.uk> References: <1433455177-21243-1-git-send-email-mark.cave-ayland@ilande.co.uk> X-SA-Exim-Connect-IP: 94.194.119.17 X-SA-Exim-Mail-From: mark.cave-ayland@ilande.co.uk X-SA-Exim-Version: 4.2.1 (built Sun, 08 Jan 2012 02:45:44 +0000) X-SA-Exim-Scanned: Yes (on s16892447.onlinehome-server.info) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 82.165.15.123 Subject: [Qemu-devel] [PATCH v2 2/4] macio: switch pmac_dma_write() over to new offset/len implementation X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org In particular, this fixes a bug whereby chains of overlapping head/tail chains would incorrectly write over each other's remainder cache. This is the access pattern used by OS X/Darwin and fixes an issue with a corrupt Darwin installation in my local tests. While we are here, rename the DBDMA_io struct property remainder to head_remainder for clarification. Signed-off-by: Mark Cave-Ayland Reviewed-by: John Snow --- hw/ide/macio.c | 116 ++++++++++++++++++++------------------------ include/hw/ppc/mac_dbdma.h | 3 +- 2 files changed, 55 insertions(+), 64 deletions(-) diff --git a/hw/ide/macio.c b/hw/ide/macio.c index 52ee4ac..85e315f 100644 --- a/hw/ide/macio.c +++ b/hw/ide/macio.c @@ -86,7 +86,7 @@ static void pmac_dma_read(BlockBackend *blk, MACIO_DPRINTF("--- DMA unaligned head: sector %" PRId64 ", " "discarding %zu bytes\n", sector_num, head_bytes); - qemu_iovec_add(&io->iov, &io->remainder, head_bytes); + qemu_iovec_add(&io->iov, &io->head_remainder, head_bytes); bytes += offset & (align - 1); offset = offset & ~(align - 1); @@ -100,7 +100,7 @@ static void pmac_dma_read(BlockBackend *blk, MACIO_DPRINTF("--- DMA unaligned tail: sector %" PRId64 ", " "discarding bytes %zu\n", sector_num, tail_bytes); - qemu_iovec_add(&io->iov, &io->remainder, align - tail_bytes); + qemu_iovec_add(&io->iov, &io->tail_remainder, align - tail_bytes); bytes = ROUND_UP(bytes, align); } @@ -117,7 +117,7 @@ static void pmac_dma_read(BlockBackend *blk, } static void pmac_dma_write(BlockBackend *blk, - int64_t sector_num, int nb_sectors, + int64_t offset, int bytes, void (*cb)(void *opaque, int ret), void *opaque) { DBDMA_io *io = opaque; @@ -125,90 +125,80 @@ static void pmac_dma_write(BlockBackend *blk, IDEState *s = idebus_active_if(&m->bus); dma_addr_t dma_addr, dma_len; void *mem; - int nsector, remainder; - int extra = 0; + 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); - if (io->remainder_len > 0) { - /* Return remainder of request */ - int transfer = MIN(io->remainder_len, io->len); + sector_num = (offset >> 9); + nsector = (io->len >> 9); - MACIO_DPRINTF("--- processing write remainder %x\n", transfer); - cpu_physical_memory_read(io->addr, - &io->remainder + (0x200 - transfer), - transfer); + MACIO_DPRINTF("--- DMA write transfer (0x%" HWADDR_PRIx ",0x%x): " + "sector_num: %" PRId64 ", nsector: %d\n", io->addr, io->len, + sector_num, nsector); - io->remainder_len -= transfer; - io->len -= transfer; - io->addr += transfer; + dma_addr = io->addr; + dma_len = io->len; + mem = dma_memory_map(&address_space_memory, dma_addr, &dma_len, + DMA_DIRECTION_TO_DEVICE); - s->io_buffer_index += transfer; - s->io_buffer_size -= transfer; + if (offset & (align - 1)) { + head_bytes = offset & (align - 1); + sector_num = ((offset & ~(align - 1)) >> 9); - if (io->remainder_len != 0) { - /* Still waiting for remainder */ - return; - } + MACIO_DPRINTF("--- DMA unaligned head: pre-reading head sector %" + PRId64 "\n", sector_num); - MACIO_DPRINTF("--> prepending bounce buffer with size 0x200\n"); + blk_pread(s->blk, (sector_num << 9), &io->head_remainder, align); - /* Sector transfer complete - prepend to request */ - qemu_iovec_add(&io->iov, &io->remainder, 0x200); - extra = 1; - } + qemu_iovec_add(&io->iov, &io->head_remainder, head_bytes); + qemu_iovec_add(&io->iov, mem, io->len); - if (s->drive_kind == IDE_CD) { - sector_num = (int64_t)(s->lba << 2) + (s->io_buffer_index >> 9); - } else { - sector_num = ide_get_sector(s) + (s->io_buffer_index >> 9); + bytes += offset & (align - 1); + offset = offset & ~(align - 1); + + unaligned_head = true; } - nsector = (io->len >> 9); - remainder = io->len - (nsector << 9); + if ((offset + bytes) & (align - 1)) { + tail_bytes = (offset + bytes) & (align - 1); + sector_num = (((offset + bytes) & ~(align - 1)) >> 9); - MACIO_DPRINTF("--- DMA write transfer - addr: %" HWADDR_PRIx " len: %x\n", - io->addr, io->len); - MACIO_DPRINTF("xxx remainder: %x\n", remainder); - MACIO_DPRINTF("xxx sector_num: %"PRIx64" nsector: %x\n", - sector_num, nsector); + MACIO_DPRINTF("--- DMA unaligned tail: pre-reading tail sector %" + PRId64 "\n", sector_num); - dma_addr = io->addr; - dma_len = io->len; - mem = dma_memory_map(&address_space_memory, dma_addr, &dma_len, - DMA_DIRECTION_TO_DEVICE); + blk_pread(s->blk, (sector_num << 9), &io->tail_remainder, align); - if (!remainder) { - MACIO_DPRINTF("--- DMA write aligned - addr: %" HWADDR_PRIx - " len: %x\n", io->addr, io->len); - qemu_iovec_add(&io->iov, mem, io->len); - } else { - /* Write up to last complete sector */ - MACIO_DPRINTF("--- DMA write unaligned - addr: %" HWADDR_PRIx - " len: %x\n", io->addr, (nsector << 9)); - qemu_iovec_add(&io->iov, mem, (nsector << 9)); + if (!unaligned_head) { + qemu_iovec_add(&io->iov, mem, io->len); + } - MACIO_DPRINTF("--- DMA write read - bounce addr: %p " - "remainder_len: %x\n", &io->remainder, remainder); - cpu_physical_memory_read(io->addr + (nsector << 9), &io->remainder, - remainder); + qemu_iovec_add(&io->iov, &io->tail_remainder + tail_bytes, + align - tail_bytes); + + bytes = ROUND_UP(bytes, align); - io->remainder_len = 0x200 - remainder; + unaligned_tail = true; + } - MACIO_DPRINTF("xxx remainder_len: %x\n", io->remainder_len); + if (!unaligned_head && !unaligned_tail) { + qemu_iovec_add(&io->iov, mem, io->len); } - s->io_buffer_size -= ((nsector + extra) << 9); - s->io_buffer_index += ((nsector + extra) << 9); + 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", sector_num, nsector + extra); + MACIO_DPRINTF("--- Block write transfer - sector_num: %" PRIx64 " " + "nsector: %x\n", (offset >> 9), (bytes >> 9)); - m->aiocb = blk_aio_writev(blk, sector_num, &io->iov, nsector + extra, cb, - io); + m->aiocb = blk_aio_writev(blk, (offset >> 9), &io->iov, (bytes >> 9), + cb, io); } static void pmac_ide_atapi_transfer_cb(void *opaque, int ret) @@ -340,7 +330,7 @@ static void pmac_ide_transfer_cb(void *opaque, int ret) pmac_dma_read(s->blk, offset, io->len, pmac_ide_transfer_cb, io); break; case IDE_DMA_WRITE: - pmac_dma_write(s->blk, sector_num, nsector, pmac_ide_transfer_cb, io); + pmac_dma_write(s->blk, offset, io->len, pmac_ide_transfer_cb, io); break; case IDE_DMA_TRIM: MACIO_DPRINTF("TRIM command issued!"); diff --git a/include/hw/ppc/mac_dbdma.h b/include/hw/ppc/mac_dbdma.h index c580327..7f247fa 100644 --- a/include/hw/ppc/mac_dbdma.h +++ b/include/hw/ppc/mac_dbdma.h @@ -40,7 +40,8 @@ struct DBDMA_io { /* DMA is in progress, don't start another one */ bool processing; /* unaligned last sector of a request */ - uint8_t remainder[0x200]; + uint8_t head_remainder[0x200]; + uint8_t tail_remainder[0x200]; int remainder_len; QEMUIOVector iov; };