[patch 2.5.31] transparent PCI-to-PCI bridges

[patch 2.5.31] transparent PCI-to-PCI bridges

[Manfred Spraul]

Ivan wrote:
> +	/*
> +	 * The PCI-to-PCI bridge spec requires that subtractive decoding
> +	 * (i.e. transparent) bridge must have programming interface code
> +	 * of 0x01.
> +	 */ 
> +	if (dev->class & 1) {
> +		printk("Transparent bridge - %s\n", dev->name);

Why not
	if ((dev->class & 0xff) == 0x01)

Is the lowest bit an indicator of subtractive decoding, or is 
Progif==0x01 the indicator of subtractive decoding?
The code and the comment should match.

--
	Manfred

Re: [patch 2.5.31] transparent PCI-to-PCI bridges

[Ivan Kokshaysky]

On Sun, Aug 25, 2002 at 06:55:59PM +0200, Manfred Spraul wrote:
> Why not
> 	if ((dev->class & 0xff) == 0x01)
> 
> Is the lowest bit an indicator of subtractive decoding, or is 
> Progif==0x01 the indicator of subtractive decoding?

The latter.

> The code and the comment should match.

Ok. Updated patch appended.

Ivan.

--- 2.5.31/arch/i386/pci/fixup.c	Sun Aug 11 05:41:21 2002
+++ linux/arch/i386/pci/fixup.c	Mon Aug 26 17:48:37 2002
@@ -166,6 +166,23 @@ static void __init pci_fixup_via_northbr
 	}
 }
 
+/*
+ * For some weird reasons Intel decided that certain parts of their
+ * 815, 845 and some other chipsets must look like PCI-to-PCI bridges
+ * while they are obviously not. The 82801 family (AA, AB, BAM/CAM,
+ * BA/CA/DB and E) PCI bridges are actually HUB-to-PCI ones, according
+ * to Intel terminology. These devices do forward all addresses from
+ * system to PCI bus no matter what are their window settings, so they are
+ * "transparent" (or subtractive decoding) from programmers point of view.
+ * Indicate this by setting ProgIf to 0x01.
+ */
+static void __init pci_fixup_transparent_bridge(struct pci_dev *dev)
+{
+	if ((dev->class >> 8) == PCI_CLASS_BRIDGE_PCI &&
+	    (dev->device & 0xff00) == 0x2400)
+		dev->class = (dev->class & ~0xff) | 1;
+}
+
 struct pci_fixup pcibios_fixups[] = {
 	{ PCI_FIXUP_HEADER,	PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_82451NX,	pci_fixup_i450nx },
 	{ PCI_FIXUP_HEADER,	PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_82454GX,	pci_fixup_i450gx },
@@ -183,5 +200,6 @@ struct pci_fixup pcibios_fixups[] = {
 	{ PCI_FIXUP_HEADER,	PCI_VENDOR_ID_VIA,	PCI_DEVICE_ID_VIA_8361,	        pci_fixup_via_northbridge_bug },
 	{ PCI_FIXUP_HEADER,	PCI_VENDOR_ID_VIA,	PCI_DEVICE_ID_VIA_8367_0,	pci_fixup_via_northbridge_bug },
 	{ PCI_FIXUP_HEADER,	PCI_VENDOR_ID_NCR,	PCI_DEVICE_ID_NCR_53C810,	pci_fixup_ncr53c810 },
+	{ PCI_FIXUP_HEADER,	PCI_VENDOR_ID_INTEL,	PCI_ANY_ID,			pci_fixup_transparent_bridge },
 	{ 0 }
 };
--- 2.5.31/drivers/pci/quirks.c	Sun Aug 11 05:41:17 2002
+++ linux/drivers/pci/quirks.c	Mon Aug 26 17:50:16 2002
@@ -471,6 +471,11 @@ static void __init quirk_dunord ( struct
 	r -> end = 0xffffff;
 }
 
+static void __init quirk_transparent_bridge(struct pci_dev *dev)
+{
+	dev->class = (dev->class & ~0xff) | 1;
+}
+
 /*
  *  The main table of quirks.
  */
@@ -525,6 +530,12 @@ static struct pci_fixup pci_fixups[] __i
 
 	{ PCI_FIXUP_FINAL, 	PCI_VENDOR_ID_AMD,	PCI_DEVICE_ID_AMD_VIPER_7410,	quirk_amd_ioapic },
 	{ PCI_FIXUP_FINAL,	PCI_VENDOR_ID_AMD,	PCI_DEVICE_ID_AMD_FE_GATE_700C, quirk_amd_ordering },
+	/*
+	 * i82380FB mobile docking controller: its PCI-to-PCI bridge
+	 * is subtractive decoding (transparent), and does indicate this
+	 * in the ProgIf. Unfortunately, in the wrong bit - 7 instead of 0.
+	 */
+	{ PCI_FIXUP_HEADER,	PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_82380FB,	quirk_transparent_bridge },
 
 	{ 0 }
 };
