Problem with pci_fixups in drivers/pci/probe.c

Problem with pci_fixups in drivers/pci/probe.c

[alan]

I have an HP zv5200z laptop running a x86_64 kernel.

In order to get cardbus devices to work, I had to patch the kernel to get 
the cardbus to actually see those devices.  (We will call this patch 
"cardbus.patch".)

I went to patch the current FC5T1 kernel and found that the patch no 
longer applied.  On further investigation I found that the patch had been 
added to the kernel, but some helpful soul had added a subroutine that 
made it absolutely worthless to anyone on x86 architecture.

in drivers/pci/probe.c at line 408 there is the following code:

        /* Attempts to fix that up are really dangerous unless
           we're going to re-assign all bus numbers. */
        if (!pcibios_assign_all_busses())
                return;

The problem with this is that for x86 and x86_64 this function is defined 
as 0.

include/asm-x86_64/pci.h:18:#define pcibios_assign_all_busses() 0
include/asm-i386/pci.h:14:extern unsigned int pcibios_assign_all_busses(void);
include/asm-i386/pci.h:16:#define pcibios_assign_all_busses()   0
include/asm-ia64/pci.h:18:#define pcibios_assign_all_busses()     0

This "fix" makes the patch absolutely useless to me.

Is there a way to get this to only run the fixup where needed or to make 
this patch less problematic or just remove the test entirely?

My cardbus devices will thank you.

-- 
"George W. Bush -- Bringing back the Sixties one Nixon at a time."

Re: Problem with pci_fixups in drivers/pci/probe.c

[Dominik Brodowski]

Hi,

On Thu, Jan 05, 2006 at 03:46:35PM -0800, alan wrote:
> I went to patch the current FC5T1 kernel and found that the patch no 
> longer applied.  On further investigation I found that the patch had been 
> added to the kernel, but some helpful soul had added a subroutine that 
> made it absolutely worthless to anyone on x86 architecture.

See
http://www.kernel.org/git/gitweb.cgi?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=12f44f46bc9c6dc79255e661b085797af395d8da
and
http://bugzilla.kernel.org/show_bug.cgi?id=5557
for more information on this issue.

> My cardbus devices will thank you.

Well, you can add "pci=assign-busses" as boot parameter, and then the
cardbus devices should work. Alternatively, I'd propose making
pcibios_assign_all_busses default to 1 on x86-64 (at least), possibly also on
x86.

	Dominik

Re: Problem with pci_fixups in drivers/pci/probe.c

[Alan Cox]

On Iau, 2006-01-05 at 15:46 -0800, alan wrote:
> include/asm-x86_64/pci.h:18:#define pcibios_assign_all_busses() 0
> include/asm-i386/pci.h:14:extern unsigned int pcibios_assign_all_busses(void);
> include/asm-i386/pci.h:16:#define pcibios_assign_all_busses()   0
> include/asm-ia64/pci.h:18:#define pcibios_assign_all_busses()     0
> 
> This "fix" makes the patch absolutely useless to me.

Your system work sif this is set to 1 ?

If so you might want to use the DMI layer to make that function return 0
except for matches on problem systems.

Alan

[PATCH] PCI/Cardbus cards hidden, needs pci=assign-busses to fix

[Bernhard Kaindl]

Hi,

"In some cases, especially on modern laptops with a lot of PCI and cardbus
bridges, we're unable to assign correct secondary/subordinate bus numbers
to all cardbus bridges due to BIOS limitations unless we are using
"pci=assign-busses" boot option." -- Ivan Kokshaysky (from a patch comment)

Without it, Cardbus cards inserted are never seen by PCI because the
parent PCI-PCI Bridge of the Cardbus bridge will not pass and translate
Type 1 PCI configuration cycles correctly and the system will fail to
find and initialise the PCI devices in the system.

Reference: PCI-PCI Bridges: PCI Configuration Cycles and PCI Bus Numbering:
http://www.science.unitn.it/~fiorella/guidelinux/tlk/node72.html

The reason for this is that:
 ``All PCI busses located behind a PCI-PCI bridge must reside between the
