Re: [Qemu-devel] [PATCH] pci: fix bridge IO/BASE

[Michael S. Tsirkin]

On Sun, Mar 04, 2012 at 10:27:24AM +0000, Blue Swirl wrote:
> On Sun, Mar 4, 2012 at 09:46, Michael S. Tsirkin <address@hidden> 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_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 <address@hidden>
> > Signed-off-by: Michael S. Tsirkin <address@hidden>
> >
> > 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 = 0x00000000ffd19e18 NPC = 0x00000000ffd19e1c
> Stopping execution
> 
> This unassigned memory exception is triggered because CMD646 IDE I/O
> registers are not accessible:

So here's a theory: the issue is that both device IO
and bridge IO have the same priority.
Bridge is initialized with IO at 0 to 4K
and so we have two devices at the same priority.
And flipping a type bit affects this,  by chance?

I tried the following and it seems to help.
But the real fix IMO is to either disable the bridge ranges
at reset, in hardware, or have the BIOS do this.

Avi, could you pls comment on my analysis?

diff --git a/hw/pci.c b/hw/pci.c
index 6d08cef..286383a 100644
--- a/hw/pci.c
+++ b/hw/pci.c
@@ -986,7 +986,7 @@ static void pci_update_mappings(PCIDevice *d)
         r->addr = new_addr;
         if (r->addr != PCI_BAR_UNMAPPED) {
             memory_region_add_subregion_overlap(r->address_space,
-                                                r->addr, r->memory, 1);
+                                                r->addr, r->memory, 2);
         }
     }
 }