--- 2.5.31/drivers/pci/probe.c	Sun Aug 11 05:41:21 2002
+++ linux/drivers/pci/probe.c	Mon Aug 26 17:38:51 2002
@@ -128,6 +128,18 @@ void __devinit pci_read_bridge_bases(str
 	if (!dev)		/* It's a host bus, nothing to read */
 		return;
 
+	/*
+	 * The PCI-to-PCI bridge spec requires that subtractive decoding
+	 * (i.e. transparent) bridge must have programming interface code
+	 * of 0x01.
+	 */ 
+	if ((dev->class & 0xff) == 0x01) {
+		printk("Transparent bridge - %s\n", dev->name);
+		for(i = 0; i < 3; i++)
+			child->resource[i] = child->parent->resource[i];
+		return;
+	}
+
 	for(i=0; i<3; i++)
 		child->resource[i] = &dev->resource[PCI_BRIDGE_RESOURCES+i];
 
@@ -149,13 +161,6 @@ void __devinit pci_read_bridge_bases(str
 		res->flags = (io_base_lo & PCI_IO_RANGE_TYPE_MASK) | IORESOURCE_IO;
 		res->start = base;
 		res->end = limit + 0xfff;
-	} else {
-		/*
-		 * Ugh. We don't know enough about this bridge. Just assume
-		 * that it's entirely transparent.
-		 */
-		printk(KERN_ERR "Unknown bridge resource %d: assuming transparent\n", 0);
-		child->resource[0] = child->parent->resource[0];
 	}
 
 	res = child->resource[1];
@@ -167,10 +172,6 @@ void __devinit pci_read_bridge_bases(str
 		res->flags = (mem_base_lo & PCI_MEMORY_RANGE_TYPE_MASK) | IORESOURCE_MEM;
 		res->start = base;
 		res->end = limit + 0xfffff;
-	} else {
-		/* See comment above. Same thing */
-		printk(KERN_ERR "Unknown bridge resource %d: assuming transparent\n", 1);
-		child->resource[1] = child->parent->resource[1];
 	}
 
 	res = child->resource[2];
@@ -197,10 +198,6 @@ void __devinit pci_read_bridge_bases(str
 		res->flags = (mem_base_lo & PCI_MEMORY_RANGE_TYPE_MASK) | IORESOURCE_MEM | IORESOURCE_PREFETCH;
 		res->start = base;
 		res->end = limit + 0xfffff;
-	} else {
-		/* See comments above */
-		printk(KERN_ERR "Unknown bridge resource %d: assuming transparent\n", 2);
-		child->resource[2] = child->parent->resource[2];
 	}
 }
 

Re: [patch 2.5.31] transparent PCI-to-PCI bridges

[Linus Torvalds]

On Mon, 26 Aug 2002, Ivan Kokshaysky wrote:
> 
> Ok. Updated patch appended.

Please don't do it like this: I hate code that changes standard PCI data 
structure meaning (in this case the "class" thing) behind peoples back.. 
Some other user might well want to look at the original class member, and 
rewriting it to a magic value just to make the transparency check pass 
sounds like a bad idea to me.

So instead, I would suggest that you add a single-bit "transparent" field
to the PCI structure, and initialize it to "(class & 0xff) == 1" when
initializing the device data. Then, any fixup can just set the transparent
bit to 1.

That would make the code more robust, and more readable in my opionion.

Ok?

		Linus

Re: [patch 2.5.31] transparent PCI-to-PCI bridges

[Benjamin Herrenschmidt]

>> Why not
>> 	if ((dev->class & 0xff) == 0x01)
>> 
>> Is the lowest bit an indicator of subtractive decoding, or is 
>> Progif==0x01 the indicator of subtractive decoding?
>
>The latter.
>
>> The code and the comment should match.
>
>Ok. Updated patch appended.

While we are at it, I still think the loop copying parent resource
pointers in the case of a transparent bridge should copy the 4
resource pointers of the parent and not only 3.
The structure pci_bus has 4 slots, let's copy them all, we really
don't need to care about the fact that the parent is a PCI<->PCI bridge
(using 3 slots), a Cardbus bridge, or a host bridge or whatever wants to
define a slighly different layout for those resources at this point, we
just want _all_ of the parent resources to be copied.

There are archs where host bridges may define 4 resources, I don't
see how it would break anything to take care of copying them all
and not only the first 3 ones ;) I know some code in setup-bus.c
won't cope well with such a layout, but it typically happens on arch
like PPC that don't use setup-bus.

Ben.

Re: [patch 2.5.31] transparent PCI-to-PCI bridges

[Ivan Kokshaysky]

On Mon, Aug 26, 2002 at 10:42:34AM -0700, Linus Torvalds wrote:
> Please don't do it like this: I hate code that changes standard PCI data 
> structure meaning (in this case the "class" thing) behind peoples back.. 

Fairly speaking, I wasn't happy about that either - that's why
I tried to change only a single bit in the first patch.
Note that this stuff was intended for 2.4 in the first place :-)

> So instead, I would suggest that you add a single-bit "transparent" field
> to the PCI structure, and initialize it to "(class & 0xff) == 1" when
> initializing the device data. Then, any fixup can just set the transparent
> bit to 1.
> 
> That would make the code more robust, and more readable in my opionion.
> 
> Ok?

Agreed.
I would even go further and add a "quirks" field to the struct
pci_dev (probably 16 bits would be enough). Thus we can define
specific bits for common PCI bugs - not only QUIRK_TRANSPARENT_BRIDGE
but also things like QUIRK_BROKEN_MWI or even
QUIRK_SOME_COMMON_PCI_IDE_BUG and so on. This would allow generic code
to check a single bit instead of looking through several
vendor/device ID lists.

Thoughts?

Ivan.

Re: [patch 2.5.31] transparent PCI-to-PCI bridges

[Linus Torvalds]

On Wed, 28 Aug 2002, Ivan Kokshaysky wrote:
>
> I would even go further and add a "quirks" field to the struct
> pci_dev (probably 16 bits would be enough).

Why add a bitfield, and not just add single bitmembers?

Other than that, I certainly wouldn't mind. 

		Linus

Re: [patch 2.5.31] transparent PCI-to-PCI bridges

[Ivan Kokshaysky]

On Mon, Aug 26, 2002 at 10:12:24PM +0200, Benjamin Herrenschmidt wrote:
> While we are at it, I still think the loop copying parent resource
> pointers in the case of a transparent bridge should copy the 4
> resource pointers of the parent and not only 3.

I agree that hardcoding the resource numbers is bad.
Instead, I suggest the following:
s/bus->resource[0]/bus->io/
s/bus->resource[1]/bus->mem/
s/bus->resource[2]/bus->pref_mem/
s/bus->resource[3]//

There are only 3 _bus_ resources - the PCI works this way.

Please don't propose your arch specific hacks to generic code.

Ivan.

Re: [patch 2.5.31] transparent PCI-to-PCI bridges

[Benjamin Herrenschmidt]

