From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51303) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wxzrs-0002Ik-Rk for qemu-devel@nongnu.org; Fri, 20 Jun 2014 10:32:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wxzrn-0006Vq-B4 for qemu-devel@nongnu.org; Fri, 20 Jun 2014 10:32:40 -0400 Message-ID: <53A44548.7020605@ilande.co.uk> Date: Fri, 20 Jun 2014 15:29:28 +0100 From: Mark Cave-Ayland MIME-Version: 1.0 References: <1401885899-16524-1-git-send-email-agraf@suse.de> <1401885899-16524-76-git-send-email-agraf@suse.de> In-Reply-To: <1401885899-16524-76-git-send-email-agraf@suse.de> Content-Type: multipart/mixed; boundary="------------050404080900020409000806" Subject: Re: [Qemu-devel] [PULL 075/118] macio: handle non-block ATAPI DMA transfers List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Graf , BALATON Zoltan Cc: "qemu-ppc@nongnu.org" , qemu-devel This is a multi-part message in MIME format. --------------050404080900020409000806 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit On 04/06/14 13:44, Alexander Graf wrote: > From: Mark Cave-Ayland > > Currently the macio DMA routines assume that all DMA requests are for read/write > block transfers. This is not always the case for ATAPI, for example when > requesting a TOC where the response is generated directly in the IDE buffer. > > Detect these non-block ATAPI DMA transfers (where no lba is specified in the > command) and copy the results directly into RAM as indicated by the DBDMA > descriptor. This fixes CDROM access under MorphOS. > > Signed-off-by: Mark Cave-Ayland > Signed-off-by: Alexander Graf I've just done a complete round of OpenBIOS tests and it looks as if this patch introduces random hang failures (60% or so) into my Darwin image boot tests. The issue seems to be related as to the timing of the DMA callback relative to reception of the IDE command; if the IDE transfer is invoked first (as happens in Darwin) then we hang after sending the IDE command which is likely because the DMA is not being completed correctly. There is also another difference with MorphOS in that when the IDE transfer callback is invoked first then s->packet_transfer_size == 0 and so the memory copy would always copy 0 bytes. After some experimentation, I've come up with the attached patch which should retain the DMA memory copy, but instead invokes a further round of pmac_ide_atapi_transfer_cb() with io->len == 0 and s->io_buffer_size == 0 which should terminate the DMA request correctly. At the very least it seems to fix the hang on boot with my Darwin images. Zoltan, please can you test the attached patch to see if this still allows MorphOS to boot? ATB, Mark. --------------050404080900020409000806 Content-Type: text/x-diff; name="qemu-macio-ppc.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="qemu-macio-ppc.patch" diff --git a/hw/ide/macio.c b/hw/ide/macio.c index c14a1dd..39bc7fd 100644 --- a/hw/ide/macio.c +++ b/hw/ide/macio.c @@ -367,27 +367,21 @@ static void pmac_ide_transfer(DBDMA_io *io) s->io_buffer_size = 0; if (s->drive_kind == IDE_CD) { + bdrv_acct_start(s->bs, &s->acct, io->len, BDRV_ACCT_READ); + /* Handle non-block ATAPI DMA 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-block ATAPI DMA transfer size: %d\n", - s->io_buffer_size); + io->len); - /* Copy ATAPI buffer directly to RAM and finish */ + /* Copy ATAPI buffer directly to RAM */ 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-block ATAPI DMA transfer\n"); - bdrv_acct_done(s->bs, &s->acct); - io->dma_end(io); - return; + io->len); + + /* Finish on next callback */ + io->len = 0; } - bdrv_acct_start(s->bs, &s->acct, io->len, BDRV_ACCT_READ); pmac_ide_atapi_transfer_cb(io, 0); return; } --------------050404080900020409000806--