Subject: MPACPI fix (for those who had trouble with it, urgent!)
To: None <current-users@netbsd.org, port-i386@netbsd.org,>
From: Takayoshi Kochi <kochi@netbsd.org>
List: current-users
Date: 05/04/2004 11:31:44
----Next_Part(Tue_May__4_11:31:44_2004_697)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

Hi,

I looked into the problem of MPACPI related panics and am trying to
fix it.  If you had any panic with the existing MPACPI code,
please try the patch attached to this message ASAP, so that
we can make it for 2.0-release.

And if you still see any panic with this patch, please let me know.
What I need to investigate is, quoting from what fvdl wrote on
March 29:

> 1) The name of the motherboard you have
> 2) Boot message output (preferably with MPVERBOSE switched on)
> 3) The output of acpidump. acpidump can be found in
>   pkgsrc/sysutils/acpidump

As well as failure report, any success reports are also welcome!

The problem seems that the logic finding subordinate bus number is
somewhat fragile and sometimes it can fail to find the number on some
ACPI BIOS implementation.
In the patch I tried to make the bus discovery logic more robust.

I tested this on two systems (dual P3 VIA chipset motherboard,
and P4 HT 865PE-based motherboard) and they are ok.

BTW, Frank, have you got any reply to the mail?
If you already have any DSDT that caused panic, I'd like to
investigate it.

---
Takayoshi Kochi

----Next_Part(Tue_May__4_11:31:44_2004_697)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: inline; filename="mpacpi.diff"

Index: arch/x86/x86/mpacpi.c
===================================================================
RCS file: /cvsroot/src/sys/arch/x86/x86/mpacpi.c,v
retrieving revision 1.23
diff -u -r1.23 mpacpi.c
--- arch/x86/x86/mpacpi.c	25 Apr 2004 11:25:35 -0000	1.23
+++ arch/x86/x86/mpacpi.c	4 May 2004 02:08:19 -0000
@@ -64,7 +64,8 @@
 #include <dev/pci/pcivar.h>
 #include <dev/pci/ppbreg.h>
 
-
+#include <dev/acpi/acpica.h>
+#include <dev/acpi/acpireg.h>
 #include <dev/acpi/acpivar.h>
 #include <dev/acpi/acpi_madt.h>
 
@@ -78,14 +79,9 @@
 #if NPCI > 0
 struct mpacpi_pcibus {
 	TAILQ_ENTRY(mpacpi_pcibus) mpr_list;
-	ACPI_HANDLE *mpr_handle;		/* Same thing really, but.. */
-	int mpr_bus;
-	int mpr_level;
-};
-
-struct mpacpi_walk_status {
-	struct mpacpi_pcibus *mpw_mpr;
-	struct acpi_softc *mpw_acpi;
+	ACPI_HANDLE mpr_handle;		/* Same thing really, but.. */
+	ACPI_BUFFER mpr_buf;		/* preserve _PRT */
+	int mpr_bus;			/* PCI bus number */
 };
 
 TAILQ_HEAD(, mpacpi_pcibus) mpacpi_pcibusses;
@@ -95,22 +91,19 @@
 int mpacpi_print(void *, const char *);
 int mpacpi_match(struct device *, struct cfdata *, void *);
 
-/*
- * acpi_madt_walk callbacks
- */
+/* acpi_madt_walk callbacks */
 static ACPI_STATUS mpacpi_count(APIC_HEADER *, void *);
 static ACPI_STATUS mpacpi_config_cpu(APIC_HEADER *, void *);
 static ACPI_STATUS mpacpi_config_ioapic(APIC_HEADER *, void *);
 static ACPI_STATUS mpacpi_nonpci_intr(APIC_HEADER *, void *);
 
 #if NPCI > 0
-/*
- * Callbacks for the device namespace walk.
- */
+/* Callbacks for the ACPI namespace walk */
 static ACPI_STATUS mpacpi_pcibus_cb(ACPI_HANDLE, UINT32, void *, void **);
+static int mpacpi_derive_bus(ACPI_HANDLE, struct acpi_softc *);
+
 static int mpacpi_pcircount(struct mpacpi_pcibus *);
 static int mpacpi_pciroute(struct mpacpi_pcibus *);
-static ACPI_STATUS mpacpi_pcihier_cb(ACPI_HANDLE, UINT32, void *, void **);
 static int mpacpi_find_pcibusses(struct acpi_softc *);
 
 static void mpacpi_print_pci_intr(int);
@@ -128,9 +121,7 @@
 #if NPCI > 0
 int mpacpi_npci;
 int mpacpi_maxpci;
-static int mpacpi_maxbuslevel;
 static int mpacpi_npciroots;