>On Mon, Aug 26, 2002 at 10:12:24PM +0200, Benjamin Herrenschmidt wrote:
>> While we are at it, I still think the loop copying parent resource
>> pointers in the case of a transparent bridge should copy the 4
>> resource pointers of the parent and not only 3.
>
>I agree that hardcoding the resource numbers is bad.
>Instead, I suggest the following:
>s/bus->resource[0]/bus->io/
>s/bus->resource[1]/bus->mem/
>s/bus->resource[2]/bus->pref_mem/
>s/bus->resource[3]//
>
>There are only 3 _bus_ resources - the PCI works this way.
>
>Please don't propose your arch specific hacks to generic code.

I still don't agree, nothing in the _PCI_ force you to have
only 3 resources. a PCI 2 PCI bridge has 3 resources, here we
agree, but absolutely _nothing_ prevents a host bridge for
exposing more than one memory range, and that DOES happen
in a few real world cases like on pmac.

So we have this situation:

 - Most of the common PCI code can deal with an arbitrary
number & ordering of resources in the pci_bus structure
 - The pci_bus structure already holds 4 resource pointers
 - Copying the all 4 instead of just 3 will not have any
side effect on machines that expose only 2 or 3 host bridge
resources.

I really don't understand your point here. Nothing defines
that a host bridge has to comply with the resource layout of
a PCI<->PCI bridge, why limit ourselves arbitrarily here ?
Especially since we _already_ have 4, not 3 resource slots
in the pci_bus structure...

Ben.

Re: [patch 2.5.31] transparent PCI-to-PCI bridges

[Linus Torvalds]

On Wed, 28 Aug 2002, Benjamin Herrenschmidt wrote:

> >On Mon, Aug 26, 2002 at 10:12:24PM +0200, Benjamin Herrenschmidt wrote:
> >> While we are at it, I still think the loop copying parent resource
> >> pointers in the case of a transparent bridge should copy the 4
> >> resource pointers of the parent and not only 3.
> >
> >I agree that hardcoding the resource numbers is bad.
> >Instead, I suggest the following:
> >s/bus->resource[0]/bus->io/
> >s/bus->resource[1]/bus->mem/
> >s/bus->resource[2]/bus->pref_mem/
> >s/bus->resource[3]//
> >
> >There are only 3 _bus_ resources - the PCI works this way.
> >
> >Please don't propose your arch specific hacks to generic code.
> 
> I still don't agree, nothing in the _PCI_ force you to have
> only 3 resources.

I agree. There is absolutely no reason to artificially limit the "bus" 
structure to only three resoruces (and with hardcoded behaviour at that).

There are examples of bridges that are very common and that can bridge at
least four resources: every single CarsBus bridge does that _today_. Right
now Linux only uses three of the 4 resources, but that's because we've
never needed to use more. The fourth one is allocated in case some cardbus
driver were to want to use it..

In fact, even regular PCI bridges often bridge more than three resources: 
many have things like VGA pass-through etc, which would actually add up to 
at least _five_ regions that they bridge (the regular three, and the added 
VGA IO and MEM regions). Again, Linux doesn't actually care about this 
right now, but again it is absolutely _wrong_ to artificially limit Linux 
internally to some made-up (and wrong) notion of what a bridge is.

In short: if anything, we may at some point make the number of resources
_larger_, not smaller.

		Linus

Re: [patch 2.5.31] transparent PCI-to-PCI bridges

[Benjamin Herrenschmidt]

>I agree. There is absolutely no reason to artificially limit the "bus" 
>structure to only three resoruces (and with hardcoded behaviour at that).
>
> .../...
>
>In short: if anything, we may at some point make the number of resources
>_larger_, not smaller.

What about removing that hard coded 4 where ever it lives, and either
do a #define (that could eventually be overriden by arch) or do a
dynamically allocated array ? If you are ok with that, I'll put that
on my list of things to do when I get back to moving the pmac drivers
to 2.5

Ben.

Re: [patch 2.5.31] transparent PCI-to-PCI bridges

[Ivan Kokshaysky]

On Wed, Aug 28, 2002 at 10:35:38AM -0700, Linus Torvalds wrote:
> I agree. There is absolutely no reason to artificially limit the "bus" 
> structure to only three resoruces (and with hardcoded behaviour at that).

I disagree. There was a very good reason for doing that - you can build
PCI bus tree (up to 256 buses) _only_ with PCI-to-PCI bridges, and I'm
absolutely sure that it will be true until PCI goes EOL.
IMHO, tying the PCI bus to the PCI-to-PCI bridge makes sense -
it's generic and can be used as abstraction.
The code in setup-bus.c is all about that - you can start embedded
system with completely uninitialized PCI and initialize it with
just pci_assign_unassigned_resources(). Don't know why i386 and ppc32
don't use this - on alpha we are using it for years. Oh, well... ;-)
Note that we don't care about "transparent" bridges at all - we
have everything properly allocated in the regular bridge windows.

> There are examples of bridges that are very common and that can bridge at
> least four resources: every single CarsBus bridge does that _today_. Right
> now Linux only uses three of the 4 resources, but that's because we've
> never needed to use more. The fourth one is allocated in case some cardbus
> driver were to want to use it..

True. But the cardbus bridge can handle only one device and the resource
allocation is up to cardbus driver. The only way to extend the bus
is to place the PCI-to-PCI bridge behind a cardbus bridge, and then
we're returning to the starting point.

> In fact, even regular PCI bridges often bridge more than three resources: 
> many have things like VGA pass-through etc, which would actually add up to 
> at least _five_ regions that they bridge (the regular three, and the added 
> VGA IO and MEM regions). Again, Linux doesn't actually care about this 
> right now, but again it is absolutely _wrong_ to artificially limit Linux 
> internally to some made-up (and wrong) notion of what a bridge is.

Hmm. Regular bridges also have ISA mode; should we have 64(!) IO resources
then it's enabled?
I think that PCIBIOS_MIN_[IO,MEM] and pcibios_align_resource handle this
legacy crap just nicely.

