--6c2NcOVqGQ03X4Wi
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Fri, Mar 09, 2007 at 06:24:59PM +0100, Quentin Garnier wrote:
> On Fri, Mar 09, 2007 at 05:51:09PM +0100, Manuel Bouyer wrote:
> > Hi,
> > an ACPI kernel doesn't boot on the dell 2950, it panics in pci_make_tag()
> > because this function is called with bus 0 device 0xffff function 0xffff.
> > With the attached patch it boots and I didn't notice any bad effects.
> > Does it make sense ?
> 
> The low word encodes the function, thus the test should against 8.
> 
> The reason why this happens is found in DSDT, in method INIS (in scope
> \_SB):  it is called by _INI method of \_SB (thus very early), and aims
> at deactivating PCIe-to-PCI bridges under some condition.
> 
> While it's not a bad idea to sanity check the values we pass to
> pci_make_tag, it's probably better to skip the device if it is not
> active.  When dev_list is populated in that function, there is a call to
> AcpiGetObjectInfo():  you should immediately test for its status before
> you go on with it, using the same test you can find in mpacpi_pcibus_cb
> (look for ACPI_STA_OK).
> 
> That means re-organising the for loop a bit, malloc'ing dev after that
> test.

OK, so what about the attached patch ? It works for me.

-- 
Manuel Bouyer <bouyer@antioche.eu.org>
     NetBSD: 26 ans d'experience feront toujours la difference
--

--6c2NcOVqGQ03X4Wi
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=diff

Index: x86/mpacpi.c
===================================================================
RCS file: /cvsroot/src/sys/arch/x86/x86/mpacpi.c,v
retrieving revision 1.46
diff -u -r1.46 mpacpi.c
--- x86/mpacpi.c	5 Mar 2007 16:51:03 -0000	1.46
+++ x86/mpacpi.c	28 Mar 2007 20:04:14 -0000
@@ -80,6 +80,8 @@
 
 #include "locators.h"
 
+#define ACPI_STA_OK (ACPI_STA_DEV_PRESENT|ACPI_STA_DEV_ENABLED|ACPI_STA_DEV_OK)
+
 /* XXX room for PCI-to-PCI bus */
 #define BUS_BUFFER (16)
 
@@ -475,11 +477,25 @@
 
 	/* first, search parent root bus */
 	for (current = handle;; current = parent) {
-		dev = malloc(sizeof(struct ac_dev), M_TEMP, M_WAITOK|M_ZERO);
-		if (dev == NULL)
+		buf.Pointer = NULL;
+		buf.Length = ACPI_ALLOCATE_BUFFER;
+		rv = AcpiGetObjectInfo(current, &buf);
+		if (ACPI_FAILURE(rv))
 			return -1;
-		dev->handle = current;
-		TAILQ_INSERT_HEAD(&dev_list, dev, list);
+
+		devinfo = buf.Pointer;
+		/* add this device to the list only if it's active */
+		if ((devinfo->Valid & ACPI_VALID_STA) == 0 ||
+		    (devinfo->CurrentStatus & ACPI_STA_OK) == ACPI_STA_OK) {
+			AcpiOsFree(buf.Pointer);
+			dev = malloc(sizeof(struct ac_dev), M_TEMP,
+			    M_WAITOK|M_ZERO);
+			if (dev == NULL)
+				return -1;
+			dev->handle = current;
+			TAILQ_INSERT_HEAD(&dev_list, dev, list);
+		}
+		AcpiOsFree(buf.Pointer);
 
 		rv = AcpiGetParent(current, &parent);
 		if (ACPI_FAILURE(rv))
@@ -492,6 +508,7 @@
 			return -1;
 
 		devinfo = buf.Pointer;
+
 		if (acpi_match_hid(devinfo, pciroot_hid)) {
 			rv = acpi_eval_integer(parent, METHOD_NAME__BBN, &val);
 			AcpiOsFree(buf.Pointer);
@@ -570,8 +587,6 @@
 
 	devinfo = buf.Pointer;
 
-#define ACPI_STA_OK (ACPI_STA_DEV_PRESENT|ACPI_STA_DEV_ENABLED|ACPI_STA_DEV_OK)
-
 	/* if this device is not active, ignore it */
 	if ((devinfo->Valid & ACPI_VALID_STA) &&
 	    (devinfo->CurrentStatus & ACPI_STA_OK) != ACPI_STA_OK)

--6c2NcOVqGQ03X4Wi--