Subject: Re: port-i386/12880: Some BIOSs lie about the interrupt router device
To: None <netbsd-bugs@netbsd.org>
From: Dave Sainty <dave@dtsp.co.nz>
List: netbsd-bugs
Date: 05/11/2001 10:01:24
Dave Sainty writes:

> Masanori Kanaoka writes:
> 
> > Hi,
> > 
> >      From: minoura@netbsd.org (MINOURA Makoto)
> >   Subject: Re: port-i386/12880: \
> > 	   Some BIOSs lie about the interrupt router device
> >      Date: 09 May 2001 22:09:17 +0900
> > 
> > $ Is this a popular bug?  If so, the patch is nice to have by
> > $ default, I think.
> > 
> > If so,I also think the patch is nice.
> > But I think that adding condition(below) when using PCIBIOS said
> > 
> >     - router location is not 000:00:0  
> > 
> > rather than define ICU_BOGUS and #ifndef ICU_BOGUS.
> > 
> > How about next patch?
> 
> I definitely agree that this is more like what should be committed :)
> In light of your evidence it does seem like a) something needs to be
> done, and b) checking for 000:00:0 is a good heuristic.  (Sample size
> of two, extrapolated :)
> 
> One other change that I think makes sense would be to check the
> compatibility entry...  If the compatibility entry is valid then I'd
> expect that the device entry is valid too, even if 000:00:0.
> 
> And I guess if it's going to be "right", the quirk table check should
> really be there...  And more comments to explain what is going on :)
> 
> Cheers,
> 
> Dave

Meant to send the above and below to netbsd-bugs@netbsd.org too...

--- src/sys/arch/i386/pci/pci_intr_fixup.c.orig	Sun Apr 22 02:30:41 2001
+++ src/sys/arch/i386/pci/pci_intr_fixup.c	Fri May 11 01:20:21 2001
@@ -716,8 +716,21 @@
 	 * specified by the PIR Table, and use the compat ID,
 	 * if present.  Otherwise, we have to look for the router
 	 * ourselves (the PCI-ISA bridge).
+	 *
+	 * A number of buggy BIOS implementations leave the router
+	 * entry as 000:00:0, which is typically not the correct
+	 * device/function.  If the router device address is set to
+	 * this value, and the compatible router entry is undefined
+	 * (zero is the correct value to indicate undefined), then we
+	 * work on the basis it is most likely an error, and search
+	 * the entire device-space of bus 0 (but obviously starting
+	 * with 000:00:0, in case that really is the right one).
 	 */
-	if (pcibios_pir_header.signature != 0) {
+	if (pcibios_pir_header.signature != 0 &&
+	    (pcibios_pir_header.router_bus != 0 ||
+	     PIR_DEVFUNC_DEVICE(pcibios_pir_header.router_devfunc) != 0 ||
+	     PIR_DEVFUNC_FUNCTION(pcibios_pir_header.router_devfunc) != 0 ||
+	     pcibios_pir_header.compat_router != 0)) {
 		icutag = pci_make_tag(pc, pcibios_pir_header.router_bus,
 		    PIR_DEVFUNC_DEVICE(pcibios_pir_header.router_devfunc),
 		    PIR_DEVFUNC_FUNCTION(pcibios_pir_header.router_devfunc));
@@ -740,6 +753,10 @@
 		 * router.
 		 */
 		for (device = 0; device < maxdevs; device++) {
+			const struct pci_quirkdata *qd;
+			int function, nfuncs;
+			pcireg_t bhlcr;
+
 			icutag = pci_make_tag(pc, 0, device, 0);
 			icuid = pci_conf_read(pc, icutag, PCI_ID_REG);
 
@@ -750,10 +767,45 @@
 			if (PCI_VENDOR(icuid) == 0)
 				continue;
 
-			piit = pciintr_icu_lookup(icuid);
+			qd = pci_lookup_quirkdata(PCI_VENDOR(icuid),
+			    PCI_PRODUCT(icuid));
+
+			bhlcr = pci_conf_read(pc, icutag, PCI_BHLC_REG);
+			if (PCI_HDRTYPE_MULTIFN(bhlcr) ||
+			    (qd != NULL &&
+			     (qd->quirks & PCI_QUIRK_MULTIFUNCTION) != 0))
+				nfuncs = 8;
+			else
+				nfuncs = 1;
+
+			for (function = 0; function < nfuncs; function++) {
+				icutag = pci_make_tag(pc, 0, device, function);
+				icuid = pci_conf_read(pc, icutag, PCI_ID_REG);
+
+				/* Invalid vendor ID value? */
+				if (PCI_VENDOR(icuid) == PCI_VENDOR_INVALID)
+					continue;
+				/* Not invalid, but we've done this ~forever */
+				if (PCI_VENDOR(icuid) == 0)
+					continue;
+
+				piit = pciintr_icu_lookup(icuid);
+				if (piit != NULL)
+					break;
+			}
+
 			if (piit != NULL)
 				break;
 		}
+
+		/*
+		 * Invalidate the ICU ID.  If we failed to find the
+		 * interrupt router (piit == NULL) we don't want to
+		 * display a spurious device address below containing
+		 * the product information of the last device we
+		 * looked at.
+		 */
+		icuid = 0;
 	}
 
 	if (piit == NULL) {