From patchwork Sun Dec 13 19:06:05 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Juan Quintela X-Patchwork-Id: 41044 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [199.232.76.165]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 91650B6F2B for ; Mon, 14 Dec 2009 06:07:43 +1100 (EST) Received: from localhost ([127.0.0.1]:35147 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NJtnA-00088h-6q for incoming@patchwork.ozlabs.org; Sun, 13 Dec 2009 14:07:40 -0500 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NJtmc-00087B-CV for qemu-devel@nongnu.org; Sun, 13 Dec 2009 14:07:06 -0500 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1NJtmW-00081e-Ou for qemu-devel@nongnu.org; Sun, 13 Dec 2009 14:07:06 -0500 Received: from [199.232.76.173] (port=59741 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NJtmW-00081T-HX for qemu-devel@nongnu.org; Sun, 13 Dec 2009 14:07:00 -0500 Received: from mx1.redhat.com ([209.132.183.28]:23340) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NJtmV-0007jP-PB for qemu-devel@nongnu.org; Sun, 13 Dec 2009 14:07:00 -0500 Received: from int-mx08.intmail.prod.int.phx2.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id nBDJ6vg3005997 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Sun, 13 Dec 2009 14:06:57 -0500 Received: from localhost (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by int-mx08.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id nBDJ6tBi027214; Sun, 13 Dec 2009 14:06:55 -0500 From: Juan Quintela To: Igor Kovalenko In-Reply-To: (Igor Kovalenko's message of "Sat, 12 Dec 2009 16:22:37 +0300") References: <4B22461602000099000327DE@collaborate.seakr.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) Date: Sun, 13 Dec 2009 20:06:05 +0100 Message-ID: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.67 on 10.5.11.21 X-detected-operating-system: by monty-python.gnu.org: Genre and OS details not recognized. Cc: Blue Swirl , qemu-devel@nongnu.org, Nick Couchman Subject: [Qemu-devel] Re: Bug in Sparc64/IDE Code X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Igor Kovalenko wrote: > On Sat, Dec 12, 2009 at 3:18 PM, Igor Kovalenko > wrote: >> On Sat, Dec 12, 2009 at 1:12 PM, Blue Swirl wrote: >>> On Fri, Dec 11, 2009 at 10:16 PM, Nick Couchman wrote: >>>> In working to try to get Sparc64 system emulation developed, we seem to have run into an issue with the IDE code in Qemu.  The OpenBIOS folks have been working quite a few issues with the OpenBIOS code that need to be resolved in order to boot 64-bit Solaris kernels correctly, but the most recent issue indicates that the IDE code for the Sparc64 emulator is reading from and writing to the wrong memory locations.  The end result is the following output when trying to boot off an ISO image in Qemu: >>> >>>> bmdma_cmd_writeb: 0x00000054 >>>> bmdma: writeb 0x701 : 0xd7 >>>> bmdma: writeb 0x702 : 0x79 >>>> bmdma: writeb 0x703 : 0xfe >>>> bmdma_addr_writew: 0x0000ddef >>>> bmdma_addr_writew: 0x0000b12b >>>> bmdma_cmd_writeb: 0x000000da >>>> bmdma: writeb 0x709 : 0x95 >>>> Segmentation fault >>> >>> I can't reproduce this with milaX 0.3.1, QEMU git HEAD and OpenBIOS >>> svn r644. The bug could be that the BMDMA address may need BE to LE >>> conversion, or OpenBIOS could just clobber BMDMA registers with >>> garbage (the DMA address candidates 0xddefb12b and 0xb12bddef do not >>> look valid). >>> >>> Another possibility is that the PCI host bridge should have an IOMMU >>> which is not implemented yet, but I doubt we are at that stage. >>> >>> Could you run QEMU in a GDB session and send the backtrace from the segfault? >>> >> >> There seems to be an issue with pci_from_bm cast: bm->unit is not >> assigned anywhere >> in the code so it is zero for second unit, and pci_from_bm returns >> wrong address. >> Crash happens writing to address mapped for second unit. > > This appears to be a regression in cmd646. After removal of pci_dev from > BMDMAState structure we cannot do much to work around this issue. > > The problem here is that we cannot rely on bm->unit value since it is getting > changed while dma operations are in progress, f.e. it is set to -1 on > dma cancel. > Thus we cannot get to pci_dev from BMDMAState passed to i/o read/write > callbacks. > > Juan, can you please take a look at this issue? I don't have a sparc setup, but could you try this patch? It also fixes the test for bm. From 6d2dfb38804fdf869b59f75768c8376951f81bb5 Mon Sep 17 00:00:00 2001 From: Juan Quintela Date: Sun, 13 Dec 2009 20:03:30 +0100 Subject: [PATCH] cmd646: pass pci_dev as it needs it Instead of doing tricks to get the pci_dev, just pass it in the 1st place. Patch is a bit longer that reverting the pci_dev field, but it states more clearly (IMHO) what we are doing. It also fixes the bm test, now that you told me that ->unit is not always valid. Could you test and tell me the result? Thanks in advance. Later, Juan. Signed-off-by: Juan Quintela --- hw/ide/cmd646.c | 64 ++++++++++++++++++++++++++++++++++++++---------------- 1 files changed, 45 insertions(+), 19 deletions(-) diff --git a/hw/ide/cmd646.c b/hw/ide/cmd646.c index 3b7c606..f5edc14 100644 --- a/hw/ide/cmd646.c +++ b/hw/ide/cmd646.c @@ -68,19 +68,9 @@ static void ide_map(PCIDevice *pci_dev, int region_num, } } -static PCIIDEState *pci_from_bm(BMDMAState *bm) +static uint32_t bmdma_readb_common(PCIIDEState *pci_dev, BMDMAState *bm, + uint32_t addr) { - if (bm->unit == 0) { - return container_of(bm, PCIIDEState, bmdma[0]); - } else { - return container_of(bm, PCIIDEState, bmdma[1]); - } -} - -static uint32_t bmdma_readb(void *opaque, uint32_t addr) -{ - BMDMAState *bm = opaque; - PCIIDEState *pci_dev = pci_from_bm(bm); uint32_t val; switch(addr & 3) { @@ -94,7 +84,7 @@ static uint32_t bmdma_readb(void *opaque, uint32_t addr) val = bm->status; break; case 3: - if (bm->unit == 0) { + if (bm == &pci_dev->bmdma[0]) { val = pci_dev->dev.config[UDIDETCR0]; } else { val = pci_dev->dev.config[UDIDETCR1]; @@ -110,10 +100,25 @@ static uint32_t bmdma_readb(void *opaque, uint32_t addr) return val; } -static void bmdma_writeb(void *opaque, uint32_t addr, uint32_t val) +static uint32_t bmdma_readb_0(void *opaque, uint32_t addr) +{ + PCIIDEState *pci_dev = opaque; + BMDMAState *bm = &pci_dev->bmdma[0]; + + return bmdma_readb_common(pci_dev, bm, addr); +} + +static uint32_t bmdma_readb_1(void *opaque, uint32_t addr) +{ + PCIIDEState *pci_dev = opaque; + BMDMAState *bm = &pci_dev->bmdma[1]; + + return bmdma_readb_common(pci_dev, bm, addr); +} + +static void bmdma_writeb_common(PCIIDEState *pci_dev, BMDMAState *bm, + uint32_t addr, uint32_t val) { - BMDMAState *bm = opaque; - PCIIDEState *pci_dev = pci_from_bm(bm); #ifdef DEBUG_IDE printf("bmdma: writeb 0x%02x : 0x%02x\n", addr, val); #endif @@ -127,7 +132,7 @@ static void bmdma_writeb(void *opaque, uint32_t addr, uint32_t val) bm->status = (val & 0x60) | (bm->status & 1) | (bm->status & ~val & 0x06); break; case 3: - if (bm->unit == 0) + if (bm == &pci_dev->bmdma[0]) pci_dev->dev.config[UDIDETCR0] = val; else pci_dev->dev.config[UDIDETCR1] = val; @@ -135,6 +140,22 @@ static void bmdma_writeb(void *opaque, uint32_t addr, uint32_t val) } } +static void bmdma_writeb_0(void *opaque, uint32_t addr, uint32_t val) +{ + PCIIDEState *pci_dev = opaque; + BMDMAState *bm = &pci_dev->bmdma[0]; + + bmdma_writeb_common(pci_dev, bm, addr, val); +} + +static void bmdma_writeb_1(void *opaque, uint32_t addr, uint32_t val) +{ + PCIIDEState *pci_dev = opaque; + BMDMAState *bm = &pci_dev->bmdma[1]; + + bmdma_writeb_common(pci_dev, bm, addr, val); +} + static void bmdma_map(PCIDevice *pci_dev, int region_num, pcibus_t addr, pcibus_t size, int type) { @@ -149,8 +170,13 @@ static void bmdma_map(PCIDevice *pci_dev, int region_num, register_ioport_write(addr, 1, 1, bmdma_cmd_writeb, bm); - register_ioport_write(addr + 1, 3, 1, bmdma_writeb, bm); - register_ioport_read(addr, 4, 1, bmdma_readb, bm); + if (i == 0) { + register_ioport_write(addr + 1, 3, 1, bmdma_writeb_0, d); + register_ioport_read(addr, 4, 1, bmdma_readb_0, d); + } else { + register_ioport_write(addr + 1, 3, 1, bmdma_writeb_1, d); + register_ioport_read(addr, 4, 1, bmdma_readb_1, d); + } register_ioport_write(addr + 4, 4, 1, bmdma_addr_writeb, bm); register_ioport_read(addr + 4, 4, 1, bmdma_addr_readb, bm);