> Date: Wed, 19 Oct 2016 21:51:47 +0200 (CEST)
> From: Mark Kettenis <[email protected]>
>
> 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.
This has been in snaps for a couple of days now. Anybody bold enough
to ok this?
> 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;
>
>