From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54797) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WlGGY-0001eB-6I for qemu-devel@nongnu.org; Fri, 16 May 2014 07:25:37 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WlGGQ-0004YU-NS for qemu-devel@nongnu.org; Fri, 16 May 2014 07:25:30 -0400 Message-ID: <5375F4F2.9030804@ilande.co.uk> Date: Fri, 16 May 2014 12:22:26 +0100 From: Mark Cave-Ayland MIME-Version: 1.0 References: <5366CA94.3030803@ilande.co.uk> <536A328F.6070100@ilande.co.uk> <536A6F3C.1030708@ilande.co.uk> <5370F620.7050206@ilande.co.uk> <5371303F.1060903@ilande.co.uk> <5372F741.9080108@ilande.co.uk> <5373CD82.6000407@ilande.co.uk> <53748952.9080909@ilande.co.uk> <537513FA.6060705@ilande.co.uk> In-Reply-To: Content-Type: multipart/mixed; boundary="------------060806080506070503070601" Subject: Re: [Qemu-devel] [Qemu-ppc] macio ide question/bug report List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: BALATON Zoltan Cc: qemu-ppc@nongnu.org, Alexander Graf , qemu-devel@nongnu.org This is a multi-part message in MIME format. --------------060806080506070503070601 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit On 15/05/14 21:28, BALATON Zoltan wrote: > On Thu, 15 May 2014, BALATON Zoltan wrote: >> On Thu, 15 May 2014, BALATON Zoltan wrote: >>> confusing.) Do you think that replacing io->len in your patch with >>> s->io_buffer_size would be the correct thing to do? That looks reasonable, as the MIN() will help prevent excessive memory clobber. >> Probably that's not enough. I've tried it and then it gets to here: > > I should've also included these lines too to make it more clear what did > I change: Yes, this is definitely helpful. It appears that cmd_read_toc_pma_atip() returns 0x20 bytes of data (can you confirm this?), while the DMA engine is configured to transfer 0x324 bytes of data - perhaps this is the maximum possible size of a TOC?. So the existing code determines there are still 0x324 - 0x20 = 0x304 bytes remaining for the DMA request and falls into the unaligned code which is definitely not what we want. Perhaps we need to assume for a non-IO DMA request that the result will only be a single ATAPI reply packet? Attached is another version of the patch for you to experiment with which makes your s->io_buffer_size change but also moves the logic into pmac_ide_transfer() so that we don't inadvertently drop into the unaligned code. ATB, Mark. --------------060806080506070503070601 Content-Type: text/x-diff; name="qemu-macio-atapi-v2.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="qemu-macio-atapi-v2.patch" diff --git a/hw/ide/macio.c b/hw/ide/macio.c index da94580..0f68124 100644 --- a/hw/ide/macio.c +++ b/hw/ide/macio.c @@ -337,6 +337,24 @@ static void pmac_ide_transfer(DBDMA_io *io) s->io_buffer_size = 0; if (s->drive_kind == IDE_CD) { + + /* Handle non-IO DMA ATAPI transfers */ + if (s->lba == -1) { + s->io_buffer_size = MIN(io->len, s->packet_transfer_size); + bdrv_acct_start(s->bs, &s->acct, s->io_buffer_size, BDRV_ACCT_READ); + MACIO_DPRINTF("non-IO ATAPI DMA transfer size: %d\n", s->io_buffer_size); + + /* Copy ATAPI buffer directly to RAM and finish */ + cpu_physical_memory_write(io->addr, s->io_buffer, s->io_buffer_size); + ide_atapi_cmd_ok(s); + m->dma_active = false; + + MACIO_DPRINTF("end of non-IO ATAPI DMA transfer\n"); + bdrv_acct_done(s->bs, &s->acct); + io->dma_end(io); + return; + } + bdrv_acct_start(s->bs, &s->acct, io->len, BDRV_ACCT_READ); pmac_ide_atapi_transfer_cb(io, 0); return; --------------060806080506070503070601--