Patchwork pci: fix bridge IO/BASE

login
register
mail settings
Submitter Blue Swirl
Date March 4, 2012, 7:51 p.m.
Message ID <CAAu8pHtL87=n1DadcwdraYvtf6GSm7p8ZETLdTHO5uqf=j03Qg@mail.gmail.com>
Download mbox | patch
Permalink /patch/144520/
State New
Headers show

Comments

Blue Swirl - March 4, 2012, 7:51 p.m.
On Sun, Mar 4, 2012 at 17:35, Michael S. Tsirkin <mst@redhat.com> 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 <mst@redhat.com> 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 <mst@redhat.com> 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 <mst@redhat.com> 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 <mst@redhat.com> 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 <mst@redhat.com> 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 <mark.cave-ayland@ilande.co.uk>
>> >> >> >> >> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>> >> >> >> >> >
>> >> >> >> >> > 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.

>> >
>> >> 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
Michael S. Tsirkin - March 4, 2012, 8:02 p.m.
On Sun, Mar 04, 2012 at 07:51:02PM +0000, Blue Swirl wrote:
> On Sun, Mar 4, 2012 at 17:35, Michael S. Tsirkin <mst@redhat.com> 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 <mst@redhat.com> 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 <mst@redhat.com> 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 <mst@redhat.com> 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 <mst@redhat.com> 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 <mst@redhat.com> 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 <mark.cave-ayland@ilande.co.uk>
> >> >> >> >> >> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >> >> >> >> >> >
> >> >> >> >> >> > 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?



> >> >
> >> >> 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: <3f957e2dc8477f00f6d3a9491d81399ee750c725.1330890410.git.blauwirbel@gmail.com>
> From: Blue Swirl <blauwirbel@gmail.com>
> 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 <blauwirbel@gmail.com>
> ---
>  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
>
Blue Swirl - March 4, 2012, 8:32 p.m.
On Sun, Mar 4, 2012 at 20:02, Michael S. Tsirkin <mst@redhat.com> 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 <mst@redhat.com> 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 <mst@redhat.com> 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 <mst@redhat.com> 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 <mst@redhat.com> 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 <mst@redhat.com> 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 <mst@redhat.com> 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 <mark.cave-ayland@ilande.co.uk>
>> >> >> >> >> >> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>> >> >> >> >> >> >
>> >> >> >> >> >> > 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. This conflicts with bridge
spec 3.2.4.9. Maybe the device predates the specification.

>> >> >
>> >> >> 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: <3f957e2dc8477f00f6d3a9491d81399ee750c725.1330890410.git.blauwirbel@gmail.com>
>> From: Blue Swirl <blauwirbel@gmail.com>
>> 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 <blauwirbel@gmail.com>
>> ---
>>  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
>>
>
Michael S. Tsirkin - March 4, 2012, 9:28 p.m.
On Sun, Mar 04, 2012 at 08:32:26PM +0000, Blue Swirl wrote:
> On Sun, Mar 4, 2012 at 20:02, Michael S. Tsirkin <mst@redhat.com> 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 <mst@redhat.com> 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 <mst@redhat.com> 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 <mst@redhat.com> 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 <mst@redhat.com> 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 <mst@redhat.com> 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 <mst@redhat.com> 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 <mark.cave-ayland@ilande.co.uk>
> >> >> >> >> >> >> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >> >> >> >> >> >> >
> >> >> >> >> >> >> > 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.

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

> 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: <3f957e2dc8477f00f6d3a9491d81399ee750c725.1330890410.git.blauwirbel@gmail.com>
> >> From: Blue Swirl <blauwirbel@gmail.com>
> >> 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 <blauwirbel@gmail.com>
> >> ---
> >>  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
> >>
> >
Blue Swirl - March 4, 2012, 9:54 p.m.
On Sun, Mar 4, 2012 at 21:28, Michael S. Tsirkin <mst@redhat.com> 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 <mst@redhat.com> 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 <mst@redhat.com> 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 <mst@redhat.com> 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 <mst@redhat.com> 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 <mst@redhat.com> 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 <mst@redhat.com> 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 <mst@redhat.com> 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 <mark.cave-ayland@ilande.co.uk>
>> >> >> >> >> >> >> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>> >> >> >> >> >> >> >
>> >> >> >> >> >> >> > 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: <3f957e2dc8477f00f6d3a9491d81399ee750c725.1330890410.git.blauwirbel@gmail.com>
>> >> From: Blue Swirl <blauwirbel@gmail.com>
>> >> 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 <blauwirbel@gmail.com>
>> >> ---
>> >>  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
>> >>
>> >
Michael S. Tsirkin - March 4, 2012, 10:29 p.m.
On Sun, Mar 04, 2012 at 09:54:02PM +0000, Blue Swirl wrote:
> >> 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.

So we are in agreement then?
The device seems to (more or less) go by the spec,
but openbios gets confused, apparently by the absence of BARs.

Can you try debugging openbios to see what is wrong?
Blue Swirl - March 5, 2012, 6:34 p.m.
On Sun, Mar 4, 2012 at 22:29, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Sun, Mar 04, 2012 at 09:54:02PM +0000, Blue Swirl wrote:
>> >> 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.
>
> So we are in agreement then?
> The device seems to (more or less) go by the spec,

QEMU is correct here as is your original commit. The only change
(beside bug fixes for the problems you found, thanks) should be for
is_bridge.

> but openbios gets confused, apparently by the absence of BARs.
>
> Can you try debugging openbios to see what is wrong?

On second thought, I think 32 bit I/O was confusing OpenBIOS, probably
the host bridge device not so much. PCI handling in OpenBIOS should be
fixed.

> --
> MST
Michael S. Tsirkin - March 6, 2012, 1:42 p.m.
On Mon, Mar 05, 2012 at 06:34:51PM +0000, Blue Swirl wrote:
> On Sun, Mar 4, 2012 at 22:29, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Sun, Mar 04, 2012 at 09:54:02PM +0000, Blue Swirl wrote:
> >> >> 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.
> >
> > So we are in agreement then?
> > The device seems to (more or less) go by the spec,
> 
> QEMU is correct here as is your original commit. The only change
> (beside bug fixes for the problems you found, thanks) should be for
> is_bridge.

You mean remove is_bridge? Right.

> > but openbios gets confused, apparently by the absence of BARs.
> >
> > Can you try debugging openbios to see what is wrong?
> 
> On second thought, I think 32 bit I/O was confusing OpenBIOS, probably
> the host bridge device not so much. PCI handling in OpenBIOS should be
> fixed.

We can make IO 32 bit. Want to try?
Anyway, so I won't revert this in core for now?

> > --
> > MST

Patch

From 3f957e2dc8477f00f6d3a9491d81399ee750c725 Mon Sep 17 00:00:00 2001
Message-Id: <3f957e2dc8477f00f6d3a9491d81399ee750c725.1330890410.git.blauwirbel@gmail.com>
From: Blue Swirl <blauwirbel@gmail.com>
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 <blauwirbel@gmail.com>
---
 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