PCI device order problem

PCI device order problem

[H. J. Lu]

In arch/i386/kernel/pci-pc.c, there are

/*
 * Sort the device list according to PCI BIOS. Nasty hack, but since some
 * fool forgot to define the `correct' device order in the PCI BIOS specs
 * and we want to be (possibly bug-to-bug ;-]) compatible with older kernels 
 * which used BIOS ordering, we are bound to do this... 
 */

static void __devinit pcibios_sort(void)

The problem is on my MB:

00:00.0 Host bridge: Intel Corp. e7500 [Plumas] DRAM Controller (rev 03)
00:00.1 Class ff00: Intel Corp. e7500 [Plumas] DRAM Controller Error Reporting ( rev 03)
00:03.0 PCI bridge: Intel Corp. e7500 [Plumas] HI_C Virtual PCI Bridge (F0) (rev 03)
00:03.1 Class ff00: Intel Corp. e7500 [Plumas] HI_C Virtual PCI Bridge (F1) (rev 03)
00:1d.0 USB Controller: Intel Corp. 82801CA/CAM USB (Hub  (rev 02)
00:1d.1 USB Controller: Intel Corp. 82801CA/CAM USB (Hub  (rev 02)
00:1e.0 PCI bridge: Intel Corp. 82801BA/CA/DB PCI Bridge (rev 42)
00:1f.0 ISA bridge: Intel Corp. 82801CA ISA Bridge (LPC) (rev 02)
00:1f.1 IDE interface: Intel Corp. 82801CA IDE U100 (rev 02)
00:1f.3 SMBus: Intel Corp. 82801CA/CAM SMBus (rev 02)
01:0c.0 VGA compatible controller: ATI Technologies Inc Rage XL (rev 27)
02:1c.0 PIC: Intel Corp. 82870P2 P64H2 I/OxAPIC (rev 03)
02:1d.0 PCI bridge: Intel Corp. 82870P2 P64H2 Hub PCI Bridge (rev 03)
02:1e.0 PIC: Intel Corp. 82870P2 P64H2 I/OxAPIC (rev 03)
02:1f.0 PCI bridge: Intel Corp. 82870P2 P64H2 Hub PCI Bridge (rev 03)
03:07.0 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01)
03:07.1 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01)
03:08.0 RAID bus controller: 3ware Inc 3ware 7000-series ATA-RAID (rev 01)

Eth1 becomes:
03:07.0 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01)

and eth0 becomes:
03:07.1 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01)

Is that a good idea to have an option to sort the PCI device by PCI bus and
slot numbers?


H.J.

Re: PCI device order problem

[Jeff Garzik]

H. J. Lu wrote:

>In arch/i386/kernel/pci-pc.c, there are
>
>/*
> * Sort the device list according to PCI BIOS. Nasty hack, but since some
> * fool forgot to define the `correct' device order in the PCI BIOS specs
> * and we want to be (possibly bug-to-bug ;-]) compatible with older kernels 
> * which used BIOS ordering, we are bound to do this... 
> */
>
>static void __devinit pcibios_sort(void)
>
>The problem is on my MB:
>
>00:00.0 Host bridge: Intel Corp. e7500 [Plumas] DRAM Controller (rev 03)
>00:00.1 Class ff00: Intel Corp. e7500 [Plumas] DRAM Controller Error Reporting ( rev 03)
>00:03.0 PCI bridge: Intel Corp. e7500 [Plumas] HI_C Virtual PCI Bridge (F0) (rev 03)
>00:03.1 Class ff00: Intel Corp. e7500 [Plumas] HI_C Virtual PCI Bridge (F1) (rev 03)
>00:1d.0 USB Controller: Intel Corp. 82801CA/CAM USB (Hub  (rev 02)
>00:1d.1 USB Controller: Intel Corp. 82801CA/CAM USB (Hub  (rev 02)
>00:1e.0 PCI bridge: Intel Corp. 82801BA/CA/DB PCI Bridge (rev 42)
>00:1f.0 ISA bridge: Intel Corp. 82801CA ISA Bridge (LPC) (rev 02)
>00:1f.1 IDE interface: Intel Corp. 82801CA IDE U100 (rev 02)
>00:1f.3 SMBus: Intel Corp. 82801CA/CAM SMBus (rev 02)
>01:0c.0 VGA compatible controller: ATI Technologies Inc Rage XL (rev 27)
>02:1c.0 PIC: Intel Corp. 82870P2 P64H2 I/OxAPIC (rev 03)
>02:1d.0 PCI bridge: Intel Corp. 82870P2 P64H2 Hub PCI Bridge (rev 03)
>02:1e.0 PIC: Intel Corp. 82870P2 P64H2 I/OxAPIC (rev 03)
>02:1f.0 PCI bridge: Intel Corp. 82870P2 P64H2 Hub PCI Bridge (rev 03)
>03:07.0 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01)
>03:07.1 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01)
>03:08.0 RAID bus controller: 3ware Inc 3ware 7000-series ATA-RAID (rev 01)
>
>Eth1 becomes:
>03:07.0 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01)
>
>and eth0 becomes:
>03:07.1 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01)
>
>Is that a good idea to have an option to sort the PCI device by PCI bus and
>slot numbers?
>  
>

Without answering your specific question, but addressing $subject, what 
problem is caused by the PCI device order you see?

You can use ETHTOOL_GDRVINFO to get the PCI bus info for an ethernet 
interface, so you know at all times what device is associated with what 
PCI device.  And among other ways you are notified of device 
addition/removal by the execution of /sbin/hotplug for each ethernet 
interface.

    Jeff

Re: PCI device order problem

[H. J. Lu]