> In short: if anything, we may at some point make the number of resources
> _larger_, not smaller.

Well, the pci_bus itself does not have any own resources - there are 4
pointers to the real resources of the pci controller (host, pci or cardbus
bridge etc.)
So I suggest leaving only _one_ pointer, but to an array of resources
(either pci_dev or arch specific). This will work.

Not that I'm quite happy with this right now.

I can't consider Ben's request for 4th resource slot as an argument because
- he's trying to build child<->parent relationship on a resource
  level between CPU and PCI address spaces. Which is generally impossible
  (consider sparse addressing on alpha or io on parisc)
- probably I have problems counting to 4 - as Ben said in earlier threads,
  the ppc32 host bridge has 1 io and 2 memory ranges. ;-)

Ivan.

Re: [patch 2.5.31] transparent PCI-to-PCI bridges

[Benjamin Herrenschmidt]

>On Wed, Aug 28, 2002 at 10:35:38AM -0700, Linus Torvalds wrote:
>> I agree. There is absolutely no reason to artificially limit the "bus" 
>> structure to only three resoruces (and with hardcoded behaviour at that).
>
>I disagree. There was a very good reason for doing that - you can build
>PCI bus tree (up to 256 buses) _only_ with PCI-to-PCI bridges, and I'm
>absolutely sure that it will be true until PCI goes EOL.
>IMHO, tying the PCI bus to the PCI-to-PCI bridge makes sense -
>it's generic and can be used as abstraction.
>The code in setup-bus.c is all about that - you can start embedded
>system with completely uninitialized PCI and initialize it with
>just pci_assign_unassigned_resources(). Don't know why i386 and ppc32
>don't use this - on alpha we are using it for years. Oh, well... ;-)
>Note that we don't care about "transparent" bridges at all - we
>have everything properly allocated in the regular bridge windows.

THen why did the embedded folks at montavista redid their own
pci_auto version (it Marcelo tree since latest merge) ? Not that
I agree with them here, I haven't digged into that code yet and
I'm pretty sure it could ultimately be replaced by setup-bus, but
in it's current incarnation, setup-bus is definitely not suitable
for PPC32. I must also make sure on ppc32's like pmac and CHRP that
some critical devices do _NOT_ get moved around by the PCI code.

Then, _again_, regarding your limit of pci_bus to the capabiliti
of a P2P bridge, I would have no problem with that except that
you are also limiting the host bridge, and my problem is with
the host bridge.

Nothingin the PCI spec prevents my host bridge from declaring
scattered regions (and that is the case on some PPCs). So the
pci_bus structure of the root of that PCI domain must have
one resource per region, and wanting to absolutely stick the
layout of a P2P bridge to it makes no sense to me. (Both
the amount of resources in the pci_bus structure, but also
relying on the fixed ordering).

If I have a transparent PCI bridge connected to such a host
that expose several scattered regions, I, of course, want
that transparent bridge to copy down pointers to all parent
resources.

Of course, a "normal" P2P bridge would only expose 3 resources,
there is no problem with that.

I don't see why you insist on picking the P2P bridge as the basis
of the definition of a pci_bus structure. It's a limiting definition,
and again, the code wouldn't be much more complex, more bloated or
whatever by beeing slightly more generic...
>
>> There are examples of bridges that are very common and that can bridge at
>> least four resources: every single CarsBus bridge does that _today_. Right
>> now Linux only uses three of the 4 resources, but that's because we've
>> never needed to use more. The fourth one is allocated in case some cardbus
>> driver were to want to use it..
>
>True. But the cardbus bridge can handle only one device and the resource
>allocation is up to cardbus driver. The only way to extend the bus
>is to place the PCI-to-PCI bridge behind a cardbus bridge, and then
>we're returning to the starting point.

Well, I would argue why bother re-configuring a bridge that would have
properly been configured by the firmware in the first place, but
well... :)

>> In fact, even regular PCI bridges often bridge more than three resources: 
>> many have things like VGA pass-through etc, which would actually add up to 
>> at least _five_ regions that they bridge (the regular three, and the added 
>> VGA IO and MEM regions). Again, Linux doesn't actually care about this 
>> right now, but again it is absolutely _wrong_ to artificially limit Linux 
>> internally to some made-up (and wrong) notion of what a bridge is.
>
>Hmm. Regular bridges also have ISA mode; should we have 64(!) IO resources
>then it's enabled?
>I think that PCIBIOS_MIN_[IO,MEM] and pcibios_align_resource handle this
>legacy crap just nicely.

>> In short: if anything, we may at some point make the number of resources
>> _larger_, not smaller.
>
>Well, the pci_bus itself does not have any own resources - there are 4
>pointers to the real resources of the pci controller (host, pci or cardbus
>bridge etc.)
>So I suggest leaving only _one_ pointer, but to an array of resources
>(either pci_dev or arch specific). This will work.

And a count then... but that would work, yes.

>Not that I'm quite happy with this right now.
>
>I can't consider Ben's request for 4th resource slot as an argument because
>- he's trying to build child<->parent relationship on a resource
>  level between CPU and PCI address spaces. Which is generally impossible
>  (consider sparse addressing on alpha or io on parisc)

Hrm... Even if I did that, why wouldn't you consider a change making the
generic code slightly more generic to acomodate my construction ? You
would reject it just because your arch can't do the same ? :) Anyway,
While we do have root (CPU) memory & IO resources as parent of all resources
on PPC, it is not the reason why I need this 4th resource. It's for
describing the host bridge pci_bus of a given bus domain, the fact that
these host bridge resources are parented to root CPU resources isn't
relevant here

>- probably I have problems counting to 4 - as Ben said in earlier threads,
>  the ppc32 host bridge has 1 io and 2 memory ranges. ;-)

