Hi, Bjorn,
On Wed, Jul 10, 2024 at 5:24 AM Bjorn Helgaas <helgaas@xxxxxxxxxx> 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@xxxxxxxxxxxxxxx>
> > Signed-off-by: Sheng Wu <wusheng@xxxxxxxxxxx>
> > Signed-off-by: Huacai Chen <chenhuacai@xxxxxxxxxxx>
> > ---
> > 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
> >
