On Wed, Oct 19, 2016 at 09:51:47PM +0200, Mark Kettenis wrote:
> 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.
Tested the diff together with the timer diff from Joris on MacBookAir6,2.
Does no blow up the machine. pci/ppb lines in dmesg change with this
(less Thunderbolt lines), dmesg diff below.
Regards,
Joerg
--- dmesg.old Thu Oct 20 19:32:44 2016
+++ dmesg.new Thu Oct 20 19:31:51 2016
@@ -1,4 +1,4 @@
-/bsd: OpenBSD 6.0-current (GENERIC.MP) #0: Wed Oct 19 20:30:41 CEST 2016
+/bsd: OpenBSD 6.0-current (GENERIC.MP) #0: Thu Oct 20 19:18:57 CEST 2016
/bsd: y...@marvin.hq.umaxx.net:/usr/src/sys/arch/amd64/compile/GENERIC.MP
/bsd: RTC BIOS diagnostic error
ff<clock_battery,ROM_cksum,config_unit,memory_size,fixed_disk,invalid_time>
/bsd: real mem = 8509276160 (8115MB)
@@ -17,7 +17,7 @@
/bsd: acpihpet0 at acpi0: 14318179 Hz
/bsd: acpimadt0 at acpi0 addr 0xfee00000: PC-AT compat
/bsd: cpu0 at mainbus0: apid 0 (boot processor)
-/bsd: cpu0: Intel(R) Core(TM) i7-4650U CPU @ 1.70GHz, 1600.21 MHz
+/bsd: cpu0: Intel(R) Core(TM) i7-4650U CPU @ 1.70GHz, 1600.28 MHz
/bsd: cpu0:
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,LONG,LAHF,ABM,PERF,ITSC,FSGSBASE,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,SENSOR,ARAT
/bsd: cpu0: 256KB 64b/line 8-way L2 cache
/bsd: cpu0: smt 0, core 0, package 0
@@ -25,17 +25,17 @@
/bsd: cpu0: apic clock running at 100MHz
/bsd: cpu0: mwait min=64, max=64, C-substates=0.2.1.2.4.1.1.1, IBE
/bsd: cpu1 at mainbus0: apid 2 (application processor)
-/bsd: cpu1: Intel(R) Core(TM) i7-4650U CPU @ 1.70GHz, 1600.00 MHz
+/bsd: cpu1: Intel(R) Core(TM) i7-4650U CPU @ 1.70GHz, 1600.01 MHz
/bsd: cpu1:
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,LONG,LAHF,ABM,PERF,ITSC,FSGSBASE,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,SENSOR,ARAT
/bsd: cpu1: 256KB 64b/line 8-way L2 cache
/bsd: cpu1: smt 0, core 1, package 0
/bsd: cpu2 at mainbus0: apid 1 (application processor)
-/bsd: cpu2: Intel(R) Core(TM) i7-4650U CPU @ 1.70GHz, 1600.00 MHz
+/bsd: cpu2: Intel(R) Core(TM) i7-4650U CPU @ 1.70GHz, 1600.01 MHz
/bsd: cpu2:
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,LONG,LAHF,ABM,PERF,ITSC,FSGSBASE,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,SENSOR,ARAT
/bsd: cpu2: 256KB 64b/line 8-way L2 cache
/bsd: cpu2: smt 1, core 0, package 0
/bsd: cpu3 at mainbus0: apid 3 (application processor)
-/bsd: cpu3: Intel(R) Core(TM) i7-4650U CPU @ 1.70GHz, 1600.00 MHz
+/bsd: cpu3: Intel(R) Core(TM) i7-4650U CPU @ 1.70GHz, 1600.01 MHz
/bsd: cpu3:
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,DS,ACPI,MMX,FXSR,SSE,SSE2,SS,HTT,TM,PBE,SSE3,PCLMUL,DTES64,MWAIT,DS-CPL,VMX,SMX,EST,TM2,SSSE3,SDBG,FMA3,CX16,xTPR,PDCM,PCID,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,DEADLINE,AES,XSAVE,AVX,F16C,RDRAND,NXE,PAGE1GB,LONG,LAHF,ABM,PERF,ITSC,FSGSBASE,BMI1,HLE,AVX2,SMEP,BMI2,ERMS,INVPCID,RTM,SENSOR,ARAT
/bsd: cpu3: 256KB 64b/line 8-way L2 cache
/bsd: cpu3: smt 1, core 1, package 0
@@ -92,22 +92,9 @@
/bsd: "Broadcom BCM4360" rev 0x03 at pci3 dev 0 function 0 not configured
/bsd: ppb3 at pci0 dev 28 function 4 "Intel 8 Series PCIE" rev 0xe4: msi
/bsd: pci4 at ppb3 bus 5
-/bsd: ppb4 at pci4 dev 0 function 0 "Intel DSL3510 Thunderbolt" rev 0x03
-/bsd: pci5 at ppb4 bus 6
-/bsd: ppb5 at pci5 dev 0 function 0 "Intel DSL3510 Thunderbolt" rev 0x03: msi
-/bsd: pci6 at ppb5 bus 7
-/bsd: "Intel DSL3510 Thunderbolt" rev 0x03 at pci6 dev 0 function 0 not
configured
-/bsd: ppb6 at pci5 dev 3 function 0 "Intel DSL3510 Thunderbolt" rev 0x03: msi
-/bsd: pci7 at ppb6 bus 8
-/bsd: ppb7 at pci5 dev 4 function 0 "Intel DSL3510 Thunderbolt" rev 0x03: msi
-/bsd: pci8 at ppb7 bus 57
-/bsd: ppb8 at pci5 dev 5 function 0 "Intel DSL3510 Thunderbolt" rev 0x03: msi
-/bsd: pci9 at ppb8 bus 106
-/bsd: ppb9 at pci5 dev 6 function 0 "Intel DSL3510 Thunderbolt" rev 0x03: msi
-/bsd: pci10 at ppb9 bus 107
-/bsd: ppb10 at pci0 dev 28 function 5 "Intel 8 Series PCIE" rev 0xe4: msi
-/bsd: pci11 at ppb10 bus 4
-/bsd: ahci0 at pci11 dev 0 function 0 "Samsung S4LN053X01" rev 0x01: apic 2
int 16, AHCI 1.3
+/bsd: ppb4 at pci0 dev 28 function 5 "Intel 8 Series PCIE" rev 0xe4: msi
+/bsd: pci5 at ppb4 bus 4
+/bsd: ahci0 at pci5 dev 0 function 0 "Samsung S4LN053X01" rev 0x01: apic 2 int
16, AHCI 1.3
/bsd: ahci0: port 0: 6.0Gb/s
/bsd: scsibus1 at ahci0: 32 targets
/bsd: sd0 at scsibus1 targ 0 lun 0: <ATA, APPLE SSD SM0512, UXM2> SCSI3
0/direct fixed naa.5002538655584d30
@@ -115,7 +102,7 @@
/bsd: pcib0 at pci0 dev 31 function 0 "Intel 8 Series LPC" rev 0x04
/bsd: ichiic0 at pci0 dev 31 function 3 "Intel 8 Series SMBus" rev 0x04: apic
2 int 18
/bsd: iic0 at ichiic0
-/bsd: iic0: addr 0x2c 03=fc 05=65 06=40 71=06 72=80 86=7c 90=37 91=1e 92=26
93=3b 94=6a 95=5c 96=6d 97=86 98=3d 99=1b 9a=9d 9f=7c a0=7f a1=b5 a2=bf a3=7b
a4=28 a5=cf a6=64 a7=2d words 00=0000 01=0000 02=00fc 03=fc00 04=0065 05=6540
06=4000 07=0000
+/bsd: iic0: addr 0x2c 03=fc 05=68 06=40 71=06 72=80 86=7c 90=37 91=1e 92=26
93=3b 94=6a 95=5c 96=6d 97=86 98=3d 99=1b 9a=9d 9f=7c a0=7f a1=b5 a2=bf a3=7b
a4=28 a5=cf a6=64 a7=2d words 00=0000 01=0000 02=00fc 03=fc00 04=0068 05=6840
06=4000 07=0000
/bsd: isa0 at pcib0
/bsd: isadma0 at isa0
/bsd: com0 at isa0 port 0x3f8/8 irq 4: ns8250, no fifo
>
> 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;
>
