From patchwork Sat May 10 12:30:56 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: BALATON Zoltan X-Patchwork-Id: 347667 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 24A1E1400A0 for ; Sat, 10 May 2014 22:31:31 +1000 (EST) Received: from localhost ([::1]:57494 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wj6R7-0003Ny-Fc for incoming@patchwork.ozlabs.org; Sat, 10 May 2014 08:31:29 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40638) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wj6Qq-00036S-0c for qemu-devel@nongnu.org; Sat, 10 May 2014 08:31:16 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wj6Ql-0000qM-HM for qemu-devel@nongnu.org; Sat, 10 May 2014 08:31:11 -0400 Received: from jedlik.phy.bme.hu ([152.66.102.83]:56804) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wj6Ql-0000q2-6A; Sat, 10 May 2014 08:31:07 -0400 Received: by jedlik.phy.bme.hu (Postfix, from userid 501) id F354760091; Sat, 10 May 2014 14:30:56 +0200 (CEST) Date: Sat, 10 May 2014 14:30:56 +0200 (CEST) From: BALATON Zoltan X-X-Sender: balaton@jedlik.phy.bme.hu To: Mark Cave-Ayland In-Reply-To: <536A6F3C.1030708@ilande.co.uk> Message-ID: References: <5366CA94.3030803@ilande.co.uk> <536A328F.6070100@ilande.co.uk> <536A6F3C.1030708@ilande.co.uk> User-Agent: Alpine 2.02 (LMD 1266 2009-07-14) MIME-Version: 1.0 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 152.66.102.83 Cc: qemu-ppc@nongnu.org, Alexander Graf , qemu-devel@nongnu.org Subject: Re: [Qemu-devel] [Qemu-ppc] macio ide question/bug report 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 On Wed, 7 May 2014, Mark Cave-Ayland wrote: > On 07/05/14 18:00, BALATON Zoltan wrote: >> On Wed, 7 May 2014, Mark Cave-Ayland wrote: >>> I'm not sure if your problem related to s->lba == -1 should be solved >>> just for macio or higher up in the block layer, but the block people >>> will be on qemu-devel and not qemu-ppc so you should definitely CC the >>> main list to get their feedback on this. Cc'd the devel list now, the original question can be found here: http://lists.nongnu.org/archive/html/qemu-ppc/2014-05/msg00003.html >> I think it's handled by the block layer but the translation in macio >> breaks it and converts it to -4 which causes an error so probably it >> should only be solved for macio. But I hope Alex can look at it and tell >> for sure or maybe solve it. > > Okay. So in that case it could just be a macio bug. > >>> Those ISOs only boot for me with the default -M g3beige, but darwin is >>> a huge culprit for these unaligned accesses. My point was that if you >>> are making changes in this area, if you can still boot the darwin >>> images then that would be a good test that any changes you make are >>> working correctly :) >> >> Ah, OK. They boot for me without a -M parameter with my changes (to the >> point saying "available for installation:" and then empty as I did not >> specify a hard disk image) too, but I was testing with -M mac99 as that >> was what I changed. I think they still "work" as before. > > But... the block patchset link I sent you has the potential to fix this > anyway, since if the unaligned accesses can be passed directly up to the > block layer then in theory this problem should go away as this whole chunk of > code should no longer be required. > > A good test for this would be to try reverting this patch: > https://github.com/agraf/qemu/commit/063a333911f1bb09fcc1a4925c8ebf5c88fc2b63 > which is Alex's original alignment fix. If you find with that with this patch > reverted darwin boots as before, it means that the block alignment patches in > core are working correctly and hopefully you may find that it fixes your > MorphOS problem. > > But again, you really should get this on qemu-devel to get the definitive > answer from the block guys. 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 pmac_ide_transfer(ATAPI) lba=0, buffer_index=0, len=800 pmac_ide_transfer(ATAPI) lba=1, buffer_index=0, len=800 pmac_ide_transfer(ATAPI) lba=1, buffer_index=0, len=800 pmac_ide_transfer(ATAPI) lba=2, buffer_index=0, len=800 pmac_ide_transfer(ATAPI) lba=2, buffer_index=0, len=800 pmac_ide_transfer(ATAPI) lba=0, buffer_index=0, len=2000 pmac_ide_transfer(ATAPI) lba=f2, buffer_index=0, len=800 pmac_ide_transfer(ATAPI) lba=f3, buffer_index=0, len=800 pmac_ide_transfer(ATAPI) lba=1ac, buffer_index=0, len=800 pmac_ide_transfer(ATAPI) lba=9ae, buffer_index=0, len=800 pmac_ide_transfer(ATAPI) lba=9af, buffer_index=0, len=400 pmac_ide_transfer(ATAPI) lba=9ae, buffer_index=0, len=400 pmac_ide_transfer(ATAPI) lba=9ae, buffer_index=400, len=400 pmac_ide_transfer(ATAPI) lba=9af, buffer_index=0, len=800 pmac_ide_transfer(ATAPI) lba=11ae, buffer_index=0, len=800 pmac_ide_transfer(ATAPI) lba=11af, buffer_index=0, len=1000 [...] pmac_ide_transfer(ATAPI) lba=3858e, buffer_index=0, len=2000 pmac_ide_transfer(ATAPI) lba=3856e, buffer_index=0, len=2000 pmac_ide_transfer(ATAPI) lba=38572, buffer_index=0, len=4000 pmac_ide_transfer(ATAPI) lba=38590, buffer_index=0, len=4000 pmac_ide_transfer(ATAPI) lba=38584, buffer_index=0, len=1000 pmac_ide_transfer(ATAPI) lba=38586, buffer_index=0, len=3000 pmac_ide_transfer(ATAPI) lba=3857e, buffer_index=0, len=3000 pmac_ide_transfer(ATAPI) lba=38566, buffer_index=0, len=4000 pmac_ide_transfer(ATAPI) lba=38550, buffer_index=0, len=1000 pmac_ide_transfer(ATAPI) lba=38552, buffer_index=0, len=4000 pmac_ide_transfer(ATAPI) lba=38536, buffer_index=0, len=4000 pmac_ide_transfer(ATAPI) lba=3852c, buffer_index=0, len=5000 pmac_ide_transfer(ATAPI) lba=385a6, buffer_index=0, len=4000 pmac_ide_transfer(ATAPI) lba=a724, buffer_index=0, len=2000 pmac_ide_transfer(ATAPI) lba=3854a, buffer_index=0, len=3000 pmac_ide_transfer(ATAPI) lba=1654, buffer_index=0, len=1000 pmac_ide_transfer(ATAPI) lba=3b1cc, buffer_index=0, len=8000 pmac_ide_transfer(ATAPI) lba=3b3a8, buffer_index=0, len=8000 pmac_ide_transfer(ATAPI) lba=3b3a0, buffer_index=0, len=4000 pmac_ide_transfer(ATAPI) lba=3b3a8, buffer_index=0, len=2000 pmac_ide_transfer(ATAPI) lba=38546, buffer_index=0, len=2000 pmac_ide_transfer(ATAPI) lba=3854a, buffer_index=0, len=8000 pmac_ide_transfer(ATAPI) lba=3b3d8, buffer_index=0, len=8000 pmac_ide_transfer(ATAPI) lba=3b3e8, buffer_index=0, len=7000 pmac_ide_transfer(ATAPI) lba=3b3ca, buffer_index=0, len=7000 pmac_ide_transfer(ATAPI) lba=3b3d8, buffer_index=0, len=1000 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: 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 If these make sense to anyone and can tell what could be the problem please share your thoughts. Regards, BALATON Zoltan 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) {