-static int mpacpi_npciknown;
 #endif
 
 static int mpacpi_intr_index;
@@ -365,6 +356,9 @@
 
 #if NPCI > 0
 
+/*
+ * Find all PCI busses from ACPI namespace and construct mpacpi_pcibusses list.
+ */
 static int
 mpacpi_find_pcibusses(struct acpi_softc *acpi)
 {
@@ -378,160 +372,193 @@
 	return 0;
 }
 
+static const char * const pciroot_hid[] = {
+	"PNP0A03",
+	NULL
+};
+
+static int
+mpacpi_derive_bus(ACPI_HANDLE handle, struct acpi_softc *acpi)
+{
+	ACPI_HANDLE parent, current;
+	ACPI_STATUS rv;
+	ACPI_INTEGER val;
+	ACPI_DEVICE_INFO *devinfo;
+	ACPI_BUFFER buf;
+	struct ac_dev {
+		TAILQ_ENTRY(ac_dev) list;
+		ACPI_HANDLE handle;
+	};
+	TAILQ_HEAD(, ac_dev) dev_list;
+	struct ac_dev *dev;
+	pcireg_t binf, class;
+	pcitag_t tag;
+	int bus;
+
+	bus = -1;
+	TAILQ_INIT(&dev_list);
+
+	/* 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)
+			return -1;
+		dev->handle = current;
+		TAILQ_INSERT_HEAD(&dev_list, dev, list);
+
+		rv = AcpiGetParent(current, &parent);
+		if (ACPI_FAILURE(rv))
+			return -1;
+
+		buf.Pointer = NULL;
+		buf.Length = ACPI_ALLOCATE_BUFFER;
+		rv = AcpiGetObjectInfo(parent, &buf);
+		if (ACPI_FAILURE(rv))
+			return -1;
+
+		devinfo = buf.Pointer;
+		if (acpi_match_hid(devinfo, pciroot_hid)) {
+			rv = acpi_eval_integer(current, METHOD_NAME__BBN, &val);
+			AcpiOsFree(buf.Pointer);
+			if (ACPI_SUCCESS(rv))
+				bus = ACPI_LOWORD(val);
+			else
+				/* assume bus = 0 */
+				bus = 0;
+			break;
+		}
+
+		AcpiOsFree(buf.Pointer);
+	}
+
+	/*
+	 * second, we step down from the root to the target
+	 * with resolving the bus number
+	 */
+	TAILQ_FOREACH(dev, &dev_list, list) {
+		rv = acpi_eval_integer(dev->handle, METHOD_NAME__ADR, &val);
+		if (ACPI_FAILURE(rv))
+			continue;	/* XXX */
+
+		tag = pci_make_tag(acpi->sc_pc, bus,
+		    ACPI_HIWORD(val), ACPI_LOWORD(val));
+
+		/* check if this is a bridge device */
+		class = pci_conf_read(acpi->sc_pc, tag, PCI_CLASS_REG);
+		if (PCI_CLASS(class) != PCI_CLASS_BRIDGE ||
+		    PCI_SUBCLASS(class) != PCI_SUBCLASS_BRIDGE_PCI)
+			continue;
+
+		/* if this is a bridge, get secondary bus */
+		binf = pci_conf_read(acpi->sc_pc, tag, PPB_REG_BUSINFO);
+		bus = PPB_BUSINFO_SECONDARY(binf);
+	}
+
+	/* cleanup */
+	while (!TAILQ_EMPTY(&dev_list)) {
+		dev = TAILQ_FIRST(&dev_list);
+		TAILQ_REMOVE(&dev_list, dev, list);
+		free(dev, M_TEMP);
+	}
+
+	return bus;
+}
+
 /*
  * Callback function for a namespace walk through ACPI space, finding all
- * PCI root busses.
+ * PCI root and subordinate busses.
  */
 static ACPI_STATUS
-mpacpi_pcibus_cb(ACPI_HANDLE handle, UINT32 level, void *ct, void **status)
+mpacpi_pcibus_cb(ACPI_HANDLE handle, UINT32 level, void *p, void **status)
 {
-	ACPI_STATUS ret;
+	ACPI_STATUS rv;
 	ACPI_BUFFER buf;
 	ACPI_INTEGER val;
+	ACPI_DEVICE_INFO *devinfo;
 	struct mpacpi_pcibus *mpr;
+	struct acpi_softc *acpi = p;
 
-	ret = acpi_get(handle, &buf, AcpiGetIrqRoutingTable);
-	if (ACPI_FAILURE(ret))
+	buf.Pointer = NULL;
+	buf.Length = ACPI_ALLOCATE_BUFFER;
+
+	/* get _HID, _CID and _STA */
+	rv = AcpiGetObjectInfo(handle, &buf);
+	if (ACPI_FAILURE(rv))
 		return AE_OK;
-	AcpiOsFree(buf.Pointer);
+
+	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)
+		goto out;
 
 	mpr = malloc(sizeof (struct mpacpi_pcibus), M_TEMP, M_WAITOK|M_ZERO);
