tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Missing ACPI PCI devices in acpi_pcidev_scan



Hi Christoph,

As AcpiWalkNamespace performs a DFS (with both pre and post order visitor callbacks), the parent-child information could be stored during the ACPI walk done by acpi_build_tree (in acpi.c), provided that struct acpi_devnode is modified to contain that information. What do you think?

I appreciate it very much if you get acpi bus scanning done.

With the attached patch, all PCI devices in the ACPI namespace should be
detected by acpi_pcidev_scan().  This was more complex than I thought:
the _BBN only applies to the PCI bus segment of the PCI host bridge.
For PCI devices that are behind PCI-to-PCI bridges, getting the bus
segment number is more tricky.  By the way, I could not test that part
since there are no PCI devices behind PCI-to-PCI bridges in the ACPI
namespace of the two boxes that I use for testing.

The file acpica/OsdHardware.c already contained PCI bus scanning code
for ACPI.  The function AcpiOsDerivePciId() in that file is supposed to
do just that.  There was a bug in this function (the PCI address in the
returned PCI address is the address of the parent PCI-to-PCI bridge),
and moreover the interface has changed in recent ACPICA.  So I adapted
the implementation to make it easily compatible with the latest ACPICA,
as well as directly usable by acpi_pcidev_scan().  The main function
doing the PCI bus scan has been moved from acpica/OsdHardware.c to
acpi_pci.c.

Since AcpiOsDerivePciId() is supposed to be called at ACPI's PCI_Config
region initialization, there is surely a way to obtain the PCI id
directly from ACPI (instead of re-executing AcpiOsDerivePciId()), but I
do not know how to do that.

I should also mention that AcpiOsReadPciConfiguration() is used to read
the PCI configuration space, but this function does not take the segment
group (aka PCI domain) into account.


The way you propose sounds right to me.

This was about storing the parent-child relationships in acpi_devnode.
The AcpiWalkNamespace function in -current does not have the new
interface with both pre-order and post-order callbacks.  According to
ACPICA's changelog [1], this was added in november 2009.  I tried using
a stack (SLIST) to do it with the old interface, but the code was
becoming ugly, whereas it would be trivial with the new interface, so I
guess it's not worth it.

Best regards,

Grégoire

[1] http://www.acpica.org/download/changes.txt

