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

[Blue Swirl]

On Sun, Mar 4, 2012 at 21:28, Michael S. Tsirkin <address@hidden> wrote:
> On Sun, Mar 04, 2012 at 08:32:26PM +0000, Blue Swirl wrote:
>> On Sun, Mar 4, 2012 at 20:02, Michael S. Tsirkin <address@hidden> wrote:
>> > On Sun, Mar 04, 2012 at 07:51:02PM +0000, Blue Swirl wrote:
>> >> On Sun, Mar 4, 2012 at 17:35, Michael S. Tsirkin <address@hidden> wrote:
>> >> > On Sun, Mar 04, 2012 at 05:07:34PM +0000, Blue Swirl wrote:
>> >> >> On Sun, Mar 4, 2012 at 15:22, Michael S. Tsirkin <address@hidden> 
>> >> >> wrote:
>> >> >> > On Sun, Mar 04, 2012 at 02:35:28PM +0000, Blue Swirl wrote:
>> >> >> >> On Sun, Mar 4, 2012 at 14:23, Michael S. Tsirkin <address@hidden> 
>> >> >> >> 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 
>> >> >> >> >> <address@hidden> wrote:
>> >> >> >> >> > On Sun, Mar 04, 2012 at 12:37:57PM +0000, Blue Swirl wrote:
>> >> >> >> >> >> On Sun, Mar 4, 2012 at 12:21, Michael S. Tsirkin 
>> >> >> >> >> >> <address@hidden> 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 
>> >> >> >> >> >> >> <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
>> >> >> >> >> >> >
>> >> >> >> >> >> > 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.  Please 
>> >> >> >> >> >> > report
>> >> >> >> >> >> > --11114-- it at 
>> >> >> >> >> >> > http://valgrind.org/support/bug_reports.html.
>> >> >> >> >> >> > ==11114== Invalid read of size 4
>> >> >> >> >> >> > ==11114==    at 0x2A68C0: pci_apb_init (apb_pci.c:350)
>> >> >> >> >> >> > ==11114==    by 0x2F7A84: sun4uv_init (sun4u.c:779)
>> >> >> >> >> >> > ==11114==    by 0x13D716: main (vl.c:3397)
>> >> >> >> >> >> > ==11114==  Address 0x156c7d30 is 0 bytes after a block of 
>> >> >> >> >> >> > size 64
>> >> >> >> >> >> > alloc'd
>> >> >> >> >> >> > ==11114==    at 0x557DD69: malloc (vg_replace_malloc.c:236)
>> >> >> >> >> >> > ==11114==    by 0x225F56: malloc_and_trace (vl.c:2156)
>> >> >> >> >> >> > ==11114==    by 0x584AFEC: ??? (in 
>> >> >> >> >> >> > /lib/libglib-2.0.so.0.2800.8)
>> >> >> >> >> >> > ==11114==    by 0x584B528: g_malloc0 (in 
>> >> >> >> >> >> > /lib/libglib-2.0.so.0.2800.8)
>> >> >> >> >> >> > ==11114==    by 0x19C50C: qemu_allocate_irqs (irq.c:47)
>> >> >> >> >> >> > ==11114==    by 0x2F7A4C: sun4uv_init (sun4u.c:778)
>> >> >> >> >> >> > ==11114==    by 0x13D716: main (vl.c:3397)
>> >> >> >> >> >> > ==11114==
>> >> >> >> >> >> > apb: here
>> >> >> >> >> >> > ==11114== Warning: client switching stacks?  SP change: 
>> >> >> >> >> >> > 0xfec42cbc -->
>> >> >> >> >> >> > 0x16894008
>> >> >> >> >> >> > ==11114==          to suppress, use: 
>> >> >> >> >> >> > --max-stackframe=398791500 or
>> >> >> >> >> >> > greater
>> >> >> >> >> >> > ==11114== Warning: client switching stacks?  SP change: 
>> >> >> >> >> >> > 0x16893fa0 -->
>> >> >> >> >> >> > 0xfec42cc0
>> >> >> >> >> >> > ==11114==          to suppress, use: 
>> >> >> >> >> >> > --max-stackframe=398791392 or
>> >> >> >> >> >> > greater
>> >> >> >> >> >> > ==11114== Warning: client switching stacks?  SP change: 
>> >> >> >> >> >> > 0xfec42fe0 -->
>> >> >> >> >> >> > 0x16893fd0
>> >> >> >> >> >> > ==11114==          to suppress, use: 
>> >> >> >> >> >> > --max-stackframe=398790640 or
>> >> >> >> >> >> > greater
>> >> >> >> >> >> > ==11114==          further instances of this message will 
>> >> >> >> >> >> > not be shown.
>> >> >> >> >> >> > QEMU 1.0.50 monitor - type 'help' for more information
>> >> >> >> >> >> > (qemu) ==11114== Thread 2:
>> >> >> >> >> >> > ==11114== Conditional jump or move depends on uninitialised 
>> >> >> >> >> >> > value(s)
>> >> >> >> >> >> > ==11114==    at 0x2A8351: compute_all_sub (cc_helper.c:37)
>> >> >> >> >> >> > ==11114==    by 0x2A8782: helper_compute_psr 
>> >> >> >> >> >> > (cc_helper.c:470)
>> >> >> >> >> >> > ==11114==    by 0x9AD9A19: ???
>> >> >> >> >> >> > ==11114==
>> >> >> >> >> >> > ==11114== Conditional jump or move depends on uninitialised 
>> >> >> >> >> >> > value(s)
>> >> >> >> >> >> > ==11114==    at 0x2A827C: compute_all_sub_xcc 
>> >> >> >> >> >> > (cc_helper.c:60)
>> >> >> >> >> >> > ==11114==    by 0x2A8795: helper_compute_psr 
>> >> >> >> >> >> > (cc_helper.c:473)
>> >> >> >> >> >> > ==11114==    by 0x9AD9A19: ???
>> >> >> >> >> >> > ==11114==
>> >> >> >> >> >> > ==11114== Conditional jump or move depends on uninitialised 
>> >> >> >> >> >> > value(s)
>> >> >> >> >> >> > ==11114==    at 0x2A8296: compute_all_sub_xcc 
>> >> >> >> >> >> > (cc_helper.c:295)
>> >> >> >> >> >> > ==11114==    by 0x2A8795: helper_compute_psr 
>> >> >> >> >> >> > (cc_helper.c:473)
>> >> >> >> >> >> > ==11114==    by 0x9AD9A19: ???
>> >> >> >> >> >> > ==11114==
>> >> >> >> >> >> >
>> >> >> >> >> >> > Is the above a problem?
>> >> >> >> >> >>
>> >> >> >> >> >> 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 = qemu_allocate_irqs(cpu_set_irq, env, MAX_PILS);
>> >> >> >> >> >
>> >> >> >> >> > then passed to apb:
>> >> >> >> >> >
>> >> >> >> >> >    pci_bus = pci_apb_init(APB_SPECIAL_BASE, APB_MEM_BASE, irq, 
>> >> >> >> >> > &pci_bus2,
>> >> >> >> >> >                           &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 = 0; i < 32; i++) {
>> >> >> >> >> >        sysbus_connect_irq(s, i, pic[i]);
>> >> >> >> >> >    }
>> >> >> >> >>
>> >> >> >> >> 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?
>> >> >> >>
>> >> >> >> 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 have most if not all manuals, downloaded from sun.com, but I'm not
>> >> >> sure if they can be redistributed.
>> >> >
>> >> > Okay ...
>> >> > Let's change the link to point to some other place which has them?
>> >> >
>> >> >> > I am looking at 19.3.1 PCI Configuration 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?
>> >> >>
>> >> >> It's the host bridge and the device class is PCI_CLASS_BRIDGE_HOST.
>> >> >
>> >> > Yes. But the *header* type is 0 (NORMAL)
>> >> > while the code in pci_init_mask_bridge
>> >> > which is the only user of the is_bridge register
>> >> > initializes a type 1 (BRIDGE) header.
>> >> >
>> >> > So it just happens to do a vaguely correct thing.
>> >>
>> >> Well, that is still according to device spec.
>> >
>> > I tried to find anything in the spec that says any register
>> > after 0x10 is implemented but failed.
>> > Can you tell me which chater and what it says?
>>
>> 19.3.1.10 tells that the header type is 0, as you noted too. Still,
>> the register layout matches bridge spec instead, for example there are
>> bus number registers in place of BAR 2.
>
> Sorry I don't see this in 19.3.1
> Where are these registers documented?
> In my spec all registers from 0x10 on are greyed out which
> it says above means 'not implemented'?
> My spec also says 'Base Address' for 0x10 - 0x27.
>
> I see bus number and subordinate bus number
> registers at 0x40 and 0x41, this is outside
> the configuration header. The spec also says
> they are unused.