secondary bus number and the subordinate bus number (inclusive).''

"pci=assign-busses" makes pcibios_assign_all_busses return 1 and this
turns on PCI renumbering during PCI probing.

Alan suggested to use DMI to make that function cause that on problem systems:

On Fri, 6 Jan 2006, Alan Cox wrote:
[...]
> 
> Your system works if this is set to 1 ?
> 
> If so you might want to use the DMI layer to make that function return 0
> except for matches on problem systems.
> 
> Alan

It is trivial to do, simply match the problem systems using DMI and
set the PCI_ASSIGN_ALL_BUSSES bit which is returned by the function:

This is the initial, working patch which I put together using cut and paste
from ACPI init where it is used for ACPI blacklisting:

--- arch/i386/pci/common.c
+++ arch/i386/pci/common.c
@@ -8,6 +8,7 @@
 #include <linux/pci.h>
 #include <linux/ioport.h>
 #include <linux/init.h>
+#include <linux/dmi.h>
 
 #include <asm/acpi.h>
 #include <asm/segment.h>
@@ -120,11 +121,40 @@
 	pci_read_bridge_bases(b);
 }
 
+/*
+ * Enable renumbering of PCI bus# ranges to reach all PCI busses (Cardbus)
+ */
+static int __init assign_all_busses(struct dmi_system_id *d)
+{
+	pci_probe |= PCI_ASSIGN_ALL_BUSSES;
+		printk(KERN_INFO "%s detected: enabling PCI bus# renumbering"
+			" (pci=assign-busses)\n", d->ident);
+	return 0;
+}
+
+static struct dmi_system_id __initdata pciprobe_dmi_table[] = {
+	/*
+	 * Laptops which need pci=assign-busses to see Cardbus cards
+	 */
+#ifdef __i386__
+	{
+	 .callback = assign_all_busses,
+	 .ident = "Samsung X20 Laptop",
+	 .matches = {
+		     DMI_MATCH(DMI_SYS_VENDOR, "Samsung Electronics"),
+		     DMI_MATCH(DMI_PRODUCT_NAME, "SX20S"),
+		     },
+	 },
+#endif				/* __i386__ */
+	{}
+};
 
 struct pci_bus * __devinit pcibios_scan_root(int busnum)
 {
 	struct pci_bus *bus = NULL;
 
+	dmi_check_system(pciprobe_dmi_table);
+
 	while ((bus = pci_find_next_bus(bus)) != NULL) {
 		if (bus->number == busnum) {
 			/* Already scanned */

Straightforward, I just copied everything from kernel/acpi/boot.c.

The only question for me was where to put it. I put it directly before
scanning PCI bus into pcibios_scan_root() because it's called from legacy,
acpi and numa and so it can be one place for all systems and configurations
which may need it.

AMD64 Laptops are also affected and fixed by assign-busses, and the code is
also incuded from arch/x86_64/pci/ that place will also work for x86_64 kernels,
I only ifdef'-ed the x86-only Laptop in this example.

Comments? - Can it be included in the next kernels?

Thanks,
Bernhard

PS:

Affected and known or assumed to be fixed with it are (found by googling):

* ASUS Z71V and L3s
* Samsung X20
* Compaq R3140us and all Compaq R3000 series laptops with TI1620 Controller,
  also Compaq R4000 series (from a kernel.org bugreport)
* HP zv5000z (AMD64 3700+, known that fixup_parent_subordinate_busnr fixes it)
* HP zv5200z
* IBM ThinkPad 240
* An IBM ThinkPad (1.8 GHz Pentium M) debugged by Pavel Machek
  gives the correspondig message which detects the possible problem.
* MSI S260 / Medion SIM 2100 MD 95600

Russell King wrote once:
>
> The assumption that the PCI BIOS will sanely assign the PCI bus numbers
> and that Linux does not need to reassign them is looking increasingly
> incorrect - most of the Cardbus "why can't the system see my card"
> are resolved by passing "pci=assign-busses", which causes the PCI
> subsystem to renumber all PCI busses.
>
> So far, no one who has tried this solution has reported any additional
> problems that I'm aware of.

Of course, I would not want to do this at this stage of the release.

To know if pci=assign-busses and the DMI check could fix your cardus,
you have two possibiltes. With kernels newer than 2.6.13.3, you look
closely at your boot messages (or dmesg output if they are still there)
to check if a message like this is there:

PCI: Bus #10 (-#13) may be hidden behind transparent bridge #02 (-#0f) (try 'pci=assign-busses') 

If you are running an older kernel, you can check your lspci output:

lspci -v| grep -e subor -e ^0|grep -B1 subor

If it looks like (output reduced to be more readable):

0000:00:1c.0 PCI bridge: PCI Express
        Bus: primary=00, secondary=02, subordinate=02

0000:00:1e.0 PCI bridge: Intel 82801 Mobile PCI Bridge
        Bus: primary=00, secondary=06, subordinate=06

0000:06:09.0 CardBus bridge: Ricoh Co Ltd RL5c476 II
        Bus: primary=06, secondary=07, subordinate=0a

In this example, the bus range (secordary to subordinate)
of the parent bridge of the CardBus Brige (in the middle)
is 06-06, but the bus range of the CardBus Brige below is
07-0a, and that is outside of 06-06, so the devices there
(you PCI Cardbus cards) cannot get the PCI messages.

The PCI bridge in the middle is the parent of the CardBus
bridge, because the CardBus bridge has primary=06, and that
is is within the bus rage of the PCI brige about (06-06).

That ment that Cardbus controller itself was detected, but
a PCI CardBus card inserted into get not PCI messages routed.

If your System is fixed with pci=assign-busses and works
flawlessy and dmidecode is installed (part of pmtools)
you can run this command to send me your system's DMI data
so that it can be added to the table and it will be possible
to enable it automatically on the systems where it's indicated
to use it:

su -c dmidecode | mail -s "This system works fine with pci=assign-busses" bk@suse.de

(best if you send the mail from an account where I can answer,
and any other way to get the output of dmidecode will work)

PPS:

I'd also like to suggest a patch like this for the detection function
to information with systems need pci=assign-busses into the DMI table:

--- drivers/pci/probe.c
+++ drivers/pci/probe.c
@@ -585,13 +585,15 @@
 		    (child->number > bus->subordinate) ||
 		    (child->number < bus->number) ||
 		    (child->subordinate < bus->number)) {
-			printk(KERN_WARNING "PCI: Bus #%02x (-#%02x) may be "
+			printk(KERN_WARNING "PCI: Bus #%02x (-#%02x) is "
 			       "hidden behind%s bridge #%02x (-#%02x)%s\n",
 			       child->number, child->subordinate,
 			       bus->self->transparent ? " transparent" : " ",
 			       bus->number, bus->subordinate,
 			       pcibios_assign_all_busses() ? " " :
 			       " (try 'pci=assign-busses')");
+			printk(KERN_WARNING "Please report the result to "
+			       "linux-kernel to fix this future(using DMI)");
 		}
 		bus = bus->parent;
 	}

(best may be a short dedicated email address instead of l-k)

Re: [PATCH] PCI/Cardbus cards hidden, needs pci=assign-busses to fix

[Andrew Morton]

Bernhard Kaindl <bk@suse.de> wrote:
>
> "In some cases, especially on modern laptops with a lot of PCI and cardbus
>  bridges, we're unable to assign correct secondary/subordinate bus numbers
>  to all cardbus bridges due to BIOS limitations unless we are using
>  "pci=assign-busses" boot option." -- Ivan Kokshaysky (from a patch comment)
> 
>  Without it, Cardbus cards inserted are never seen by PCI because the
>  parent PCI-PCI Bridge of the Cardbus bridge will not pass and translate
>  Type 1 PCI configuration cycles correctly and the system will fail to
>  find and initialise the PCI devices in the system.
> 
>  Reference: PCI-PCI Bridges: PCI Configuration Cycles and PCI Bus Numbering:
>  http://www.science.unitn.it/~fiorella/guidelinux/tlk/node72.html
> 
>  The reason for this is that:
>   ``All PCI busses located behind a PCI-PCI bridge must reside between the
>  secondary bus number and the subordinate bus number (inclusive).''
> 
>  "pci=assign-busses" makes pcibios_assign_all_busses return 1 and this
>  turns on PCI renumbering during PCI probing.
> 
>  Alan suggested to use DMI to make that function cause that on problem systems:
>
> [ snip patch which uses a DMI table to auto-set "pci=assign-busses" ]
>

I guess if this is the only way in which we can do this, and nobody has any
better solutions then sure, it'll get people's machines going.  We'll be
forever patching that table though.

But _does_ anyone have any better solutions?


(Minor squawks: please avoid the running-dialogue-mixed-up-with-patch-segments,
please prepare patches in `patch -p1' form and please send me a Signed-off-by:
for this work, as per section 11 of Documentation/SubmittingPatches, thanks).

Re: [PATCH] PCI/Cardbus cards hidden, needs pci=assign-busses to fix

[Greg KH]

On Sat, Feb 18, 2006 at 01:41:02AM -0800, Andrew Morton wrote:
> Bernhard Kaindl <bk@suse.de> wrote:
> >
> > "In some cases, especially on modern laptops with a lot of PCI and cardbus
> >  bridges, we're unable to assign correct secondary/subordinate bus numbers
> >  to all cardbus bridges due to BIOS limitations unless we are using
> >  "pci=assign-busses" boot option." -- Ivan Kokshaysky (from a patch comment)
> > 
> >  Without it, Cardbus cards inserted are never seen by PCI because the
> >  parent PCI-PCI Bridge of the Cardbus bridge will not pass and translate
> >  Type 1 PCI configuration cycles correctly and the system will fail to
> >  find and initialise the PCI devices in the system.
> > 
> >  Reference: PCI-PCI Bridges: PCI Configuration Cycles and PCI Bus Numbering:
> >  http://www.science.unitn.it/~fiorella/guidelinux/tlk/node72.html
> > 
> >  The reason for this is that:
> >   ``All PCI busses located behind a PCI-PCI bridge must reside between the
> >  secondary bus number and the subordinate bus number (inclusive).''
> > 
> >  "pci=assign-busses" makes pcibios_assign_all_busses return 1 and this
> >  turns on PCI renumbering during PCI probing.
> > 
> >  Alan suggested to use DMI to make that function cause that on problem systems:
> >
> > [ snip patch which uses a DMI table to auto-set "pci=assign-busses" ]
> >
> 
> I guess if this is the only way in which we can do this, and nobody has any
> better solutions then sure, it'll get people's machines going.  We'll be
> forever patching that table though.
> 
> But _does_ anyone have any better solutions?

This patch might not be needed at all, as per Kristen's recent comments
on the linux-pci mailing list, and her small patch that is already in
the -mm tree.

I'll let the discussion continue on that list.

thanks,

greg k-h

Re: [PATCH] PCI/Cardbus cards hidden, needs pci=assign-busses to fix

[Dominik Brodowski]

On Thu, Feb 23, 2006 at 05:47:55PM -0800, Greg KH wrote:
> > I guess if this is the only way in which we can do this, and nobody has any
> > better solutions then sure, it'll get people's machines going.  We'll be
> > forever patching that table though.
> > 
> > But _does_ anyone have any better solutions?
> 
> This patch might not be needed at all, as per Kristen's recent comments
> on the linux-pci mailing list, and her small patch that is already in
> the -mm tree.

If you mean

Subject:    [patch 3/4] pci: really fix parent's subordinate busnr

then no, that patch is not going to help: you can call
pci_fixup_parent_subordinate_busnr() as often as you want; it won't do
anything unless pcibios_assign_all_busses() is true.

	Dominik

[PATCH] Cardbus cards hidden, fixup parent subordinate# carefully

[Bernhard Kaindl]

On Sat, 18 Feb 2006, Andrew Morton wrote:
> Bernhard Kaindl <bk@suse.de> wrote:
> >
> > "In some cases, especially on modern laptops with a lot of PCI and cardbus
> >  bridges, we're unable to assign correct secondary/subordinate bus numbers
> >  to all cardbus bridges due to BIOS limitations unless we are using
> >  "pci=assign-busses" boot option." -- Ivan Kokshaysky (from a patch comment)
[...]
> > [ snip patch which uses a DMI table to auto-set "pci=assign-busses" ]
> 
> I guess if this is the only way in which we can do this, and nobody has any
> better solutions then sure, it'll get people's machines going.  We'll be
> forever patching that table though.
> 
> But _does_ anyone have any better solutions?

Now, yes: The patch is in the git-pcmcia-2.6 tree for testing it in -mm
and I include it here also - should peope wish to add it to their trees too.

Comments welcome!

Thanks,
Bernhard

------------------------------------------------------------------------

There are currently two ways to fix the problem of hidden cardbus cards:
One is to use

setpci -s 0:a.0 SUBORDINATE_BUS=0x0A (or similar)

On Presario R3000/R4000 series and HP zv5000 series, this is the standard.

Using pci=assign-busses is the other, but as ACPI may contain references
to PCI bus numbers and as files like xorg.conf file can also contain
references to PCI devices, it can break installations:

https://bugzilla.novell.com/show_bug.cgi?id=146438

It would also be a big change to enable it by default on many machines.

The other possiblity is to fixup the parent subordinate numbers after
we completed the initial PCI scan, so that we can do checks to prevent
overlapping bus numbers when trying to increase the subordinate number.

I created a kernel module for testing code which does that and it
was tested on Presario R3000 laptops and a Samsung X20 notebook.

I then trimmed it down to call it from the CardBus probe function,
before the socket is registered and the bus(es) below it are scanned.

>From that location, we can directly check the parent of the CardBus
bridge and that simplifies the code dramatically, so I could merge
it into one small function.

It was tested on my X20 and (it seems) a R3000 series notebook:
https://bugzilla.novell.com/show_bug.cgi?id=146438#c31

Conclusion:
-----------

We apparently have a better solution, and since the full PCI renumbering
could cause problems which I mentioned above, I think we should revert my
previous patch which was committed to 2.6.17-rc1:

commit 8c4b2cf9af9b4ecc29d4f0ec4ecc8e94dc4432d7
Author: Bernhard Kaindl <bk@suse.de>
Date:   Sat Feb 18 01:36:55 2006 -0800

Regarding the waring message patched in that commit: It gives a few
(partly) false warnings, because it's not specific enough in its tests.
So that part should be reverted too and I'll propose a fix for this
warning check, preferably depending on this patch (we could skip
cardbus there then), in separate mail.

Short patch comment:

Fixup the subordinate number of the parent of CardBus bridges carefully
before the the slots are scanned by PCI, to make the CardBus cards and
child busses which may otherwise be hidden from PCI scans seen.

Affected systems (from an internet search), supposed to be fixed:
* ASUS Z71V and L3s
* Samsung X20 (fixed in latest BIOS, but older BIOSes are affected)
* Compaq R3140us and all Compaq R3000 series (AMD64-based Laptops),
  also Compaq R4000 series
* HP zv5000z (AMD64 3700+, known that fixup_parent_subordinate_busnr fixes it)
* HP zv5200z
* HP SE2000L amd64 Turino laptop
* IBM ThinkPad 240
* An IBM ThinkPad (1.8 GHz Pentium M) debugged by Pavel Machek
  gives the message which detects the problem.
* MSI S260 / Medion SIM 2100 MD 95600

Tested-by: Andreas Schneider   <mail@cynapses.org>
Signed-off-by: Bernhard Kaindl <bk@suse.de>
Signed-off-by: Dominik Brodowski <linux@dominikbrodowski.net>

--- linux-2.6.16/drivers/pcmcia/yenta_socket.c
+++ linux-2.6.16/drivers/pcmcia/yenta_socket.c	2006/04/08 11:51:20
@@ -998,6 +998,78 @@
 	config_writew(socket, CB_BRIDGE_CONTROL, bridge);
 }

+/**
+ * yenta_fixup_parent_subordinate - Fix subordinate bus# of the parent bridge
+ * @cardbus_bridge: The PCI bus which the CardBus bridge bridges to
+ *
+ * Checks if devices on the bus which the CardBus bridge bridges to would be
+ * invisible during PCI scans because of a misconfigured subordinate number
+ * of the parent brige - some BIOSes seem to be too lazy to set it right.
+ * Does the fixup carefully by checking how far it can go without conflicts.
+ * See http://bugzilla.kernel.org/show_bug.cgi?id=2944 for more information.
+ */
+static void yenta_fixup_parent_bridge(struct pci_bus *cardbus_bridge)
+{
+	struct list_head *tmp;
+	/* Our starting point is the max PCI bus number */
+	unsigned char upper_limit = 0xff;
+ 	/*
+	 * We only check and fix the parent bridge: All systems which need
+	 * this fixup that have been reviewed are laptops and the only bridge
+	 * which needed fixing was the parent bridge of the CardBus bridge:
+	 */
+	struct pci_bus *bridge_to_fix = cardbus_bridge->parent;
+
+	/* Check bus numbers are already set up correctly: */
+	if (bridge_to_fix->subordinate >= cardbus_bridge->subordinate)
+		return; /* The subordinate number is ok, nothing to do */
+
+	if (!bridge_to_fix->parent)
+		return; /* Root bridges are ok */
+
+	/* stay within the limits of the bus range of the parent: */
+	upper_limit = bridge_to_fix->parent->subordinate;
+
+	/* check the bus ranges of all silbling bridges to prevent overlap */
+	list_for_each(tmp, &bridge_to_fix->parent->children) {
+		struct pci_bus * silbling = pci_bus_b(tmp);
+		/*
+		 * If the silbling has a higher secondary bus number
+		 * and it's secondary is equal or smaller than our
+		 * current upper limit, set the new upper limit to
+		 * the bus number below the silbling's range:
+		 */
+		if (silbling->secondary > bridge_to_fix->subordinate
+		    && silbling->secondary <= upper_limit)
+			upper_limit = silbling->secondary - 1;
+	}
+
+	/* Show that the wanted subordinate number is not possible: */
+	if (cardbus_bridge->subordinate > upper_limit)
+		printk(KERN_WARNING "Yenta: Upper limit for fixing this "
+			"bridge's parent bridge: #%02x\n", upper_limit);
+
+	/* If we have room to increase the bridge's subordinate number, */
+	if (bridge_to_fix->subordinate < upper_limit) {
+
+		/* use the highest number of the hidden bus, within limits */
+		unsigned char subordinate_to_assign =
+			min(cardbus_bridge->subordinate, upper_limit);
+
+		printk(KERN_INFO "Yenta: Raising subordinate bus# of parent "
+			"bus (#%02x) from #%02x to #%02x\n",
+			bridge_to_fix->number,
+			bridge_to_fix->subordinate, subordinate_to_assign);
+
+		/* Save the new subordinate in the bus struct of the bridge */
+		bridge_to_fix->subordinate = subordinate_to_assign;
+
+		/* and update the PCI config space with the new subordinate */
+		pci_write_config_byte(bridge_to_fix->self,
+			PCI_SUBORDINATE_BUS, bridge_to_fix->subordinate);
+	}
+}
+
 /*
  * Initialize a cardbus controller. Make sure we have a usable
  * interrupt, and that we can map the cardbus area. Fill in the
@@ -1113,6 +1184,8 @@
 	yenta_get_socket_capabilities(socket, isa_interrupts);
 	printk(KERN_INFO "Socket status: %08x\n", cb_readl(socket, CB_SOCKET_STATE));

+	yenta_fixup_parent_bridge(dev->subordinate);
+
 	/* Register it with the pcmcia layer.. */
 	ret = pcmcia_register_socket(&socket->socket);
 	if (ret == 0) {