Index: sys/dev/acpi/acpi_pci.c
===================================================================
RCS file: /cvsroot/src/sys/dev/acpi/acpi_pci.c,v
retrieving revision 1.2
diff -u -p -r1.2 acpi_pci.c
--- sys/dev/acpi/acpi_pci.c     4 Dec 2009 10:42:39 -0000       1.2
+++ sys/dev/acpi/acpi_pci.c     12 Feb 2010 15:43:59 -0000
@@ -49,6 +49,9 @@ __KERNEL_RCSID(0, "$NetBSD: acpi_pci.c,v
 #include <dev/acpi/acpivar.h>
 #include <dev/acpi/acpi_pci.h>
 
+#define _COMPONENT          ACPI_BUS_COMPONENT
+ACPI_MODULE_NAME            ("acpi_pci")
+
 struct acpi_pcidev;
 
 static TAILQ_HEAD(, acpi_pcidev) acpi_pcidevlist =
@@ -56,78 +59,73 @@ static TAILQ_HEAD(, acpi_pcidev) acpi_pc
 
 struct acpi_pcidev {
        struct acpi_devnode *ap_node;
-       uint32_t ap_pciseg;
-       uint32_t ap_pcibus;
-       uint32_t ap_pcidev;
-       uint32_t ap_pcifunc;
+       uint16_t ap_pciseg;
+       uint16_t ap_pcibus;
+       uint16_t ap_pcidev;
+       uint16_t ap_pcifunc;
        bool ap_pcihost;
        TAILQ_ENTRY(acpi_pcidev) ap_list;
 };
 
+/*
+ * Regarding PCI Segment Groups, the ACPI spec says:
+ *
+ * "The optional _SEG object is located under a PCI host bridge and
+ * evaluates to an integer that describes the PCI Segment Group (see PCI
+ * Firmware Specification v3.0)."
+ *
+ * "PCI Segment Group supports more than 256 buses in a system by allowing
+ * the reuse of the PCI bus numbers.  Within each PCI Segment Group, the bus
+ * numbers for the PCI buses must be unique. PCI buses in different PCI
+ * Segment Group are permitted to have the same bus number."
+ */
 
-static const char * const acpi_pcidev_ids[] = {
-       "PNP0A??",      /* ACPI PCI host controllers */
-       NULL
-};
+/*
+ * Regarding PCI Base Bus Numbers, the ACPI spec says:
+ *
+ * "For multi-root PCI platforms, the _BBN object evaluates to the PCI bus
+ * number that the BIOS assigns.  This is needed to access a PCI_Config
+ * operation region for the specified bus.  The _BBN object is located under
+ * a PCI host bridge and must be unique for every host bridge within a
+ * segment since it is the PCI bus number."
+ */
 
 static bool
 acpi_pcidev_add(struct acpi_softc *sc, struct acpi_devnode *ad)
 {
        struct acpi_pcidev *ap;
+       ACPI_PCI_ID pciid;
        ACPI_STATUS rv;
-       ACPI_INTEGER seg, bus, addr;
+       bool root;
 
        /*
-        * ACPI spec: "The _BBN object is located under a
-        * PCI host bridge and must be unique for every
-        * host bridge within a segment since it is the PCI bus number."
+        * ACPI spec: "If _SEG does not exist, OSPM assumes that all PCI bus
+        * segments are in PCI Segment Group 0."
         */
-       rv = acpi_eval_integer(ad->ad_handle, "_BBN", &bus);
-       if (ACPI_FAILURE(rv))
-               return false;
+       pciid.Segment = 0;
+
        /*
-         * The ACPI address (_ADR) is equal to: (device << 16) | function.
+        * For mono-root PCI platforms, the PCI bus segment of the PCI root
+        * bridge has number zero.
         */
-       rv = acpi_eval_integer(ad->ad_handle, "_ADR", &addr);
+       pciid.Bus = 0;
+
+       rv = acpi_pcidev_pciid(ad->ad_handle, &pciid, &root);
        if (ACPI_FAILURE(rv))
                return false;
 
-       /*
-        * ACPI spec: "The optional _SEG object is located under a PCI host
-        * bridge and evaluates to an integer that describes the
-        * PCI Segment Group (see PCI Firmware Specification v3.0)."
-        *
-        * "PCI Segment Group supports more than 256 buses
-        * in a system by allowing the reuse of the PCI bus numbers.
-        * Within each PCI Segment Group, the bus numbers for the PCI
-        * buses must be unique. PCI buses in different PCI Segment
-        * Group are permitted to have the same bus number."
-        */
-       rv = acpi_eval_integer(ad->ad_handle, "_SEG", &seg);
-       if (ACPI_FAILURE(rv)) {
-               /*
-                * ACPI spec: "If _SEG does not exist, OSPM assumes that all
-                * PCI bus segments are in PCI Segment Group 0."
-                */
-               seg = 0;
-       }
-
        ap = kmem_alloc(sizeof(*ap), KM_SLEEP);
        if (ap == NULL) {
                aprint_error("%s: kmem_alloc failed\n", __func__);
                return false;
        }
 
-       if (acpi_match_hid(ad->ad_devinfo, acpi_pcidev_ids))
-               ap->ap_pcihost = true;
-       else
-               ap->ap_pcihost = false;
-
        ap->ap_node = ad;
-       ap->ap_pciseg = seg;
-       ap->ap_pcibus = bus;
-       ap->ap_pcidev = addr >> 16;
-       ap->ap_pcifunc = addr & 0xffff;
+       ap->ap_pciseg = pciid.Segment;
+       ap->ap_pcibus = pciid.Bus;
+       ap->ap_pcidev = pciid.Device;
+       ap->ap_pcifunc = pciid.Function;
+       ap->ap_pcihost = root;
 
        TAILQ_INSERT_TAIL(&acpi_pcidevlist, ap, ap_list);
 
@@ -137,7 +135,16 @@ acpi_pcidev_add(struct acpi_softc *sc, s
 static void
 acpi_pcidev_print(struct acpi_pcidev *ap)
 {
+#ifdef ACPI_PCI_DEBUG
+       aprint_debug(" ");
+       if (ap->ap_pcihost)
+               aprint_debug("*");
+       aprint_debug("%s@%"PRIx16":%"PRIx16":%"PRIx16":%"PRIx16,
+           ap->ap_node->ad_name,
+           ap->ap_pciseg, ap->ap_pcibus, ap->ap_pcidev, ap->ap_pcifunc);
+#else
        aprint_debug(" %s", ap->ap_node->ad_name);
+#endif
 }
 
 int
@@ -212,3 +219,110 @@ acpi_pcidev_find(u_int segment, u_int bu
        *handlep = hdl;
        return (hdl != NULL) ? AE_OK : AE_NOT_FOUND;
 }
+
+/*
+ * acpi_pcidev_pciid:
+ *
+ *     Get PCI id (segment, bus, device, function) of an ACPI handle.
+ *     If the ACPI handle is successfully recognized as a PCI device,
+ *     fill *pciid and *root and return AE_OK. Otherwise, return a
+ *     failure exception code.  Recursive implementation.
+ *
+ *     The fields pciid->Segment and pciid->Bus are used as default
+ *     values for the PCI root bridge if the latter does not have a
+ *     _SEG or a _BBN integer object.
+ */
+ACPI_STATUS
+acpi_pcidev_pciid(ACPI_HANDLE handle, ACPI_PCI_ID *pciid, bool *root)
+{
+       ACPI_HANDLE phandle;
+       ACPI_INTEGER address;
+       /* Temporary variables declared static to limit stack usage. */
+       static ACPI_DEVICE_INFO *devinfo;
+       static ACPI_INTEGER val;
+       static ACPI_STATUS rv;
+       static uint8_t valb;
+
+       /* Get object info. */
+       rv = AcpiGetObjectInfo(handle, &devinfo);
+       if (ACPI_FAILURE(rv))
+               return rv;
+
+       if (devinfo->Type != ACPI_TYPE_DEVICE ||
+           !(devinfo->Valid & ACPI_VALID_ADR))
+               return AE_TYPE;
+
+       if (devinfo->Flags & ACPI_PCI_ROOT_BRIDGE) {
+               pciid->Device = ACPI_HIWORD(ACPI_LODWORD(devinfo->Address));
+               pciid->Function = ACPI_LOWORD(ACPI_LODWORD(devinfo->Address));
+               ACPI_FREE(devinfo);
+
+               rv = acpi_eval_integer(handle, METHOD_NAME__BBN, &val);
+               if (ACPI_SUCCESS(rv))
+                       pciid->Bus = ACPI_LOWORD(val);
+
+               /*
+                * ACPI spec: "The lower 16 bits of _SEG returned integer is
+                * the PCI Segment Group number.  Other bits are reserved."
+                */
+               rv = acpi_eval_integer(handle, METHOD_NAME__SEG, &val);
+               if (ACPI_SUCCESS(rv))
+                       pciid->Segment = ACPI_LOWORD(val);
+
+               *root = true;
+               return AE_OK;
+       }
+
+       /* Save address and free devinfo before recursive call. */
+       address = devinfo->Address;
+       ACPI_FREE(devinfo);
+
+       /*
+        * This is an ACPI device with an address.  We recognize it as a PCI
+        * device if its parent is a PCI root bridge or a PCI-to-PCI bridge.
+        */
+       rv = AcpiGetParent(handle, &phandle);
+       if (ACPI_FAILURE(rv))
+               return rv;
+
+       rv = acpi_pcidev_pciid(phandle, pciid, root);
+       if (ACPI_FAILURE(rv))
+               return rv;
+
+       if (*root) {
+               /*
+                * Our segment# and bus# are the same as the PCI root's ones.
+                */
+               pciid->Device = ACPI_HIWORD(ACPI_LODWORD(address));
+               pciid->Function = ACPI_LOWORD(ACPI_LODWORD(address));
+               *root = false;
+               return AE_OK;
+       }
+
+       /*
+        * Not a PCI root bridge.  Check that it is a PCI-to-PCI bridge.
+        */
+
+       /* read HDR_TYPE register */
+       rv = AcpiOsReadPciConfiguration(pciid, 0x0e, &valb, 8);
+       if (ACPI_FAILURE(rv))
+               return rv;
+       /* mask multifunction bit & check bridge type */
+       if ((valb & 0x7f) != 1 && (valb & 0x7f) != 2)
+               return AE_TYPE;
+
+       /*
+        * The parent is a PCI-to-PCI bridge.  Our bus# is the secondary bus#
+        * of the bridge.
+        */
+
+       /* read SECONDARY_BUS register */
+       rv = AcpiOsReadPciConfiguration(pciid, 0x19, &valb, 8);
+       if (ACPI_FAILURE(rv))
+               return rv;
+
+       pciid->Bus = valb;
+       pciid->Device = ACPI_HIWORD(ACPI_LODWORD(address));
+       pciid->Function = ACPI_LOWORD(ACPI_LODWORD(address));
+       return AE_OK;
+}
Index: sys/dev/acpi/acpi_pci.h
===================================================================
RCS file: /cvsroot/src/sys/dev/acpi/acpi_pci.h,v
retrieving revision 1.2
diff -u -p -r1.2 acpi_pci.h
--- sys/dev/acpi/acpi_pci.h     4 Dec 2009 10:42:39 -0000       1.2
+++ sys/dev/acpi/acpi_pci.h     12 Feb 2010 15:43:59 -0000
@@ -33,5 +33,6 @@
 
 int acpi_pcidev_scan(struct acpi_softc *);
 ACPI_STATUS acpi_pcidev_find(u_int, u_int, u_int, u_int, ACPI_HANDLE *);
+ACPI_STATUS acpi_pcidev_pciid(ACPI_HANDLE, ACPI_PCI_ID *, bool *);
 
 #endif /* _SYS_DEV_ACPI_ACPI_PCI_H */
Index: sys/dev/acpi/acpica/OsdHardware.c
===================================================================
RCS file: /cvsroot/src/sys/dev/acpi/acpica/OsdHardware.c,v
retrieving revision 1.5
diff -u -p -r1.5 OsdHardware.c
--- sys/dev/acpi/acpica/OsdHardware.c   15 Sep 2009 19:41:30 -0000      1.5
+++ sys/dev/acpi/acpica/OsdHardware.c   12 Feb 2010 15:44:00 -0000
@@ -51,6 +51,7 @@ __KERNEL_RCSID(0, "$NetBSD: OsdHardware.
 
 #include <dev/acpi/acpica.h>
 #include <dev/acpi/acpivar.h>
+#include <dev/acpi/acpi_pci.h>
 
 #include <machine/acpi_machdep.h>
 
@@ -276,72 +277,30 @@ AcpiOsWritePciConfiguration(ACPI_PCI_ID 
        return AE_OK;
 }
 
-/* get PCI bus# from root bridge recursively */
-static int
-get_bus_number(
-    ACPI_HANDLE        rhandle,
-    ACPI_HANDLE        chandle,
-    ACPI_PCI_ID        **PciId)
-{
-       ACPI_HANDLE handle;
-       ACPI_STATUS rv;
-       ACPI_OBJECT_TYPE type;
-       ACPI_PCI_ID *id;
-       ACPI_INTEGER v;
-       int bus;
-
-       id = *PciId;
-
-       rv = AcpiGetParent(chandle, &handle);
-       if (ACPI_FAILURE(rv))
-               return 0;
-
-       /*
-        * When handle == rhandle, we have valid PciId->Bus
-        * which was obtained from _BBN in evrgnini.c
-        * so we don't have to reevaluate _BBN.
-        */
-       if (handle != rhandle) {
-               bus = get_bus_number(rhandle, handle, PciId);
-
-               rv = AcpiGetType(handle, &type);
-               if (ACPI_FAILURE(rv) || type != ACPI_TYPE_DEVICE)
-                       return bus;
-
-               rv = acpi_eval_integer(handle, METHOD_NAME__ADR, &v);
-
-               if (ACPI_FAILURE(rv))
-                       return bus;
-
-               id->Bus = bus;
-               id->Device = ACPI_HIWORD((ACPI_INTEGER)v);
-               id->Function = ACPI_LOWORD((ACPI_INTEGER)v);
-
-               /* read HDR_TYPE register */
-               rv = AcpiOsReadPciConfiguration(id, 0x0e, &v, 8);
-               if (ACPI_SUCCESS(rv) &&
-                       /* mask multifunction bit & check bridge type */
-                       ((v & 0x7f) == 1 || (v & 0x7f) == 2)) {
-                       /* read SECONDARY_BUS register */
-                       rv = AcpiOsReadPciConfiguration(id, 0x19, &v, 8);
-                       if (ACPI_SUCCESS(rv))
-                               id->Bus = v;
-               }
-       }
-
-       return id->Bus;
-}
-
 /*
  * AcpiOsDerivePciId:
  *
- * Derive correct PCI bus# by traversing bridges
+ *     Derive correct PCI bus# by traversing bridges.
+ *
+ *     In recent ACPICA, this function does not take as argument the
+ *     handle to the PCI root bridge.  To facilitate the upgrade of
+ *     ACPICA in the future, we do not use this argument here (nor in
+ *     acpi_pcidev_pciid).
  */
-void
+ACPI_STATUS
 AcpiOsDerivePciId(
     ACPI_HANDLE        rhandle,
-    ACPI_HANDLE        chandle,
+    ACPI_HANDLE        handle,
     ACPI_PCI_ID        **PciId)
 {
-       (*PciId)->Bus = get_bus_number(rhandle, chandle, PciId);
+       ACPI_PCI_ID pciid;
+       ACPI_STATUS rv;
+       bool root;
+
+       pciid = **PciId;
+       rv = acpi_pcidev_pciid(handle, &pciid, &root);
+       if (ACPI_FAILURE(rv))
+               return rv;
+       (*PciId)->Bus = pciid.Bus;
+       return AE_OK;
 }
Index: sys/external/intel-public/acpica/dist/include/acpiosxf.h
===================================================================
RCS file: 
/cvsroot/src/sys/external/intel-public/acpica/dist/include/acpiosxf.h,v
retrieving revision 1.2
diff -u -p -r1.2 acpiosxf.h
--- sys/external/intel-public/acpica/dist/include/acpiosxf.h    18 Aug 2009 
16:33:11 -0000      1.2
+++ sys/external/intel-public/acpica/dist/include/acpiosxf.h    12 Feb 2010 
15:44:04 -0000
@@ -408,7 +408,7 @@ AcpiOsWritePciConfiguration (
 /*
  * Interim function needed for PCI IRQ routing
  */
-void
+ACPI_STATUS
 AcpiOsDerivePciId(
     ACPI_HANDLE             Rhandle,
     ACPI_HANDLE             Chandle,


Home | Main Index | Thread Index | Old Index