| Message ID | 20240612065315.2048110-1-chenhuacai@loongson.cn |
|---|---|
| State | New |
| Headers | show |
| Series | PCI: loongson: Add LS7A MSI enablement quirk | expand |
Hello, > LS7A chipset can be used as a downstream bridge which connected to a > high-level host bridge. In this case DEV_LS7A_PCIE_PORT5 is used as the > upward port. We should always enable MSI caps of this port, otherwise > downstream devices cannot use MSI. Applied to controller/loongson, thank you! [1/1] PCI/DPC: Fix use-after-free on concurrent DPC and hot-removal https://git.kernel.org/pci/pci/c/b69d24a763b Krzysztof
Hello, [...] > [1/1] PCI/DPC: Fix use-after-free on concurrent DPC and hot-removal > https://git.kernel.org/pci/pci/c/b69d24a763b Sorry, this should be: [1/1] PCI: loongson: Add LS7A MSI enablement quirk https://git.kernel.org/pci/pci/c/b69d24a763b Krzysztof
On Wed, Jun 12, 2024 at 02:53:15PM +0800, Huacai Chen wrote: > LS7A chipset can be used as a downstream bridge which connected to a > high-level host bridge. In this case DEV_LS7A_PCIE_PORT5 is used as the > upward port. We should always enable MSI caps of this port, otherwise > downstream devices cannot use MSI. Can you clarify this topology a bit? Since DEV_LS7A_PCIE_PORT5 apparently has a class of PCI_CLASS_BRIDGE_HOST, I guess that in PCIe terms, it is basically a PCI host bridge (Root Complex, if you prefer) that is materialized as a PCI Endpoint? I'm curious about what's going on here because the normal PCI MSI support should set PCI_MSI_FLAGS_ENABLE since it's completely specified by the spec, which says it controls whether *this function* can use MSI. But in this case PCI_MSI_FLAGS_ENABLE seems to have non-architected behavior of controlling MSI from *other* devices below this host bridge? That's a little bit weird too because MSI looks like DMA to any bridges upstream from the device that is using MSI, and the Bus Master Enable bit in those bridges controls whether they forward those MSI DMA accesses upstream. And of course the PCI core should already make sure those bridges have Bus Master Enable set when downstream devices use MSI. > Cc: <stable@vger.kernel.org> > Signed-off-by: Sheng Wu <wusheng@loongson.cn> > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn> > --- > drivers/pci/controller/pci-loongson.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c > index 8b34ccff073a..ffc581605834 100644 > --- a/drivers/pci/controller/pci-loongson.c > +++ b/drivers/pci/controller/pci-loongson.c > @@ -163,6 +163,18 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, > DEV_LS7A_HDMI, loongson_pci_pin_quirk); > > +static void loongson_pci_msi_quirk(struct pci_dev *dev) > +{ > + u16 val, class = dev->class >> 8; > + > + if (class == PCI_CLASS_BRIDGE_HOST) { > + pci_read_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, &val); > + val |= PCI_MSI_FLAGS_ENABLE; > + pci_write_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, val); > + } > +} > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, DEV_LS7A_PCIE_PORT5, loongson_pci_msi_quirk); > + > static struct loongson_pci *pci_bus_to_loongson_pci(struct pci_bus *bus) > { > struct pci_config_window *cfg; > -- > 2.43.0 >
Hi, Bjorn, On Wed, Jul 10, 2024 at 5:24 AM Bjorn Helgaas <helgaas@kernel.org> wrote: > > On Wed, Jun 12, 2024 at 02:53:15PM +0800, Huacai Chen wrote: > > LS7A chipset can be used as a downstream bridge which connected to a > > high-level host bridge. In this case DEV_LS7A_PCIE_PORT5 is used as the > > upward port. We should always enable MSI caps of this port, otherwise > > downstream devices cannot use MSI. > > Can you clarify this topology a bit? Since DEV_LS7A_PCIE_PORT5 > apparently has a class of PCI_CLASS_BRIDGE_HOST, I guess that in PCIe > terms, it is basically a PCI host bridge (Root Complex, if you prefer) > that is materialized as a PCI Endpoint? Now most of the existing LoongArch CPUs don't have an integrated PCIe RC, instead they have HyperTransport controllers. But the latest CPU (Loongson-3C6000) has an integrated PCIe RC and removed HyperTransport. LS7A bridge can work together with both old (HT) CPUs and new (PCIe) CPUs. If it is connected to an old CPU, its upstream port is a HT port, and DEV_LS7A_PCIE_PORT5 works as a normal downstream PCIe port. If it is connected to a new CPU, DEV_LS7A_PCIE_PORT5 works as an upstream port (the class code becomes PCI_CLASS_BRIDGE_HOST) and the HT port is idle. > > I'm curious about what's going on here because the normal PCI MSI > support should set PCI_MSI_FLAGS_ENABLE since it's completely > specified by the spec, which says it controls whether *this function* > can use MSI. > > But in this case PCI_MSI_FLAGS_ENABLE seems to have non-architected > behavior of controlling MSI from *other* devices below this host > bridge? That's a little bit weird too because MSI looks like DMA to > any bridges upstream from the device that is using MSI, and the Bus > Master Enable bit in those bridges controls whether they forward those > MSI DMA accesses upstream. And of course the PCI core should already > make sure those bridges have Bus Master Enable set when downstream > devices use MSI. In my opinion this is a hardware bug, when DEV_LS7A_PCIE_PORT5 works as a host bridge, it should enable MSI automatically. But unfortunately hardware doesn't behave like this, so we need a quirk here. Huacai > > > Cc: <stable@vger.kernel.org> > > Signed-off-by: Sheng Wu <wusheng@loongson.cn> > > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn> > > --- > > drivers/pci/controller/pci-loongson.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c > > index 8b34ccff073a..ffc581605834 100644 > > --- a/drivers/pci/controller/pci-loongson.c > > +++ b/drivers/pci/controller/pci-loongson.c > > @@ -163,6 +163,18 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, > > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, > > DEV_LS7A_HDMI, loongson_pci_pin_quirk); > > > > +static void loongson_pci_msi_quirk(struct pci_dev *dev) > > +{ > > + u16 val, class = dev->class >> 8; > > + > > + if (class == PCI_CLASS_BRIDGE_HOST) { > > + pci_read_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, &val); > > + val |= PCI_MSI_FLAGS_ENABLE; > > + pci_write_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, val); > > + } > > +} > > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, DEV_LS7A_PCIE_PORT5, loongson_pci_msi_quirk); > > + > > static struct loongson_pci *pci_bus_to_loongson_pci(struct pci_bus *bus) > > { > > struct pci_config_window *cfg; > > -- > > 2.43.0 > >
On Wed, Jul 10, 2024 at 11:04:24AM +0800, Huacai Chen wrote: > Hi, Bjorn, > > On Wed, Jul 10, 2024 at 5:24 AM Bjorn Helgaas <helgaas@kernel.org> wrote: > > > > On Wed, Jun 12, 2024 at 02:53:15PM +0800, Huacai Chen wrote: > > > LS7A chipset can be used as a downstream bridge which connected to a > > > high-level host bridge. In this case DEV_LS7A_PCIE_PORT5 is used as the > > > upward port. We should always enable MSI caps of this port, otherwise > > > downstream devices cannot use MSI. > > > > Can you clarify this topology a bit? Since DEV_LS7A_PCIE_PORT5 > > apparently has a class of PCI_CLASS_BRIDGE_HOST, I guess that in PCIe > > terms, it is basically a PCI host bridge (Root Complex, if you prefer) > > that is materialized as a PCI Endpoint? > > Now most of the existing LoongArch CPUs don't have an integrated PCIe > RC, instead they have HyperTransport controllers. But the latest CPU > (Loongson-3C6000) has an integrated PCIe RC and removed > HyperTransport. > > LS7A bridge can work together with both old (HT) CPUs and new (PCIe) > CPUs. If it is connected to an old CPU, its upstream port is a HT > port, and DEV_LS7A_PCIE_PORT5 works as a normal downstream PCIe port. > If it is connected to a new CPU, DEV_LS7A_PCIE_PORT5 works as an > upstream port (the class code becomes PCI_CLASS_BRIDGE_HOST) and the > HT port is idle. What does lspci look like for both the old HT and the new PCIe CPUs? With the old HT CPU, I imagine this: [LS7A includes a HT port that doesn't appear as a PCI device and basically implements a PCIe Root Complex] 00:00.0 Root Port to [bus 01-1f] (DEV_LS7A_PCIE_PORT5) With a new PCIe CPU, if DEV_LS7A_PCIE_PORT5 is a PCIe Upstream Port, it would be part of a switch, so I'm imagining something like this: 00:00.0 Root Port to [bus 01-1f] (integrated into Loongson-3C6000) 01:00.0 Upstream Port to [bus 02-1f] (DEV_LS7A_PCIE_PORT5) 02:00.0 Downstream Port to [bus 03-1f] (part of the LS7A switch) In both cases, 00:00.0 and 01:00.0 (DEV_LS7A_PCIE_PORT5) would be a Type 1 device that is enumerated as a PCI-to-PCI bridge, which would normally have a Class Code of 0x0604 (PCI_CLASS_BRIDGE_PCI). But you're saying DEV_LS7A_PCIE_PORT5 has a Class Code of PCI_CLASS_BRIDGE_HOST, which is 0x0600. That would normally be a Type 0 device and would not have a secondary bus. > > I'm curious about what's going on here because the normal PCI MSI > > support should set PCI_MSI_FLAGS_ENABLE since it's completely > > specified by the spec, which says it controls whether *this function* > > can use MSI. > > > > But in this case PCI_MSI_FLAGS_ENABLE seems to have non-architected > > behavior of controlling MSI from *other* devices below this host > > bridge? That's a little bit weird too because MSI looks like DMA to > > any bridges upstream from the device that is using MSI, and the Bus > > Master Enable bit in those bridges controls whether they forward those > > MSI DMA accesses upstream. And of course the PCI core should already > > make sure those bridges have Bus Master Enable set when downstream > > devices use MSI. > > In my opinion this is a hardware bug, when DEV_LS7A_PCIE_PORT5 works > as a host bridge, it should enable MSI automatically. But > unfortunately hardware doesn't behave like this, so we need a quirk > here. I'm fine with the quirk to work around this issue. But the commit log is confusing. > > > Cc: <stable@vger.kernel.org> > > > Signed-off-by: Sheng Wu <wusheng@loongson.cn> > > > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn> > > > --- > > > drivers/pci/controller/pci-loongson.c | 12 ++++++++++++ > > > 1 file changed, 12 insertions(+) > > > > > > diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c > > > index 8b34ccff073a..ffc581605834 100644 > > > --- a/drivers/pci/controller/pci-loongson.c > > > +++ b/drivers/pci/controller/pci-loongson.c > > > @@ -163,6 +163,18 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, > > > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, > > > DEV_LS7A_HDMI, loongson_pci_pin_quirk); > > > > > > +static void loongson_pci_msi_quirk(struct pci_dev *dev) > > > +{ > > > + u16 val, class = dev->class >> 8; > > > + > > > + if (class == PCI_CLASS_BRIDGE_HOST) { > > > + pci_read_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, &val); > > > + val |= PCI_MSI_FLAGS_ENABLE; > > > + pci_write_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, val); > > > + } > > > +} > > > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, DEV_LS7A_PCIE_PORT5, loongson_pci_msi_quirk); > > > + > > > static struct loongson_pci *pci_bus_to_loongson_pci(struct pci_bus *bus) > > > { > > > struct pci_config_window *cfg; > > > -- > > > 2.43.0 > > >
Hi, Bjorn, Sorry for the late reply. On Thu, Jul 11, 2024 at 3:48 AM Bjorn Helgaas <helgaas@kernel.org> wrote: > > On Wed, Jul 10, 2024 at 11:04:24AM +0800, Huacai Chen wrote: > > Hi, Bjorn, > > > > On Wed, Jul 10, 2024 at 5:24 AM Bjorn Helgaas <helgaas@kernel.org> wrote: > > > > > > On Wed, Jun 12, 2024 at 02:53:15PM +0800, Huacai Chen wrote: > > > > LS7A chipset can be used as a downstream bridge which connected to a > > > > high-level host bridge. In this case DEV_LS7A_PCIE_PORT5 is used as the > > > > upward port. We should always enable MSI caps of this port, otherwise > > > > downstream devices cannot use MSI. > > > > > > Can you clarify this topology a bit? Since DEV_LS7A_PCIE_PORT5 > > > apparently has a class of PCI_CLASS_BRIDGE_HOST, I guess that in PCIe > > > terms, it is basically a PCI host bridge (Root Complex, if you prefer) > > > that is materialized as a PCI Endpoint? > > > > Now most of the existing LoongArch CPUs don't have an integrated PCIe > > RC, instead they have HyperTransport controllers. But the latest CPU > > (Loongson-3C6000) has an integrated PCIe RC and removed > > HyperTransport. > > > > LS7A bridge can work together with both old (HT) CPUs and new (PCIe) > > CPUs. If it is connected to an old CPU, its upstream port is a HT > > port, and DEV_LS7A_PCIE_PORT5 works as a normal downstream PCIe port. > > If it is connected to a new CPU, DEV_LS7A_PCIE_PORT5 works as an > > upstream port (the class code becomes PCI_CLASS_BRIDGE_HOST) and the > > HT port is idle. > > What does lspci look like for both the old HT and the new PCIe CPUs? When LS7A connect to HT, 00:00.0 Host bridge: Loongson Technology LLC Hyper Transport Bridge Controller 00:00.1 Host bridge: Loongson Technology LLC Hyper Transport Bridge Controller (rev 01) 00:00.2 Host bridge: Loongson Technology LLC Device 7a20 (rev 01) 00:00.3 Host bridge: Loongson Technology LLC Device 7a30 00:03.0 Ethernet controller: Loongson Technology LLC Device 7a13 00:04.0 USB controller: Loongson Technology LLC OHCI USB Controller (rev 02) 00:04.1 USB controller: Loongson Technology LLC EHCI USB Controller (rev 02) 00:05.0 USB controller: Loongson Technology LLC OHCI USB Controller (rev 02) 00:05.1 USB controller: Loongson Technology LLC EHCI USB Controller (rev 02) 00:07.0 Audio device: Loongson Technology LLC HDA (High Definition Audio) Controller 00:08.0 SATA controller: Loongson Technology LLC Device 7a18 00:09.0 PCI bridge: Loongson Technology LLC Device 7a49 00:0d.0 PCI bridge: Loongson Technology LLC Device 7a49 00:0f.0 PCI bridge: Loongson Technology LLC Device 7a69 00:10.0 PCI bridge: Loongson Technology LLC Device 7a59 00:13.0 PCI bridge: Loongson Technology LLC Device 7a59 00:16.0 System peripheral: Loongson Technology LLC Device 7a1b 00:19.0 USB controller: Loongson Technology LLC Device 7a34 02:00.0 Non-Volatile memory controller: Shenzhen Longsys Electronics Co., Ltd. SM2263EN/SM2263XT-based OEM NVME SSD (DRAM-less) (rev 03) 04:00.0 VGA compatible controller: Advanced Micro Devices, Inc. [AMD/ATI] Oland [Radeon HD 8570 / R5 430 OEM / R7 240/340 / Radeon 520 OEM] (rev 87) 04:00.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI] Oland/Hainan/Cape Verde/Pitcairn HDMI Audio [Radeon HD 7000 Series] -[0000:00]-+-00.0 0014:7a00 +-00.1 0014:7a10 +-00.2 0014:7a20 +-00.3 0014:7a30 +-03.0 0014:7a13 +-04.0 0014:7a24 +-04.1 0014:7a14 +-05.0 0014:7a24 +-05.1 0014:7a14 +-07.0 0014:7a07 +-08.0 0014:7a18 +-09.0-[01]-- +-0d.0-[02]----00.0 1d97:2263 +-0f.0-[03]-- +-10.0-[04]--+-00.0 1002:6611 | \-00.1 1002:aab0 +-13.0-[05]-- +-16.0 0014:7a1b \-19.0 0014:7a34 DEV_LS7A_PCIE_PORT5 is 00:13.0 When LS7A connect to PCIe, 00:00.0 Host bridge: Loongson Technology LLC Device 7a59 00:03.0 Ethernet controller: Loongson Technology LLC Device 7a13 00:04.0 USB controller: Loongson Technology LLC OHCI USB Controller (rev 02) 00:04.1 USB controller: Loongson Technology LLC EHCI USB Controller (rev 02) 00:05.0 USB controller: Loongson Technology LLC OHCI USB Controller (rev 02) 00:05.1 USB controller: Loongson Technology LLC EHCI USB Controller (rev 02) 00:06.0 Multimedia video controller: Loongson Technology LLC Device 7a25 (rev 01) 00:06.1 VGA compatible controller: Loongson Technology LLC Device 7a36 (rev 02) 00:06.2 Audio device: Loongson Technology LLC Device 7a37 00:07.0 Audio device: Loongson Technology LLC HDA (High Definition Audio) Controller 00:08.0 SATA controller: Loongson Technology LLC Device 7a18 00:09.0 PCI bridge: Loongson Technology LLC Device 7a49 00:0a.0 PCI bridge: Loongson Technology LLC Device 7a39 00:0b.0 PCI bridge: Loongson Technology LLC Device 7a39 00:0c.0 PCI bridge: Loongson Technology LLC Device 7a39 00:0d.0 PCI bridge: Loongson Technology LLC Device 7a49 00:0f.0 PCI bridge: Loongson Technology LLC Device 7a69 00:10.0 PCI bridge: Loongson Technology LLC Device 7a59 00:16.0 System peripheral: Loongson Technology LLC Device 7a1b 00:17.0 ISA bridge: Loongson Technology LLC LPC Controller (rev 01) 00:19.0 USB controller: Loongson Technology LLC Device 7a34 00:1c.0 PCI bridge: Loongson Technology LLC Device 3c09 00:1d.0 IOMMU: Loongson Technology LLC Device 3c0f 00:1e.0 PCI bridge: Loongson Technology LLC Device 3c09 02:00.0 Ethernet controller: Device 1f0a:6801 (rev 01) 08:00.0 PCI bridge: Loongson Technology LLC Device 3c19 08:01.0 PCI bridge: Loongson Technology LLC Device 3c29 08:02.0 PCI bridge: Loongson Technology LLC Device 3c29 0c:00.0 PCI bridge: Loongson Technology LLC Device 3c19 0c:01.0 PCI bridge: Loongson Technology LLC Device 3c19 0c:04.0 IOMMU: Loongson Technology LLC Device 3c0f -[0000:00]-+-00.0 0014:7a59 +-03.0 0014:7a13 +-04.0 0014:7a24 +-04.1 0014:7a14 +-05.0 0014:7a24 +-05.1 0014:7a14 +-06.0 0014:7a25 +-06.1 0014:7a36 +-06.2 0014:7a37 +-07.0 0014:7a07 +-08.0 0014:7a18 +-09.0-[01]-- +-0a.0-[02]----00.0 1f0a:6801 +-0b.0-[03]-- +-0c.0-[04]-- +-0d.0-[05]-- +-0f.0-[06]-- +-10.0-[07]-- +-16.0 0014:7a1b +-17.0 0014:7a0c +-19.0 0014:7a34 +-1c.0-[08-0b]--+-00.0-[09]-- | +-01.0-[0a]-- | \-02.0-[0b]-- +-1d.0 0014:3c0f \-1e.0-[0c-0e]--+-00.0-[0d]-- +-01.0-[0e]-- \-04.0 0014:3c0f DEV_LS7A_PCIE_PORT5 becomes 00:00.0 Huacai > > With the old HT CPU, I imagine this: > > [LS7A includes a HT port that doesn't appear as a PCI device and > basically implements a PCIe Root Complex] > 00:00.0 Root Port to [bus 01-1f] (DEV_LS7A_PCIE_PORT5) > > With a new PCIe CPU, if DEV_LS7A_PCIE_PORT5 is a PCIe Upstream Port, > it would be part of a switch, so I'm imagining something like this: > > 00:00.0 Root Port to [bus 01-1f] (integrated into Loongson-3C6000) > 01:00.0 Upstream Port to [bus 02-1f] (DEV_LS7A_PCIE_PORT5) > 02:00.0 Downstream Port to [bus 03-1f] (part of the LS7A switch) > > In both cases, 00:00.0 and 01:00.0 (DEV_LS7A_PCIE_PORT5) would be a > Type 1 device that is enumerated as a PCI-to-PCI bridge, which would > normally have a Class Code of 0x0604 (PCI_CLASS_BRIDGE_PCI). > > But you're saying DEV_LS7A_PCIE_PORT5 has a Class Code of > PCI_CLASS_BRIDGE_HOST, which is 0x0600. That would normally be a Type > 0 device and would not have a secondary bus. > > > > I'm curious about what's going on here because the normal PCI MSI > > > support should set PCI_MSI_FLAGS_ENABLE since it's completely > > > specified by the spec, which says it controls whether *this function* > > > can use MSI. > > > > > > But in this case PCI_MSI_FLAGS_ENABLE seems to have non-architected > > > behavior of controlling MSI from *other* devices below this host > > > bridge? That's a little bit weird too because MSI looks like DMA to > > > any bridges upstream from the device that is using MSI, and the Bus > > > Master Enable bit in those bridges controls whether they forward those > > > MSI DMA accesses upstream. And of course the PCI core should already > > > make sure those bridges have Bus Master Enable set when downstream > > > devices use MSI. > > > > In my opinion this is a hardware bug, when DEV_LS7A_PCIE_PORT5 works > > as a host bridge, it should enable MSI automatically. But > > unfortunately hardware doesn't behave like this, so we need a quirk > > here. > > I'm fine with the quirk to work around this issue. But the commit log > is confusing. > > > > > Cc: <stable@vger.kernel.org> > > > > Signed-off-by: Sheng Wu <wusheng@loongson.cn> > > > > Signed-off-by: Huacai Chen <chenhuacai@loongson.cn> > > > > --- > > > > drivers/pci/controller/pci-loongson.c | 12 ++++++++++++ > > > > 1 file changed, 12 insertions(+) > > > > > > > > diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c > > > > index 8b34ccff073a..ffc581605834 100644 > > > > --- a/drivers/pci/controller/pci-loongson.c > > > > +++ b/drivers/pci/controller/pci-loongson.c > > > > @@ -163,6 +163,18 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, > > > > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, > > > > DEV_LS7A_HDMI, loongson_pci_pin_quirk); > > > > > > > > +static void loongson_pci_msi_quirk(struct pci_dev *dev) > > > > +{ > > > > + u16 val, class = dev->class >> 8; > > > > + > > > > + if (class == PCI_CLASS_BRIDGE_HOST) { > > > > + pci_read_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, &val); > > > > + val |= PCI_MSI_FLAGS_ENABLE; > > > > + pci_write_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, val); > > > > + } > > > > +} > > > > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, DEV_LS7A_PCIE_PORT5, loongson_pci_msi_quirk); > > > > + > > > > static struct loongson_pci *pci_bus_to_loongson_pci(struct pci_bus *bus) > > > > { > > > > struct pci_config_window *cfg; > > > > -- > > > > 2.43.0 > > > > >
On Thu, Jul 18, 2024 at 09:01:17PM +0800, Huacai Chen wrote: > On Thu, Jul 11, 2024 at 3:48 AM Bjorn Helgaas <helgaas@kernel.org> wrote: > > On Wed, Jul 10, 2024 at 11:04:24AM +0800, Huacai Chen wrote: > > > On Wed, Jul 10, 2024 at 5:24 AM Bjorn Helgaas <helgaas@kernel.org> wrote: > > > > > > > > On Wed, Jun 12, 2024 at 02:53:15PM +0800, Huacai Chen wrote: > > > > > LS7A chipset can be used as a downstream bridge which connected to a > > > > > high-level host bridge. In this case DEV_LS7A_PCIE_PORT5 is used as the > > > > > upward port. We should always enable MSI caps of this port, otherwise > > > > > downstream devices cannot use MSI. > > > > > > > > Can you clarify this topology a bit? Since DEV_LS7A_PCIE_PORT5 > > > > apparently has a class of PCI_CLASS_BRIDGE_HOST, I guess that in PCIe > > > > terms, it is basically a PCI host bridge (Root Complex, if you prefer) > > > > that is materialized as a PCI Endpoint? > > > > > > Now most of the existing LoongArch CPUs don't have an integrated PCIe > > > RC, instead they have HyperTransport controllers. But the latest CPU > > > (Loongson-3C6000) has an integrated PCIe RC and removed > > > HyperTransport. > > > > > > LS7A bridge can work together with both old (HT) CPUs and new (PCIe) > > > CPUs. If it is connected to an old CPU, its upstream port is a HT > > > port, and DEV_LS7A_PCIE_PORT5 works as a normal downstream PCIe port. > > > If it is connected to a new CPU, DEV_LS7A_PCIE_PORT5 works as an > > > upstream port (the class code becomes PCI_CLASS_BRIDGE_HOST) and the > > > HT port is idle. > > > > What does lspci look like for both the old HT and the new PCIe CPUs? > When LS7A connect to HT, > > 00:00.0 Host bridge: Loongson Technology LLC Hyper Transport Bridge Controller > 00:00.1 Host bridge: Loongson Technology LLC Hyper Transport Bridge > Controller (rev 01) > 00:00.2 Host bridge: Loongson Technology LLC Device 7a20 (rev 01) > 00:00.3 Host bridge: Loongson Technology LLC Device 7a30 > 00:03.0 Ethernet controller: Loongson Technology LLC Device 7a13 > 00:04.0 USB controller: Loongson Technology LLC OHCI USB Controller (rev 02) > 00:04.1 USB controller: Loongson Technology LLC EHCI USB Controller (rev 02) > 00:05.0 USB controller: Loongson Technology LLC OHCI USB Controller (rev 02) > 00:05.1 USB controller: Loongson Technology LLC EHCI USB Controller (rev 02) > 00:07.0 Audio device: Loongson Technology LLC HDA (High Definition > Audio) Controller > 00:08.0 SATA controller: Loongson Technology LLC Device 7a18 > 00:09.0 PCI bridge: Loongson Technology LLC Device 7a49 > 00:0d.0 PCI bridge: Loongson Technology LLC Device 7a49 > 00:0f.0 PCI bridge: Loongson Technology LLC Device 7a69 > 00:10.0 PCI bridge: Loongson Technology LLC Device 7a59 > 00:13.0 PCI bridge: Loongson Technology LLC Device 7a59 > 00:16.0 System peripheral: Loongson Technology LLC Device 7a1b > 00:19.0 USB controller: Loongson Technology LLC Device 7a34 > 02:00.0 Non-Volatile memory controller: Shenzhen Longsys Electronics > Co., Ltd. SM2263EN/SM2263XT-based OEM NVME SSD (DRAM-less) (rev 03) > 04:00.0 VGA compatible controller: Advanced Micro Devices, Inc. > [AMD/ATI] Oland [Radeon HD 8570 / R5 430 OEM / R7 240/340 / Radeon 520 > OEM] (rev 87) > 04:00.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI] > Oland/Hainan/Cape Verde/Pitcairn HDMI Audio [Radeon HD 7000 Series] > > -[0000:00]-+-00.0 0014:7a00 > +-00.1 0014:7a10 > +-00.2 0014:7a20 > +-00.3 0014:7a30 > +-03.0 0014:7a13 > +-04.0 0014:7a24 > +-04.1 0014:7a14 > +-05.0 0014:7a24 > +-05.1 0014:7a14 > +-07.0 0014:7a07 > +-08.0 0014:7a18 > +-09.0-[01]-- > +-0d.0-[02]----00.0 1d97:2263 > +-0f.0-[03]-- > +-10.0-[04]--+-00.0 1002:6611 > | \-00.1 1002:aab0 > +-13.0-[05]-- > +-16.0 0014:7a1b > \-19.0 0014:7a34 > > DEV_LS7A_PCIE_PORT5 is 00:13.0 In this case, the DEV_LS7A_PCIE_PORT5 at 00:13.0 is a Header Type 1 (PCI-to-PCI bridge). I assume it has a PCIe Capability that identifies it as Root Port, and the secondary bus 05 is probably a PCIe Link leading to a slot. I found "lspci -v" output similar to this at https://linux-hardware.org/?probe=ad154077da&log=lspci_all > When LS7A connect to PCIe, > > 00:00.0 Host bridge: Loongson Technology LLC Device 7a59 > 00:03.0 Ethernet controller: Loongson Technology LLC Device 7a13 > 00:04.0 USB controller: Loongson Technology LLC OHCI USB Controller (rev 02) > 00:04.1 USB controller: Loongson Technology LLC EHCI USB Controller (rev 02) > 00:05.0 USB controller: Loongson Technology LLC OHCI USB Controller (rev 02) > 00:05.1 USB controller: Loongson Technology LLC EHCI USB Controller (rev 02) > 00:06.0 Multimedia video controller: Loongson Technology LLC Device > 7a25 (rev 01) > 00:06.1 VGA compatible controller: Loongson Technology LLC Device 7a36 (rev 02) > 00:06.2 Audio device: Loongson Technology LLC Device 7a37 > 00:07.0 Audio device: Loongson Technology LLC HDA (High Definition > Audio) Controller > 00:08.0 SATA controller: Loongson Technology LLC Device 7a18 > 00:09.0 PCI bridge: Loongson Technology LLC Device 7a49 > 00:0a.0 PCI bridge: Loongson Technology LLC Device 7a39 > 00:0b.0 PCI bridge: Loongson Technology LLC Device 7a39 > 00:0c.0 PCI bridge: Loongson Technology LLC Device 7a39 > 00:0d.0 PCI bridge: Loongson Technology LLC Device 7a49 > 00:0f.0 PCI bridge: Loongson Technology LLC Device 7a69 > 00:10.0 PCI bridge: Loongson Technology LLC Device 7a59 > 00:16.0 System peripheral: Loongson Technology LLC Device 7a1b > 00:17.0 ISA bridge: Loongson Technology LLC LPC Controller (rev 01) > 00:19.0 USB controller: Loongson Technology LLC Device 7a34 > 00:1c.0 PCI bridge: Loongson Technology LLC Device 3c09 > 00:1d.0 IOMMU: Loongson Technology LLC Device 3c0f > 00:1e.0 PCI bridge: Loongson Technology LLC Device 3c09 > 02:00.0 Ethernet controller: Device 1f0a:6801 (rev 01) > 08:00.0 PCI bridge: Loongson Technology LLC Device 3c19 > 08:01.0 PCI bridge: Loongson Technology LLC Device 3c29 > 08:02.0 PCI bridge: Loongson Technology LLC Device 3c29 > 0c:00.0 PCI bridge: Loongson Technology LLC Device 3c19 > 0c:01.0 PCI bridge: Loongson Technology LLC Device 3c19 > 0c:04.0 IOMMU: Loongson Technology LLC Device 3c0f > > -[0000:00]-+-00.0 0014:7a59 > +-03.0 0014:7a13 > +-04.0 0014:7a24 > +-04.1 0014:7a14 > +-05.0 0014:7a24 > +-05.1 0014:7a14 > +-06.0 0014:7a25 > +-06.1 0014:7a36 > +-06.2 0014:7a37 > +-07.0 0014:7a07 > +-08.0 0014:7a18 > +-09.0-[01]-- > +-0a.0-[02]----00.0 1f0a:6801 > +-0b.0-[03]-- > +-0c.0-[04]-- > +-0d.0-[05]-- > +-0f.0-[06]-- > +-10.0-[07]-- > +-16.0 0014:7a1b > +-17.0 0014:7a0c > +-19.0 0014:7a34 > +-1c.0-[08-0b]--+-00.0-[09]-- > | +-01.0-[0a]-- > | \-02.0-[0b]-- > +-1d.0 0014:3c0f > \-1e.0-[0c-0e]--+-00.0-[0d]-- > +-01.0-[0e]-- > \-04.0 0014:3c0f > > DEV_LS7A_PCIE_PORT5 becomes 00:00.0 In this case, the DEV_LS7A_PCIE_PORT5 at 00:00.0 looks like a Header Type 0 device (not a bridge), and the bus 00 it is on is not a normal PCIe Link. Since it doesn't connect to a PCIe Link, 00:00.0 might not have a PCIe Capability at all, or it could be an RCiEP. Or it's possible it has a PCIe Capability left over from the old model that says it's a Root Port, although this would potentially confuse an OS. The other bridges (00:09.0, 00:0a.0, etc) are probably PCIe Root Ports, which are logically part of the Root Complex, so the Root Complex would be implemented partly in the new CPU and partly in the LS7A. The connection between the new CPU and the LS7A might be electrically similar to PCIe, but it's not part of a PCIe hierarchy that Linux sees. That connection would have to be managed in some non-PCI way, the same as all PCI host bridges, i.e., by firmware, ACPI, or a Linux native host bridge driver like pci-loongson.c. Here's the current commit log: The LS7A chipset can be used as a downstream bridge that connects to a high-level host bridge, and in such case the DEV_LS7A_PCIE_PORT5 is used as the upstream port. Thus, always enable MSI caps of this port, otherwise downstream devices cannot use MSI. In this configuration, DEV_LS7A_PCIE_PORT5 is not a PCI-to-PCI bridge itself. It might be some sort of downstream end of a connection from the CPU, but that's not really relevant here. And DEV_LS7A_PCIE_PORT5 doesn't have any devices directly downstream from it. I think something like this would be clearer: The LS7A chipset can be used as part of a PCIe Root Complex with Loongson-3C6000 and similar CPUs. In this case, DEV_LS7A_PCIE_PORT5 has a PCI_CLASS_BRIDGE_HOST class code, and it is a Type 0 Function whose config space provides access to Root Complex registers. The DEV_LS7A_PCIE_PORT5 has an MSI Capability, and its MSI Enable bit must be set before other devices below the Root Complex can use MSI. This is not the standard PCI behavior of MSI Enable, so the normal PCI MSI code does not set it. Set the DEV_LS7A_PCIE_PORT5 MSI Enable bit via a quirk so other devices below the Root Complex can use MSI. What do you think?
On Fri, Jul 19, 2024 at 7:03 AM Bjorn Helgaas <helgaas@kernel.org> wrote: > > On Thu, Jul 18, 2024 at 09:01:17PM +0800, Huacai Chen wrote: > > On Thu, Jul 11, 2024 at 3:48 AM Bjorn Helgaas <helgaas@kernel.org> wrote: > > > On Wed, Jul 10, 2024 at 11:04:24AM +0800, Huacai Chen wrote: > > > > On Wed, Jul 10, 2024 at 5:24 AM Bjorn Helgaas <helgaas@kernel.org> wrote: > > > > > > > > > > On Wed, Jun 12, 2024 at 02:53:15PM +0800, Huacai Chen wrote: > > > > > > LS7A chipset can be used as a downstream bridge which connected to a > > > > > > high-level host bridge. In this case DEV_LS7A_PCIE_PORT5 is used as the > > > > > > upward port. We should always enable MSI caps of this port, otherwise > > > > > > downstream devices cannot use MSI. > > > > > > > > > > Can you clarify this topology a bit? Since DEV_LS7A_PCIE_PORT5 > > > > > apparently has a class of PCI_CLASS_BRIDGE_HOST, I guess that in PCIe > > > > > terms, it is basically a PCI host bridge (Root Complex, if you prefer) > > > > > that is materialized as a PCI Endpoint? > > > > > > > > Now most of the existing LoongArch CPUs don't have an integrated PCIe > > > > RC, instead they have HyperTransport controllers. But the latest CPU > > > > (Loongson-3C6000) has an integrated PCIe RC and removed > > > > HyperTransport. > > > > > > > > LS7A bridge can work together with both old (HT) CPUs and new (PCIe) > > > > CPUs. If it is connected to an old CPU, its upstream port is a HT > > > > port, and DEV_LS7A_PCIE_PORT5 works as a normal downstream PCIe port. > > > > If it is connected to a new CPU, DEV_LS7A_PCIE_PORT5 works as an > > > > upstream port (the class code becomes PCI_CLASS_BRIDGE_HOST) and the > > > > HT port is idle. > > > > > > What does lspci look like for both the old HT and the new PCIe CPUs? > > When LS7A connect to HT, > > > > 00:00.0 Host bridge: Loongson Technology LLC Hyper Transport Bridge Controller > > 00:00.1 Host bridge: Loongson Technology LLC Hyper Transport Bridge > > Controller (rev 01) > > 00:00.2 Host bridge: Loongson Technology LLC Device 7a20 (rev 01) > > 00:00.3 Host bridge: Loongson Technology LLC Device 7a30 > > 00:03.0 Ethernet controller: Loongson Technology LLC Device 7a13 > > 00:04.0 USB controller: Loongson Technology LLC OHCI USB Controller (rev 02) > > 00:04.1 USB controller: Loongson Technology LLC EHCI USB Controller (rev 02) > > 00:05.0 USB controller: Loongson Technology LLC OHCI USB Controller (rev 02) > > 00:05.1 USB controller: Loongson Technology LLC EHCI USB Controller (rev 02) > > 00:07.0 Audio device: Loongson Technology LLC HDA (High Definition > > Audio) Controller > > 00:08.0 SATA controller: Loongson Technology LLC Device 7a18 > > 00:09.0 PCI bridge: Loongson Technology LLC Device 7a49 > > 00:0d.0 PCI bridge: Loongson Technology LLC Device 7a49 > > 00:0f.0 PCI bridge: Loongson Technology LLC Device 7a69 > > 00:10.0 PCI bridge: Loongson Technology LLC Device 7a59 > > 00:13.0 PCI bridge: Loongson Technology LLC Device 7a59 > > 00:16.0 System peripheral: Loongson Technology LLC Device 7a1b > > 00:19.0 USB controller: Loongson Technology LLC Device 7a34 > > 02:00.0 Non-Volatile memory controller: Shenzhen Longsys Electronics > > Co., Ltd. SM2263EN/SM2263XT-based OEM NVME SSD (DRAM-less) (rev 03) > > 04:00.0 VGA compatible controller: Advanced Micro Devices, Inc. > > [AMD/ATI] Oland [Radeon HD 8570 / R5 430 OEM / R7 240/340 / Radeon 520 > > OEM] (rev 87) > > 04:00.1 Audio device: Advanced Micro Devices, Inc. [AMD/ATI] > > Oland/Hainan/Cape Verde/Pitcairn HDMI Audio [Radeon HD 7000 Series] > > > > -[0000:00]-+-00.0 0014:7a00 > > +-00.1 0014:7a10 > > +-00.2 0014:7a20 > > +-00.3 0014:7a30 > > +-03.0 0014:7a13 > > +-04.0 0014:7a24 > > +-04.1 0014:7a14 > > +-05.0 0014:7a24 > > +-05.1 0014:7a14 > > +-07.0 0014:7a07 > > +-08.0 0014:7a18 > > +-09.0-[01]-- > > +-0d.0-[02]----00.0 1d97:2263 > > +-0f.0-[03]-- > > +-10.0-[04]--+-00.0 1002:6611 > > | \-00.1 1002:aab0 > > +-13.0-[05]-- > > +-16.0 0014:7a1b > > \-19.0 0014:7a34 > > > > DEV_LS7A_PCIE_PORT5 is 00:13.0 > > In this case, the DEV_LS7A_PCIE_PORT5 at 00:13.0 is a Header Type 1 > (PCI-to-PCI bridge). I assume it has a PCIe Capability that > identifies it as Root Port, and the secondary bus 05 is probably a > PCIe Link leading to a slot. > > I found "lspci -v" output similar to this at > https://linux-hardware.org/?probe=ad154077da&log=lspci_all > > > When LS7A connect to PCIe, > > > > 00:00.0 Host bridge: Loongson Technology LLC Device 7a59 > > 00:03.0 Ethernet controller: Loongson Technology LLC Device 7a13 > > 00:04.0 USB controller: Loongson Technology LLC OHCI USB Controller (rev 02) > > 00:04.1 USB controller: Loongson Technology LLC EHCI USB Controller (rev 02) > > 00:05.0 USB controller: Loongson Technology LLC OHCI USB Controller (rev 02) > > 00:05.1 USB controller: Loongson Technology LLC EHCI USB Controller (rev 02) > > 00:06.0 Multimedia video controller: Loongson Technology LLC Device > > 7a25 (rev 01) > > 00:06.1 VGA compatible controller: Loongson Technology LLC Device 7a36 (rev 02) > > 00:06.2 Audio device: Loongson Technology LLC Device 7a37 > > 00:07.0 Audio device: Loongson Technology LLC HDA (High Definition > > Audio) Controller > > 00:08.0 SATA controller: Loongson Technology LLC Device 7a18 > > 00:09.0 PCI bridge: Loongson Technology LLC Device 7a49 > > 00:0a.0 PCI bridge: Loongson Technology LLC Device 7a39 > > 00:0b.0 PCI bridge: Loongson Technology LLC Device 7a39 > > 00:0c.0 PCI bridge: Loongson Technology LLC Device 7a39 > > 00:0d.0 PCI bridge: Loongson Technology LLC Device 7a49 > > 00:0f.0 PCI bridge: Loongson Technology LLC Device 7a69 > > 00:10.0 PCI bridge: Loongson Technology LLC Device 7a59 > > 00:16.0 System peripheral: Loongson Technology LLC Device 7a1b > > 00:17.0 ISA bridge: Loongson Technology LLC LPC Controller (rev 01) > > 00:19.0 USB controller: Loongson Technology LLC Device 7a34 > > 00:1c.0 PCI bridge: Loongson Technology LLC Device 3c09 > > 00:1d.0 IOMMU: Loongson Technology LLC Device 3c0f > > 00:1e.0 PCI bridge: Loongson Technology LLC Device 3c09 > > 02:00.0 Ethernet controller: Device 1f0a:6801 (rev 01) > > 08:00.0 PCI bridge: Loongson Technology LLC Device 3c19 > > 08:01.0 PCI bridge: Loongson Technology LLC Device 3c29 > > 08:02.0 PCI bridge: Loongson Technology LLC Device 3c29 > > 0c:00.0 PCI bridge: Loongson Technology LLC Device 3c19 > > 0c:01.0 PCI bridge: Loongson Technology LLC Device 3c19 > > 0c:04.0 IOMMU: Loongson Technology LLC Device 3c0f > > > > -[0000:00]-+-00.0 0014:7a59 > > +-03.0 0014:7a13 > > +-04.0 0014:7a24 > > +-04.1 0014:7a14 > > +-05.0 0014:7a24 > > +-05.1 0014:7a14 > > +-06.0 0014:7a25 > > +-06.1 0014:7a36 > > +-06.2 0014:7a37 > > +-07.0 0014:7a07 > > +-08.0 0014:7a18 > > +-09.0-[01]-- > > +-0a.0-[02]----00.0 1f0a:6801 > > +-0b.0-[03]-- > > +-0c.0-[04]-- > > +-0d.0-[05]-- > > +-0f.0-[06]-- > > +-10.0-[07]-- > > +-16.0 0014:7a1b > > +-17.0 0014:7a0c > > +-19.0 0014:7a34 > > +-1c.0-[08-0b]--+-00.0-[09]-- > > | +-01.0-[0a]-- > > | \-02.0-[0b]-- > > +-1d.0 0014:3c0f > > \-1e.0-[0c-0e]--+-00.0-[0d]-- > > +-01.0-[0e]-- > > \-04.0 0014:3c0f > > > > DEV_LS7A_PCIE_PORT5 becomes 00:00.0 > > In this case, the DEV_LS7A_PCIE_PORT5 at 00:00.0 looks like a Header > Type 0 device (not a bridge), and the bus 00 it is on is not a normal > PCIe Link. Since it doesn't connect to a PCIe Link, 00:00.0 might not > have a PCIe Capability at all, or it could be an RCiEP. Or it's > possible it has a PCIe Capability left over from the old model that > says it's a Root Port, although this would potentially confuse an OS. > > The other bridges (00:09.0, 00:0a.0, etc) are probably PCIe Root > Ports, which are logically part of the Root Complex, so the Root > Complex would be implemented partly in the new CPU and partly in the > LS7A. > > The connection between the new CPU and the LS7A might be electrically > similar to PCIe, but it's not part of a PCIe hierarchy that Linux > sees. That connection would have to be managed in some non-PCI way, > the same as all PCI host bridges, i.e., by firmware, ACPI, or a Linux > native host bridge driver like pci-loongson.c. > > Here's the current commit log: > > The LS7A chipset can be used as a downstream bridge that connects to > a high-level host bridge, and in such case the DEV_LS7A_PCIE_PORT5 > is used as the upstream port. > > Thus, always enable MSI caps of this port, otherwise downstream > devices cannot use MSI. > > In this configuration, DEV_LS7A_PCIE_PORT5 is not a PCI-to-PCI bridge > itself. It might be some sort of downstream end of a connection from > the CPU, but that's not really relevant here. And DEV_LS7A_PCIE_PORT5 > doesn't have any devices directly downstream from it. > > I think something like this would be clearer: > > The LS7A chipset can be used as part of a PCIe Root Complex with > Loongson-3C6000 and similar CPUs. In this case, DEV_LS7A_PCIE_PORT5 > has a PCI_CLASS_BRIDGE_HOST class code, and it is a Type 0 Function > whose config space provides access to Root Complex registers. > > The DEV_LS7A_PCIE_PORT5 has an MSI Capability, and its MSI Enable > bit must be set before other devices below the Root Complex can use > MSI. This is not the standard PCI behavior of MSI Enable, so the > normal PCI MSI code does not set it. > > Set the DEV_LS7A_PCIE_PORT5 MSI Enable bit via a quirk so other > devices below the Root Complex can use MSI. > > What do you think? Yes, that's more better. Huacai
diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c index 8b34ccff073a..ffc581605834 100644 --- a/drivers/pci/controller/pci-loongson.c +++ b/drivers/pci/controller/pci-loongson.c @@ -163,6 +163,18 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, DEV_LS7A_HDMI, loongson_pci_pin_quirk); +static void loongson_pci_msi_quirk(struct pci_dev *dev) +{ + u16 val, class = dev->class >> 8; + + if (class == PCI_CLASS_BRIDGE_HOST) { + pci_read_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, &val); + val |= PCI_MSI_FLAGS_ENABLE; + pci_write_config_word(dev, dev->msi_cap + PCI_MSI_FLAGS, val); + } +} +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, DEV_LS7A_PCIE_PORT5, loongson_pci_msi_quirk); + static struct loongson_pci *pci_bus_to_loongson_pci(struct pci_bus *bus) { struct pci_config_window *cfg;