No, the pmac bridge has 1 IO and N memory ranges. It appears that in most
cases I dealt with on machines I actually own, I could coalesce the ranges
provided by the firmware to 1 IO and 2 memory (though those 2 memory are
both non-prefetchable), I know that when I wrote that code, I had a couple
of reports of users with problems because they had more.
Our arch code can and will eventually define host bridges with 4 resources
(and potentially more if we allowed it to). 

However, I do agree that hard-coding "4" (or "3") is no good here. Why not
just #define the number of resource slots of a pci_bus structure ? Couldn't
it be done so asm-xxx/pci.h can override it ?

>Ivan.

Re: [patch 2.5.31] transparent PCI-to-PCI bridges

[Linus Torvalds]

On Fri, 30 Aug 2002, Ivan Kokshaysky wrote:
> 
> I disagree. There was a very good reason for doing that - you can build
> PCI bus tree (up to 256 buses) _only_ with PCI-to-PCI bridges

Your logic is so flawed that it's scary.

Your logic is "I _can_ do it this way, thus everybody else must do it this
way". Where the _hell_ is the logic there?

I pointed out a _fact_: there are PCI bridges that bridge more than 3 
resources. Argue with that until you turn blue in the face, I don't care. 
I'm not talking about theory, I'm talking about bridges that I _have_ in 
my machines, and that may have been set up by the firmware. 

The fact that you could have built hardware and firmware that didn't have 
more than three resources is _immaterial_, since it has nothing to do with 
the realities of today.

		Linus

Re: [patch 2.5.31] transparent PCI-to-PCI bridges

[Benjamin Herrenschmidt]

>Obviously, this window + 128M range of the PCI card won't fit in the
>256M region of the host bridge. So, the only way to get all this working
>is to set AGP bridge window such that it overlaps both regions
>of the host bridge. Like this:
>0x80000000-0x87ffffff           - PCI card, fb
>0x88000000-0xf00fffff           - AGP bridge, memory window
>        0x88000000-0x8fffffff   - AGP card, fb
>        0x90000000-0xefffffff   - empty
>        0xf0000000-0xf0000fff   - AGP card, MMIO
>0xf0100000-0xf0100fff           - PCI card, MMIO
>
>I don't see how you can cope with it using 2 separate bus resources -
>the memory resource of the AGP bridge wouldn't have a valid parent.
>OTOH, if you expose a _single_ memory resource of the host bridge
>(0x80000000-0xf0ffffff) and write your pcibios_align_resource()
>such that it won't allow allocation of non-bridge resources in the
>0x90000000-0xefffffff range, everything is fine.
>
>Please prove me that I'm wrong. :-)

First, a simple problem: You are showing a possible problem caused
by a given PCI host & bridge setup. That's not my point. My point
is that I _do_ have setups with N MMIO regions and want the kernel
to be able to deal with that. I'm not introducing any limitation to
the code, I want the code to be generic enough to cope with a setup
that exist (as the host is configured by my firmware).

Also, in your example, if I expose a single memory resource, then I
lie since the host bridge in this example would not forward addresses
"between" the 2 ranges, thus the kernel would potentially allocate
space for unassigned devices in that non-decoded range.

I want my host pci_bus structure to expose what it is really forwarding.
That's as simple as that. If your host is configured in a more "sane",
way, then good. I'm not forcing anybody to have 2 MMIO regions ;)
I just want that dawn code to deal with cases where I do have them
(or more). Again, that code isn't about setting up the bus, it's
about coping with an existing setup when building the resource tree.

Regarding your above example, it just don't happen in real life.
First, we have AGP as a separate PCI host domain on pmac ;) Then,
the firmware can configures host bridges with large enough regions
to deal with what is needed by the card.

Ben.

[patch 2.5.32] transparent PCI-to-PCI bridges

[Ivan Kokshaysky]

On Tue, Aug 27, 2002 at 06:29:23PM -0700, Linus Torvalds wrote:
> Why add a bitfield, and not just add single bitmembers?

Modified as you suggested. For now, "transparent" is the only field
added to the struct pci_dev. Adding other fields certainly looks
promising, but it will take some time...

Ivan.

--- 2.5.32/drivers/pci/quirks.c	Tue Aug 27 23:26:32 2002
+++ linux/drivers/pci/quirks.c	Fri Aug 30 18:32:32 2002
@@ -471,6 +471,11 @@ static void __init quirk_dunord ( struct
 	r -> end = 0xffffff;
 }
 
+static void __init quirk_transparent_bridge(struct pci_dev *dev)
+{
+	dev->transparent = 1;
+}
+
 /*
  *  The main table of quirks.
  */
@@ -525,6 +530,13 @@ static struct pci_fixup pci_fixups[] __i
 
 	{ PCI_FIXUP_FINAL, 	PCI_VENDOR_ID_AMD,	PCI_DEVICE_ID_AMD_VIPER_7410,	quirk_amd_ioapic },
 	{ PCI_FIXUP_FINAL,	PCI_VENDOR_ID_AMD,	PCI_DEVICE_ID_AMD_FE_GATE_700C, quirk_amd_ordering },
+	/*
+	 * i82380FB mobile docking controller: its PCI-to-PCI bridge
+	 * is subtractive decoding (transparent), and does indicate this
+	 * in the ProgIf. Unfortunately, the ProgIf value is wrong - 0x80
+	 * instead of 0x01.
+	 */
+	{ PCI_FIXUP_HEADER,	PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_82380FB,	quirk_transparent_bridge },
 
 	{ 0 }
 };