Oh, I somehow read that they were in bridge locations 0x18 and 0x19.
Perhaps the is_bridge property should be removed after OpenBIOS no
longer wants to write to the registers.

> Are we looking at the same spec? Mine is copyright 1997.

Yes, Part No.: 805-0087-01.

>> This conflicts with bridge
>> spec 3.2.4.9. Maybe the device predates the specification.
>
> bridge spec 1.0 was released in 1994, 1.1 in 1998.
> This spec is from 1997, but the device might be older.
>
>
>> >> >> >
>> >> >> >> and there it says that the device is
>> >> >> >> found in the configuration space.
>> >> >> >>
>> >> >> >> 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
>> >> >>
>> >> >> IIRC otherwise some registers are not writable.
>> >> >
>> >> > Yes but which ones? I looked at the manual and
>> >> > it does not list any registers. Playing with code,
>> >> > it looks like we just need to make *some*
>> >> > BAR writeable. I tried with
>> >> > pci_set_long(d->wmask + PCI_BASE_ADDRESS_0, 0xfffffff0);
>> >> > to
>> >> > pci_set_long(d->wmask + PCI_BASE_ADDRESS_5, 0xfffffff0);
>> >> >
>> >> > and any one of these makes bios get at least to
>> >> > the prompt.
>> >>
>> >> I now know the root cause of the problem. OpenBIOS programs the BARs
>> >> somewhat correctly just by accident. The initial io_base and mem_base
>> >> for BARs are not correct, but because the host bridge BARs (and also 6
>> >> of which 4 are not even BARs!) are programmed first, the bases
>> >> happened to settle to values that happen to work. The commit revealed
>> >> the problem since the settling didn't happen. The mask changes just
>> >> let the host bridge setup continue to do the magic.
>> >>
>> >> By just changing OpenBIOS (see attached patch), I can get the devices
>> >> to work (assuming that VGA is a separate problem). There's no need to
>> >> change QEMU.
>> >>
>> >> >> >
>> >> >> >> >
>> >> >> >> >
>> >> >> >> > --
>> >> >> >> > MST
>> >
>> >> From 3f957e2dc8477f00f6d3a9491d81399ee750c725 Mon Sep 17 00:00:00 2001
>> >> Message-Id: <address@hidden>
>> >> From: Blue Swirl <address@hidden>
>> >> Date: Sun, 4 Mar 2012 19:46:38 +0000
>> >> Subject: [PATCH] pci: fix BAR setup
>> >>
>> >> A change in QEMU on how PCI bridges are setup revealed
>> >> a bug in OpenBIOS PCI setup. On Sparc64, the BARs just
>> >> happened to get somewhat correct values by accident before
>> >> the commit but not after the change.
>> >>
>> >> Avoid to set up BARs for host bridge. Fix bridge
>> >> check, this lead to setting up 6 BARs instead of more
>> >> correct 2. If a bridge doesn't have any devices behind it,
>> >> disable it entirely. Fix Sparc64 PCI memory base.
>> >>
>> >> Signed-off-by: Blue Swirl <address@hidden>
>> >> ---
>> >>  arch/sparc64/openbios.c |    2 +-
>> >>  drivers/pci.c           |   67 
>> >> ++++++++++++++++++++++++++++++++++------------
>> >>  drivers/pci.h           |    7 +++++
>> >>  3 files changed, 57 insertions(+), 19 deletions(-)
>> >>
>> >> diff --git a/arch/sparc64/openbios.c b/arch/sparc64/openbios.c
>> >> index ac709fe..a1544a8 100644
>> >> --- a/arch/sparc64/openbios.c
>> >> +++ b/arch/sparc64/openbios.c
>> >> @@ -64,7 +64,7 @@ static const struct hwdef hwdefs[] = {
>> >>              .cfg_base = APB_SPECIAL_BASE,
>> >>              .cfg_len = 0x2000000,
>> >>              .host_mem_base = APB_MEM_BASE,
>> >> -            .pci_mem_base = 0,
>> >> +            .pci_mem_base = 0x10000000,
>> >>              .mem_len = 0x10000000,
>> >>              .io_base = APB_SPECIAL_BASE + 0x2000000ULL, // PCI Bus I/O 
>> >> space
>> >>              .io_len = 0x10000,
>> >> diff --git a/drivers/pci.c b/drivers/pci.c
>> >> index f8c6414..6ed0c03 100644
>> >> --- a/drivers/pci.c
>> >> +++ b/drivers/pci.c
>> >> @@ -966,11 +966,18 @@ static void ob_pci_configure_bar(pci_addr addr, 
>> >> pci_config_t *config,
>> >>                  size = min_align;
>> >>          reloc = (reloc + size -1) & ~(size - 1);
>> >>          if (*io_base == base) {
>> >> +                PCI_DPRINTF("changing io_base from 0x%lx to 0x%x\n",
>> >> +                            *io_base, reloc + size);
>> >>                  *io_base = reloc + size;
>> >>                  reloc -= arch->io_base;
>> >>          } else {
>> >> +                PCI_DPRINTF("changing mem_base from 0x%lx to 0x%x\n",
>> >> +                            *mem_base, reloc + size);
>> >>                  *mem_base = reloc + size;
>> >>          }
>> >> +        PCI_DPRINTF("Configuring BARs for %s: reloc 0x%x omask 0x%x "
>> >> +                    "io_base 0x%lx mem_base 0x%lx\n",
>> >> +                    config->path, reloc, *p_omask, *io_base, *mem_base);
>> >>          pci_config_write32(addr, config_addr, reloc | *p_omask);
>> >>          config->assigned[reg] = reloc | *p_omask;
>> >>  }
>> >> @@ -1021,26 +1028,30 @@ ob_pci_configure(pci_addr addr, pci_config_t 
>> >> *config, int num_regs, int rom_bar,
>> >>          pci_config_write16(addr, PCI_COMMAND, cmd);
>> >>  }
>> >>
>> >> -static void ob_configure_pci_device(const char* parent_path,
>> >> -        int *bus_num, unsigned long *mem_base, unsigned long *io_base,
>> >> -        int bus, int devnum, int fn, int *p_is_multi);
>> >> +static int ob_configure_pci_device(const char* parent_path,
>> >> +                                   int *bus_num, unsigned long *mem_base,
>> >> +                                   unsigned long *io_base, int bus, int 
>> >> devnum,
>> >> +                                   int fn, int *p_is_multi);
>> >>
>> >> -static void ob_scan_pci_bus(int *bus_num, unsigned long *mem_base,
>> >> -                            unsigned long *io_base, const char *path,
>> >> -                            int bus)
>> >> +static int ob_scan_pci_bus(int *bus_num, unsigned long *mem_base,
>> >> +                           unsigned long *io_base, const char *path,
>> >> +                           int bus)
>> >>  {
>> >> -     int devnum, fn, is_multi;
>> >> +        int devnum, fn, is_multi, ndevices = 0;
>> >>
>> >>       PCI_DPRINTF("\nScanning bus %d at %s...\n", bus, path);
>> >>
>> >>       for (devnum = 0; devnum < 32; devnum++) {
>> >>               is_multi = 0;
>> >>               for (fn = 0; fn==0 || (is_multi && fn<8); fn++) {
>> >> -                 ob_configure_pci_device(path, bus_num, mem_base, 
>> >> io_base,
>> >> -                         bus, devnum, fn, &is_multi);
>> >> +                 ndevices += ob_configure_pci_device(path, bus_num,
>> >> +                                                        mem_base, 
>> >> io_base,
>> >> +                                                        bus, devnum, fn,
>> >> +                                                        &is_multi);
>> >>
>> >>               }
>> >>       }
>> >> +        return ndevices;
>> >>  }
>> >>
>> >>  static void ob_configure_pci_bridge(pci_addr addr,
>> >> @@ -1048,6 +1059,9 @@ static void ob_configure_pci_bridge(pci_addr addr,
>> >>                                      unsigned long *io_base,
>> >>                                      int primary_bus, pci_config_t 
>> >> *config)
>> >>  {
>> >> +    int ndevices;
>> >> +    uint8_t command;
>> >> +
>> >>      config->primary_bus = primary_bus;
>> >>      pci_config_write8(addr, PCI_PRIMARY_BUS, config->primary_bus);
>> >>
>> >> @@ -1062,16 +1076,30 @@ static void ob_configure_pci_bridge(pci_addr addr,
>> >>
>> >>      /* make pci bridge parent device, prepare for recursion */
>> >>
>> >> -    ob_scan_pci_bus(bus_num, mem_base, io_base,
>> >> -                    config->path, config->secondary_bus);
>> >> +    ndevices = ob_scan_pci_bus(bus_num, mem_base, io_base,
>> >> +                               config->path, config->secondary_bus);
>> >> +    if (!ndevices) {
>> >> +        /* no devices, disable bridging */
>> >> +        PCI_DPRINTF("disabling bridge %s\n", config->path);
>> >> +        command = pci_config_read8(addr, PCI_COMMAND);
>> >> +        command &= ~(PCI_COMMAND_IO | PCI_COMMAND_MEMORY |
>> >> +                     PCI_COMMAND_VGA_PALETTE | PCI_COMMAND_MASTER);
>> >> +        pci_config_write8(addr, PCI_COMMAND, command);
>> >> +        pci_config_write8(addr, PCI_IO_BASE, 0);
>> >> +        pci_config_write8(addr, PCI_IO_LIMIT, 0);
>> >> +        pci_config_write8(addr, PCI_MEMORY_BASE, 0);
>> >> +        pci_config_write8(addr, PCI_MEMORY_LIMIT, 0);
>> >> +        return;
>> >> +    }
>> >>
>> >>      /* bus scan updates *bus_num to last revealed pci bus number */
>> >>      config->subordinate_bus = *bus_num;
>> >>      pci_config_write8(addr, PCI_SUBORDINATE_BUS, 
>> >> config->subordinate_bus);
>> >>
>> >> -    PCI_DPRINTF("bridge %s PCI bus primary=%d secondary=%d 
>> >> subordinate=%d\n",
>> >> -            config->path, config->primary_bus, config->secondary_bus,
>> >> -            config->subordinate_bus);
>> >> +    PCI_DPRINTF("bridge %s PCI bus primary=%d secondary=%d 
>> >> subordinate=%d"
>> >> +                " ndev=%d\n",
>> >> +                config->path, config->primary_bus, config->secondary_bus,
>> >> +                config->subordinate_bus, ndevices);
>> >>
>> >>      pci_set_bus_range(config);
>> >>  }
>> >> @@ -1117,7 +1145,7 @@ static int ob_pci_read_identification(int bus, int 
>> >> devnum, int fn,
>> >>      return 1;
>> >>  }
>> >>
>> >> -static void ob_configure_pci_device(const char* parent_path,
>> >> +static int ob_configure_pci_device(const char* parent_path,
>> >>          int *bus_num, unsigned long *mem_base, unsigned long *io_base,
>> >>          int bus, int devnum, int fn, int *p_is_multi)
>> >>  {
>> >> @@ -1133,7 +1161,7 @@ static void ob_configure_pci_device(const char* 
>> >> parent_path,
>> >>      int is_host_bridge = 0;
>> >>
>> >>      if (!ob_pci_read_identification(bus, devnum, fn, &vid, &did, &class, 
>> >> &subclass)) {
>> >> -        return;
>> >> +        return 0;
>> >>      }
>> >>
>> >>      addr = PCI_ADDR(bus, devnum, fn);
>> >> @@ -1195,16 +1223,18 @@ static void ob_configure_pci_device(const char* 
>> >> parent_path,
>> >>
>> >>          if (get_property(phandle, "vendor-id", NULL)) {
>> >>              PCI_DPRINTF("host bridge already configured\n");
>> >> -            return;
>> >> +            return 0;
>> >>          }
>> >>      }
>> >>
>> >>      activate_dev(phandle);
>> >>
>> >> -    if (htype & PCI_HEADER_TYPE_BRIDGE) {
>> >> +    if (htype & PCI_HEADER_TYPE_BRIDGE || (class == 
>> >> PCI_BASE_CLASS_BRIDGE)) {
>> >> +        PCI_DPRINTF("Bridge 2 bars, htype %x\n", htype);
>> >>          num_bars = 2;
>> >>          rom_bar  = PCI_ROM_ADDRESS1;
>> >>      } else {
>> >> +        PCI_DPRINTF("Device 6 bars, htype %x\n", htype);
>> >>          num_bars = 6;
>> >>          rom_bar  = PCI_ROM_ADDRESS;
>> >>      }
>> >> @@ -1240,6 +1270,7 @@ static void ob_configure_pci_device(const char* 
>> >> parent_path,
>> >>
>> >>          ob_configure_pci_bridge(addr, bus_num, mem_base, io_base, bus, 
>> >> &config);
>> >>      }
>> >> +    return 1;
>> >>  }
>> >>
>> >>  int ob_pci_init(void)
>> >> diff --git a/drivers/pci.h b/drivers/pci.h
>> >> index 0f6ae1f..4314507 100644
>> >> --- a/drivers/pci.h
>> >> +++ b/drivers/pci.h
>> >> @@ -7,6 +7,8 @@
>> >>  #define PCI_COMMAND          0x04
>> >>  #define  PCI_COMMAND_IO              0x01
>> >>  #define  PCI_COMMAND_MEMORY  0x02
>> >> +#define  PCI_COMMAND_MASTER     0x4     /* Enable bus mastering */
>> >> +#define  PCI_COMMAND_VGA_PALETTE 0x20   /* Enable palette snooping */
>> >>
>> >>  #define PCI_STATUS              0x06    /* 16 bits */
>> >>  #define  PCI_STATUS_CAP_LIST    0x10    /* Support Capability List */
>> >> @@ -44,6 +46,11 @@
>> >>  #define PCI_BASE_ADDR_4              0x20
>> >>  #define PCI_BASE_ADDR_5              0x24
>> >>
>> >> +#define PCI_IO_BASE             0x1c    /* I/O range behind the bridge */
>> >> +#define PCI_IO_LIMIT            0x1d
>> >> +#define PCI_MEMORY_BASE         0x20    /* Memory range behind */
>> >> +#define PCI_MEMORY_LIMIT        0x22
>> >> +
>> >>  #define PCI_SUBSYSTEM_VENDOR_ID 0x2c
>> >>  #define PCI_SUBSYSTEM_ID        0x2e
>> >>
>> >> --
>> >> 1.7.2.5
>> >>
>> >