From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51849) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wjt5r-0000mO-Ss for qemu-devel@nongnu.org; Mon, 12 May 2014 12:28:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wjt5k-0002Nv-CJ for qemu-devel@nongnu.org; Mon, 12 May 2014 12:28:47 -0400 Message-ID: <5370F620.7050206@ilande.co.uk> Date: Mon, 12 May 2014 17:26:08 +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> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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 On 10/05/14 13:30, BALATON Zoltan wrote: > That patch would be 80fc95d8bdaf3392106b131a97ca701fd374489a in QEMU > master. I've tried reverting it and Darwin still boots (without -M > mac99) up to the point where it asks to install as before but I don't > know how good a test is this as I'm not sure it does unaligned accesses > up to this point. I see accesses like this: > > pmac_ide_transfer(ATAPI) lba=ffffffff, buffer_index=30, len=930 > pmac_ide_transfer(ATAPI) lba=0, buffer_index=0, len=930 Here is an example of an unaligned access, where the length is not a multiple of 0x200 (512 bytes) which was required for the block layer DMA APIs at the time it was written. If you can get to the same point with the patch reverted, then it means Kevin's patchset for byte-level rather than sector-level granularity for block requests is working. What's important is the sum of the start_input size descriptors now matches the size of the ATA request, so the dma_* callbacks can now be used directly with byte precision rather than clobbering the memory up to the next multiple of 0x200. > This however does not fix MorphOS which fails as before. I've tried this > patch too (on top of the previous revert) but that did not work either: > > diff --git a/hw/ide/macio.c b/hw/ide/macio.c > index d3395f4..bce14fc 100644 > --- a/hw/ide/macio.c > +++ b/hw/ide/macio.c > @@ -56,6 +56,7 @@ static void pmac_ide_atapi_transfer_cb(void *opaque, > int ret) > DBDMA_io *io = opaque; > MACIOIDEState *m = io->opaque; > IDEState *s = idebus_active_if(&m->bus); > + int64_t sector_num; > > if (ret < 0) { > m->aiocb = NULL; > @@ -107,13 +108,16 @@ static void pmac_ide_atapi_transfer_cb(void > *opaque, int r > qemu_sglist_add(&s->sg, io->addr, io->len); > io->addr += io->len; > io->len = 0; > - > - MACIO_DPRINTF("sector_num=%d size=%d, cmd_cmd=%d\n", > - (s->lba << 2) + (s->io_buffer_index >> 9), > + if (s->lba >= 0) > + sector_num = (s->lba << 2) + (s->io_buffer_index >> 9); > + else > + sector_num = s->lba; > + MACIO_DPRINTF("sector_num=%ld size=%d, cmd_cmd=%d\n", > + sector_num, > s->packet_transfer_size, s->dma_cmd); > > m->aiocb = dma_bdrv_read(s->bs, &s->sg, > - (int64_t)(s->lba << 2) + > (s->io_buffer_index >> 9) > + sector_num, > pmac_ide_atapi_transfer_cb, io); > return; > > This still produces an error in MorphOS as before: > > DBDMA: writel 0x0000000000000d0c <= 0x00e5ac80 > DBDMA: channel 0x1a reg 0x3 > DBDMA: dbdma_cmdptr_load 0x00e5ac80 > ATAPI limit=0x8000 packet: 43 00 00 00 00 00 00 03 24 00 00 00 > > DBDMA: DBDMA_run_bh > DBDMA: writel 0x0000000000000d00 <= 0x80008000 > DBDMA: channel 0x1a reg 0x0 > DBDMA: status 0x00008400 > DBDMA: readl 0x0000000000000d00 => 0x80008000 > DBDMA: channel 0x1a reg 0x0 > DBDMA: DBDMA_run_bh > DBDMA: channel_run > dbdma_cmd 0x7f7dddff5ae0 > req_count 0x0324 > command 0x3000 > phy_addr 0x00e7dddc > cmd_dep 0x00000000 > res_count 0x0000 > xfer_status 0x0000 > DBDMA: start_input > DBDMA: addr 0xe7dddc key 0x0 > pmac_ide_transfer(ATAPI) lba=ffffffff, buffer_index=0, len=324 ^^^^^ > io_buffer_size = 0 > io->len = 0x324 > sector_num=-1 size=20, cmd_cmd=0 > atapi_cmd_error: sense=0x5 asc=0x21 > done DMA > DBDMA: dbdma_end > > Interestingly a similar read works for Darwin which does this: > > ATAPI limit=0x0 packet: bb 00 ff ff 00 00 00 00 00 00 00 00 > ATAPI limit=0xfffe packet: 43 02 00 00 00 00 00 ff fe 80 00 00 > reply: tx_size=48 elem_tx_size=0 index=0 > byte_count_limit=65534 > status=0x58 > reply: tx_size=0 elem_tx_size=0 index=48 > status=0x50 > ATAPI limit=0x30 packet: 43 02 00 00 00 00 00 00 30 80 00 00 > reply: tx_size=48 elem_tx_size=0 index=0 > byte_count_limit=48 > status=0x58 > reply: tx_size=0 elem_tx_size=0 index=48 > status=0x50 > DBDMA: readl 0x0000000000000d04 => 0x00000000 > DBDMA: channel 0x1a reg 0x1 > DBDMA: writel 0x0000000000000d08 <= 0x00000000 > DBDMA: channel 0x1a reg 0x2 > DBDMA: writel 0x0000000000000d0c <= 0x011f8010 > DBDMA: channel 0x1a reg 0x3 > DBDMA: dbdma_cmdptr_load 0x011f8010 > DBDMA: writel 0x0000000000000d00 <= 0x90009000 > DBDMA: channel 0x1a reg 0x0 > DBDMA: status 0x00009400 > DBDMA: DBDMA_run_bh > DBDMA: channel_run > dbdma_cmd 0x7ffa05173c60 > req_count 0x0930 > command 0x2000 > phy_addr 0x017cc000 > cmd_dep 0x00000000 > res_count 0x0000 > xfer_status 0x0000 > DBDMA: start_input > DBDMA: addr 0x17cc000 key 0x0 > pmac_ide_transfer(ATAPI) lba=ffffffff, buffer_index=30, len=930 > waiting for data (0x3 - 0x930 - 50) ^^^^^ > ATAPI limit=0x930 packet: be 00 00 00 00 00 00 00 01 f8 00 00 > read dma: LBA=0 nb_sectors=1 > > DBDMA: DBDMA_run_bh > DBDMA: channel_run > dbdma_cmd 0x7ffa05173c60 > req_count 0x0930 > command 0x2000 > phy_addr 0x017cc000 > cmd_dep 0x00000000 > res_count 0x0000 > xfer_status 0x0000 > DBDMA: start_input > DBDMA: addr 0x17cc000 key 0x0 > pmac_ide_transfer(ATAPI) lba=0, buffer_index=0, len=930 > io_buffer_size = 0 > io->len = 0x930 > sector_num=0 size=2352, cmd_cmd=0 > io_buffer_size = 0x930 > end of transfer > end of DMA > done DMA > DBDMA: dbdma_end From the limited block layer knowledge I have, it seems that lba == -1 is a default value meaning the ATA request hasn't been submitted. Take a look at the lines I marked above: it looks to me as if in the 2nd request, the DMA controller is running but realises that it is still waiting for the ATA request to complete before continuing which is why you see "waiting for data" followed by that ATAPI packet. In your first case, pmac_ide_transfer() incorrectly believes the ATA request has already completed which is why it tries to continue and then chokes on the invalid lba value of -1. HTH, Mark.