On Thu, Oct 24, 2002 at 07:49:41PM -0400, Jeff Garzik wrote:
> H. J. Lu wrote:
> 
> >In arch/i386/kernel/pci-pc.c, there are
> >
> >/*
> > * Sort the device list according to PCI BIOS. Nasty hack, but since some
> > * fool forgot to define the `correct' device order in the PCI BIOS specs
> > * and we want to be (possibly bug-to-bug ;-]) compatible with older kernels 
> > * which used BIOS ordering, we are bound to do this... 
> > */
> >
> >static void __devinit pcibios_sort(void)
> >
> >The problem is on my MB:
> >
> >00:00.0 Host bridge: Intel Corp. e7500 [Plumas] DRAM Controller (rev 03)
> >00:00.1 Class ff00: Intel Corp. e7500 [Plumas] DRAM Controller Error Reporting ( rev 03)
> >00:03.0 PCI bridge: Intel Corp. e7500 [Plumas] HI_C Virtual PCI Bridge (F0) (rev 03)
> >00:03.1 Class ff00: Intel Corp. e7500 [Plumas] HI_C Virtual PCI Bridge (F1) (rev 03)
> >00:1d.0 USB Controller: Intel Corp. 82801CA/CAM USB (Hub  (rev 02)
> >00:1d.1 USB Controller: Intel Corp. 82801CA/CAM USB (Hub  (rev 02)
> >00:1e.0 PCI bridge: Intel Corp. 82801BA/CA/DB PCI Bridge (rev 42)
> >00:1f.0 ISA bridge: Intel Corp. 82801CA ISA Bridge (LPC) (rev 02)
> >00:1f.1 IDE interface: Intel Corp. 82801CA IDE U100 (rev 02)
> >00:1f.3 SMBus: Intel Corp. 82801CA/CAM SMBus (rev 02)
> >01:0c.0 VGA compatible controller: ATI Technologies Inc Rage XL (rev 27)
> >02:1c.0 PIC: Intel Corp. 82870P2 P64H2 I/OxAPIC (rev 03)
> >02:1d.0 PCI bridge: Intel Corp. 82870P2 P64H2 Hub PCI Bridge (rev 03)
> >02:1e.0 PIC: Intel Corp. 82870P2 P64H2 I/OxAPIC (rev 03)
> >02:1f.0 PCI bridge: Intel Corp. 82870P2 P64H2 Hub PCI Bridge (rev 03)
> >03:07.0 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01)
> >03:07.1 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01)
> >03:08.0 RAID bus controller: 3ware Inc 3ware 7000-series ATA-RAID (rev 01)
> >
> >Eth1 becomes:
> >03:07.0 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01)
> >
> >and eth0 becomes:
> >03:07.1 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01)
> >
> >Is that a good idea to have an option to sort the PCI device by PCI bus and
> >slot numbers?
> >  
> >
> 
> Without answering your specific question, but addressing $subject, what 
> problem is caused by the PCI device order you see?

It is different from the hardware documentation. The hardware manual says
it has 2 NICs, NIC 1 (03:07.0) and NIC2 (03:07.1), which makes senses
to me. NIC 1 is a special one which supports IPMI over LAN. Since we
only use one NIC now, we'd like to use NIC 1 and call it eth0.


H.J.

Re: PCI device order problem

[Jeff Garzik]

H. J. Lu wrote:

>On Thu, Oct 24, 2002 at 07:49:41PM -0400, Jeff Garzik wrote:
>  
>
>>H. J. Lu wrote:
>>
>>    
>>
>>>In arch/i386/kernel/pci-pc.c, there are
>>>
>>>/*
>>>* Sort the device list according to PCI BIOS. Nasty hack, but since some
>>>* fool forgot to define the `correct' device order in the PCI BIOS specs
>>>* and we want to be (possibly bug-to-bug ;-]) compatible with older kernels 
>>>* which used BIOS ordering, we are bound to do this... 
>>>*/
>>>
>>>static void __devinit pcibios_sort(void)
>>>
>>>The problem is on my MB:
>>>
>>>00:00.0 Host bridge: Intel Corp. e7500 [Plumas] DRAM Controller (rev 03)
>>>00:00.1 Class ff00: Intel Corp. e7500 [Plumas] DRAM Controller Error Reporting ( rev 03)
>>>00:03.0 PCI bridge: Intel Corp. e7500 [Plumas] HI_C Virtual PCI Bridge (F0) (rev 03)
>>>00:03.1 Class ff00: Intel Corp. e7500 [Plumas] HI_C Virtual PCI Bridge (F1) (rev 03)
>>>00:1d.0 USB Controller: Intel Corp. 82801CA/CAM USB (Hub  (rev 02)
>>>00:1d.1 USB Controller: Intel Corp. 82801CA/CAM USB (Hub  (rev 02)
>>>00:1e.0 PCI bridge: Intel Corp. 82801BA/CA/DB PCI Bridge (rev 42)
>>>00:1f.0 ISA bridge: Intel Corp. 82801CA ISA Bridge (LPC) (rev 02)
>>>00:1f.1 IDE interface: Intel Corp. 82801CA IDE U100 (rev 02)
>>>00:1f.3 SMBus: Intel Corp. 82801CA/CAM SMBus (rev 02)
>>>01:0c.0 VGA compatible controller: ATI Technologies Inc Rage XL (rev 27)
>>>02:1c.0 PIC: Intel Corp. 82870P2 P64H2 I/OxAPIC (rev 03)
>>>02:1d.0 PCI bridge: Intel Corp. 82870P2 P64H2 Hub PCI Bridge (rev 03)
>>>02:1e.0 PIC: Intel Corp. 82870P2 P64H2 I/OxAPIC (rev 03)
>>>02:1f.0 PCI bridge: Intel Corp. 82870P2 P64H2 Hub PCI Bridge (rev 03)
>>>03:07.0 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01)
>>>03:07.1 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01)
>>>03:08.0 RAID bus controller: 3ware Inc 3ware 7000-series ATA-RAID (rev 01)
>>>
>>>Eth1 becomes:
>>>03:07.0 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01)
>>>
>>>and eth0 becomes:
>>>03:07.1 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01)
>>>
>>>Is that a good idea to have an option to sort the PCI device by PCI bus and
>>>slot numbers?
>>> 
>>>
>>>      
>>>
>>Without answering your specific question, but addressing $subject, what 
>>problem is caused by the PCI device order you see?
>>    
>>
>
>It is different from the hardware documentation. The hardware manual says
>it has 2 NICs, NIC 1 (03:07.0) and NIC2 (03:07.1), which makes senses
>to me. NIC 1 is a special one which supports IPMI over LAN. Since we
>only use one NIC now, we'd like to use NIC 1 and call it eth0.
>  
>

