From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:40902) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S4BTu-0005G9-5p for qemu-devel@nongnu.org; Sun, 04 Mar 2012 08:28:12 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1S4BTr-00076l-42 for qemu-devel@nongnu.org; Sun, 04 Mar 2012 08:28:09 -0500 Received: from mx1.redhat.com ([209.132.183.28]:27837) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1S4BTq-00076f-PQ for qemu-devel@nongnu.org; Sun, 04 Mar 2012 08:28:07 -0500 Date: Sun, 4 Mar 2012 15:28:05 +0200 From: "Michael S. Tsirkin" Message-ID: <20120304132805.GC12047@redhat.com> References: <20120304094614.GA8158@redhat.com> <20120304122100.GA11207@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 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 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 wr= ote: > >> > 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_TYPE_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. =A0Please report > > --11114-- it at http://valgrind.org/support/bug_reports.html. > > =3D=3D11114=3D=3D Invalid read of size 4 > > =3D=3D11114=3D=3D =A0 =A0at 0x2A68C0: pci_apb_init (apb_pci.c:350) > > =3D=3D11114=3D=3D =A0 =A0by 0x2F7A84: sun4uv_init (sun4u.c:779) > > =3D=3D11114=3D=3D =A0 =A0by 0x13D716: main (vl.c:3397) > > =3D=3D11114=3D=3D =A0Address 0x156c7d30 is 0 bytes after a block of s= ize 64 > > alloc'd > > =3D=3D11114=3D=3D =A0 =A0at 0x557DD69: malloc (vg_replace_malloc.c:23= 6) > > =3D=3D11114=3D=3D =A0 =A0by 0x225F56: malloc_and_trace (vl.c:2156) > > =3D=3D11114=3D=3D =A0 =A0by 0x584AFEC: ??? (in /lib/libglib-2.0.so.0.= 2800.8) > > =3D=3D11114=3D=3D =A0 =A0by 0x584B528: g_malloc0 (in /lib/libglib-2.0= .so.0.2800.8) > > =3D=3D11114=3D=3D =A0 =A0by 0x19C50C: qemu_allocate_irqs (irq.c:47) > > =3D=3D11114=3D=3D =A0 =A0by 0x2F7A4C: sun4uv_init (sun4u.c:778) > > =3D=3D11114=3D=3D =A0 =A0by 0x13D716: main (vl.c:3397) > > =3D=3D11114=3D=3D > > apb: here > > =3D=3D11114=3D=3D Warning: client switching stacks? =A0SP change: 0xf= ec42cbc --> > > 0x16894008 > > =3D=3D11114=3D=3D =A0 =A0 =A0 =A0 =A0to suppress, use: --max-stackfra= me=3D398791500 or > > greater > > =3D=3D11114=3D=3D Warning: client switching stacks? =A0SP change: 0x1= 6893fa0 --> > > 0xfec42cc0 > > =3D=3D11114=3D=3D =A0 =A0 =A0 =A0 =A0to suppress, use: --max-stackfra= me=3D398791392 or > > greater > > =3D=3D11114=3D=3D Warning: client switching stacks? =A0SP change: 0xf= ec42fe0 --> > > 0x16893fd0 > > =3D=3D11114=3D=3D =A0 =A0 =A0 =A0 =A0to suppress, use: --max-stackfra= me=3D398790640 or > > greater > > =3D=3D11114=3D=3D =A0 =A0 =A0 =A0 =A0further instances of this messag= e 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 uninitialised v= alue(s) > > =3D=3D11114=3D=3D =A0 =A0at 0x2A8351: compute_all_sub (cc_helper.c:37= ) > > =3D=3D11114=3D=3D =A0 =A0by 0x2A8782: helper_compute_psr (cc_helper.c= :470) > > =3D=3D11114=3D=3D =A0 =A0by 0x9AD9A19: ??? > > =3D=3D11114=3D=3D > > =3D=3D11114=3D=3D Conditional jump or move depends on uninitialised v= alue(s) > > =3D=3D11114=3D=3D =A0 =A0at 0x2A827C: compute_all_sub_xcc (cc_helper.= c:60) > > =3D=3D11114=3D=3D =A0 =A0by 0x2A8795: helper_compute_psr (cc_helper.c= :473) > > =3D=3D11114=3D=3D =A0 =A0by 0x9AD9A19: ??? > > =3D=3D11114=3D=3D > > =3D=3D11114=3D=3D Conditional jump or move depends on uninitialised v= alue(s) > > =3D=3D11114=3D=3D =A0 =A0at 0x2A8296: compute_all_sub_xcc (cc_helper.= c:295) > > =3D=3D11114=3D=3D =A0 =A0by 0x2A8795: helper_compute_psr (cc_helper.c= :473) > > =3D=3D11114=3D=3D =A0 =A0by 0x9AD9A19: ??? > > =3D=3D11114=3D=3D > > > > Is the above a problem? >=20 > It looks like Sparc does not reset registers at CPU reset. Nice catch. Invalid read and address after block are also worrying. irqs are allocated with #define MAX_PILS 16 irq =3D qemu_allocate_irqs(cpu_set_irq, env, MAX_PILS); then passed to apb: pci_bus =3D pci_apb_init(APB_SPECIAL_BASE, APB_MEM_BASE, irq, &pci_bu= s2, &pci_bus3); which does: PCIBus *pci_apb_init(target_phys_addr_t special_base, target_phys_addr_t mem_base, qemu_irq *pic, PCIBus **bus2, PCIBus **bus3) and for (i =3D 0; i < 32; i++) { sysbus_connect_irq(s, i, pic[i]); } > >> This unassigned memory exception is triggered because CMD646 IDE I/O > >> registers are not accessible: > >> > >> (qemu) info pci > >> =A0 Bus =A00, device =A0 0, function 0: > >> =A0 =A0 Host bridge: PCI device 108e:a000 > >> =A0 =A0 =A0 id "" > >> =A0 Bus =A00, device =A0 1, function 0: > >> =A0 =A0 PCI bridge: PCI device 108e:5000 > >> =A0 =A0 =A0 BUS 0. > >> =A0 =A0 =A0 secondary bus 1. > >> =A0 =A0 =A0 subordinate bus 1. > >> =A0 =A0 =A0 IO range [0x0000, 0x0fff] > >> =A0 =A0 =A0 memory range [0x00000000, 0x000fffff] > >> =A0 =A0 =A0 prefetchable memory range [0x00000000, 0x000fffff] > >> =A0 =A0 =A0 id "" > >> =A0 Bus =A00, device =A0 1, function 1: > >> =A0 =A0 PCI bridge: PCI device 108e:5000 > >> =A0 =A0 =A0 BUS 0. > >> =A0 =A0 =A0 secondary bus 2. > >> =A0 =A0 =A0 subordinate bus 2. > >> =A0 =A0 =A0 IO range [0x0000, 0x0fff] > >> =A0 =A0 =A0 memory range [0x00000000, 0x000fffff] > >> =A0 =A0 =A0 prefetchable memory range [0x00000000, 0x000fffff] > >> =A0 =A0 =A0 id "" > >> =A0 Bus =A00, device =A0 2, function 0: > >> =A0 =A0 VGA controller: PCI device 1234:1111 > >> =A0 =A0 =A0 BAR0: 32 bit prefetchable memory at 0x00800000 [0x00ffff= ff]. > >> =A0 =A0 =A0 BAR6: 32 bit memory at 0x01000000 [0x0100ffff]. > >> =A0 =A0 =A0 id "" > >> =A0 Bus =A00, device =A0 3, function 0: > >> =A0 =A0 Bridge: PCI device 108e:1000 > >> =A0 =A0 =A0 BAR0: 32 bit memory at 0x02000000 [0x02ffffff]. > >> =A0 =A0 =A0 BAR1: 32 bit memory at 0x03000000 [0x037fffff]. > >> =A0 =A0 =A0 id "" > >> =A0 Bus =A00, device =A0 4, function 0: > >> =A0 =A0 Ethernet controller: PCI device 10ec:8029 > >> =A0 =A0 =A0 IRQ 0. > >> =A0 =A0 =A0 BAR0: I/O at 0xffffffffffffffff [0x00fe]. > >> =A0 =A0 =A0 BAR6: 32 bit memory at 0x03800000 [0x0380ffff]. > >> =A0 =A0 =A0 id "" > >> =A0 Bus =A00, device =A0 5, function 0: > >> =A0 =A0 IDE controller: PCI device 1095:0646 > >> =A0 =A0 =A0 IRQ 1. > >> =A0 =A0 =A0 BAR0: I/O at 0xffffffffffffffff [0x0006]. > >> =A0 =A0 =A0 BAR1: I/O at 0xffffffffffffffff [0x0002]. > >> =A0 =A0 =A0 BAR2: I/O at 0xffffffffffffffff [0x0006]. > >> =A0 =A0 =A0 BAR3: I/O at 0xffffffffffffffff [0x0002]. > >> =A0 =A0 =A0 BAR4: I/O at 0xffffffffffffffff [0x000e]. > >> =A0 =A0 =A0 id "" > >> (qemu) info mtree > >> memory > >> 0000000000000000-7ffffffffffffffe (prio 0, RW): system > >> =A0 0000000000000000-0000000007ffffff (prio 0, RW): sun4u.ram > >> =A0 000001fe00000000-000001fe0000ffff (prio 0, RW): apb-config > >> =A0 000001fe01000000-000001fe01ffffff (prio 0, RW): apb-pci-config > >> =A0 000001fe02000000-000001fe0200ffff (prio 0, RW): apb-pci-ioport > >> =A0 000001ff00000000-000001ffffffffff (prio 0, RW): pci-mmio > >> =A0 =A0 000001ff00000000-000001ff000fffff (prio 1, RW): alias > >> pci_bridge_mem @pci_bridge_pci 0000000000000000-00000000000fffff > >> =A0 =A0 000001ff00000000-000001ff000fffff (prio 1, RW): alias > >> pci_bridge_pref_mem @pci_bridge_pci 0000000000000000-00000000000ffff= f > >> =A0 =A0 000001ff00000000-000001ff000fffff (prio 1, RW): alias > >> pci_bridge_mem @pci_bridge_pci 0000000000000000-00000000000fffff > >> =A0 =A0 000001ff00000000-000001ff000fffff (prio 1, RW): alias > >> pci_bridge_pref_mem @pci_bridge_pci 0000000000000000-00000000000ffff= f > >> =A0 =A0 000001ff000a0000-000001ff000affff (prio 2, RW): alias vga.ch= ain4 > >> @vga.vram 0000000000000000-000000000000ffff > >> =A0 =A0 000001ff000a0000-000001ff000bffff (prio 1, RW): vga-lowmem > >> =A0 =A0 000001ff00800000-000001ff00ffffff (prio 1, RW): vga.vram > >> =A0 =A0 000001ff01000000-000001ff0100ffff (prio 1, RW): vga.rom > >> =A0 =A0 000001ff02000000-000001ff02ffffff (prio 1, RW): isa-mmio > >> =A0 =A0 000001ff03000000-000001ff037fffff (prio 1, RW): isa-mmio > >> =A0 =A0 000001ff03800000-000001ff0380ffff (prio 1, RW): ne2000.rom > >> =A0 000001fff0000000-000001fff03fffff (prio 0, R-): sun4u.prom > >> pci_bridge_pci > >> 0000000000000000-7ffffffffffffffe (prio 0, RW): pci_bridge_pci > >> pci_bridge_pci > >> 0000000000000000-7ffffffffffffffe (prio 0, RW): pci_bridge_pci > >> vga.vram > >> 0000000000800000-0000000000ffffff (prio 1, RW): vga.vram > >> I/O > >> 0000000000000000-000000000000ffff (prio 0, RW): io > >> =A0 0000000000000000-0000000000000fff (prio 1, RW): alias pci_bridge= _io > >> @pci_bridge_io 0000000000000000-0000000000000fff > >> =A0 0000000000000000-0000000000000fff (prio 1, RW): alias pci_bridge= _io > >> @pci_bridge_io 0000000000000000-0000000000000fff > >> =A0 0000000000000060-0000000000000060 (prio 0, RW): i8042-data > >> =A0 0000000000000064-0000000000000064 (prio 0, RW): i8042-cmd > >> =A0 0000000000000074-0000000000000077 (prio 0, RW): m48t59 > >> =A0 00000000000001ce-00000000000001ce (prio 0, RW): alias vbe @vbe > >> 00000000000001ce-00000000000001ce > >> =A0 00000000000001d0-00000000000001d0 (prio 0, RW): alias vbe @vbe > >> 00000000000001d0-00000000000001d0 > >> =A0 0000000000000378-000000000000037f (prio 0, RW): alias parallel > >> @parallel 0000000000000378-000000000000037f > >> =A0 00000000000003b4-00000000000003b5 (prio 0, RW): alias vga @vga > >> 00000000000003b4-00000000000003b5 > >> =A0 00000000000003ba-00000000000003ba (prio 0, RW): alias vga @vga > >> 00000000000003ba-00000000000003ba > >> =A0 00000000000003c0-00000000000003cf (prio 0, RW): alias vga @vga > >> 00000000000003c0-00000000000003cf > >> =A0 00000000000003d4-00000000000003d5 (prio 0, RW): alias vga @vga > >> 00000000000003d4-00000000000003d5 > >> =A0 00000000000003da-00000000000003da (prio 0, RW): alias vga @vga > >> 00000000000003da-00000000000003da > >> =A0 00000000000003f1-00000000000003f5 (prio 0, RW): alias fdc @fdc > >> 00000000000003f1-00000000000003f5 > >> =A0 00000000000003f7-00000000000003f7 (prio 0, RW): alias fdc @fdc > >> 00000000000003f7-00000000000003f7 > >> =A0 00000000000003f8-00000000000003ff (prio 0, RW): serial > >> =A0 0000000000000510-0000000000000511 (prio 0, RW): fwcfg > > > > And with type 32 range it looks like this: > > > > 0000000000000000-7ffffffffffffffe (prio 0, RW): system > > =A00000000000000000-0000000007ffffff (prio 0, RW): sun4u.ram > > =A0000001fe00000000-000001fe0000ffff (prio 0, RW): apb-config > > =A0000001fe01000000-000001fe01ffffff (prio 0, RW): apb-pci-config > > =A0000001fe02000000-000001fe0200ffff (prio 0, RW): apb-pci-ioport > > =A0000001ff00000000-000001ffffffffff (prio 0, RW): pci-mmio > > =A0 =A0000001ff00000000-000001ff000fffff (prio 1, RW): alias pci_brid= ge_mem > > @pci_bridge_pci 0000000000000000-00000000000fffff > > =A0 =A0000001ff00000000-000001ff000fffff (prio 1, RW): alias > > pci_bridge_pref_mem @pci_bridge_pci 0000000000000000-00000000000fffff > > =A0 =A0000001ff00000000-000001ff000fffff (prio 1, RW): alias pci_brid= ge_mem > > @pci_bridge_pci 0000000000000000-00000000000fffff > > =A0 =A0000001ff00000000-000001ff000fffff (prio 1, RW): alias > > pci_bridge_pref_mem @pci_bridge_pci 0000000000000000-00000000000fffff > > =A0 =A0000001ff000a0000-000001ff000affff (prio 2, RW): alias vga.chai= n4 > > @vga.vram 0000000000000000-000000000000ffff > > =A0 =A0000001ff000a0000-000001ff000bffff (prio 1, RW): vga-lowmem > > =A0 =A0000001ff00800000-000001ff00ffffff (prio 1, RW): vga.vram > > =A0 =A0000001ff01000000-000001ff0100ffff (prio 1, RW): vga.rom > > =A0 =A0000001ff02000000-000001ff02ffffff (prio 1, RW): isa-mmio > > =A0 =A0000001ff03000000-000001ff037fffff (prio 1, RW): isa-mmio > > =A0000001fff0000000-000001fff03fffff (prio 0, R-): sun4u.prom > > pci_bridge_pci > > 0000000000000000-7ffffffffffffffe (prio 0, RW): pci_bridge_pci > > pci_bridge_pci > > 0000000000000000-7ffffffffffffffe (prio 0, RW): pci_bridge_pci > > vga.vram > > 0000000000800000-0000000000ffffff (prio 1, RW): vga.vram > > I/O > > 0000000000000000-000000000000ffff (prio 0, RW): io > > =A00000000000000000-0000000000000fff (prio 1, RW): alias pci_bridge_i= o > > @pci_bridge_io 0000000000000000-0000000000000fff > > =A00000000000000000-0000000000000fff (prio 1, RW): alias pci_bridge_i= o > > @pci_bridge_io 0000000000000000-0000000000000fff > > =A00000000000000060-0000000000000060 (prio 0, RW): i8042-data > > =A00000000000000064-0000000000000064 (prio 0, RW): i8042-cmd > > =A00000000000000074-0000000000000077 (prio 0, RW): m48t59 > > =A000000000000001ce-00000000000001ce (prio 0, RW): vbe > > =A000000000000001d0-00000000000001d0 (prio 0, RW): vbe > > =A00000000000000378-000000000000037f (prio 0, RW): parallel > > =A000000000000003b4-00000000000003b5 (prio 0, RW): vga > > =A000000000000003ba-00000000000003ba (prio 0, RW): vga > > =A000000000000003c0-00000000000003cf (prio 0, RW): vga > > =A000000000000003d4-00000000000003d5 (prio 0, RW): vga > > =A000000000000003da-00000000000003da (prio 0, RW): vga > > =A000000000000003f1-00000000000003f5 (prio 0, RW): fdc > > =A000000000000003f7-00000000000003f7 (prio 0, RW): fdc > > =A000000000000003f8-00000000000003ff (prio 0, RW): serial > > =A00000000000000400-00000000000004ff (prio 1, RW): ne2000 > > =A00000000000000500-0000000000000507 (prio 1, RW): cmd646-data > > =A00000000000000510-0000000000000511 (prio 0, RW): fwcfg > > =A00000000000000580-0000000000000583 (prio 1, RW): cmd646-cmd > > =A00000000000000600-0000000000000607 (prio 1, RW): cmd646-data > > =A00000000000000680-0000000000000683 (prio 1, RW): cmd646-cmd > > =A00000000000000700-000000000000070f (prio 1, RW): cmd646-bmdma > > =A0 =A00000000000000700-0000000000000703 (prio 0, RW): cmd646-bmdma-b= us > > =A0 =A00000000000000704-0000000000000707 (prio 0, RW): cmd646-bmdma-i= oport > > =A0 =A00000000000000708-000000000000070b (prio 0, RW): cmd646-bmdma-b= us > > =A0 =A0000000000000070c-000000000000070f (prio 0, RW): cmd646-bmdma-i= oport >=20 > This looks better. >=20 > > > > Sill trying to understand what all this means. > > > > > >> > =A0hw/pci.c | =A0 =A04 ++-- > >> > =A01 files changed, 2 insertions(+), 2 deletions(-) > >> > > >> > diff --git a/hw/pci.c b/hw/pci.c > >> > index fee27fc..6d08cef 100644 > >> > --- a/hw/pci.c > >> > +++ b/hw/pci.c > >> > @@ -633,8 +633,8 @@ static void pci_init_mask_bridge(PCIDevice *d) > >> > =A0 =A0 memset(d->wmask + PCI_PREF_BASE_UPPER32, 0xff, 8); > >> > > >> > =A0 =A0 /* Supported memory and i/o types */ > >> > - =A0 =A0d->config[PCI_IO_BASE] |=3D PCI_IO_RANGE_TYPE_32; > >> > - =A0 =A0d->config[PCI_IO_LIMIT] |=3D PCI_IO_RANGE_TYPE_32; > >> > + =A0 =A0d->config[PCI_IO_BASE] |=3D PCI_IO_RANGE_TYPE_16; > >> > + =A0 =A0d->config[PCI_IO_LIMIT] |=3D PCI_IO_RANGE_TYPE_16; > >> > =A0 =A0 pci_word_test_and_set_mask(d->config + PCI_PREF_MEMORY_BAS= E, > >> > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0PCI= _PREF_RANGE_TYPE_64); > >> > =A0 =A0 pci_word_test_and_set_mask(d->config + PCI_PREF_MEMORY_LIM= IT, > >> > -- > >> > 1.7.9.111.gf3fb0