From patchwork Thu May 15 19:22:34 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mark Cave-Ayland X-Patchwork-Id: 349344 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 15C9F140077 for ; Fri, 16 May 2014 05:26:08 +1000 (EST) Received: from localhost ([::1]:60154 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wl1I5-0007Tz-Pz for incoming@patchwork.ozlabs.org; Thu, 15 May 2014 15:26:05 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54171) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wl1HS-0006vI-4U for qemu-devel@nongnu.org; Thu, 15 May 2014 15:25:33 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wl1HK-0004FC-If for qemu-devel@nongnu.org; Thu, 15 May 2014 15:25:26 -0400 Received: from s16892447.onlinehome-server.info ([82.165.15.123]:45026) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wl1H5-0003f4-8B; Thu, 15 May 2014 15:25:03 -0400 Received: from 4e5674b0.skybroadband.com ([78.86.116.176] helo=[192.168.1.78]) by s16892447.onlinehome-server.info with esmtpsa (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.76) (envelope-from ) id 1Wl1Gx-0007xs-5A; Thu, 15 May 2014 20:24:56 +0100 Message-ID: <537513FA.6060705@ilande.co.uk> Date: Thu, 15 May 2014 20:22:34 +0100 From: Mark Cave-Ayland User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Icedove/24.5.0 MIME-Version: 1.0 To: BALATON Zoltan , Alexander Graf 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> In-Reply-To: X-SA-Exim-Connect-IP: 78.86.116.176 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 Cc: qemu-ppc@nongnu.org, 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 15/05/14 18:28, BALATON Zoltan wrote: >> If you place a breakpoint on pmac_ide_transfer() then its invocation >> of pmac_ide_atapi_transfer_cb() should be the first iteration which >> sets up the DMA request, and the second invocation should be the >> (failing) callback from the dma_bdrv_*() functions. But as I mentioned >> before, I think the problem is that these functions shouldn't be >> called in the first place when requesting a TOC. > > OK, I've done that and stopped at the first invocation of > pmac_ide_atapi_transfer_cb. Here is a backtrace and the contents of some > data structures: > > #0 pmac_ide_atapi_transfer_cb (opaque=0x5555563ccc68, ret=0) > at hw/ide/macio.c:55 > #1 0x00005555556da6d0 in pmac_ide_transfer (io=0x5555563ccc68) > at hw/ide/macio.c:225 > #2 0x00005555556eeee2 in start_input (ch=0x5555563ccc18, key=0, addr= > 14777932, req_count=804, is_last=1) at hw/misc/macio/mac_dbdma.c:334 > #3 0x00005555556ef444 in channel_run (ch=0x5555563ccc18) > at hw/misc/macio/mac_dbdma.c:489 > #4 0x00005555556ef599 in DBDMA_run (s=0x5555563cba40) > at hw/misc/macio/mac_dbdma.c:531 > #5 0x00005555556ef5f4 in DBDMA_run_bh (opaque=0x5555563cba40) > at hw/misc/macio/mac_dbdma.c:542 > #6 0x00005555555f8200 in aio_bh_poll (ctx=0x55555637fc00) at async.c:81 > #7 0x00005555555f7e59 in aio_poll (ctx=0x55555637fc00, blocking=false) > at aio-posix.c:188 > #8 0x00005555555f8617 in aio_ctx_dispatch (source=0x55555637fc00, > callback= > 0x0, user_data=0x0) at async.c:205 > #9 0x00007ffff78ca6d5 in g_main_context_dispatch () > from /lib64/libglib-2.0.so.0 > #10 0x00005555557a0fde in glib_pollfds_poll () at main-loop.c:190 > #11 0x00005555557a10de in os_host_main_loop_wait (timeout=15883632) > at main-loop.c:235 > #12 0x00005555557a11b1 in main_loop_wait (nonblocking=0) at main-loop.c:484 > #13 0x000055555584422c in main_loop () at vl.c:2075 > #14 0x000055555584bcbf in main (argc=32, argv=0x7fffffffdc48, envp= > 0x7fffffffdd50) at vl.c:4556 (lots cut) So that looks like the correct request based upon it's size so what happened when you stepped into pmac_ide_atapi_transfer_cb()...? > My testing was done with commit 80fc95d8bdaf3392106b131a97ca701fd374489a > already reverted as that was established before that it is not needed > any more which simplifies pmac_ide_atapi_transfer_cb() quite a bit Sadly I've now found that this isn't the case (email to the qemu-devel list to follow, but I've run out of time today) :( > but I > still can't understand the flow of this function and don't see where > should I add a condition to handle this lba=-1 case that happens with > READ_TOC using DMA. The reason I don't understand it is that the > different fields (sizes and indexes) in these structures that are used > in this callback don't make sense to me and I don't know how to use > cpu_physical_memory_write() to copy data from the ide buffer to the > guest memory as was suggested by Mark. Now that the problem is fairly > well understood, wouldn't it be easier to one of you who understands > what's happening to create a patch, instead of trying to explain all > this to me? Well my understanding is that it's impossible to boot a MorphOS image directly and requires all sorts of tricks with debuggers etc. Unless you can provide a "run this and it breaks" test case, then the time barrier for trying to fix bugs like this is exceptionally high. You mention that you don't understand the fields and sizes, so explain which ones you don't understand and ask. Search around for guides to how ATAPI/IDE works, and compare with gdb output to find the correlation with the IDEState variables. Have you tried looking at the header file for cpu_physical_memory_write()? Even to someone who had never seen this function before Alex's patches, it seems fairly obvious how the API should work. I'm afraid that there really is no alternative to spending the time getting stuck into the code, experimenting, adding printf()'s in exciting places, and then asking specific questions to the mailing list. > This part was last touched by Alexander Graf so I assume he knows how it > works and it would not be too hard for him to fix it. I'm happy to help > testing and providing more debugging info as needed but I'm not sure I > want to detangle it and figure out the whole block layer and DBDMA > without any documentation to be able to fix it myself. Would it be > possible to find some time for it in the foreseeable future? (That might > still be sooner than me wrapping my head around it.) Please see above. FWIW based upon the your gdb output I've put together the following patch which compiles, but that's all I can vouch for it. Further testing and debugging will have to be done by you, although I will try and respond to specific questions where possible. ATB, Mark. diff --git a/hw/ide/macio.c b/hw/ide/macio.c index da94580..96c2556 100644 --- a/hw/ide/macio.c +++ b/hw/ide/macio.c @@ -111,6 +111,14 @@ static void pmac_ide_atapi_transfer_cb(void *opaque, int ret) return; } + if (s->packet_transfer_size && s->lba == -1) { + MACIO_DPRINTF("non-IO ATAPI DMA transfer size: %d\n", io->len); + /* Copy ATAPI buffer directly to RAM and finish */ + cpu_physical_memory_write(io->addr, s->io_buffer, io->len); + s->packet_transfer_size -= io->len; + MACIO_DPRINTF("end of non-IO ATAPI DMA transfer\n"); + } + if (!s->packet_transfer_size) { MACIO_DPRINTF("end of transfer\n"); ide_atapi_cmd_ok(s);