The bus number it reports will be totally bogus for devices behind PCI
bridges. As a consequence AML will peek and poke at registers of the
wrong device. This is what caused the suspend issues with Joris'
Macbook.
The diff below attempts to fix this by using the mapping between AML
nodes and PCI devices that we establish. Because _INI methods may end
up executing AML that ends up calling aml_rdpciaddr(), the mapping is
now established before we execute _INI. I'm not 100% sure this is
correct as there is a possibility that _INI will poke some PCI bridges
in a way that changes the mapping. Only one way to find out...
The code also deals with devices that aren't there in a slightly
different way. They are now included in the node -> PCI mapping, but
they're still not included in the list of PCI device nodes that we
create. Writing to config space for devices that aren't there is
implemented as a no-op. Reading will return an all-ones pattern.
So far I've only tested this on my x1. More testing is needed.
Index: dev/acpi/acpi.c
===================================================================
RCS file: /cvs/src/sys/dev/acpi/acpi.c,v
retrieving revision 1.316
diff -u -p -r1.316 acpi.c
--- dev/acpi/acpi.c 18 Sep 2016 23:56:45 -0000 1.316
+++ dev/acpi/acpi.c 19 Oct 2016 19:42:51 -0000
@@ -614,6 +614,7 @@ acpi_getpci(struct aml_node *node, void
pci->dev = ACPI_ADR_PCIDEV(val);
pci->fun = ACPI_ADR_PCIFUN(val);
pci->node = node;
+ node->pci = pci;
pci->sub = -1;
dnprintf(10, "%.2x:%.2x.%x -> %s\n",
@@ -639,17 +640,12 @@ acpi_getpci(struct aml_node *node, void
pci->_s4w = -1;
/* Check if PCI device exists */
- if (pci->dev > 0x1F || pci->fun > 7) {
- free(pci, M_DEVBUF, sizeof(*pci));
- return (1);
- }
+ if (pci->dev > 31 || pci->fun > 7)
+ return 1;
tag = pci_make_tag(pc, pci->bus, pci->dev, pci->fun);
reg = pci_conf_read(pc, tag, PCI_ID_REG);
- if (PCI_VENDOR(reg) == PCI_VENDOR_INVALID) {
- free(pci, M_DEVBUF, sizeof(*pci));
- return (1);
- }
- node->pci = pci;
+ if (PCI_VENDOR(reg) == PCI_VENDOR_INVALID)
+ return 1;
TAILQ_INSERT_TAIL(&acpi_pcidevs, pci, next);
@@ -1066,11 +1062,11 @@ acpi_attach(struct device *parent, struc
config_found_sm(self, &aaa, acpi_print, acpi_submatch);
}
+ /* get PCI mapping */
+ aml_walknodes(&aml_root, AML_WALK_PRE, acpi_getpci, sc);
+
/* initialize runtime environment */
aml_find_node(&aml_root, "_INI", acpi_inidev, sc);
-
- /* Get PCI mapping */
- aml_walknodes(&aml_root, AML_WALK_PRE, acpi_getpci, sc);
/* attach pci interrupt routing tables */
aml_find_node(&aml_root, "_PRT", acpi_foundprt, sc);
Index: dev/acpi/dsdt.c
===================================================================
RCS file: /cvs/src/sys/dev/acpi/dsdt.c,v
retrieving revision 1.225
diff -u -p -r1.225 dsdt.c
--- dev/acpi/dsdt.c 27 Sep 2016 10:04:19 -0000 1.225
+++ dev/acpi/dsdt.c 19 Oct 2016 19:42:51 -0000
@@ -2216,21 +2216,16 @@ aml_rdpciaddr(struct aml_node *pcidev, u
{
int64_t res;
- if (aml_evalinteger(acpi_softc, pcidev, "_ADR", 0, NULL, &res) == 0) {
- addr->fun = res & 0xFFFF;
- addr->dev = res >> 16;
- }
- while (pcidev != NULL) {
- /* HID device (PCI or PCIE root): eval _BBN */
- if (__aml_search(pcidev, "_HID", 0)) {
- if (aml_evalinteger(acpi_softc, pcidev, "_BBN", 0,
NULL, &res) == 0) {
- addr->bus = res;
- break;
- }
- }
- pcidev = pcidev->parent;
- }
- return (0);
+ if (pcidev->pci == NULL)
+ return -1;
+
+ if (aml_evalinteger(acpi_softc, pcidev, "_ADR", 0, NULL, &res))
+ return -1;
+
+ addr->fun = res & 0xffff;
+ addr->dev = res >> 16;
+ addr->bus = pcidev->pci->bus;
+ return 0;
}
/* Read/Write from opregion object */
@@ -2272,7 +2267,12 @@ aml_rwgas(struct aml_value *rgn, int bpo
if (rgn->v_opregion.iospace == GAS_PCI_CFG_SPACE) {
/* Get PCI Root Address for this opregion */
- aml_rdpciaddr(rgn->node->parent, &pi);
+ if (aml_rdpciaddr(rgn->node->parent, &pi)) {
+ if (mode == ACPI_IOREAD)
+ pi.fun = 0xffff;
+ else
+ return;
+ }
}
tbit = &tmp.v_integer;