--- 2.5.32/drivers/pci/probe.c	Tue Aug 27 23:26:36 2002
+++ linux/drivers/pci/probe.c	Fri Aug 30 18:20:44 2002
@@ -128,6 +128,13 @@ void __devinit pci_read_bridge_bases(str
 	if (!dev)		/* It's a host bus, nothing to read */
 		return;
 
+	if (dev->transparent) {
+		printk("Transparent bridge - %s\n", dev->name);
+		for(i = 0; i < 3; i++)
+			child->resource[i] = child->parent->resource[i];
+		return;
+	}
+
 	for(i=0; i<3; i++)
 		child->resource[i] = &dev->resource[PCI_BRIDGE_RESOURCES+i];
 
@@ -149,13 +156,6 @@ void __devinit pci_read_bridge_bases(str
 		res->flags = (io_base_lo & PCI_IO_RANGE_TYPE_MASK) | IORESOURCE_IO;
 		res->start = base;
 		res->end = limit + 0xfff;
-	} else {
-		/*
-		 * Ugh. We don't know enough about this bridge. Just assume
-		 * that it's entirely transparent.
-		 */
-		printk(KERN_ERR "Unknown bridge resource %d: assuming transparent\n", 0);
-		child->resource[0] = child->parent->resource[0];
 	}
 
 	res = child->resource[1];
@@ -167,10 +167,6 @@ void __devinit pci_read_bridge_bases(str
 		res->flags = (mem_base_lo & PCI_MEMORY_RANGE_TYPE_MASK) | IORESOURCE_MEM;
 		res->start = base;
 		res->end = limit + 0xfffff;
-	} else {
-		/* See comment above. Same thing */
-		printk(KERN_ERR "Unknown bridge resource %d: assuming transparent\n", 1);
-		child->resource[1] = child->parent->resource[1];
 	}
 
 	res = child->resource[2];
@@ -197,10 +193,6 @@ void __devinit pci_read_bridge_bases(str
 		res->flags = (mem_base_lo & PCI_MEMORY_RANGE_TYPE_MASK) | IORESOURCE_MEM | IORESOURCE_PREFETCH;
 		res->start = base;
 		res->end = limit + 0xfffff;
-	} else {
-		/* See comments above */
-		printk(KERN_ERR "Unknown bridge resource %d: assuming transparent\n", 2);
-		child->resource[2] = child->parent->resource[2];
 	}
 }
 
@@ -388,6 +380,10 @@ int pci_setup_device(struct pci_dev * de
 	case PCI_HEADER_TYPE_BRIDGE:		    /* bridge header */
 		if (class != PCI_CLASS_BRIDGE_PCI)
 			goto bad;
+		/* The PCI-to-PCI bridge spec requires that subtractive
+		   decoding (i.e. transparent) bridge must have programming
+		   interface code of 0x01. */ 
+		dev->transparent = ((class & 0xff) == 1);
 		pci_read_bases(dev, 2, PCI_ROM_ADDRESS1);
 		break;
 
--- 2.5.32/arch/i386/pci/fixup.c	Tue Aug 27 23:26:35 2002
+++ linux/arch/i386/pci/fixup.c	Fri Aug 30 18:23:04 2002
@@ -166,6 +166,22 @@ static void __init pci_fixup_via_northbr
 	}
 }
 
+/*
+ * For some weird reasons Intel decided that certain parts of their
+ * 815, 845 and some other chipsets must look like PCI-to-PCI bridges
+ * while they are obviously not. The 82801 family (AA, AB, BAM/CAM,
+ * BA/CA/DB and E) PCI bridges are actually HUB-to-PCI ones, according
+ * to Intel terminology. These devices do forward all addresses from
+ * system to PCI bus no matter what are their window settings, so they are
+ * "transparent" (or subtractive decoding) from programmers point of view.
+ */
+static void __init pci_fixup_transparent_bridge(struct pci_dev *dev)
+{
+	if ((dev->class >> 8) == PCI_CLASS_BRIDGE_PCI &&
+	    (dev->device & 0xff00) == 0x2400)
+		dev->transparent = 1;
+}
+
 struct pci_fixup pcibios_fixups[] = {
 	{ PCI_FIXUP_HEADER,	PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_82451NX,	pci_fixup_i450nx },
 	{ PCI_FIXUP_HEADER,	PCI_VENDOR_ID_INTEL,	PCI_DEVICE_ID_INTEL_82454GX,	pci_fixup_i450gx },
@@ -183,5 +199,6 @@ struct pci_fixup pcibios_fixups[] = {
 	{ PCI_FIXUP_HEADER,	PCI_VENDOR_ID_VIA,	PCI_DEVICE_ID_VIA_8361,	        pci_fixup_via_northbridge_bug },
 	{ PCI_FIXUP_HEADER,	PCI_VENDOR_ID_VIA,	PCI_DEVICE_ID_VIA_8367_0,	pci_fixup_via_northbridge_bug },
 	{ PCI_FIXUP_HEADER,	PCI_VENDOR_ID_NCR,	PCI_DEVICE_ID_NCR_53C810,	pci_fixup_ncr53c810 },
+	{ PCI_FIXUP_HEADER,	PCI_VENDOR_ID_INTEL,	PCI_ANY_ID,			pci_fixup_transparent_bridge },
 	{ 0 }
 };
--- 2.5.32/include/linux/pci.h	Tue Aug 27 23:26:42 2002
+++ linux/include/linux/pci.h	Fri Aug 30 18:08:09 2002
@@ -385,6 +385,9 @@ struct pci_dev {
 	int		ro;		/* ISAPnP: read only */
 	unsigned short	regs;		/* ISAPnP: supported registers */
 
+	/* These fields are used by common fixups */
+	unsigned short	transparent:1;	/* Transparent PCI bridge */
+
 	int (*prepare)(struct pci_dev *dev);	/* ISAPnP hooks */
 	int (*activate)(struct pci_dev *dev);
 	int (*deactivate)(struct pci_dev *dev);

Re: [patch 2.5.31] transparent PCI-to-PCI bridges

[Ivan Kokshaysky]

On Fri, Aug 30, 2002 at 11:28:25AM +0200, Benjamin Herrenschmidt wrote:
> THen why did the embedded folks at montavista redid their own
> pci_auto version (it Marcelo tree since latest merge) ? Not that
> I agree with them here, I haven't digged into that code yet and
> I'm pretty sure it could ultimately be replaced by setup-bus, but
> in it's current incarnation, setup-bus is definitely not suitable
> for PPC32. I must also make sure on ppc32's like pmac and CHRP that
> some critical devices do _NOT_ get moved around by the PCI code.

Indeed, 2.4 isn't usable for that. My fault - I have yet to back port
2.5 setup-bus stuff...

> I don't see why you insist on picking the P2P bridge as the basis
> of the definition of a pci_bus structure. It's a limiting definition,
> and again, the code wouldn't be much more complex, more bloated or
> whatever by beeing slightly more generic...

And less functional, unless I'm missing something fundamental.

Consider the following simplified cofiguration (not quite real life,
but very close).
Host bridge (similar to pmac, I think :-) with 2 memory regions:
256M at 0x80000000 and 16M at 0xf0000000. There is also PCI or, even better,
AGP bridge, non-transparent; two graphic cards, one PCI and other AGP, with
2 memory resources each: 128M framebuffer and 4K of MMIO registers.
The memory window of the AGP bridge must be at least 129M (128M + 4K rounded
up to 1M).
Obviously, this window + 128M range of the PCI card won't fit in the
256M region of the host bridge. So, the only way to get all this working
is to set AGP bridge window such that it overlaps both regions
of the host bridge. Like this:
0x80000000-0x87ffffff           - PCI card, fb
0x88000000-0xf00fffff           - AGP bridge, memory window
        0x88000000-0x8fffffff   - AGP card, fb
        0x90000000-0xefffffff   - empty
        0xf0000000-0xf0000fff   - AGP card, MMIO
0xf0100000-0xf0100fff           - PCI card, MMIO

I don't see how you can cope with it using 2 separate bus resources -
the memory resource of the AGP bridge wouldn't have a valid parent.
OTOH, if you expose a _single_ memory resource of the host bridge
(0x80000000-0xf0ffffff) and write your pcibios_align_resource()
such that it won't allow allocation of non-bridge resources in the
0x90000000-0xefffffff range, everything is fine.

Please prove me that I'm wrong. :-)

Ivan.

Re: [patch 2.5.31] transparent PCI-to-PCI bridges

[Ivan Kokshaysky]

On Fri, Aug 30, 2002 at 10:12:32AM -0700, Linus Torvalds wrote:
> Your logic is so flawed that it's scary.

Maybe.

> Your logic is "I _can_ do it this way, thus everybody else must do it this
> way". Where the _hell_ is the logic there?

Well, I'm just too lazy and don't want to rewrite that setup-bus stuff
once again. :-)
Seriously, I see some implementation issues with "arbitrary number of
resources" approach. By now I don't know how to deal with them.

Ivan.

Re: [patch 2.5.31] transparent PCI-to-PCI bridges

[Benjamin Herrenschmidt]

>> Your logic is so flawed that it's scary.
>
>Maybe.
>
>> Your logic is "I _can_ do it this way, thus everybody else must do it this
>> way". Where the _hell_ is the logic there?
>
>Well, I'm just too lazy and don't want to rewrite that setup-bus stuff
>once again. :-)
>Seriously, I see some implementation issues with "arbitrary number of
>resources" approach. By now I don't know how to deal with them.