-	if (mpr == NULL)
+	if (mpr == NULL) {
+		AcpiOsFree(buf.Pointer);
 		return AE_NO_MEMORY;
-	if (level == 1) {
-		ret = acpi_eval_integer(handle, METHOD_NAME__BBN, &val);
-		if (ACPI_FAILURE(ret)) {
-			mpr->mpr_bus = mpacpi_npciroots;
-			if (mp_verbose)
-				printf("mpacpi: could not get bus number for "
-				       "PCI root bus, assuming %d\n",
-				    mpr->mpr_bus);
-		} else
-			mpr->mpr_bus = ACPI_LOWORD(val);
-		mpacpi_npciroots++;
-		if (mp_verbose)
-			printf("mpacpi: found root PCI bus %d at level %u\n",
-			    mpr->mpr_bus, level);
-	} else {
-		mpr->mpr_bus = -1;
-		if (mp_verbose)
-			printf("mpacpi: found subordinate bus at level %u\n",
-			    level);
 	}
 
-	mpr->mpr_handle = handle;
-	mpr->mpr_level = (int)level;
-	TAILQ_INSERT_TAIL(&mpacpi_pcibusses, mpr, mpr_list);
-	mpacpi_npci++;
-	if ((int)level > mpacpi_maxbuslevel)
-		mpacpi_maxbuslevel = level;
-	return AE_OK;
-}
-
-static ACPI_STATUS
-mpacpi_pcihier_cb(ACPI_HANDLE handle, UINT32 level, void *ct, void **status)
-{
-	ACPI_STATUS ret;
-	ACPI_INTEGER val;
-	pcireg_t binf, class;
-	pcitag_t tag;
-	struct acpi_softc *acpi;
-	struct mpacpi_pcibus *mpr, *mparent;
-	struct mpacpi_walk_status *mpw = ct;
-	int bus;
-	int maxdev;
-
-	mparent = mpw->mpw_mpr;
-	acpi = mpw->mpw_acpi;
+	/* try get _PRT. if this fails, we're not interested in it */
+	rv = acpi_get(handle, &mpr->mpr_buf, AcpiGetIrqRoutingTable);
+	if (ACPI_FAILURE(rv)) {
+		free(mpr, M_TEMP);
+		goto out;
+	}
 
-	ret = acpi_eval_integer(handle, METHOD_NAME__ADR, &val);
-	if (ACPI_FAILURE(ret))
-		return AE_OK;
+	/* check whether this is PCI root bridge or not */
+	if (acpi_match_hid(devinfo, pciroot_hid)) {
+		/* this is PCI root bridge */
+
+		/* get the bus number */
+		rv = acpi_eval_integer(handle, METHOD_NAME__BBN, &val);
+		if (ACPI_SUCCESS(rv)) {
+			mpr->mpr_bus = ACPI_LOWORD(val);
+		} else {
+			/*
+			 * This often happens on systems which have only
+			 * one PCI root bus, assuming 0 will be ok.
+			 *
+			 * If there is a system that have multiple PCI
+			 * root but doesn't describe _BBN for every root,
+			 * the ASL is *broken*.
+			 */
+			if (mpacpi_npciroots != 0)
+				panic("mpacpi: ASL is broken");
 
-#if !defined(PCI_CONF_MODE)
-	maxdev = ( pci_mode == 1 ) ? 32 : 16;
-#elif (PCI_CONF_MODE == 1)
-	maxdev = 32;
-#elif (PCI_CONF_MODE == 2)
-	maxdev = 16;
-#endif
-
-	if ( mparent->mpr_bus >= 256 || ACPI_HIWORD(val) >= maxdev
-	     || ACPI_LOWORD(val) >= 8 ) {
-		if (mp_verbose) {
-			ACPI_BUFFER             Buffer;
-			ACPI_DEVICE_INFO        *Info;
-			ACPI_STATUS             Status;
-			ACPI_NATIVE_UINT        i;
-
-			printf("mpacpi: bad device found at bus %x dev %x, function %x\n",
-			       mparent->mpr_bus,
-			       ACPI_HIWORD(val),
-			       ACPI_LOWORD(val));
-
-			
-			Buffer.Length = ACPI_ALLOCATE_LOCAL_BUFFER;
-			
-			Status = AcpiGetObjectInfo (handle, &Buffer);
-			if (ACPI_SUCCESS (Status))
-			{
-				Info = Buffer.Pointer;
-				AcpiOsPrintf ("HID: %s, ADR: %8.8X%8.8X, Status %8.8X\n",
-					      Info->HardwareId.Value,
-					      ACPI_FORMAT_UINT64 (Info->Address),
-					      Info->CurrentStatus);
-
-				if (Info->Valid & ACPI_VALID_CID)
-				{
-					for (i = 0; i < Info->CompatibilityId.Count; i++)
-					{
-						AcpiOsPrintf ("CID #%d: %s\n", (UINT32) i, Info->CompatibilityId.Id[i].Value);
-					}
-				}
-				
-				ACPI_MEM_FREE (Info);
-			}
-			else
-			{
-				AcpiOsPrintf ("%s\n", AcpiFormatException (Status));
-			}
-			
+			printf("mpacpi: could not get bus number, "
+				    "assuming bus 0\n");
+			mpr->mpr_bus = 0;
 		}
-		return AE_OK;
-	}
-	
-	tag = pci_make_tag(acpi->sc_pc, mparent->mpr_bus,
-	    ACPI_HIWORD(val), ACPI_LOWORD(val));
-	class = pci_conf_read(acpi->sc_pc, tag, PCI_CLASS_REG);
-	if (PCI_CLASS(class) != PCI_CLASS_BRIDGE ||
-	    PCI_SUBCLASS(class) != PCI_SUBCLASS_BRIDGE_PCI)
-		return AE_OK;
-
-	TAILQ_FOREACH(mpr, &mpacpi_pcibusses, mpr_list)
-		if (mpr->mpr_handle == handle)
-			break;
-	if (mpr != NULL && mpr->mpr_bus != -1)
-		return AE_OK;
+		if (mp_verbose)
+			printf("mpacpi: found root PCI bus %d at level %u\n",
+			    mpr->mpr_bus, level);
+		mpacpi_npciroots++;
+	} else {
+		/* this is subordinate PCI bus (behind PCI-to-PCI bridge) */
 
-	binf = pci_conf_read(acpi->sc_pc, tag, PPB_REG_BUSINFO);
-	bus = PPB_BUSINFO_SECONDARY(binf);
+		/* we have no direct method to get the bus number... */
+		mpr->mpr_bus = mpacpi_derive_bus(handle, acpi);
 
-	if (mpr == NULL) {
+		if (mpr->mpr_bus < 0)
+			panic("failed to derive bus number");
 		if (mp_verbose)
-			printf("mpacpi: PCI bus %d has no ACPI handle; "
-			       "ignoring.\n", bus);
-		return AE_OK;
+			printf("mpacpi: found subordinate bus %d at level %u\n",
+			    mpr->mpr_bus, level);
 	}
 
-	mpr->mpr_bus = bus;
-	mpacpi_npciknown++;
-	if (mp_verbose)
-		printf("mpacpi: found subordinate PCI bus %d\n",
-		    mpr->mpr_bus);
+	mpr->mpr_handle = handle;
+	TAILQ_INSERT_TAIL(&mpacpi_pcibusses, mpr, mpr_list);
 
 	if (mpr->mpr_bus > mpacpi_maxpci)
 		mpacpi_maxpci = mpr->mpr_bus;
 
+	mpacpi_npci++;
+
+ out:
+	AcpiOsFree(buf.Pointer);
 	return AE_OK;
 }
 
@@ -541,8 +568,6 @@
 static int
 mpacpi_pciroute(struct mpacpi_pcibus *mpr)
 {
-	ACPI_STATUS ret;
-	ACPI_BUFFER buf;
 	ACPI_PCI_ROUTING_TABLE *ptrp;
 	char *p;
 	struct mp_intr_map *mpi;
@@ -551,10 +576,6 @@
 	unsigned dev;
 	int pin;
 
-	ret = acpi_get(mpr->mpr_handle, &buf, AcpiGetIrqRoutingTable);
-	if (ACPI_FAILURE(ret))
-		return -1;
-
 	if (mp_verbose)
 		printf("mpacpi: configuring PCI bus %d int routing\n",
 		    mpr->mpr_bus);
@@ -567,7 +588,7 @@
 	mpb->mb_intr_cfg = NULL;
 	mpb->mb_data = 0;
 
-	for (p = buf.Pointer; ; p += ptrp->Length) {
+	for (p = mpr->mpr_buf.Pointer; ; p += ptrp->Length) {
 		ptrp = (ACPI_PCI_ROUTING_TABLE *)p;
 		if (ptrp->Length == 0)
 			break;
@@ -599,37 +620,32 @@
 		mpi->global_int = ptrp->SourceIndex;
 		mpb->mb_intrs = mpi;
 	}
-	AcpiOsFree(buf.Pointer);
 	return 0;
 }
 
+/*
+ * Count number of elements in _PRT
+ */
 static int
 mpacpi_pcircount(struct mpacpi_pcibus *mpr)
 {
 	int count = 0;
-	ACPI_STATUS ret;
-	ACPI_BUFFER buf;
 	ACPI_PCI_ROUTING_TABLE *PrtElement;
 	UINT8 *Buffer;
 
-	ret = acpi_get(mpr->mpr_handle, &buf, AcpiGetIrqRoutingTable);
-	if (!ACPI_FAILURE(ret)) {
-		for (Buffer = buf.Pointer; ; Buffer += PrtElement->Length) {
-			PrtElement = (ACPI_PCI_ROUTING_TABLE *)Buffer;
-			if (PrtElement->Length == 0)
-				break;
-			count++;
-		}
-		AcpiOsFree(buf.Pointer);
+	for (Buffer = mpr->mpr_buf.Pointer;; Buffer += PrtElement->Length) {
+		PrtElement = (ACPI_PCI_ROUTING_TABLE *)Buffer;
+		if (PrtElement->Length == 0)
+			break;
+		count++;
 	}
+
 	return count;
 }
-
 #endif
 
 /*
- * Set up the interrupt config lists, in the same format as the mpbios
- * does.
+ * Set up the interrupt config lists, in the same format as the mpbios does.
  */
 static void
 mpacpi_config_irouting(struct acpi_softc *acpi)
@@ -637,46 +653,18 @@
 #if NPCI > 0
 	struct mpacpi_pcibus *mpr;
 #endif
-	int nintr, known;
+	int nintr;
 	int i, index;
 	struct mp_bus *mbp;
 	struct mp_intr_map *mpi;
 	struct ioapic_softc *ioapic;
-	struct mpacpi_walk_status mpw;
 
 	nintr = mpacpi_nintsrc + NUM_LEGACY_IRQS - 1;
-	mpacpi_npciknown = mpacpi_npciroots;
 #if NPCI > 0
 	TAILQ_FOREACH(mpr, &mpacpi_pcibusses, mpr_list) {
 		nintr += mpacpi_pcircount(mpr);
 	}
 
-	for (;;) {
-		known = mpacpi_npciknown;
-		TAILQ_FOREACH(mpr, &mpacpi_pcibusses, mpr_list) {
-			if (mpr->mpr_bus != -1) {
-				mpw.mpw_acpi = acpi;
-				mpw.mpw_mpr = mpr;
-				AcpiWalkNamespace(ACPI_TYPE_DEVICE,
-				    mpr->mpr_handle, 1, mpacpi_pcihier_cb,
-				    &mpw, NULL);
-			}
-		}
-		if (mpacpi_npciknown == mpacpi_npci) {
-			if (mp_verbose)
-				printf("mpacpi: resolved all PCI buses\n");
-			break;
-		}
-		if (mpacpi_npciknown == known) {
-			/*
-			 * Too committed at this point to find a graceful
-			 * way out.
-			 */
-			panic("mpacpi: couldn't find all PCI bus numbers");
-			break;
-		}
-	}
-
 	mp_isa_bus = mpacpi_maxpci + 1;
 #else
 	mp_isa_bus = 0;
@@ -820,13 +808,12 @@
 }
 
 
-
 int
 mpacpi_find_interrupts(void *self)
 {
 	ACPI_OBJECT_LIST arglist;
 	ACPI_OBJECT arg;
-	ACPI_STATUS ret;
+	ACPI_STATUS rv;
 	struct acpi_softc *acpi = self;
 	int i;
 
@@ -852,8 +839,8 @@
 	arglist.Pointer = &arg;
 	arg.Type = ACPI_TYPE_INTEGER;
 	arg.Integer.Value = 1;	/* I/O APIC mode (0 = PIC, 2 = IOSAPIC) */
-	ret = AcpiEvaluateObject(NULL, "\\_PIC", &arglist, NULL);
-	if (ACPI_FAILURE(ret)) {
+	rv = AcpiEvaluateObject(NULL, "\\_PIC", &arglist, NULL);
+	if (ACPI_FAILURE(rv)) {
 		if (mp_verbose)
 			printf("mpacpi: switch to APIC mode failed\n");
 		return 0;

----Next_Part(Tue_May__4_11:31:44_2004_697)----