> Date: Wed, 19 Oct 2016 21:51:47 +0200 (CEST)
> From: Mark Kettenis <mark.kette...@xs4all.nl>
> 
> 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;
> 
> 

Reply via email to