Ok, well, you should have said that first ;)

Because what I'm asking (the change to pci_read_bridge_bases()) for
copying all transparent bridge resources will just not affect setup-bus,
so you shouldn't have a problem with it. Currently, setup-bus cannot deal
with my hosts already, copying all 4 resources won't make this worse ;)
If your arch can use setup-bus today, it won't be harmed as it won't
have anything in that 4th resource anyway.

Now, regarding improving setup-bus to deal with such weird cases and stop
relying on fixed resource layout, well... that will be my job or the
job for whoever wants to make it work on PPC (or other arch with similar
need). It's something I have on my long-term todolist, but no time to
dedicate to yet right now.

Ben.

Re: [patch 2.5.31] transparent PCI-to-PCI bridges

[Benjamin Herrenschmidt]

>Obviously, this window + 128M range of the PCI card won't fit in the
>256M region of the host bridge. So, the only way to get all this working
>is to set AGP bridge window such that it overlaps both regions
>of the host bridge. Like this:
>0x80000000-0x87ffffff           - PCI card, fb
>0x88000000-0xf00fffff           - AGP bridge, memory window
>        0x88000000-0x8fffffff   - AGP card, fb
>        0x90000000-0xefffffff   - empty
>        0xf0000000-0xf0000fff   - AGP card, MMIO
>0xf0100000-0xf0100fff           - PCI card, MMIO
>
>I don't see how you can cope with it using 2 separate bus resources -
>the memory resource of the AGP bridge wouldn't have a valid parent.
>OTOH, if you expose a _single_ memory resource of the host bridge
>(0x80000000-0xf0ffffff) and write your pcibios_align_resource()
>such that it won't allow allocation of non-bridge resources in the
>0x90000000-0xefffffff range, everything is fine.
>
>Please prove me that I'm wrong. :-)

First, a simple problem: You are showing a possible problem caused
by a given PCI host & bridge setup. That's not my point. My point
is that I _do_ have setups with N MMIO regions and want the kernel
to be able to deal with that. I'm not introducing any limitation to
the code, I want the code to be generic enough to cope with a setup
that exist (as the host is configured by my firmware).

Also, in your example, if I expose a single memory resource, then I
lie since the host bridge in this example would not forward addresses
"between" the 2 ranges, thus the kernel would potentially allocate
space for unassigned devices in that non-decoded range.

I want my host pci_bus structure to expose what it is really forwarding.
That's as simple as that. If your host is configured in a more "sane",
way, then good. I'm not forcing anybody to have 2 MMIO regions ;)
I just want that dawn code to deal with cases where I do have them
(or more). Again, that code isn't about setting up the bus, it's
about coping with an existing setup when building the resource tree.

Regarding your above example, it just don't happen in real life.
First, we have AGP as a separate PCI host domain on pmac ;) Then,
the firmware can configures host bridges with large enough regions
to deal with what is needed by the card.

Ben.

