From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:56718) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S4DGS-0003Op-Rh for qemu-devel@nongnu.org; Sun, 04 Mar 2012 10:22:44 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S4DG5-0001Ua-Sz for qemu-devel@nongnu.org; Sun, 04 Mar 2012 10:22:24 -0500 Received: from mx1.redhat.com ([209.132.183.28]:49057) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S4DG5-0001Ty-Ka for qemu-devel@nongnu.org; Sun, 04 Mar 2012 10:22:01 -0500 Date: Sun, 4 Mar 2012 17:22:01 +0200 From: "Michael S. Tsirkin" Message-ID: <20120304152201.GC12776@redhat.com> References: <20120304094614.GA8158@redhat.com> <20120304122100.GA11207@redhat.com> <20120304132805.GC12047@redhat.com> <20120304142333.GA12900@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] pci: fix bridge IO/BASE List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: Mark Cave-Ayland , qemu-devel@nongnu.org, Anthony Liguori On Sun, Mar 04, 2012 at 02:35:28PM +0000, Blue Swirl wrote: > On Sun, Mar 4, 2012 at 14:23, Michael S. Tsirkin wrote= : > > On Sun, Mar 04, 2012 at 01:38:38PM +0000, Blue Swirl wrote: > >> On Sun, Mar 4, 2012 at 13:28, Michael S. Tsirkin wr= ote: > >> > On Sun, Mar 04, 2012 at 12:37:57PM +0000, Blue Swirl wrote: > >> >> On Sun, Mar 4, 2012 at 12:21, Michael S. Tsirkin = wrote: > >> >> > On Sun, Mar 04, 2012 at 10:27:24AM +0000, Blue Swirl wrote: > >> >> >> On Sun, Mar 4, 2012 at 09:46, Michael S. Tsirkin wrote: > >> >> >> > commit 5caef97a16010f818ea8b950e2ee24ba876643ad introduced > >> >> >> > a regression: we do not make IO base/limit upper 16 > >> >> >> > bit registers writeable, so we should report a 16 bit > >> >> >> > IO range type, not a 32 bit one. > >> >> >> > Note that PCI_PREF_RANGE_TYPE_32 is 0x0, but PCI_IO_RANGE_TY= PE_32 is 0x1. > >> >> >> > > >> >> >> > In particular, this broke sparc64. > >> >> >> > > >> >> >> > Note: this just reverts to behaviour prior to the patch. > >> >> >> > Making PCI_IO_BASE_UPPER16 and PCI_IO_LIMIT_UPPER16 > >> >> >> > registers writeable should, and seems to, work just as well,= but > >> >> >> > as no system seems to actually be interested in 32 bit IO, > >> >> >> > let's not make unnecessary changes. > >> >> >> > > >> >> >> > Reported-by: Mark Cave-Ayland > >> >> >> > Signed-off-by: Michael S. Tsirkin > >> >> >> > > >> >> >> > Mark, can you confirm that this fixes the bug for you? > >> >> >> > >> >> >> No, running > >> >> >> qemu-system-sparc64 -serial stdio > >> >> >> still shows black screen and the following on console: > >> >> >> OpenBIOS for Sparc64 > >> >> >> Unhandled Exception 0x0000000000000032 > >> >> >> PC =3D 0x00000000ffd19e18 NPC =3D 0x00000000ffd19e1c > >> >> >> Stopping execution > >> >> > > >> >> > The weird thing is the range type does not seem to be accessed > >> >> > at all. So I guessed there's some memory corruption here. > >> >> > Running valgrind shows this: > >> >> > > >> >> > --11114-- WARNING: unhandled syscall: 340 > >> >> > --11114-- You may be able to write your own handler. > >> >> > --11114-- Read the file README_MISSING_SYSCALL_OR_IOCTL. > >> >> > --11114-- Nevertheless we consider this a bug. =C2=A0Please rep= ort > >> >> > --11114-- it at http://valgrind.org/support/bug_reports.html. > >> >> > =3D=3D11114=3D=3D Invalid read of size 4 > >> >> > =3D=3D11114=3D=3D =C2=A0 =C2=A0at 0x2A68C0: pci_apb_init (apb_p= ci.c:350) > >> >> > =3D=3D11114=3D=3D =C2=A0 =C2=A0by 0x2F7A84: sun4uv_init (sun4u.= c:779) > >> >> > =3D=3D11114=3D=3D =C2=A0 =C2=A0by 0x13D716: main (vl.c:3397) > >> >> > =3D=3D11114=3D=3D =C2=A0Address 0x156c7d30 is 0 bytes after a b= lock of size 64 > >> >> > alloc'd > >> >> > =3D=3D11114=3D=3D =C2=A0 =C2=A0at 0x557DD69: malloc (vg_replace= _malloc.c:236) > >> >> > =3D=3D11114=3D=3D =C2=A0 =C2=A0by 0x225F56: malloc_and_trace (v= l.c:2156) > >> >> > =3D=3D11114=3D=3D =C2=A0 =C2=A0by 0x584AFEC: ??? (in /lib/libgl= ib-2.0.so.0.2800.8) > >> >> > =3D=3D11114=3D=3D =C2=A0 =C2=A0by 0x584B528: g_malloc0 (in /lib= /libglib-2.0.so.0.2800.8) > >> >> > =3D=3D11114=3D=3D =C2=A0 =C2=A0by 0x19C50C: qemu_allocate_irqs = (irq.c:47) > >> >> > =3D=3D11114=3D=3D =C2=A0 =C2=A0by 0x2F7A4C: sun4uv_init (sun4u.= c:778) > >> >> > =3D=3D11114=3D=3D =C2=A0 =C2=A0by 0x13D716: main (vl.c:3397) > >> >> > =3D=3D11114=3D=3D > >> >> > apb: here > >> >> > =3D=3D11114=3D=3D Warning: client switching stacks? =C2=A0SP ch= ange: 0xfec42cbc --> > >> >> > 0x16894008 > >> >> > =3D=3D11114=3D=3D =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0to suppress= , use: --max-stackframe=3D398791500 or > >> >> > greater > >> >> > =3D=3D11114=3D=3D Warning: client switching stacks? =C2=A0SP ch= ange: 0x16893fa0 --> > >> >> > 0xfec42cc0 > >> >> > =3D=3D11114=3D=3D =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0to suppress= , use: --max-stackframe=3D398791392 or > >> >> > greater > >> >> > =3D=3D11114=3D=3D Warning: client switching stacks? =C2=A0SP ch= ange: 0xfec42fe0 --> > >> >> > 0x16893fd0 > >> >> > =3D=3D11114=3D=3D =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0to suppress= , use: --max-stackframe=3D398790640 or > >> >> > greater > >> >> > =3D=3D11114=3D=3D =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0further ins= tances of this message will not be shown. > >> >> > QEMU 1.0.50 monitor - type 'help' for more information > >> >> > (qemu) =3D=3D11114=3D=3D Thread 2: > >> >> > =3D=3D11114=3D=3D Conditional jump or move depends on uninitial= ised value(s) > >> >> > =3D=3D11114=3D=3D =C2=A0 =C2=A0at 0x2A8351: compute_all_sub (cc= _helper.c:37) > >> >> > =3D=3D11114=3D=3D =C2=A0 =C2=A0by 0x2A8782: helper_compute_psr = (cc_helper.c:470) > >> >> > =3D=3D11114=3D=3D =C2=A0 =C2=A0by 0x9AD9A19: ??? > >> >> > =3D=3D11114=3D=3D > >> >> > =3D=3D11114=3D=3D Conditional jump or move depends on uninitial= ised value(s) > >> >> > =3D=3D11114=3D=3D =C2=A0 =C2=A0at 0x2A827C: compute_all_sub_xcc= (cc_helper.c:60) > >> >> > =3D=3D11114=3D=3D =C2=A0 =C2=A0by 0x2A8795: helper_compute_psr = (cc_helper.c:473) > >> >> > =3D=3D11114=3D=3D =C2=A0 =C2=A0by 0x9AD9A19: ??? > >> >> > =3D=3D11114=3D=3D > >> >> > =3D=3D11114=3D=3D Conditional jump or move depends on uninitial= ised value(s) > >> >> > =3D=3D11114=3D=3D =C2=A0 =C2=A0at 0x2A8296: compute_all_sub_xcc= (cc_helper.c:295) > >> >> > =3D=3D11114=3D=3D =C2=A0 =C2=A0by 0x2A8795: helper_compute_psr = (cc_helper.c:473) > >> >> > =3D=3D11114=3D=3D =C2=A0 =C2=A0by 0x9AD9A19: ??? > >> >> > =3D=3D11114=3D=3D > >> >> > > >> >> > Is the above a problem? > >> >> > >> >> It looks like Sparc does not reset registers at CPU reset. Nice c= atch. > >> > > >> > Invalid read and address after block are also worrying. > >> > > >> > irqs are allocated with > >> > =C2=A0#define MAX_PILS 16 > >> > > >> > =C2=A0 =C2=A0irq =3D qemu_allocate_irqs(cpu_set_irq, env, MAX_PILS= ); > >> > > >> > then passed to apb: > >> > > >> > =C2=A0 =C2=A0pci_bus =3D pci_apb_init(APB_SPECIAL_BASE, APB_MEM_BA= SE, irq, &pci_bus2, > >> > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 &pci_bus3); > >> > > >> > which does: > >> > PCIBus *pci_apb_init(target_phys_addr_t special_base, > >> > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 target_phys_addr_t mem_base, > >> > =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 qemu_irq *pic, PCIBus **bus2, PCIBus **bus3) > >> > > >> > and > >> > > >> > =C2=A0 for (i =3D 0; i < 32; i++) { > >> > =C2=A0 =C2=A0 =C2=A0 =C2=A0sysbus_connect_irq(s, i, pic[i]); > >> > =C2=A0 =C2=A0} > >> > >> Awful. But using 32 for MAX_PILS does not help either. > > > > > > Could you please clarify what is the SABRE device? > > Is it, in fact, a bridge device? Or not? >=20 > Yes, it's the host bridge, also known as PBM. It's documented in > UltraSPARC IIi User's Manual Btw would be nice to host the manuals at qemu.org our code points at sun.com URLs :( I am looking at 19.3.1 PCI Con=EF=AC=81guration Space and it appears to show that this is a regular device with a couple of custom registers at pffsets 0x40 and 0x41. Why do we want to pretend it is a bridge? > and there it says that the device is > found in the configuration space. >=20 > The secondary bridges are Simbas and should be called APBs. As far as I can see from the code, it has header type NORMAL but sets is_bridge. This was done by this commit: 776e1bbb6cf4fe66a93c1a5dd814bbb650deca00 > > > > > > -- > > MST