| Message ID | 20170407132222.28300-1-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi Ard, On Fri, Apr 07, 2017 at 02:22:22PM +0100, Ard Biesheuvel wrote: > We currently derive legacy interrupt routing by matching _PRT > entries on the PCI device only, presumably under the assumption > that PRT entries always have a value of 0xffff in the function > field, meaning 'match all functions'. The spec (ACPI v6.0, sec 6.2.13) contains a note that: The PCI function number in the Address field of the _PRT packages must be 0xFFFF, indicating "any" function number or "all functions". If we need a patch like this, we need to somehow reconcile it with that spec text to make sure firmware and OS folks have a common understanding of how this is supposed to work. > This no longer holds for modern PCIe topologies, where the > legacy interrupts for different slots may be wired to different > functions on the same bridge device. For instance, on AMD Seattle, > we may have something like > > -[0000:00]-+-00.0 Advanced Micro Devices, Inc. [AMD] Device 1a00 > +-02.0 Advanced Micro Devices, Inc. [AMD] Device 1a01 > +-02.2-[01]----00.0 Renesas uPD720202 USB 3.0 Host Controller > \-02.3-[02]----00.0 Realtek RTL8169 PCIe Gigabit Ethernet > > where the _PRT describes the legacy interrupt routing as > > Name (_PRT, Package () // _PRT: PCI Routing Table > { > // slot 1: dev 2 fn 1 > Package () { 0x20001, 0x0, 0x0, 0x140 }, > Package () { 0x20001, 0x1, 0x0, 0x141 }, > Package () { 0x20001, 0x2, 0x0, 0x142 }, > Package () { 0x20001, 0x3, 0x0, 0x143 }, > > // slot 1: dev 2 fn 2 > Package () { 0x20002, 0x0, 0x0, 0x144 }, > Package () { 0x20002, 0x1, 0x0, 0x145 }, > Package () { 0x20002, 0x2, 0x0, 0x146 }, > Package () { 0x20002, 0x3, 0x0, 0x147 }, > > // slot 1: dev 2 fn 3 > Package () { 0x20003, 0x0, 0x0, 0x148 }, > Package () { 0x20003, 0x1, 0x0, 0x149 }, > Package () { 0x20003, 0x2, 0x0, 0x14a }, > Package () { 0x20003, 0x3, 0x0, 0x14b } > }) // _PRT But I think this _PRT description is incorrect and we should change the _PRT rather than the kernel. My laptop has a basically identical topology: -[0000:00]-+-00.0 Intel Corporation Sky Lake Host Bridge/DRAM Registers +-1c.0-[02]----00.0 Realtek Semiconductor Co., Ltd. Device 525a +-1c.2-[04]----00.0 Intel Corporation Wireless 8260 and the ASL looks like this (paraphrased): Device (EXP1) { Name (_ADR, 0x001C0000) Name (_PRT) { Package () { 0xFFFF, 0x00, \_SB.LNKA, 0x00 }, Package () { 0xFFFF, 0x01, \_SB.LNKB, 0x00 }, ... } } Device (EXP3) { Name (_ADR, 0x001C0002) Name (_PRT) { Package () { 0xFFFF, 0x00, \_SB.LNKC, 0x00 }, Package () { 0xFFFF, 0x01, \_SB.LNKD, 0x00 }, ... } } I assume the AMD Seattle device is on the motherboard, so I think this topology could be described via two _PRTs, one for each root port. > The current code always matches on the first entry when trying to > derive the legacy interrupt routing for the USB and Ethernet controllers > behind bridges 02.2 and 02.3, which results in the wrong entry to > be selected. > > So fix this by matching the function ID to the value in the _PRT entry > if the _PRT entries are not using 0xffff in the lower word to match > all functions. > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > drivers/acpi/pci_irq.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/acpi/pci_irq.c b/drivers/acpi/pci_irq.c > index c576a6fe4ebb..caac4ac94418 100644 > --- a/drivers/acpi/pci_irq.c > +++ b/drivers/acpi/pci_irq.c > @@ -160,6 +160,8 @@ static int acpi_pci_irq_check_entry(acpi_handle handle, struct pci_dev *dev, > struct acpi_prt_entry *entry; > > if (((prt->address >> 16) & 0xffff) != device || > + ((prt->address & 0xffff) != 0xffff && > + (prt->address & 0xffff) != PCI_FUNC(dev->devfn)) || > prt->pin + 1 != pin) > return -ENODEV; > > -- > 2.9.3 > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 7 April 2017 at 14:22, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > We currently derive legacy interrupt routing by matching _PRT > entries on the PCI device only, presumably under the assumption > that PRT entries always have a value of 0xffff in the function > field, meaning 'match all functions'. > > This no longer holds for modern PCIe topologies, where the > legacy interrupts for different slots may be wired to different > functions on the same bridge device. For instance, on AMD Seattle, > we may have something like > > -[0000:00]-+-00.0 Advanced Micro Devices, Inc. [AMD] Device 1a00 > +-02.0 Advanced Micro Devices, Inc. [AMD] Device 1a01 > +-02.2-[01]----00.0 Renesas uPD720202 USB 3.0 Host Controller > \-02.3-[02]----00.0 Realtek RTL8169 PCIe Gigabit Ethernet > > where the _PRT describes the legacy interrupt routing as > > Name (_PRT, Package () // _PRT: PCI Routing Table > { > // slot 1: dev 2 fn 1 > Package () { 0x20001, 0x0, 0x0, 0x140 }, > Package () { 0x20001, 0x1, 0x0, 0x141 }, > Package () { 0x20001, 0x2, 0x0, 0x142 }, > Package () { 0x20001, 0x3, 0x0, 0x143 }, > > // slot 1: dev 2 fn 2 > Package () { 0x20002, 0x0, 0x0, 0x144 }, > Package () { 0x20002, 0x1, 0x0, 0x145 }, > Package () { 0x20002, 0x2, 0x0, 0x146 }, > Package () { 0x20002, 0x3, 0x0, 0x147 }, > > // slot 1: dev 2 fn 3 > Package () { 0x20003, 0x0, 0x0, 0x148 }, > Package () { 0x20003, 0x1, 0x0, 0x149 }, > Package () { 0x20003, 0x2, 0x0, 0x14a }, > Package () { 0x20003, 0x3, 0x0, 0x14b } > }) // _PRT > > The current code always matches on the first entry when trying to > derive the legacy interrupt routing for the USB and Ethernet controllers > behind bridges 02.2 and 02.3, which results in the wrong entry to > be selected. > > So fix this by matching the function ID to the value in the _PRT entry > if the _PRT entries are not using 0xffff in the lower word to match > all functions. > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > drivers/acpi/pci_irq.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/acpi/pci_irq.c b/drivers/acpi/pci_irq.c > index c576a6fe4ebb..caac4ac94418 100644 > --- a/drivers/acpi/pci_irq.c > +++ b/drivers/acpi/pci_irq.c > @@ -160,6 +160,8 @@ static int acpi_pci_irq_check_entry(acpi_handle handle, struct pci_dev *dev, > struct acpi_prt_entry *entry; > > if (((prt->address >> 16) & 0xffff) != device || > + ((prt->address & 0xffff) != 0xffff && > + (prt->address & 0xffff) != PCI_FUNC(dev->devfn)) || > prt->pin + 1 != pin) > return -ENODEV; > > -- > 2.9.3 > It has been pointed out to me by Alan that the current wording of the ACPI spec requires the function field of the address to be 0xffff. Section 6.2.13 of the ACPI spec has this note Note: The PCI function number in the Address field of the _PRT packages must be 0xFFFF, indicating “any” function number or “all functions” of which I wonder if it makes sense for bridges as well. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 7 April 2017 at 18:06, Bjorn Helgaas <helgaas@kernel.org> wrote: > Hi Ard, > > On Fri, Apr 07, 2017 at 02:22:22PM +0100, Ard Biesheuvel wrote: >> We currently derive legacy interrupt routing by matching _PRT >> entries on the PCI device only, presumably under the assumption >> that PRT entries always have a value of 0xffff in the function >> field, meaning 'match all functions'. > > The spec (ACPI v6.0, sec 6.2.13) contains a note that: > > The PCI function number in the Address field of the _PRT packages > must be 0xFFFF, indicating "any" function number or "all functions". > > If we need a patch like this, we need to somehow reconcile it with > that spec text to make sure firmware and OS folks have a common > understanding of how this is supposed to work. > >> This no longer holds for modern PCIe topologies, where the >> legacy interrupts for different slots may be wired to different >> functions on the same bridge device. For instance, on AMD Seattle, >> we may have something like >> >> -[0000:00]-+-00.0 Advanced Micro Devices, Inc. [AMD] Device 1a00 >> +-02.0 Advanced Micro Devices, Inc. [AMD] Device 1a01 >> +-02.2-[01]----00.0 Renesas uPD720202 USB 3.0 Host Controller >> \-02.3-[02]----00.0 Realtek RTL8169 PCIe Gigabit Ethernet >> >> where the _PRT describes the legacy interrupt routing as >> >> Name (_PRT, Package () // _PRT: PCI Routing Table >> { >> // slot 1: dev 2 fn 1 >> Package () { 0x20001, 0x0, 0x0, 0x140 }, >> Package () { 0x20001, 0x1, 0x0, 0x141 }, >> Package () { 0x20001, 0x2, 0x0, 0x142 }, >> Package () { 0x20001, 0x3, 0x0, 0x143 }, >> >> // slot 1: dev 2 fn 2 >> Package () { 0x20002, 0x0, 0x0, 0x144 }, >> Package () { 0x20002, 0x1, 0x0, 0x145 }, >> Package () { 0x20002, 0x2, 0x0, 0x146 }, >> Package () { 0x20002, 0x3, 0x0, 0x147 }, >> >> // slot 1: dev 2 fn 3 >> Package () { 0x20003, 0x0, 0x0, 0x148 }, >> Package () { 0x20003, 0x1, 0x0, 0x149 }, >> Package () { 0x20003, 0x2, 0x0, 0x14a }, >> Package () { 0x20003, 0x3, 0x0, 0x14b } >> }) // _PRT > > But I think this _PRT description is incorrect and we should change > the _PRT rather than the kernel. My laptop has a basically identical > topology: > > -[0000:00]-+-00.0 Intel Corporation Sky Lake Host Bridge/DRAM Registers > +-1c.0-[02]----00.0 Realtek Semiconductor Co., Ltd. Device 525a > +-1c.2-[04]----00.0 Intel Corporation Wireless 8260 > > and the ASL looks like this (paraphrased): > > Device (EXP1) { > Name (_ADR, 0x001C0000) > Name (_PRT) { > Package () { 0xFFFF, 0x00, \_SB.LNKA, 0x00 }, > Package () { 0xFFFF, 0x01, \_SB.LNKB, 0x00 }, > ... > } > } > Device (EXP3) { > Name (_ADR, 0x001C0002) > Name (_PRT) { > Package () { 0xFFFF, 0x00, \_SB.LNKC, 0x00 }, > Package () { 0xFFFF, 0x01, \_SB.LNKD, 0x00 }, > ... > } > } > Thanks for the explanation. But how is this wired up into the PNP0A08 device then? IOW, how does the ACPI code in Linux discover the relation between these devices and my PCI root device? -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Apr 07, 2017 at 06:12:05PM +0100, Ard Biesheuvel wrote: > On 7 April 2017 at 18:06, Bjorn Helgaas <helgaas@kernel.org> wrote: > > Hi Ard, > > > > On Fri, Apr 07, 2017 at 02:22:22PM +0100, Ard Biesheuvel wrote: > >> We currently derive legacy interrupt routing by matching _PRT > >> entries on the PCI device only, presumably under the assumption > >> that PRT entries always have a value of 0xffff in the function > >> field, meaning 'match all functions'. > > > > The spec (ACPI v6.0, sec 6.2.13) contains a note that: > > > > The PCI function number in the Address field of the _PRT packages > > must be 0xFFFF, indicating "any" function number or "all functions". > > > > If we need a patch like this, we need to somehow reconcile it with > > that spec text to make sure firmware and OS folks have a common > > understanding of how this is supposed to work. > > > >> This no longer holds for modern PCIe topologies, where the > >> legacy interrupts for different slots may be wired to different > >> functions on the same bridge device. For instance, on AMD Seattle, > >> we may have something like > >> > >> -[0000:00]-+-00.0 Advanced Micro Devices, Inc. [AMD] Device 1a00 > >> +-02.0 Advanced Micro Devices, Inc. [AMD] Device 1a01 > >> +-02.2-[01]----00.0 Renesas uPD720202 USB 3.0 Host Controller > >> \-02.3-[02]----00.0 Realtek RTL8169 PCIe Gigabit Ethernet > >> > >> where the _PRT describes the legacy interrupt routing as > >> > >> Name (_PRT, Package () // _PRT: PCI Routing Table > >> { > >> // slot 1: dev 2 fn 1 > >> Package () { 0x20001, 0x0, 0x0, 0x140 }, > >> Package () { 0x20001, 0x1, 0x0, 0x141 }, > >> Package () { 0x20001, 0x2, 0x0, 0x142 }, > >> Package () { 0x20001, 0x3, 0x0, 0x143 }, > >> > >> // slot 1: dev 2 fn 2 > >> Package () { 0x20002, 0x0, 0x0, 0x144 }, > >> Package () { 0x20002, 0x1, 0x0, 0x145 }, > >> Package () { 0x20002, 0x2, 0x0, 0x146 }, > >> Package () { 0x20002, 0x3, 0x0, 0x147 }, > >> > >> // slot 1: dev 2 fn 3 > >> Package () { 0x20003, 0x0, 0x0, 0x148 }, > >> Package () { 0x20003, 0x1, 0x0, 0x149 }, > >> Package () { 0x20003, 0x2, 0x0, 0x14a }, > >> Package () { 0x20003, 0x3, 0x0, 0x14b } > >> }) // _PRT > > > > But I think this _PRT description is incorrect and we should change > > the _PRT rather than the kernel. My laptop has a basically identical > > topology: > > > > -[0000:00]-+-00.0 Intel Corporation Sky Lake Host Bridge/DRAM Registers > > +-1c.0-[02]----00.0 Realtek Semiconductor Co., Ltd. Device 525a > > +-1c.2-[04]----00.0 Intel Corporation Wireless 8260 > > > > and the ASL looks like this (paraphrased): > > > > Device (EXP1) { > > Name (_ADR, 0x001C0000) > > Name (_PRT) { > > Package () { 0xFFFF, 0x00, \_SB.LNKA, 0x00 }, > > Package () { 0xFFFF, 0x01, \_SB.LNKB, 0x00 }, > > ... > > } > > } > > Device (EXP3) { > > Name (_ADR, 0x001C0002) > > Name (_PRT) { > > Package () { 0xFFFF, 0x00, \_SB.LNKC, 0x00 }, > > Package () { 0xFFFF, 0x01, \_SB.LNKD, 0x00 }, > > ... > > } > > } > > > > Thanks for the explanation. But how is this wired up into the PNP0A08 > device then? IOW, how does the ACPI code in Linux discover the > relation between these devices and my PCI root device? Device (PCI0) { Name (_ADR, 0x00) Name (_HID, "PNP0A08") Device (EXP1) { ... } Device (EXP3) { ... } } -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Apr 07, 2017 at 06:12:05PM +0100, Ard Biesheuvel wrote: > On 7 April 2017 at 18:06, Bjorn Helgaas <helgaas@kernel.org> wrote: > > Hi Ard, > > > > On Fri, Apr 07, 2017 at 02:22:22PM +0100, Ard Biesheuvel wrote: > >> We currently derive legacy interrupt routing by matching _PRT > >> entries on the PCI device only, presumably under the assumption > >> that PRT entries always have a value of 0xffff in the function > >> field, meaning 'match all functions'. > > > > The spec (ACPI v6.0, sec 6.2.13) contains a note that: > > > > The PCI function number in the Address field of the _PRT packages > > must be 0xFFFF, indicating "any" function number or "all functions". > > > > If we need a patch like this, we need to somehow reconcile it with > > that spec text to make sure firmware and OS folks have a common > > understanding of how this is supposed to work. > > > >> This no longer holds for modern PCIe topologies, where the > >> legacy interrupts for different slots may be wired to different > >> functions on the same bridge device. For instance, on AMD Seattle, > >> we may have something like > >> > >> -[0000:00]-+-00.0 Advanced Micro Devices, Inc. [AMD] Device 1a00 > >> +-02.0 Advanced Micro Devices, Inc. [AMD] Device 1a01 > >> +-02.2-[01]----00.0 Renesas uPD720202 USB 3.0 Host Controller > >> \-02.3-[02]----00.0 Realtek RTL8169 PCIe Gigabit Ethernet > >> > >> where the _PRT describes the legacy interrupt routing as > >> > >> Name (_PRT, Package () // _PRT: PCI Routing Table > >> { > >> // slot 1: dev 2 fn 1 > >> Package () { 0x20001, 0x0, 0x0, 0x140 }, > >> Package () { 0x20001, 0x1, 0x0, 0x141 }, > >> Package () { 0x20001, 0x2, 0x0, 0x142 }, > >> Package () { 0x20001, 0x3, 0x0, 0x143 }, > >> > >> // slot 1: dev 2 fn 2 > >> Package () { 0x20002, 0x0, 0x0, 0x144 }, > >> Package () { 0x20002, 0x1, 0x0, 0x145 }, > >> Package () { 0x20002, 0x2, 0x0, 0x146 }, > >> Package () { 0x20002, 0x3, 0x0, 0x147 }, > >> > >> // slot 1: dev 2 fn 3 > >> Package () { 0x20003, 0x0, 0x0, 0x148 }, > >> Package () { 0x20003, 0x1, 0x0, 0x149 }, > >> Package () { 0x20003, 0x2, 0x0, 0x14a }, > >> Package () { 0x20003, 0x3, 0x0, 0x14b } > >> }) // _PRT > > > > But I think this _PRT description is incorrect and we should change > > the _PRT rather than the kernel. My laptop has a basically identical > > topology: > > > > -[0000:00]-+-00.0 Intel Corporation Sky Lake Host Bridge/DRAM Registers > > +-1c.0-[02]----00.0 Realtek Semiconductor Co., Ltd. Device 525a > > +-1c.2-[04]----00.0 Intel Corporation Wireless 8260 > > > > and the ASL looks like this (paraphrased): > > > > Device (EXP1) { > > Name (_ADR, 0x001C0000) > > Name (_PRT) { > > Package () { 0xFFFF, 0x00, \_SB.LNKA, 0x00 }, > > Package () { 0xFFFF, 0x01, \_SB.LNKB, 0x00 }, > > ... > > } > > } > > Device (EXP3) { > > Name (_ADR, 0x001C0002) > > Name (_PRT) { > > Package () { 0xFFFF, 0x00, \_SB.LNKC, 0x00 }, > > Package () { 0xFFFF, 0x01, \_SB.LNKD, 0x00 }, > > ... > > } > > } > > > > Thanks for the explanation. But how is this wired up into the PNP0A08 > device then? IOW, how does the ACPI code in Linux discover the > relation between these devices and my PCI root device? You describe the PCI hierarchy starting from PNP0A08 at root and the kernel assigns the ACPI companion through _ADR matching (see acpi_pci_find_companion()) which is what is used by _PRT parsing code to route IRQs IIUC. Lorenzo -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 7 April 2017 at 19:06, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: > On Fri, Apr 07, 2017 at 06:12:05PM +0100, Ard Biesheuvel wrote: >> On 7 April 2017 at 18:06, Bjorn Helgaas <helgaas@kernel.org> wrote: >> > Hi Ard, >> > >> > On Fri, Apr 07, 2017 at 02:22:22PM +0100, Ard Biesheuvel wrote: >> >> We currently derive legacy interrupt routing by matching _PRT >> >> entries on the PCI device only, presumably under the assumption >> >> that PRT entries always have a value of 0xffff in the function >> >> field, meaning 'match all functions'. >> > >> > The spec (ACPI v6.0, sec 6.2.13) contains a note that: >> > >> > The PCI function number in the Address field of the _PRT packages >> > must be 0xFFFF, indicating "any" function number or "all functions". >> > >> > If we need a patch like this, we need to somehow reconcile it with >> > that spec text to make sure firmware and OS folks have a common >> > understanding of how this is supposed to work. >> > >> >> This no longer holds for modern PCIe topologies, where the >> >> legacy interrupts for different slots may be wired to different >> >> functions on the same bridge device. For instance, on AMD Seattle, >> >> we may have something like >> >> >> >> -[0000:00]-+-00.0 Advanced Micro Devices, Inc. [AMD] Device 1a00 >> >> +-02.0 Advanced Micro Devices, Inc. [AMD] Device 1a01 >> >> +-02.2-[01]----00.0 Renesas uPD720202 USB 3.0 Host Controller >> >> \-02.3-[02]----00.0 Realtek RTL8169 PCIe Gigabit Ethernet >> >> >> >> where the _PRT describes the legacy interrupt routing as >> >> >> >> Name (_PRT, Package () // _PRT: PCI Routing Table >> >> { >> >> // slot 1: dev 2 fn 1 >> >> Package () { 0x20001, 0x0, 0x0, 0x140 }, >> >> Package () { 0x20001, 0x1, 0x0, 0x141 }, >> >> Package () { 0x20001, 0x2, 0x0, 0x142 }, >> >> Package () { 0x20001, 0x3, 0x0, 0x143 }, >> >> >> >> // slot 1: dev 2 fn 2 >> >> Package () { 0x20002, 0x0, 0x0, 0x144 }, >> >> Package () { 0x20002, 0x1, 0x0, 0x145 }, >> >> Package () { 0x20002, 0x2, 0x0, 0x146 }, >> >> Package () { 0x20002, 0x3, 0x0, 0x147 }, >> >> >> >> // slot 1: dev 2 fn 3 >> >> Package () { 0x20003, 0x0, 0x0, 0x148 }, >> >> Package () { 0x20003, 0x1, 0x0, 0x149 }, >> >> Package () { 0x20003, 0x2, 0x0, 0x14a }, >> >> Package () { 0x20003, 0x3, 0x0, 0x14b } >> >> }) // _PRT >> > >> > But I think this _PRT description is incorrect and we should change >> > the _PRT rather than the kernel. My laptop has a basically identical >> > topology: >> > >> > -[0000:00]-+-00.0 Intel Corporation Sky Lake Host Bridge/DRAM Registers >> > +-1c.0-[02]----00.0 Realtek Semiconductor Co., Ltd. Device 525a >> > +-1c.2-[04]----00.0 Intel Corporation Wireless 8260 >> > >> > and the ASL looks like this (paraphrased): >> > >> > Device (EXP1) { >> > Name (_ADR, 0x001C0000) >> > Name (_PRT) { >> > Package () { 0xFFFF, 0x00, \_SB.LNKA, 0x00 }, >> > Package () { 0xFFFF, 0x01, \_SB.LNKB, 0x00 }, >> > ... >> > } >> > } >> > Device (EXP3) { >> > Name (_ADR, 0x001C0002) >> > Name (_PRT) { >> > Package () { 0xFFFF, 0x00, \_SB.LNKC, 0x00 }, >> > Package () { 0xFFFF, 0x01, \_SB.LNKD, 0x00 }, >> > ... >> > } >> > } >> > >> >> Thanks for the explanation. But how is this wired up into the PNP0A08 >> device then? IOW, how does the ACPI code in Linux discover the >> relation between these devices and my PCI root device? > > You describe the PCI hierarchy starting from PNP0A08 at root and the > kernel assigns the ACPI companion through _ADR matching (see > acpi_pci_find_companion()) which is what is used by _PRT parsing > code to route IRQs IIUC. > OK, I have changed my DSDT as follows: Device (PCI0) { Name (_ADR, 0x00) Name (_HID, "PNP0A08" /* PCI Express Bus */) // _HID: Hardware ID Name (_CID, "PNP0A03" /* PCI Bus */) // _CID: Compatible ID Name (_SEG, 0x00) // _SEG: PCI Segment Name (_BBN, 0x00) // _BBN: BIOS Bus Number Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute Device (EXP1) { Name (_ADR, 0x20001) // _ADR: Address Name (_PRT, Package () // _PRT: PCI Routing Table { // slot 1: dev 2 fn 1 Package () { 0xFFFF, 0x0, 0x0, 0x140 }, Package () { 0xFFFF, 0x1, 0x0, 0x141 }, Package () { 0xFFFF, 0x2, 0x0, 0x142 }, Package () { 0xFFFF, 0x3, 0x0, 0x143 } }) // _PRT } Device (EXP2) { Name (_ADR, 0x20002) // _ADR: Address Name (_PRT, Package () // _PRT: PCI Routing Table { // slot 2: dev 2 fn 2 Package () { 0xFFFF, 0x0, 0x0, 0x144 }, Package () { 0xFFFF, 0x1, 0x0, 0x145 }, Package () { 0xFFFF, 0x2, 0x0, 0x146 }, Package () { 0xFFFF, 0x3, 0x0, 0x147 } }) // _PRT } Device (EXP3) { Name (_ADR, 0x20003) // _ADR: Address Name (_PRT, Package () // _PRT: PCI Routing Table { // slot 3: dev 2 fn 3 Package () { 0xFFFF, 0x0, 0x0, 0x148 }, Package () { 0xFFFF, 0x1, 0x0, 0x149 }, Package () { 0xFFFF, 0x2, 0x0, 0x14a }, Package () { 0xFFFF, 0x3, 0x0, 0x14b } }) // _PRT } but it does not get picked up, and I am back to [ 3.357555] pcieport 0000:00:02.2: can't derive routing for PCI INT A [ 3.370477] pcieport 0000:00:02.2: PCI INT A: no GSI [ 3.380549] pcieport 0000:00:02.3: can't derive routing for PCI INT A [ 3.393476] pcieport 0000:00:02.3: PCI INT A: no GSI Then I tried switching to Device (SLT1) { Name(_HID, EISAID("PNP0C0F")) Name(_UID, 0x1) Name(_PRS, ResourceTemplate() { Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive) { 0x140 } Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive) { 0x141 } Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive) { 0x142 } Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive) { 0x143 } }) Method (_CRS, 0) { Return (_PRS) } Method (_SRS, 1) { } Method (_DIS) { } } Device (SLT2) { Name(_HID, EISAID("PNP0C0F")) Name(_UID, 0x2) Name(_PRS, ResourceTemplate() { Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive) { 0x144 } Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive) { 0x145 } Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive) { 0x146 } Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive) { 0x147 } }) Method (_CRS, 0) { Return (_PRS) } Method (_SRS, 1) { } Method (_DIS) { } } Device (SLT3) { Name(_HID, EISAID("PNP0C0F")) Name(_UID, 0x3) Name(_PRS, ResourceTemplate() { Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive) { 0x148 } Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive) { 0x149 } Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive) { 0x14A } Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive) { 0x14B } }) Method (_CRS, 0) { Return (_PRS) } Method (_SRS, 1) { } Method (_DIS) { } } // // PCIe Root Bus // Device (PCI0) { Name (_HID, "PNP0A08" /* PCI Express Bus */) // _HID: Hardware ID Name (_CID, "PNP0A03" /* PCI Bus */) // _CID: Compatible ID Name (_SEG, 0x00) // _SEG: PCI Segment Name (_BBN, 0x00) // _BBN: BIOS Bus Number Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute Device (EXP1) { Name (_ADR, 0x20001) // _ADR: Address Name (_PRT, Package () // _PRT: PCI Routing Table { Package () { 0xFFFF, 0x0, \_SB.SLT1, 0x0 }, Package () { 0xFFFF, 0x1, \_SB.SLT1, 0x1 }, Package () { 0xFFFF, 0x2, \_SB.SLT1, 0x2 }, Package () { 0xFFFF, 0x3, \_SB.SLT1, 0x3 } }) // _PRT } Device (EXP2) { Name (_ADR, 0x20002) // _ADR: Address Name (_PRT, Package () // _PRT: PCI Routing Table { Package () { 0xFFFF, 0x0, \_SB.SLT2, 0x0 }, Package () { 0xFFFF, 0x1, \_SB.SLT2, 0x1 }, Package () { 0xFFFF, 0x2, \_SB.SLT2, 0x2 }, Package () { 0xFFFF, 0x3, \_SB.SLT2, 0x3 } }) // _PRT } Device (EXP3) { Name (_ADR, 0x20003) // _ADR: Address Name (_PRT, Package () // _PRT: PCI Routing Table { Package () { 0xFFFF, 0x0, \_SB.SLT3, 0x0 }, Package () { 0xFFFF, 0x1, \_SB.SLT3, 0x1 }, Package () { 0xFFFF, 0x2, \_SB.SLT3, 0x2 }, Package () { 0xFFFF, 0x3, \_SB.SLT3, 0x3 } }) // _PRT } with the same result. So could we be missing anything in the arm64 implementation that prevents the companion device from being found? -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Apr 07, 2017 at 07:35:45PM +0100, Ard Biesheuvel wrote: > On 7 April 2017 at 19:06, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: > > On Fri, Apr 07, 2017 at 06:12:05PM +0100, Ard Biesheuvel wrote: > >> On 7 April 2017 at 18:06, Bjorn Helgaas <helgaas@kernel.org> wrote: > >> > Hi Ard, > >> > > >> > On Fri, Apr 07, 2017 at 02:22:22PM +0100, Ard Biesheuvel wrote: > >> >> We currently derive legacy interrupt routing by matching _PRT > >> >> entries on the PCI device only, presumably under the assumption > >> >> that PRT entries always have a value of 0xffff in the function > >> >> field, meaning 'match all functions'. > >> > > >> > The spec (ACPI v6.0, sec 6.2.13) contains a note that: > >> > > >> > The PCI function number in the Address field of the _PRT packages > >> > must be 0xFFFF, indicating "any" function number or "all functions". > >> > > >> > If we need a patch like this, we need to somehow reconcile it with > >> > that spec text to make sure firmware and OS folks have a common > >> > understanding of how this is supposed to work. > >> > > >> >> This no longer holds for modern PCIe topologies, where the > >> >> legacy interrupts for different slots may be wired to different > >> >> functions on the same bridge device. For instance, on AMD Seattle, > >> >> we may have something like > >> >> > >> >> -[0000:00]-+-00.0 Advanced Micro Devices, Inc. [AMD] Device 1a00 > >> >> +-02.0 Advanced Micro Devices, Inc. [AMD] Device 1a01 > >> >> +-02.2-[01]----00.0 Renesas uPD720202 USB 3.0 Host Controller > >> >> \-02.3-[02]----00.0 Realtek RTL8169 PCIe Gigabit Ethernet > >> >> > >> >> where the _PRT describes the legacy interrupt routing as > >> >> > >> >> Name (_PRT, Package () // _PRT: PCI Routing Table > >> >> { > >> >> // slot 1: dev 2 fn 1 > >> >> Package () { 0x20001, 0x0, 0x0, 0x140 }, > >> >> Package () { 0x20001, 0x1, 0x0, 0x141 }, > >> >> Package () { 0x20001, 0x2, 0x0, 0x142 }, > >> >> Package () { 0x20001, 0x3, 0x0, 0x143 }, > >> >> > >> >> // slot 1: dev 2 fn 2 > >> >> Package () { 0x20002, 0x0, 0x0, 0x144 }, > >> >> Package () { 0x20002, 0x1, 0x0, 0x145 }, > >> >> Package () { 0x20002, 0x2, 0x0, 0x146 }, > >> >> Package () { 0x20002, 0x3, 0x0, 0x147 }, > >> >> > >> >> // slot 1: dev 2 fn 3 > >> >> Package () { 0x20003, 0x0, 0x0, 0x148 }, > >> >> Package () { 0x20003, 0x1, 0x0, 0x149 }, > >> >> Package () { 0x20003, 0x2, 0x0, 0x14a }, > >> >> Package () { 0x20003, 0x3, 0x0, 0x14b } > >> >> }) // _PRT > >> > > >> > But I think this _PRT description is incorrect and we should change > >> > the _PRT rather than the kernel. My laptop has a basically identical > >> > topology: > >> > > >> > -[0000:00]-+-00.0 Intel Corporation Sky Lake Host Bridge/DRAM Registers > >> > +-1c.0-[02]----00.0 Realtek Semiconductor Co., Ltd. Device 525a > >> > +-1c.2-[04]----00.0 Intel Corporation Wireless 8260 > >> > > >> > and the ASL looks like this (paraphrased): > >> > > >> > Device (EXP1) { > >> > Name (_ADR, 0x001C0000) > >> > Name (_PRT) { > >> > Package () { 0xFFFF, 0x00, \_SB.LNKA, 0x00 }, > >> > Package () { 0xFFFF, 0x01, \_SB.LNKB, 0x00 }, > >> > ... > >> > } > >> > } > >> > Device (EXP3) { > >> > Name (_ADR, 0x001C0002) > >> > Name (_PRT) { > >> > Package () { 0xFFFF, 0x00, \_SB.LNKC, 0x00 }, > >> > Package () { 0xFFFF, 0x01, \_SB.LNKD, 0x00 }, > >> > ... > >> > } > >> > } > >> > > >> > >> Thanks for the explanation. But how is this wired up into the PNP0A08 > >> device then? IOW, how does the ACPI code in Linux discover the > >> relation between these devices and my PCI root device? > > > > You describe the PCI hierarchy starting from PNP0A08 at root and the > > kernel assigns the ACPI companion through _ADR matching (see > > acpi_pci_find_companion()) which is what is used by _PRT parsing > > code to route IRQs IIUC. > > > > OK, I have changed my DSDT as follows: > > > Device (PCI0) > { > Name (_ADR, 0x00) > Name (_HID, "PNP0A08" /* PCI Express Bus */) // _HID: Hardware ID > Name (_CID, "PNP0A03" /* PCI Bus */) // _CID: Compatible ID > Name (_SEG, 0x00) // _SEG: PCI Segment > Name (_BBN, 0x00) // _BBN: BIOS Bus Number > Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute > > Device (EXP1) > { > Name (_ADR, 0x20001) // _ADR: Address > Name (_PRT, Package () // _PRT: PCI Routing Table > { > // slot 1: dev 2 fn 1 > Package () { 0xFFFF, 0x0, 0x0, 0x140 }, ^ 0x2FFFF Ditto for the other entries. > Package () { 0xFFFF, 0x1, 0x0, 0x141 }, > Package () { 0xFFFF, 0x2, 0x0, 0x142 }, > Package () { 0xFFFF, 0x3, 0x0, 0x143 } > }) // _PRT > } > Device (EXP2) > { > Name (_ADR, 0x20002) // _ADR: Address > Name (_PRT, Package () // _PRT: PCI Routing Table > { > // slot 2: dev 2 fn 2 > Package () { 0xFFFF, 0x0, 0x0, 0x144 }, > Package () { 0xFFFF, 0x1, 0x0, 0x145 }, > Package () { 0xFFFF, 0x2, 0x0, 0x146 }, > Package () { 0xFFFF, 0x3, 0x0, 0x147 } > }) // _PRT > } > Device (EXP3) > { > Name (_ADR, 0x20003) // _ADR: Address > Name (_PRT, Package () // _PRT: PCI Routing Table > { > // slot 3: dev 2 fn 3 > Package () { 0xFFFF, 0x0, 0x0, 0x148 }, > Package () { 0xFFFF, 0x1, 0x0, 0x149 }, > Package () { 0xFFFF, 0x2, 0x0, 0x14a }, > Package () { 0xFFFF, 0x3, 0x0, 0x14b } > }) // _PRT > } > > but it does not get picked up, and I am back to > > [ 3.357555] pcieport 0000:00:02.2: can't derive routing for PCI INT A > [ 3.370477] pcieport 0000:00:02.2: PCI INT A: no GSI > [ 3.380549] pcieport 0000:00:02.3: can't derive routing for PCI INT A > [ 3.393476] pcieport 0000:00:02.3: PCI INT A: no GSI > > Then I tried switching to > > Device (SLT1) > { > Name(_HID, EISAID("PNP0C0F")) > Name(_UID, 0x1) > Name(_PRS, ResourceTemplate() { > Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive) { 0x140 } > Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive) { 0x141 } > Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive) { 0x142 } > Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive) { 0x143 } > }) > Method (_CRS, 0) { Return (_PRS) } > Method (_SRS, 1) { } > Method (_DIS) { } > } > Device (SLT2) > { > Name(_HID, EISAID("PNP0C0F")) > Name(_UID, 0x2) > Name(_PRS, ResourceTemplate() { > Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive) { 0x144 } > Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive) { 0x145 } > Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive) { 0x146 } > Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive) { 0x147 } > }) > Method (_CRS, 0) { Return (_PRS) } > Method (_SRS, 1) { } > Method (_DIS) { } > } > Device (SLT3) > { > Name(_HID, EISAID("PNP0C0F")) > Name(_UID, 0x3) > Name(_PRS, ResourceTemplate() { > Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive) { 0x148 } > Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive) { 0x149 } > Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive) { 0x14A } > Interrupt(ResourceProducer, Level, ActiveHigh, Exclusive) { 0x14B } > }) > Method (_CRS, 0) { Return (_PRS) } > Method (_SRS, 1) { } > Method (_DIS) { } > } > > // > // PCIe Root Bus > // > Device (PCI0) > { > Name (_HID, "PNP0A08" /* PCI Express Bus */) // _HID: Hardware ID > Name (_CID, "PNP0A03" /* PCI Bus */) // _CID: Compatible ID > Name (_SEG, 0x00) // _SEG: PCI Segment > Name (_BBN, 0x00) // _BBN: BIOS Bus Number > Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute > > Device (EXP1) > { > Name (_ADR, 0x20001) // _ADR: Address > Name (_PRT, Package () // _PRT: PCI Routing Table > { > Package () { 0xFFFF, 0x0, \_SB.SLT1, 0x0 }, > Package () { 0xFFFF, 0x1, \_SB.SLT1, 0x1 }, > Package () { 0xFFFF, 0x2, \_SB.SLT1, 0x2 }, > Package () { 0xFFFF, 0x3, \_SB.SLT1, 0x3 } > }) // _PRT > } > Device (EXP2) > { > Name (_ADR, 0x20002) // _ADR: Address > Name (_PRT, Package () // _PRT: PCI Routing Table > { > Package () { 0xFFFF, 0x0, \_SB.SLT2, 0x0 }, > Package () { 0xFFFF, 0x1, \_SB.SLT2, 0x1 }, > Package () { 0xFFFF, 0x2, \_SB.SLT2, 0x2 }, > Package () { 0xFFFF, 0x3, \_SB.SLT2, 0x3 } > }) // _PRT > } > Device (EXP3) > { > Name (_ADR, 0x20003) // _ADR: Address > Name (_PRT, Package () // _PRT: PCI Routing Table > { > Package () { 0xFFFF, 0x0, \_SB.SLT3, 0x0 }, > Package () { 0xFFFF, 0x1, \_SB.SLT3, 0x1 }, > Package () { 0xFFFF, 0x2, \_SB.SLT3, 0x2 }, > Package () { 0xFFFF, 0x3, \_SB.SLT3, 0x3 } > }) // _PRT > } > > with the same result. > > So could we be missing anything in the arm64 implementation that > prevents the companion device from being found? No and if there is that's a bug. I will help you debug it next week, mind trying the change I suggest above please ? I *think* the devfn parameter in the _PRT entry must match the device looked-up, in particular the device bits. Thanks ! Lorenzo -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 7 April 2017 at 22:36, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: > On Fri, Apr 07, 2017 at 07:35:45PM +0100, Ard Biesheuvel wrote: >> On 7 April 2017 at 19:06, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: >> > On Fri, Apr 07, 2017 at 06:12:05PM +0100, Ard Biesheuvel wrote: >> >> On 7 April 2017 at 18:06, Bjorn Helgaas <helgaas@kernel.org> wrote: >> >> > Hi Ard, >> >> > >> >> > On Fri, Apr 07, 2017 at 02:22:22PM +0100, Ard Biesheuvel wrote: >> >> >> We currently derive legacy interrupt routing by matching _PRT >> >> >> entries on the PCI device only, presumably under the assumption >> >> >> that PRT entries always have a value of 0xffff in the function >> >> >> field, meaning 'match all functions'. >> >> > >> >> > The spec (ACPI v6.0, sec 6.2.13) contains a note that: >> >> > >> >> > The PCI function number in the Address field of the _PRT packages >> >> > must be 0xFFFF, indicating "any" function number or "all functions". >> >> > >> >> > If we need a patch like this, we need to somehow reconcile it with >> >> > that spec text to make sure firmware and OS folks have a common >> >> > understanding of how this is supposed to work. >> >> > >> >> >> This no longer holds for modern PCIe topologies, where the >> >> >> legacy interrupts for different slots may be wired to different >> >> >> functions on the same bridge device. For instance, on AMD Seattle, >> >> >> we may have something like >> >> >> >> >> >> -[0000:00]-+-00.0 Advanced Micro Devices, Inc. [AMD] Device 1a00 >> >> >> +-02.0 Advanced Micro Devices, Inc. [AMD] Device 1a01 >> >> >> +-02.2-[01]----00.0 Renesas uPD720202 USB 3.0 Host Controller >> >> >> \-02.3-[02]----00.0 Realtek RTL8169 PCIe Gigabit Ethernet >> >> >> >> >> >> where the _PRT describes the legacy interrupt routing as >> >> >> >> >> >> Name (_PRT, Package () // _PRT: PCI Routing Table >> >> >> { >> >> >> // slot 1: dev 2 fn 1 >> >> >> Package () { 0x20001, 0x0, 0x0, 0x140 }, >> >> >> Package () { 0x20001, 0x1, 0x0, 0x141 }, >> >> >> Package () { 0x20001, 0x2, 0x0, 0x142 }, >> >> >> Package () { 0x20001, 0x3, 0x0, 0x143 }, >> >> >> >> >> >> // slot 1: dev 2 fn 2 >> >> >> Package () { 0x20002, 0x0, 0x0, 0x144 }, >> >> >> Package () { 0x20002, 0x1, 0x0, 0x145 }, >> >> >> Package () { 0x20002, 0x2, 0x0, 0x146 }, >> >> >> Package () { 0x20002, 0x3, 0x0, 0x147 }, >> >> >> >> >> >> // slot 1: dev 2 fn 3 >> >> >> Package () { 0x20003, 0x0, 0x0, 0x148 }, >> >> >> Package () { 0x20003, 0x1, 0x0, 0x149 }, >> >> >> Package () { 0x20003, 0x2, 0x0, 0x14a }, >> >> >> Package () { 0x20003, 0x3, 0x0, 0x14b } >> >> >> }) // _PRT >> >> > >> >> > But I think this _PRT description is incorrect and we should change >> >> > the _PRT rather than the kernel. My laptop has a basically identical >> >> > topology: >> >> > >> >> > -[0000:00]-+-00.0 Intel Corporation Sky Lake Host Bridge/DRAM Registers >> >> > +-1c.0-[02]----00.0 Realtek Semiconductor Co., Ltd. Device 525a >> >> > +-1c.2-[04]----00.0 Intel Corporation Wireless 8260 >> >> > >> >> > and the ASL looks like this (paraphrased): >> >> > >> >> > Device (EXP1) { >> >> > Name (_ADR, 0x001C0000) >> >> > Name (_PRT) { >> >> > Package () { 0xFFFF, 0x00, \_SB.LNKA, 0x00 }, >> >> > Package () { 0xFFFF, 0x01, \_SB.LNKB, 0x00 }, >> >> > ... >> >> > } >> >> > } >> >> > Device (EXP3) { >> >> > Name (_ADR, 0x001C0002) >> >> > Name (_PRT) { >> >> > Package () { 0xFFFF, 0x00, \_SB.LNKC, 0x00 }, >> >> > Package () { 0xFFFF, 0x01, \_SB.LNKD, 0x00 }, >> >> > ... >> >> > } >> >> > } >> >> > >> >> >> >> Thanks for the explanation. But how is this wired up into the PNP0A08 >> >> device then? IOW, how does the ACPI code in Linux discover the >> >> relation between these devices and my PCI root device? >> > >> > You describe the PCI hierarchy starting from PNP0A08 at root and the >> > kernel assigns the ACPI companion through _ADR matching (see >> > acpi_pci_find_companion()) which is what is used by _PRT parsing >> > code to route IRQs IIUC. >> > >> >> OK, I have changed my DSDT as follows: >> >> >> Device (PCI0) >> { >> Name (_ADR, 0x00) >> Name (_HID, "PNP0A08" /* PCI Express Bus */) // _HID: Hardware ID >> Name (_CID, "PNP0A03" /* PCI Bus */) // _CID: Compatible ID >> Name (_SEG, 0x00) // _SEG: PCI Segment >> Name (_BBN, 0x00) // _BBN: BIOS Bus Number >> Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute >> >> Device (EXP1) >> { >> Name (_ADR, 0x20001) // _ADR: Address >> Name (_PRT, Package () // _PRT: PCI Routing Table >> { >> // slot 1: dev 2 fn 1 >> Package () { 0xFFFF, 0x0, 0x0, 0x140 }, > ^ > 0x2FFFF > > Ditto for the other entries. > I had already tried that as well, but it doesn't work. Single stepping through the ACPI code, it seems like the _PRT entry is never found by acpi_get_irq_routing_table(), and so whether the contents are correct doesn't really matter at this point. >> >> So could we be missing anything in the arm64 implementation that >> prevents the companion device from being found? > > No and if there is that's a bug. I will help you debug it next week, > mind trying the change I suggest above please ? I *think* the devfn > parameter in the _PRT entry must match the device looked-up, in > particular the device bits. > Yes, let's look into this on Monday. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 7 April 2017 at 19:35, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > On 7 April 2017 at 19:06, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: >> On Fri, Apr 07, 2017 at 06:12:05PM +0100, Ard Biesheuvel wrote: >>> On 7 April 2017 at 18:06, Bjorn Helgaas <helgaas@kernel.org> wrote: [...] > > OK, I have changed my DSDT as follows: > > > Device (PCI0) > { > Name (_ADR, 0x00) > Name (_HID, "PNP0A08" /* PCI Express Bus */) // _HID: Hardware ID > Name (_CID, "PNP0A03" /* PCI Bus */) // _CID: Compatible ID > Name (_SEG, 0x00) // _SEG: PCI Segment > Name (_BBN, 0x00) // _BBN: BIOS Bus Number > Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute > > Device (EXP1) > { > Name (_ADR, 0x20001) // _ADR: Address > Name (_PRT, Package () // _PRT: PCI Routing Table > { > // slot 1: dev 2 fn 1 > Package () { 0xFFFF, 0x0, 0x0, 0x140 }, > Package () { 0xFFFF, 0x1, 0x0, 0x141 }, > Package () { 0xFFFF, 0x2, 0x0, 0x142 }, > Package () { 0xFFFF, 0x3, 0x0, 0x143 } > }) // _PRT > } > Device (EXP2) > { > Name (_ADR, 0x20002) // _ADR: Address > Name (_PRT, Package () // _PRT: PCI Routing Table > { > // slot 2: dev 2 fn 2 > Package () { 0xFFFF, 0x0, 0x0, 0x144 }, > Package () { 0xFFFF, 0x1, 0x0, 0x145 }, > Package () { 0xFFFF, 0x2, 0x0, 0x146 }, > Package () { 0xFFFF, 0x3, 0x0, 0x147 } > }) // _PRT > } > Device (EXP3) > { > Name (_ADR, 0x20003) // _ADR: Address > Name (_PRT, Package () // _PRT: PCI Routing Table > { > // slot 3: dev 2 fn 3 > Package () { 0xFFFF, 0x0, 0x0, 0x148 }, > Package () { 0xFFFF, 0x1, 0x0, 0x149 }, > Package () { 0xFFFF, 0x2, 0x0, 0x14a }, > Package () { 0xFFFF, 0x3, 0x0, 0x14b } > }) // _PRT > } > > but it does not get picked up, and I am back to > > [ 3.357555] pcieport 0000:00:02.2: can't derive routing for PCI INT A > [ 3.370477] pcieport 0000:00:02.2: PCI INT A: no GSI > [ 3.380549] pcieport 0000:00:02.3: can't derive routing for PCI INT A > [ 3.393476] pcieport 0000:00:02.3: PCI INT A: no GSI > OK, this does appear to work in fact, for the devices behind the bridges. These messages are from the pcieport driver trying to request an IRQ for the bridge device itself, which no longer works now that the _PRT methods have been moved out. So that mostly solves the problem. I'll try adding back a _PRT in the PCI root device itself, but i'm not sure if I can make any inferences about the IRQ wiring from the data i have. Thanks all, -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, Apr 08, 2017 at 12:22:15PM +0100, Ard Biesheuvel wrote: > On 7 April 2017 at 19:35, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > > On 7 April 2017 at 19:06, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: > >> On Fri, Apr 07, 2017 at 06:12:05PM +0100, Ard Biesheuvel wrote: > >>> On 7 April 2017 at 18:06, Bjorn Helgaas <helgaas@kernel.org> wrote: > [...] > > > > OK, I have changed my DSDT as follows: > > > > > > Device (PCI0) > > { > > Name (_ADR, 0x00) > > Name (_HID, "PNP0A08" /* PCI Express Bus */) // _HID: Hardware ID > > Name (_CID, "PNP0A03" /* PCI Bus */) // _CID: Compatible ID > > Name (_SEG, 0x00) // _SEG: PCI Segment > > Name (_BBN, 0x00) // _BBN: BIOS Bus Number > > Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute > > > > Device (EXP1) > > { > > Name (_ADR, 0x20001) // _ADR: Address > > Name (_PRT, Package () // _PRT: PCI Routing Table > > { > > // slot 1: dev 2 fn 1 > > Package () { 0xFFFF, 0x0, 0x0, 0x140 }, > > Package () { 0xFFFF, 0x1, 0x0, 0x141 }, > > Package () { 0xFFFF, 0x2, 0x0, 0x142 }, > > Package () { 0xFFFF, 0x3, 0x0, 0x143 } > > }) // _PRT > > } > > Device (EXP2) > > { > > Name (_ADR, 0x20002) // _ADR: Address > > Name (_PRT, Package () // _PRT: PCI Routing Table > > { > > // slot 2: dev 2 fn 2 > > Package () { 0xFFFF, 0x0, 0x0, 0x144 }, > > Package () { 0xFFFF, 0x1, 0x0, 0x145 }, > > Package () { 0xFFFF, 0x2, 0x0, 0x146 }, > > Package () { 0xFFFF, 0x3, 0x0, 0x147 } > > }) // _PRT > > } > > Device (EXP3) > > { > > Name (_ADR, 0x20003) // _ADR: Address > > Name (_PRT, Package () // _PRT: PCI Routing Table > > { > > // slot 3: dev 2 fn 3 > > Package () { 0xFFFF, 0x0, 0x0, 0x148 }, > > Package () { 0xFFFF, 0x1, 0x0, 0x149 }, > > Package () { 0xFFFF, 0x2, 0x0, 0x14a }, > > Package () { 0xFFFF, 0x3, 0x0, 0x14b } > > }) // _PRT > > } > > > > but it does not get picked up, and I am back to > > > > [ 3.357555] pcieport 0000:00:02.2: can't derive routing for PCI INT A > > [ 3.370477] pcieport 0000:00:02.2: PCI INT A: no GSI > > [ 3.380549] pcieport 0000:00:02.3: can't derive routing for PCI INT A > > [ 3.393476] pcieport 0000:00:02.3: PCI INT A: no GSI > > > > OK, this does appear to work in fact, for the devices behind the > bridges. These messages are from the pcieport driver trying to request > an IRQ for the bridge device itself, which no longer works now that > the _PRT methods have been moved out. > > So that mostly solves the problem. I'll try adding back a _PRT in the > PCI root device itself, but i'm not sure if I can make any inferences > about the IRQ wiring from the data i have. Yes and IIUC with the $SUBJECT patch applied, the endpoints would match the _PRT entry under PNP0A08 through their bridge (PCIe port device = 2 fn = X) device entry, not their own (device = 0) (ie first look-up in acpi_pci_irq_lookup() through acpi_pci_irq_find_prt_entry() should fail for the endpoints, then the same look-up is carried out with the bridge device instead and that succeeds, the _PRT entry routes IRQs for the PCIeport AND their downstream endpoint with $SUBJECT patch applied). If you add the _PRT under the PCIe port itself (table above) the PCIe port can't find an ACPI parent device to route its own IRQx, that's my reading of what's going on, I will debug it next week. Lorenzo -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> On 8 Apr 2017, at 14:22, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: > >> On Sat, Apr 08, 2017 at 12:22:15PM +0100, Ard Biesheuvel wrote: >>> On 7 April 2017 at 19:35, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: >>>> On 7 April 2017 at 19:06, Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> wrote: >>>>> On Fri, Apr 07, 2017 at 06:12:05PM +0100, Ard Biesheuvel wrote: >>>>>> On 7 April 2017 at 18:06, Bjorn Helgaas <helgaas@kernel.org> wrote: >>> [...] >>> >>> OK, I have changed my DSDT as follows: >>> >>> >>> Device (PCI0) >>> { >>> Name (_ADR, 0x00) >>> Name (_HID, "PNP0A08" /* PCI Express Bus */) // _HID: Hardware ID >>> Name (_CID, "PNP0A03" /* PCI Bus */) // _CID: Compatible ID >>> Name (_SEG, 0x00) // _SEG: PCI Segment >>> Name (_BBN, 0x00) // _BBN: BIOS Bus Number >>> Name (_CCA, 0x01) // _CCA: Cache Coherency Attribute >>> >>> Device (EXP1) >>> { >>> Name (_ADR, 0x20001) // _ADR: Address >>> Name (_PRT, Package () // _PRT: PCI Routing Table >>> { >>> // slot 1: dev 2 fn 1 >>> Package () { 0xFFFF, 0x0, 0x0, 0x140 }, >>> Package () { 0xFFFF, 0x1, 0x0, 0x141 }, >>> Package () { 0xFFFF, 0x2, 0x0, 0x142 }, >>> Package () { 0xFFFF, 0x3, 0x0, 0x143 } >>> }) // _PRT >>> } >>> Device (EXP2) >>> { >>> Name (_ADR, 0x20002) // _ADR: Address >>> Name (_PRT, Package () // _PRT: PCI Routing Table >>> { >>> // slot 2: dev 2 fn 2 >>> Package () { 0xFFFF, 0x0, 0x0, 0x144 }, >>> Package () { 0xFFFF, 0x1, 0x0, 0x145 }, >>> Package () { 0xFFFF, 0x2, 0x0, 0x146 }, >>> Package () { 0xFFFF, 0x3, 0x0, 0x147 } >>> }) // _PRT >>> } >>> Device (EXP3) >>> { >>> Name (_ADR, 0x20003) // _ADR: Address >>> Name (_PRT, Package () // _PRT: PCI Routing Table >>> { >>> // slot 3: dev 2 fn 3 >>> Package () { 0xFFFF, 0x0, 0x0, 0x148 }, >>> Package () { 0xFFFF, 0x1, 0x0, 0x149 }, >>> Package () { 0xFFFF, 0x2, 0x0, 0x14a }, >>> Package () { 0xFFFF, 0x3, 0x0, 0x14b } >>> }) // _PRT >>> } >>> >>> but it does not get picked up, and I am back to >>> >>> [ 3.357555] pcieport 0000:00:02.2: can't derive routing for PCI INT A >>> [ 3.370477] pcieport 0000:00:02.2: PCI INT A: no GSI >>> [ 3.380549] pcieport 0000:00:02.3: can't derive routing for PCI INT A >>> [ 3.393476] pcieport 0000:00:02.3: PCI INT A: no GSI >>> >> >> OK, this does appear to work in fact, for the devices behind the >> bridges. These messages are from the pcieport driver trying to request >> an IRQ for the bridge device itself, which no longer works now that >> the _PRT methods have been moved out. >> >> So that mostly solves the problem. I'll try adding back a _PRT in the >> PCI root device itself, but i'm not sure if I can make any inferences >> about the IRQ wiring from the data i have. > > Yes and IIUC with the $SUBJECT patch applied, the endpoints would match > the _PRT entry under PNP0A08 through their bridge (PCIe port device = 2 > fn = X) device entry, not their own (device = 0) (ie first look-up in > acpi_pci_irq_lookup() through acpi_pci_irq_find_prt_entry() should fail > for the endpoints, then the same look-up is carried out with the bridge > device instead and that succeeds, the _PRT entry routes IRQs for the > PCIeport AND their downstream endpoint with $SUBJECT patch applied). > > If you add the _PRT under the PCIe port itself (table above) the PCIe > port can't find an ACPI parent device to route its own IRQx, that's my > reading of what's going on, I will debug it next week. > This is my understanding as well, and i think there remains little to debug on the kernel side. Moving the three banks of legacy interrupts to separate companion devices is the correct thing to do, and works as expected for the devices behind the bridge. The only thing that is lacking now is a _PRT description in the pci root that describes how INTA is wired for the bridges themselves. I am not sure how this matters, and whether this pci h/w ever asserts that interrupt in the first place, so i am just going to map it to 0x140 for now until someone tells me otherwise. -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/acpi/pci_irq.c b/drivers/acpi/pci_irq.c index c576a6fe4ebb..caac4ac94418 100644 --- a/drivers/acpi/pci_irq.c +++ b/drivers/acpi/pci_irq.c @@ -160,6 +160,8 @@ static int acpi_pci_irq_check_entry(acpi_handle handle, struct pci_dev *dev, struct acpi_prt_entry *entry; if (((prt->address >> 16) & 0xffff) != device || + ((prt->address & 0xffff) != 0xffff && + (prt->address & 0xffff) != PCI_FUNC(dev->devfn)) || prt->pin + 1 != pin) return -ENODEV;
We currently derive legacy interrupt routing by matching _PRT entries on the PCI device only, presumably under the assumption that PRT entries always have a value of 0xffff in the function field, meaning 'match all functions'. This no longer holds for modern PCIe topologies, where the legacy interrupts for different slots may be wired to different functions on the same bridge device. For instance, on AMD Seattle, we may have something like -[0000:00]-+-00.0 Advanced Micro Devices, Inc. [AMD] Device 1a00 +-02.0 Advanced Micro Devices, Inc. [AMD] Device 1a01 +-02.2-[01]----00.0 Renesas uPD720202 USB 3.0 Host Controller \-02.3-[02]----00.0 Realtek RTL8169 PCIe Gigabit Ethernet where the _PRT describes the legacy interrupt routing as Name (_PRT, Package () // _PRT: PCI Routing Table { // slot 1: dev 2 fn 1 Package () { 0x20001, 0x0, 0x0, 0x140 }, Package () { 0x20001, 0x1, 0x0, 0x141 }, Package () { 0x20001, 0x2, 0x0, 0x142 }, Package () { 0x20001, 0x3, 0x0, 0x143 }, // slot 1: dev 2 fn 2 Package () { 0x20002, 0x0, 0x0, 0x144 }, Package () { 0x20002, 0x1, 0x0, 0x145 }, Package () { 0x20002, 0x2, 0x0, 0x146 }, Package () { 0x20002, 0x3, 0x0, 0x147 }, // slot 1: dev 2 fn 3 Package () { 0x20003, 0x0, 0x0, 0x148 }, Package () { 0x20003, 0x1, 0x0, 0x149 }, Package () { 0x20003, 0x2, 0x0, 0x14a }, Package () { 0x20003, 0x3, 0x0, 0x14b } }) // _PRT The current code always matches on the first entry when trying to derive the legacy interrupt routing for the USB and Ethernet controllers behind bridges 02.2 and 02.3, which results in the wrong entry to be selected. So fix this by matching the function ID to the value in the _PRT entry if the _PRT entries are not using 0xffff in the lower word to match all functions. Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- drivers/acpi/pci_irq.c | 2 ++ 1 file changed, 2 insertions(+) -- 2.9.3 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html