Re: [patch 2.5.31] transparent PCI-to-PCI bridges

[Ivan Kokshaysky]

On Fri, Aug 30, 2002 at 10:19:58PM +0200, Benjamin Herrenschmidt wrote:
> First, a simple problem: You are showing a possible problem caused
> by a given PCI host & bridge setup. That's not my point. My point
> is that I _do_ have setups with N MMIO regions and want the kernel
> to be able to deal with that.

It's just an example. Give me real numbers and addresses and I'll show
you configuration with _real_ hardware which might work, but won't
with your approach.

> I'm not introducing any limitation to
> the code, I want the code to be generic enough to cope with a setup
> that exist (as the host is configured by my firmware).

You won't allow windows of the PCI bridge to overlap multiple ranges decoded
by the host bridge - it's a serious limitation, I think.

> Also, in your example, if I expose a single memory resource, then I
> lie since the host bridge in this example would not forward addresses
> "between" the 2 ranges, thus the kernel would potentially allocate
> space for unassigned devices in that non-decoded range.

Nope.  Arch specific pcibios_align_resource is called on every allocation
and should take care of this - read the code.

> I want my host pci_bus structure to expose what it is really forwarding.
> That's as simple as that. If your host is configured in a more "sane",
> way, then good.

Actually, no. Some old alphas have several (8, IIRC) PCI memory windows
with a 8Mb "hole" in each.

> Regarding your above example, it just don't happen in real life.
> First, we have AGP as a separate PCI host domain on pmac ;) Then,
> the firmware can configures host bridges with large enough regions
> to deal with what is needed by the card.

Pmac firmware is really cool then. ;-) Video cards with 256M memory
regions are quite common and 512M ones already exist.

Ivan.

Re: [patch 2.5.31] transparent PCI-to-PCI bridges

[Ivan Kokshaysky]

On Sat, Aug 31, 2002 at 10:09:42AM +0200, Benjamin Herrenschmidt wrote:
> >Seriously, I see some implementation issues with "arbitrary number of
> >resources" approach. By now I don't know how to deal with them.
> 
> Ok, well, you should have said that first ;)

:-)
Well, probably I shouldn't care about it at all. The setup-bus.c could
be considered as PCI-to-PCI bridge driver which deals _only_ with
certain type of devices with fixed resource layout. If you can
make your host bridge look like this, fine. You'll be able to use some
nice tricks like reallocating host bridge resources.
If not, it's your problem. But you're still able to use routines from
setup-bus.c for any bus behind the PCI-PCI bridges.

> 
> Because what I'm asking (the change to pci_read_bridge_bases()) for
> copying all transparent bridge resources will just not affect setup-bus,
> so you shouldn't have a problem with it. Currently, setup-bus cannot deal
> with my hosts already, copying all 4 resources won't make this worse ;)

Ok, ok, I give up. :-)

> If your arch can use setup-bus today, it won't be harmed as it won't
> have anything in that 4th resource anyway.

True. Probably it's ok for 2.4, but it would be better to have something
sane for 2.5. We have already agreed that 4 is not much better than 3.
A single resource pointer and resource count fields would fork fine for
PCI and CardBus bridges, but will break most platforms where root bus
resources are randomly allocated structures.
I tend to agree about #define.

Ivan.

Re: [patch 2.5.31] transparent PCI-to-PCI bridges

[Alan Cox]

Related question while we are on the subject of bridges. I'm trying to
work out a clean way to initialize a new subtree of devices given a
bridge that suddenely has devices behind it.

This occurs in three cases I know about now 
- Easidock cardbus->PCI extender
- IBM Thinkpad hot docking bridge
- Magma PCI extended split bridge

Re: [patch 2.5.31] transparent PCI-to-PCI bridges

[Linus Torvalds]

On Sat, 31 Aug 2002, Ivan Kokshaysky wrote:
> 
> Well, I'm just too lazy and don't want to rewrite that setup-bus stuff
> once again. :-)

I'm not so much worried about setup_bus(), I worry more about special 
cases. It's easier to handle some "extended bridge" (like CardBus) if you 
already have a notion of a "generic set of resources" and don't get too 
hung up about how a standard PCI bridge looks.

		Linus

Re: [patch 2.5.31] transparent PCI-to-PCI bridges

[Linus Torvalds]

On 31 Aug 2002, Alan Cox wrote:
>
> Related question while we are on the subject of bridges. I'm trying to
> work out a clean way to initialize a new subtree of devices given a
> bridge that suddenely has devices behind it.
> 
> This occurs in three cases I know about now 
> - Easidock cardbus->PCI extender
> - IBM Thinkpad hot docking bridge
> - Magma PCI extended split bridge

pci_do_scan_bus() should do almost everything for you. Pat Mochel had some
code that made the cardbus driver basically do just this on cardbus
insertion, you might ask him.

		Linus

Re: [patch 2.5.31] transparent PCI-to-PCI bridges

[Ivan Kokshaysky]

On Sat, Aug 31, 2002 at 09:49:14AM -0700, Linus Torvalds wrote:
> 
> On 31 Aug 2002, Alan Cox wrote:
> >
> > Related question while we are on the subject of bridges. I'm trying to
> > work out a clean way to initialize a new subtree of devices given a
> > bridge that suddenely has devices behind it.
> > 
> > This occurs in three cases I know about now 
> > - Easidock cardbus->PCI extender
> > - IBM Thinkpad hot docking bridge
> > - Magma PCI extended split bridge
> 
> pci_do_scan_bus() should do almost everything for you. Pat Mochel had some
> code that made the cardbus driver basically do just this on cardbus
> insertion, you might ask him.

I guess Pat's code has something to do with a resource allocation
(haven't seen it though).
I would play with following if I have the hardware:
{
	pci_do_scan_bus(bus);
	pbus_size_bridges(bus);
	pbus_assign_resources(bus);
}
Obviously, __init qualifiers should be changed to __devinit for pbus_*
and other stuff in setup-bus.c and setup-res.c.

Ivan.