From patchwork Fri Nov 8 22:18:34 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Mark Cave-Ayland X-Patchwork-Id: 289932 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)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 057172C00C6 for ; Sat, 9 Nov 2013 09:31:58 +1100 (EST) Received: from localhost ([::1]:50697 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Veuao-0000U3-V7 for incoming@patchwork.ozlabs.org; Fri, 08 Nov 2013 17:31:54 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40961) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VeuWw-0006kb-MV for qemu-devel@nongnu.org; Fri, 08 Nov 2013 17:31:32 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VeuT3-0007qq-Qb for qemu-devel@nongnu.org; Fri, 08 Nov 2013 17:27:50 -0500 Received: from p15195424.pureserver.info ([82.165.34.74]:45308) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VeuPB-0006fj-I9; Fri, 08 Nov 2013 17:19:53 -0500 Received: from [149.241.34.189] (helo=[192.168.1.108]) by p15195424.pureserver.info with esmtpsa (TLSv1:AES256-SHA:256) (Exim 4.43) id 1VeuOz-0000Mo-BN; Fri, 08 Nov 2013 22:19:45 +0000 Message-ID: <527D633A.5020801@ilande.co.uk> Date: Fri, 08 Nov 2013 22:18:34 +0000 From: Mark Cave-Ayland User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:10.0.12) Gecko/20130116 Icedove/10.0.12 MIME-Version: 1.0 To: Alexander Graf References: <1381488828-22575-1-git-send-email-mark.cave-ayland@ilande.co.uk> <155E836F-E5EF-4F8F-AF5B-B8E0EC35553D@suse.de> In-Reply-To: <155E836F-E5EF-4F8F-AF5B-B8E0EC35553D@suse.de> X-SA-Exim-Connect-IP: 149.241.34.189 X-SA-Exim-Mail-From: mark.cave-ayland@ilande.co.uk X-SA-Exim-Version: 4.1 (built Wed, 05 Jan 2005 10:54:05 -0500) X-SA-Exim-Scanned: Yes (on p15195424.pureserver.info) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.4.x X-Received-From: 82.165.34.74 Cc: =?ISO-8859-1?Q?Herv=E9_Poussineau?= , qemu-ppc , QEMU Developers , =?ISO-8859-1?Q?Andreas_F=E4rber?= Subject: Re: [Qemu-devel] [PATCH] PPC: fix PCI configuration space MemoryRegions for grackle/uninorth 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 08/11/13 03:20, Alexander Graf wrote: > On 11.10.2013, at 12:53, Mark Cave-Ayland wrote: > >> OpenBIOS prior to SVN r1225 had a horrible bug when accessing PCI >> configuration space for PPC Mac architectures - instead of writing the PCI >> configuration data value to the data register address, it would instead write >> it to the data register address plus the PCI configuration address. >> >> For this reason, the MemoryRegions for the PCI data register for >> grackle/uninorth were extremely large in order to accomodate the entire PCI >> configuration space being accessed during OpenBIOS PCI bus enumeration. Now >> that the OpenBIOS images have been updated, reduce the MemoryRegion sizes down >> to a single 32-bit register as intended. >> >> Signed-off-by: Mark Cave-Ayland >> CC: Hervé Poussineau >> CC: Andreas Färber >> CC: Alexander Graf > > With this patch applied, mac99 emulation seems to break: > > http://award.ath.cx/results/288-alex/x86/kvm.qemu-git-tcg.ppc-debian.mac99-G4.etch.e1000.reboot/debug/serial-vm1.log > > > Alex Hi Alex, Thanks for the heads-up - with the information above I was able to reproduce this fairly easily. I had look at some of the uninorth drivers, and while it's not particularly apparent from Linux that the PCI configuration data is accessed via MMIO rather than ioport access, FreeBSD seems to suggest that this is the case: http://code.google.com/p/freebsd-head/source/browse/sys/powerpc/powermac/uninorth.c?spec=svnc6989e24706228678e454517dad4ad465a36e556&r=c6989e24706228678e454517dad4ad465a36e556#274. The key is that the QEMU uninorth host bridge contains a hack to allow PCI configuration mechanism #1 as used by OpenBIOS to work at all (see unin_get_config_reg() in hw/pci-host/uninorth.c) which is why I didn't notice it in my OpenBIOS boot tests; and in fact, the name of the uninorth PCI configuration data MemoryRegions have a "-data" rather than a "-idx" suffix is also a big clue. Hence please find a revised version of the patch which is unaltered for grackle, and only changes the MemoryRegion size for the PCI configuration address register for uninorth so that the PCI configuration data space is still accessible using MMIO. This resolves the issue for me, so if you're satisifed that it works for you then I'll post a revised version to the list. ATB, Mark. diff --git a/hw/pci-host/grackle.c b/hw/pci-host/grackle.c index 4991ec4..d70c519 100644 --- a/hw/pci-host/grackle.c +++ b/hw/pci-host/grackle.c @@ -105,9 +105,9 @@ static int pci_grackle_init_device(SysBusDevice *dev) phb = PCI_HOST_BRIDGE(dev); memory_region_init_io(&phb->conf_mem, OBJECT(dev), &pci_host_conf_le_ops, - dev, "pci-conf-idx", 0x1000); + dev, "pci-conf-idx", 4); memory_region_init_io(&phb->data_mem, OBJECT(dev), &pci_host_data_le_ops, - dev, "pci-data-idx", 0x1000); + dev, "pci-data-idx", 4); sysbus_init_mmio(dev, &phb->conf_mem); sysbus_init_mmio(dev, &phb->data_mem); diff --git a/hw/pci-host/uninorth.c b/hw/pci-host/uninorth.c index 91530cd..2238646 100644 --- a/hw/pci-host/uninorth.c +++ b/hw/pci-host/uninorth.c @@ -153,7 +153,7 @@ static int pci_unin_main_init_device(SysBusDevice *dev) h = PCI_HOST_BRIDGE(dev); memory_region_init_io(&h->conf_mem, OBJECT(h), &pci_host_conf_le_ops, - dev, "pci-conf-idx", 0x1000); + dev, "pci-conf-idx", 4); memory_region_init_io(&h->data_mem, OBJECT(h), &unin_data_ops, dev, "pci-conf-data", 0x1000); sysbus_init_mmio(dev, &h->conf_mem); @@ -171,7 +171,7 @@ static int pci_u3_agp_init_device(SysBusDevice *dev) h = PCI_HOST_BRIDGE(dev); memory_region_init_io(&h->conf_mem, OBJECT(h), &pci_host_conf_le_ops, - dev, "pci-conf-idx", 0x1000); + dev, "pci-conf-idx", 4); memory_region_init_io(&h->data_mem, OBJECT(h), &unin_data_ops, dev, "pci-conf-data", 0x1000); sysbus_init_mmio(dev, &h->conf_mem); @@ -188,7 +188,7 @@ static int pci_unin_agp_init_device(SysBusDevice *dev) h = PCI_HOST_BRIDGE(dev); memory_region_init_io(&h->conf_mem, OBJECT(h), &pci_host_conf_le_ops, - dev, "pci-conf-idx", 0x1000); + dev, "pci-conf-idx", 4); memory_region_init_io(&h->data_mem, OBJECT(h), &pci_host_data_le_ops, dev, "pci-conf-data", 0x1000); sysbus_init_mmio(dev, &h->conf_mem); @@ -204,7 +204,7 @@ static int pci_unin_internal_init_device(SysBusDevice *dev) h = PCI_HOST_BRIDGE(dev); memory_region_init_io(&h->conf_mem, OBJECT(h), &pci_host_conf_le_ops, - dev, "pci-conf-idx", 0x1000); + dev, "pci-conf-idx", 4); memory_region_init_io(&h->data_mem, OBJECT(h), &pci_host_data_le_ops, dev, "pci-conf-data", 0x1000); sysbus_init_mmio(dev, &h->conf_mem);