Well, overall, depending on ordering is error-prone (as you see).  I 
would suggest migrating to a less-fragile scheme.  nameif and ethtool 
together should get you exactly the device you need...  (though I wonder 
why simply using eth1 is so awful, if this situation is constant...)

    Jeff

Re: PCI device order problem

[H. J. Lu]

On Thu, Oct 24, 2002 at 08:14:09PM -0400, Jeff Garzik wrote:
> H. J. Lu wrote:
> 
> >On Thu, Oct 24, 2002 at 07:49:41PM -0400, Jeff Garzik wrote:
> >  
> >
> >>H. J. Lu wrote:
> >>
> >>    
> >>
> >>>In arch/i386/kernel/pci-pc.c, there are
> >>>
> >>>/*
> >>>* Sort the device list according to PCI BIOS. Nasty hack, but since some
> >>>* fool forgot to define the `correct' device order in the PCI BIOS specs
> >>>* and we want to be (possibly bug-to-bug ;-]) compatible with older kernels 
> >>>* which used BIOS ordering, we are bound to do this... 
> >>>*/
> >>>
> >>>static void __devinit pcibios_sort(void)
> >>>
> >>>The problem is on my MB:
> >>>
> >>>00:00.0 Host bridge: Intel Corp. e7500 [Plumas] DRAM Controller (rev 03)
> >>>00:00.1 Class ff00: Intel Corp. e7500 [Plumas] DRAM Controller Error Reporting ( rev 03)
> >>>00:03.0 PCI bridge: Intel Corp. e7500 [Plumas] HI_C Virtual PCI Bridge (F0) (rev 03)
> >>>00:03.1 Class ff00: Intel Corp. e7500 [Plumas] HI_C Virtual PCI Bridge (F1) (rev 03)
> >>>00:1d.0 USB Controller: Intel Corp. 82801CA/CAM USB (Hub  (rev 02)
> >>>00:1d.1 USB Controller: Intel Corp. 82801CA/CAM USB (Hub  (rev 02)
> >>>00:1e.0 PCI bridge: Intel Corp. 82801BA/CA/DB PCI Bridge (rev 42)
> >>>00:1f.0 ISA bridge: Intel Corp. 82801CA ISA Bridge (LPC) (rev 02)
> >>>00:1f.1 IDE interface: Intel Corp. 82801CA IDE U100 (rev 02)
> >>>00:1f.3 SMBus: Intel Corp. 82801CA/CAM SMBus (rev 02)
> >>>01:0c.0 VGA compatible controller: ATI Technologies Inc Rage XL (rev 27)
> >>>02:1c.0 PIC: Intel Corp. 82870P2 P64H2 I/OxAPIC (rev 03)
> >>>02:1d.0 PCI bridge: Intel Corp. 82870P2 P64H2 Hub PCI Bridge (rev 03)
> >>>02:1e.0 PIC: Intel Corp. 82870P2 P64H2 I/OxAPIC (rev 03)
> >>>02:1f.0 PCI bridge: Intel Corp. 82870P2 P64H2 Hub PCI Bridge (rev 03)
> >>>03:07.0 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01)
> >>>03:07.1 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01)
> >>>03:08.0 RAID bus controller: 3ware Inc 3ware 7000-series ATA-RAID (rev 01)
> >>>
> >>>Eth1 becomes:
> >>>03:07.0 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01)
> >>>
> >>>and eth0 becomes:
> >>>03:07.1 Ethernet controller: Intel Corp. 82546EB Gigabit Ethernet Controller (rev 01)
> >>>
> >>>Is that a good idea to have an option to sort the PCI device by PCI bus and
> >>>slot numbers?
> >>> 
> >>>
> >>>      
> >>>
> >>Without answering your specific question, but addressing $subject, what 
> >>problem is caused by the PCI device order you see?
> >>    
> >>
> >
> >It is different from the hardware documentation. The hardware manual says
> >it has 2 NICs, NIC 1 (03:07.0) and NIC2 (03:07.1), which makes senses
> >to me. NIC 1 is a special one which supports IPMI over LAN. Since we
> >only use one NIC now, we'd like to use NIC 1 and call it eth0.
> >  
> >
> 
> Well, overall, depending on ordering is error-prone (as you see).  I 
> would suggest migrating to a less-fragile scheme.  nameif and ethtool 
> together should get you exactly the device you need...  (though I wonder 
> why simply using eth1 is so awful, if this situation is constant...)

We can use eth1. It is just very confusing since Linux and hardware
manual don't agree which one is the first NIC. Also, when we upgrade
the BIOS, the BIOS order may change. As for other schemes, we don't want
to change every software which access ethX.


H.J.

Re: PCI device order problem

[Alan Cox]

On Fri, 2002-10-25 at 00:56, H. J. Lu wrote:
> It is different from the hardware documentation. The hardware manual says
> it has 2 NICs, NIC 1 (03:07.0) and NIC2 (03:07.1), which makes senses
> to me. NIC 1 is a special one which supports IPMI over LAN. Since we
> only use one NIC now, we'd like to use NIC 1 and call it eth0.

SIOCSIFNAME ioctl. You can call them "haddock" and "chips" if you really
want, or swap the eth%d names about. RH 8.0 allows you to bind an
interface to a mac address too

Re: PCI device order problem

[H. J. Lu]

On Fri, Oct 25, 2002 at 11:00:31AM +0100, Alan Cox wrote:
> On Fri, 2002-10-25 at 00:56, H. J. Lu wrote:
> > It is different from the hardware documentation. The hardware manual says
> > it has 2 NICs, NIC 1 (03:07.0) and NIC2 (03:07.1), which makes senses
> > to me. NIC 1 is a special one which supports IPMI over LAN. Since we
> > only use one NIC now, we'd like to use NIC 1 and call it eth0.
> 
> SIOCSIFNAME ioctl. You can call them "haddock" and "chips" if you really
> want, or swap the eth%d names about. RH 8.0 allows you to bind an
> interface to a mac address too
> 

It doesn't help RedHat installer over network.


H.J.

PATCH: Support PCI device sorting (Re: PCI device order problem)

[H. J. Lu]

[-- Attachment #1: Type: text/plain, Size: 90 bytes --]

Here is a patch to add an option to sort the PCI device by bus, slot
and function.


H.J.

[-- Attachment #2: linux-2.4.18-pci-order.patch --]
[-- Type: text/plain, Size: 3295 bytes --]

--- linux/arch/i386/config.in.order	Thu Oct 24 21:09:20 2002
+++ linux/arch/i386/config.in	Fri Oct 25 16:34:13 2002
@@ -257,6 +257,7 @@ else
       if [ "$CONFIG_PCI_GODIRECT" = "y" -o "$CONFIG_PCI_GOANY" = "y" ]; then
          define_bool CONFIG_PCI_DIRECT y
       fi
+      bool '  Sort device by bus, slot, function' CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC
    fi
 fi
 
--- linux/arch/i386/kernel/pci-i386.h.order	Sun Nov 11 10:09:32 2001
+++ linux/arch/i386/kernel/pci-i386.h	Fri Oct 25 19:26:51 2002
@@ -21,6 +21,7 @@
 #define PCI_ASSIGN_ROMS		0x1000
 #define PCI_BIOS_IRQ_SCAN	0x2000
 #define PCI_ASSIGN_ALL_BUSSES	0x4000
+#define PCI_BUS_SORT		0x8000
 
 extern unsigned int pci_probe;
 
--- linux/arch/i386/kernel/pci-pc.c.order	Thu Oct 24 21:09:20 2002
+++ linux/arch/i386/kernel/pci-pc.c	Fri Oct 25 19:58:18 2002
@@ -19,7 +19,11 @@
 
 #include "pci-i386.h"
 
+#ifdef CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC
+unsigned int pci_probe = PCI_PROBE_BIOS | PCI_BUS_SORT | PCI_PROBE_CONF1 | PCI_PROBE_CONF2;
+#else
 unsigned int pci_probe = PCI_PROBE_BIOS | PCI_PROBE_CONF1 | PCI_PROBE_CONF2;
+#endif
 
 int pcibios_last_bus = -1;
 struct pci_bus *pci_root_bus = NULL;
@@ -871,6 +875,53 @@ static struct pci_ops * __devinit pci_fi
 	return NULL;
 }
 
+static void __devinit pci_sort_by_bus_slot_func(void)
+{
+	LIST_HEAD(sorted_devices);
+	struct list_head *ln;
+	struct pci_dev *dev, *d;
+	int n;
+	int s, slot;
+	int f, func;
+
+	printk(KERN_INFO "PCI: Sorting device list by bus, slot, function...\n");
+
+	/* Starting from bus 0, ...  */
+	for (n=0; n <= pcibios_last_bus; n++) {
+		if (!pci_bus_exists(&pci_root_buses, n))
+			continue;
+
+		while (!list_empty(&pci_devices)) {
+			/* Find the lowest remaining PCI slot/function.  */
+			slot = INT_MAX;
+			func = INT_MAX;
+			dev = NULL;
+			for (ln=pci_devices.next; ln != &pci_devices; ln=ln->next) {
+				d = pci_dev_g(ln);
+				s = PCI_SLOT(d->devfn);
+				f = PCI_FUNC(d->devfn);
+				if (d->bus->number == n
+				    && (s < slot || (s == slot && f < func))) {
+					slot = s;
+					func = f;
+					dev = d;
+				}
+			}
+
+			if (dev) {
+				list_del(&dev->global_list);
+				list_add_tail(&dev->global_list, &sorted_devices);
+			}
+			else {
+				/* Stop if we don't find any devices on
+				   this bus.  */
+				break;
+			}
+		}
+	}
+	list_splice(&sorted_devices, &pci_devices);
+}
+
 /*
  * Sort the device list according to PCI BIOS. Nasty hack, but since some
  * fool forgot to define the `correct' device order in the PCI BIOS specs
@@ -1392,8 +1443,10 @@ void __init pcibios_init(void)
 
 	pcibios_resource_survey();
 
+	if ((pci_probe & PCI_BUS_SORT) && !(pci_probe & PCI_NO_SORT))
+		pci_sort_by_bus_slot_func();
 #ifdef CONFIG_PCI_BIOS
-	if ((pci_probe & PCI_BIOS_SORT) && !(pci_probe & PCI_NO_SORT))
+	else if ((pci_probe & PCI_BIOS_SORT) && !(pci_probe & PCI_NO_SORT))
 		pcibios_sort();
 #endif
 }
@@ -1404,6 +1457,17 @@ char * __devinit  pcibios_setup(char *st
 		pci_probe = 0;
 		return NULL;
 	}
+#ifdef CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC
+	else if (!strcmp(str, "nobussort")) {
+		pci_probe &= ~PCI_BUS_SORT;
+		return NULL;
+	}
+#else
+	else if (!strcmp(str, "bussort")) {
+		pci_probe |= PCI_BUS_SORT;
+		return NULL;
+	}
+#endif
 #ifdef CONFIG_PCI_BIOS
 	else if (!strcmp(str, "bios")) {
 		pci_probe = PCI_PROBE_BIOS;

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

[Jeff Garzik]

Well, WRT your implementation, the function you add is dead code if your 
new config variable is not set, which is not desireable at all.

WRT the overall idea, I would prefer to see what Linus and Martin Mares 
(and Ivan K) thought about it, before merging it.  The x86 PCI code is 
very touchy, and your patch has the potential to change driver probe 
order for little gain.

    Jeff

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

[H. J. Lu]

On Sat, Oct 26, 2002 at 05:12:51PM -0400, Jeff Garzik wrote:
> Well, WRT your implementation, the function you add is dead code if your 
> new config variable is not set, which is not desireable at all.

I am not sure if I understand what you were trying to say. If
CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC is not set, you should be able to 
pass "pci=bussort" to kernel to sort the PCI device by bus, slot and
function. Did I miss something?

> 
> WRT the overall idea, I would prefer to see what Linus and Martin Mares 
> (and Ivan K) thought about it, before merging it.  The x86 PCI code is 
> very touchy, and your patch has the potential to change driver probe 
> order for little gain.
> 

The whole purpose of my patch is to change PCI driver probe order in
such a way that is BIOS independent.


H.J.

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

[Jeff Garzik]

H. J. Lu wrote:

>On Sat, Oct 26, 2002 at 05:12:51PM -0400, Jeff Garzik wrote:
>  
>
>>Well, WRT your implementation, the function you add is dead code if your 
>>new config variable is not set, which is not desireable at all.
>>    
>>
>
>I am not sure if I understand what you were trying to say. If
>CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC is not set, you should be able to 
>pass "pci=bussort" to kernel to sort the PCI device by bus, slot and
>function. Did I miss something?
>  
>
The sorting function you add should be covered by the ifdef you add.

>>WRT the overall idea, I would prefer to see what Linus and Martin Mares 
>>(and Ivan K) thought about it, before merging it.  The x86 PCI code is 
>>very touchy, and your patch has the potential to change driver probe 
>>order for little gain.
>>
>>    
>>
>
>The whole purpose of my patch is to change PCI driver probe order in
>such a way that is BIOS independent.
>  
>

The purpose is irrelevant to the effect on existing drivers and systems 
-- which is unknown.  Making the probe order independent of BIOS 
ordering may very well break drivers and systems that are dependent on 
BIOS ordering.  IOW what looks nice on your system could _likely_ suck 
for others.  That's what I meant by "x86 PCI code is very touchy."

    Jeff

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

[H. J. Lu]

On Sat, Oct 26, 2002 at 05:34:57PM -0400, Jeff Garzik wrote:
> H. J. Lu wrote:
> 
> >On Sat, Oct 26, 2002 at 05:12:51PM -0400, Jeff Garzik wrote:
> >  
> >
> >>Well, WRT your implementation, the function you add is dead code if your 
> >>new config variable is not set, which is not desireable at all.
> >>    
> >>
> >
> >I am not sure if I understand what you were trying to say. If
> >CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC is not set, you should be able to 
> >pass "pci=bussort" to kernel to sort the PCI device by bus, slot and
> >function. Did I miss something?
> >  
> >
> The sorting function you add should be covered by the ifdef you add.

Have you tried? If I did that, the kernel wouldn't even compile. As I
said, when CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC isn't defined, my sorting
function is still available, just not called by default.

> 
> >>WRT the overall idea, I would prefer to see what Linus and Martin Mares 
> >>(and Ivan K) thought about it, before merging it.  The x86 PCI code is 
> >>very touchy, and your patch has the potential to change driver probe 
> >>order for little gain.
> >>
> >>    
> >>
> >
> >The whole purpose of my patch is to change PCI driver probe order in
> >such a way that is BIOS independent.
> >  
> >
> 
> The purpose is irrelevant to the effect on existing drivers and systems 
> -- which is unknown.  Making the probe order independent of BIOS 
> ordering may very well break drivers and systems that are dependent on 
> BIOS ordering.  IOW what looks nice on your system could _likely_ suck 
> for others.  That's what I meant by "x86 PCI code is very touchy."

That is why CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC is off by default and
even if it is on, you can still override it by passing "pci=nosort"
or "pci=nobussort" to kernel.



H.J.

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

[Jeff Garzik]

H. J. Lu wrote:

>On Sat, Oct 26, 2002 at 05:34:57PM -0400, Jeff Garzik wrote:
>  
>
>>H. J. Lu wrote:
>>
>>    
>>
>>>On Sat, Oct 26, 2002 at 05:12:51PM -0400, Jeff Garzik wrote:
>>> 
>>>
>>>      
>>>
>>>>Well, WRT your implementation, the function you add is dead code if your 
>>>>new config variable is not set, which is not desireable at all.
>>>>   
>>>>
>>>>        
>>>>
>>>I am not sure if I understand what you were trying to say. If
>>>CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC is not set, you should be able to 
>>>pass "pci=bussort" to kernel to sort the PCI device by bus, slot and
>>>function. Did I miss something?
>>> 
>>>
>>>      
>>>
>>The sorting function you add should be covered by the ifdef you add.
>>    
>>
>
>Have you tried? If I did that, the kernel wouldn't even compile. As I
>said, when CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC isn't defined, my sorting
>function is still available, just not called by default.
>
Which is the entire problem.  The kernel compiles and builds just fine 
right now, without your function.

If it is "just not called by default" then it clearly can be removed at 
compile time when a certain CONFIG_xxx is not defined.


>>>>WRT the overall idea, I would prefer to see what Linus and Martin Mares 
>>>>(and Ivan K) thought about it, before merging it.  The x86 PCI code is 
>>>>very touchy, and your patch has the potential to change driver probe 
>>>>order for little gain.
>>>>
>>>>        
>>>>
>>>The whole purpose of my patch is to change PCI driver probe order in
>>>such a way that is BIOS independent.
>>>
>>>      
>>>
>>The purpose is irrelevant to the effect on existing drivers and systems 
>>-- which is unknown.  Making the probe order independent of BIOS 
>>ordering may very well break drivers and systems that are dependent on 
>>BIOS ordering.  IOW what looks nice on your system could _likely_ suck 
>>for others.  That's what I meant by "x86 PCI code is very touchy."
>>    
>>
>
>That is why CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC is off by default and
>even if it is on, you can still override it by passing "pci=nosort"
>or "pci=nobussort" to kernel.
>  
>

Sigh.  Repeating, the kernel is still bloated by your sorting function 
if CONFIG_PCI_SORT_BY_BUS_SLOT is not defined.  The function should go 
away if CONFIG_PCI_SORT_BY_BUS_SLOT is not defined.

    Jeff

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

[H. J. Lu]

On Sat, Oct 26, 2002 at 06:04:00PM -0400, Jeff Garzik wrote:
> Which is the entire problem.  The kernel compiles and builds just fine 
> right now, without your function.
> 

Without my patch or my function? My patched file has

        if ((pci_probe & PCI_BUS_SORT) && !(pci_probe & PCI_NO_SORT))
                pci_sort_by_bus_slot_func();
#ifdef CONFIG_PCI_BIOS
        else if ((pci_probe & PCI_BIOS_SORT) && !(pci_probe & PCI_NO_SORT))
                pcibios_sort();
#endif

That is pci_sort_by_bus_slot_func () will be called if PCI_BUS_SORT is
set. It is independent of whether CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC is
set or not, which sets PCI_BUS_SORT. If pci_sort_by_bus_slot_func is
not defined when CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC is not set. How can
kernel compile?

> If it is "just not called by default" then it clearly can be removed at 
> compile time when a certain CONFIG_xxx is not defined.

It is controlled by PCI_BUS_SORT, not CONFIG_xxx.

> 
> 
> >>>>WRT the overall idea, I would prefer to see what Linus and Martin Mares 
> >>>>(and Ivan K) thought about it, before merging it.  The x86 PCI code is 
> >>>>very touchy, and your patch has the potential to change driver probe 
> >>>>order for little gain.
> >>>>
> >>>>        
> >>>>
> >>>The whole purpose of my patch is to change PCI driver probe order in
> >>>such a way that is BIOS independent.
> >>>
> >>>      
> >>>
> >>The purpose is irrelevant to the effect on existing drivers and systems 
> >>-- which is unknown.  Making the probe order independent of BIOS 
> >>ordering may very well break drivers and systems that are dependent on 
> >>BIOS ordering.  IOW what looks nice on your system could _likely_ suck 
> >>for others.  That's what I meant by "x86 PCI code is very touchy."
> >>    
> >>
> >
> >That is why CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC is off by default and
> >even if it is on, you can still override it by passing "pci=nosort"
> >or "pci=nobussort" to kernel.
> >  
> >
> 
> Sigh.  Repeating, the kernel is still bloated by your sorting function 
> if CONFIG_PCI_SORT_BY_BUS_SLOT is not defined.  The function should go 
> away if CONFIG_PCI_SORT_BY_BUS_SLOT is not defined.

I added pci=nobussort since it might not be safe for all MBs. Then I
added "pci=bussort". I have no problem taking out "pci=bussort". If you
don't want "pci=bussort", please say so. I can provide a new patch which
won't define pci_sort_by_bus_slot_func if CONFIG_PCI_SORT_BY_BUS_SLOT
is not set and won't have "pci=bussort" either.


H.J.

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

[Jeff Garzik]

H. J. Lu wrote:

>On Sat, Oct 26, 2002 at 06:04:00PM -0400, Jeff Garzik wrote:
>  
>
>>Which is the entire problem.  The kernel compiles and builds just fine 
>>right now, without your function.
>>
>>    
>>
>
>Without my patch or my function? My patched file has
>
>        if ((pci_probe & PCI_BUS_SORT) && !(pci_probe & PCI_NO_SORT))
>                pci_sort_by_bus_slot_func();
>#ifdef CONFIG_PCI_BIOS
>        else if ((pci_probe & PCI_BIOS_SORT) && !(pci_probe & PCI_NO_SORT))
>                pcibios_sort();
>#endif
>
>That is pci_sort_by_bus_slot_func () will be called if PCI_BUS_SORT is
>set. It is independent of whether CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC is
>set or not, which sets PCI_BUS_SORT. If pci_sort_by_bus_slot_func is
>not defined when CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC is not set. How can
>kernel compile?
>
I am obviously not being understood / not communicating myself clearly.

Your patch unconditionally adds a new function.
Don't do that.  Wrap it in an ifdef, like you propose below.



>I added pci=nobussort since it might not be safe for all MBs. Then I
>added "pci=bussort". I have no problem taking out "pci=bussort". If you
>don't want "pci=bussort", please say so. I can provide a new patch which
>won't define pci_sort_by_bus_slot_func if CONFIG_PCI_SORT_BY_BUS_SLOT
>is not set and won't have "pci=bussort" either.
>  
>

You're still missing the point here too.

Your patch has the potential to suddenly make systems unbootable, to 
suddenly reverse people's ethX interface numbering, etc.  No command 
line options in the world will save lkml from being deluged by tons of 
"my system doesn't network anymore" bug reports.

The basic point is "let's proceed with caution, and test test test 
before applying this patch."

    Jeff

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

[H. J. Lu]

On Sat, Oct 26, 2002 at 06:29:23PM -0400, Jeff Garzik wrote:
> 
> >I added pci=nobussort since it might not be safe for all MBs. Then I
> >added "pci=bussort". I have no problem taking out "pci=bussort". If you
> >don't want "pci=bussort", please say so. I can provide a new patch which
> >won't define pci_sort_by_bus_slot_func if CONFIG_PCI_SORT_BY_BUS_SLOT
> >is not set and won't have "pci=bussort" either.
> >  
> >
> 
> You're still missing the point here too.
> 
> Your patch has the potential to suddenly make systems unbootable, to 
> suddenly reverse people's ethX interface numbering, etc.  No command 

That is the whole purpose of my patch. But you will only get it when
you set CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC.

> line options in the world will save lkml from being deluged by tons of 
> "my system doesn't network anymore" bug reports.

You will only get it when you set CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC on
purpose by hand.

> 
> The basic point is "let's proceed with caution, and test test test 
> before applying this patch."

Please state clearly what you have in mind. First you were
saying you didn't like pci_sort_by_bus_slot_func defined when
CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC wass not set. Now you were
saying my patch was dangerous. Please make up your mind.


H.J.

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

[Jeff Garzik]

H. J. Lu wrote:

>>The basic point is "let's proceed with caution, and test test test 
>>before applying this patch."
>>    
>>
>
>Please state clearly what you have in mind. First you were
>saying you didn't like pci_sort_by_bus_slot_func defined when
>CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC wass not set. Now you were
>saying my patch was dangerous. Please make up your mind.
>  
>

In my first reply, I clearly separated implementation issues from 
commentary on the overall idea.  Aside from that, I don't see much value 
in further repeating what I've already said.

    Jeff

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

[Alan Cox]

On Sat, 2002-10-26 at 23:58, Jeff Garzik wrote:
> In my first reply, I clearly separated implementation issues from 
> commentary on the overall idea.  Aside from that, I don't see much value 
> in further repeating what I've already said.

Its pretty pointless code but since its done at boot time, just marking
it __init ought to be quite sufficient.

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

[H. J. Lu]

[-- Attachment #1: Type: text/plain, Size: 794 bytes --]

On Sat, Oct 26, 2002 at 06:58:49PM -0400, Jeff Garzik wrote:
> H. J. Lu wrote:
> 
> >>The basic point is "let's proceed with caution, and test test test 
> >>before applying this patch."
> >>    
> >>
> >
> >Please state clearly what you have in mind. First you were
> >saying you didn't like pci_sort_by_bus_slot_func defined when
> >CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC wass not set. Now you were
> >saying my patch was dangerous. Please make up your mind.
> >  
> >
> 
> In my first reply, I clearly separated implementation issues from 
> commentary on the overall idea.  Aside from that, I don't see much value 
> in further repeating what I've already said.
> 

Here is an alternative patch, which won't define pci_sort_by_bus_slot_func
if CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC is not set.


H.J.

[-- Attachment #2: linux-2.4.18-pci-order-2.patch --]
[-- Type: text/plain, Size: 3224 bytes --]

--- linux/arch/i386/config.in.order	Thu Oct 24 21:09:20 2002
+++ linux/arch/i386/config.in	Fri Oct 25 16:34:13 2002
@@ -257,6 +257,7 @@ else
       if [ "$CONFIG_PCI_GODIRECT" = "y" -o "$CONFIG_PCI_GOANY" = "y" ]; then
          define_bool CONFIG_PCI_DIRECT y
       fi
+      bool '  Sort device by bus, slot, function' CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC
    fi
 fi
 
--- linux/arch/i386/kernel/pci-i386.h.order	Sun Nov 11 10:09:32 2001
+++ linux/arch/i386/kernel/pci-i386.h	Fri Oct 25 19:26:51 2002
@@ -21,6 +21,7 @@
 #define PCI_ASSIGN_ROMS		0x1000
 #define PCI_BIOS_IRQ_SCAN	0x2000
 #define PCI_ASSIGN_ALL_BUSSES	0x4000
+#define PCI_BUS_SORT		0x8000
 
 extern unsigned int pci_probe;
 
--- linux/arch/i386/kernel/pci-pc.c.order	Thu Oct 24 21:09:20 2002
+++ linux/arch/i386/kernel/pci-pc.c	Sat Oct 26 16:14:00 2002
@@ -19,7 +19,11 @@
 
 #include "pci-i386.h"
 
+#ifdef CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC
+unsigned int pci_probe = PCI_PROBE_BIOS | PCI_BUS_SORT | PCI_PROBE_CONF1 | PCI_PROBE_CONF2;
+#else
 unsigned int pci_probe = PCI_PROBE_BIOS | PCI_PROBE_CONF1 | PCI_PROBE_CONF2;
+#endif
 
 int pcibios_last_bus = -1;
 struct pci_bus *pci_root_bus = NULL;
@@ -871,6 +875,55 @@ static struct pci_ops * __devinit pci_fi
 	return NULL;
 }
 
+#ifdef CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC
+static void __devinit pci_sort_by_bus_slot_func(void)
+{
+	LIST_HEAD(sorted_devices);
+	struct list_head *ln;
+	struct pci_dev *dev, *d;
+	int n;
+	int s, slot;
+	int f, func;
+
+	printk(KERN_INFO "PCI: Sorting device list by bus, slot, function...\n");
+
+	/* Starting from bus 0, ...  */
+	for (n=0; n <= pcibios_last_bus; n++) {
+		if (!pci_bus_exists(&pci_root_buses, n))
+			continue;
+
+		while (!list_empty(&pci_devices)) {
+			/* Find the lowest remaining PCI slot/function.  */
+			slot = INT_MAX;
+			func = INT_MAX;
+			dev = NULL;
+			for (ln=pci_devices.next; ln != &pci_devices; ln=ln->next) {
+				d = pci_dev_g(ln);
+				s = PCI_SLOT(d->devfn);
+				f = PCI_FUNC(d->devfn);
+				if (d->bus->number == n
+				    && (s < slot || (s == slot && f < func))) {
+					slot = s;
+					func = f;
+					dev = d;
+				}
+			}
+
+			if (dev) {
+				list_del(&dev->global_list);
+				list_add_tail(&dev->global_list, &sorted_devices);
+			}
+			else {
+				/* Stop if we don't find any devices on
+				   this bus.  */
+				break;
+			}
+		}
+	}
+	list_splice(&sorted_devices, &pci_devices);
+}
+#endif
+
 /*
  * Sort the device list according to PCI BIOS. Nasty hack, but since some
  * fool forgot to define the `correct' device order in the PCI BIOS specs
@@ -1392,6 +1445,11 @@ void __init pcibios_init(void)
 
 	pcibios_resource_survey();
 
+#ifdef CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC
+	if ((pci_probe & PCI_BUS_SORT) && !(pci_probe & PCI_NO_SORT))
+		pci_sort_by_bus_slot_func();
+	else
+#endif
 #ifdef CONFIG_PCI_BIOS
 	if ((pci_probe & PCI_BIOS_SORT) && !(pci_probe & PCI_NO_SORT))
 		pcibios_sort();
@@ -1404,6 +1462,12 @@ char * __devinit  pcibios_setup(char *st
 		pci_probe = 0;
 		return NULL;
 	}
+#ifdef CONFIG_PCI_SORT_BY_BUS_SLOT_FUNC
+	else if (!strcmp(str, "nobussort")) {
+		pci_probe &= ~PCI_BUS_SORT;
+		return NULL;
+	}
+#endif
 #ifdef CONFIG_PCI_BIOS
 	else if (!strcmp(str, "bios")) {
 		pci_probe = PCI_PROBE_BIOS;

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

[Jeff Garzik]

s/__devinit/__init/ and the implementation looks ok to me

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

[Jeff Garzik]

Jeff Garzik wrote:

> s/__devinit/__init/ and the implementation looks ok to me



...except if your patch can be called in hotplug paths...

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

[H. J. Lu]

On Sat, Oct 26, 2002 at 08:05:13PM -0400, Jeff Garzik wrote:
> Jeff Garzik wrote:
> 
> > s/__devinit/__init/ and the implementation looks ok to me
> 
> 
> 
> ...except if your patch can be called in hotplug paths...

There are plenty of __devini in arch/i386/kernel/pci-pc.c. I will leave
mine alone.


H.J.

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

[Greg KH]

On Sat, Oct 26, 2002 at 05:25:26PM -0700, H. J. Lu wrote:
> On Sat, Oct 26, 2002 at 08:05:13PM -0400, Jeff Garzik wrote:
> > Jeff Garzik wrote:
> > 
> > > s/__devinit/__init/ and the implementation looks ok to me
> > 
> > 
> > 
> > ...except if your patch can be called in hotplug paths...
> 
> There are plenty of __devini in arch/i386/kernel/pci-pc.c. I will leave
> mine alone.

That is because those functions can be called in PCI hotplug paths,
since yours is only called during init, it should be marked as such.

thanks,

greg k-h

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

[H. J. Lu]

On Sun, Oct 27, 2002 at 09:42:49AM -0800, Greg KH wrote:
> On Sat, Oct 26, 2002 at 05:25:26PM -0700, H. J. Lu wrote:
> > On Sat, Oct 26, 2002 at 08:05:13PM -0400, Jeff Garzik wrote:
> > > Jeff Garzik wrote:
> > > 
> > > > s/__devinit/__init/ and the implementation looks ok to me
> > > 
> > > 
> > > 
> > > ...except if your patch can be called in hotplug paths...
> > 
> > There are plenty of __devini in arch/i386/kernel/pci-pc.c. I will leave
> > mine alone.
> 
> That is because those functions can be called in PCI hotplug paths,
> since yours is only called during init, it should be marked as such.
> 

Are you telling me that pcibios_sort will be called by PCI hotplug?


H.J.

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

[Jeff Garzik]

H. J. Lu wrote:

>On Sun, Oct 27, 2002 at 09:42:49AM -0800, Greg KH wrote:
>  
>
>>On Sat, Oct 26, 2002 at 05:25:26PM -0700, H. J. Lu wrote:
>>    
>>
>>>On Sat, Oct 26, 2002 at 08:05:13PM -0400, Jeff Garzik wrote:
>>>      
>>>
>>>>Jeff Garzik wrote:
>>>>
>>>>        
>>>>
>>>>>s/__devinit/__init/ and the implementation looks ok to me
>>>>>          
>>>>>
>>>>
>>>>...except if your patch can be called in hotplug paths...
>>>>        
>>>>
>>>There are plenty of __devini in arch/i386/kernel/pci-pc.c. I will leave
>>>mine alone.
>>>      
>>>
>>That is because those functions can be called in PCI hotplug paths,
>>since yours is only called during init, it should be marked as such.
>>
>>    
>>
>
>Are you telling me that pcibios_sort will be called by PCI hotplug?
>  
>

no, he's saying __init is correct, as Alan and I said.

    Jeff

Re: PATCH: Support PCI device sorting (Re: PCI device order problem)

[Alan Cox]

On Sun, 2002-10-27 at 01:05, Jeff Garzik wrote:
> Jeff Garzik wrote:
> 
> > s/__devinit/__init/ and the implementation looks ok to me
> 
> 
> 
> ...except if your patch can be called in hotplug paths...

We can't go around re-sorting the PCI devices. The pci lists are not
locked sanely as it is