| Message ID | 20210909080927.164709-1-nathan@nathanrossi.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | [v2] PCI: Add ACS errata for Pericom PI7C9X2G404 switch | expand |
On Thu, 09 Sep 2021 08:09:27 +0000 Nathan Rossi <nathan@nathanrossi.com> wrote: > From: Nathan Rossi <nathan.rossi@digi.com> > > The Pericom PI7C9X2G404 PCIe switch has an errata for ACS P2P Request > Redirect behaviour when used in the cut-through forwarding mode. The > recommended work around for this issue is to use the switch in store and > forward mode. The errata results in packets being queued and not being > delivered upstream, this can be observed as very poor downstream device > performance and/or dropped device generated data/interrupts. > > This change adds a fixup specific to this switch that when enabling or > resuming the downstream port it checks if it has enabled ACS P2P Request > Redirect, and if so changes the device (via the upstream port) to use > the store and forward operating mode. > > Signed-off-by: Nathan Rossi <nathan.rossi@digi.com> Thanks for pursuing this! I took a stab at it[1] some time ago but I tried to pursue link balancing, I didn't have the datasheet to realize we could simply put the device in a different mode and blindly assumed the mode was inherit to the unbalanced link config. Link balancing turned out to be more complicated than I cared or had time to pursue. Also related to this device, there is a kernel bz for this issue that might provide more details[2], Bjorn will probably want to add that reference to the commit log. I'll see if I can find the card from that bz to test here. Thanks, Alex [1]https://lore.kernel.org/all/20161026175156.23495.12980.stgit@gimli.home/ [2]https://bugzilla.kernel.org/show_bug.cgi?id=177471 > --- > Changes in v2: > - Added DECLARE_PCI_FIXUP_RESUME to handle applying fixup upon resume as > switch operation may have been reset or ACS configuration may have > changed > --- > drivers/pci/quirks.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 48 insertions(+) > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > index e5089af8ad..5849b7046b 100644 > --- a/drivers/pci/quirks.c > +++ b/drivers/pci/quirks.c > @@ -5790,3 +5790,51 @@ static void apex_pci_fixup_class(struct pci_dev *pdev) > } > DECLARE_PCI_FIXUP_CLASS_HEADER(0x1ac1, 0x089a, > PCI_CLASS_NOT_DEFINED, 8, apex_pci_fixup_class); > + > +/* > + * Pericom PI7C9X2G404 switch errata E5 - ACS P2P Request Redirect is not > + * functional > + * > + * When ACS P2P Request Redirect is enabled and bandwidth is not balanced > + * between upstream and downstream ports, packets are queued in an internal > + * buffer until CPLD packet. The workaround is to use the switch in store and > + * forward mode. > + */ > +#define PI7C9X2G404_MODE_REG 0x74 > +#define PI7C9X2G404_STORE_FORWARD_MODE BIT(0) > +static void pci_fixup_pericom_acs_store_forward(struct pci_dev *pdev) > +{ > + struct pci_dev *upstream; > + u16 val; > + > + /* Downstream ports only */ > + if (pci_pcie_type(pdev) != PCI_EXP_TYPE_DOWNSTREAM) > + return; > + > + /* Check for ACS P2P Request Redirect use */ > + if (!pdev->acs_cap) > + return; > + pci_read_config_word(pdev, pdev->acs_cap + PCI_ACS_CTRL, &val); > + if (!(val & PCI_ACS_RR)) > + return; > + > + upstream = pci_upstream_bridge(pdev); > + if (!upstream) > + return; > + > + pci_read_config_word(upstream, PI7C9X2G404_MODE_REG, &val); > + if (!(val & PI7C9X2G404_STORE_FORWARD_MODE)) { > + pci_info(upstream, "Setting PI7C9X2G404 store-forward mode\n"); > + /* Enable store-foward mode */ > + pci_write_config_word(upstream, PI7C9X2G404_MODE_REG, val | > + PI7C9X2G404_STORE_FORWARD_MODE); > + } > +} > +/* > + * Apply fixup on enable and on resume, in order to apply the fix up whenever > + * ACS configuration changes or switch mode is reset > + */ > +DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_PERICOM, 0x2404, > + pci_fixup_pericom_acs_store_forward); > +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_PERICOM, 0x2404, > + pci_fixup_pericom_acs_store_forward); > --- > 2.33.0 >
On Thu, 9 Sep 2021 09:39:11 -0600 Alex Williamson <alex.williamson@redhat.com> wrote: > On Thu, 09 Sep 2021 08:09:27 +0000 > Nathan Rossi <nathan@nathanrossi.com> wrote: > > > From: Nathan Rossi <nathan.rossi@digi.com> > > > > The Pericom PI7C9X2G404 PCIe switch has an errata for ACS P2P Request > > Redirect behaviour when used in the cut-through forwarding mode. The > > recommended work around for this issue is to use the switch in store and > > forward mode. The errata results in packets being queued and not being > > delivered upstream, this can be observed as very poor downstream device > > performance and/or dropped device generated data/interrupts. > > > > This change adds a fixup specific to this switch that when enabling or > > resuming the downstream port it checks if it has enabled ACS P2P Request > > Redirect, and if so changes the device (via the upstream port) to use > > the store and forward operating mode. > > > > Signed-off-by: Nathan Rossi <nathan.rossi@digi.com> > > > Thanks for pursuing this! I took a stab at it[1] some time ago but I > tried to pursue link balancing, I didn't have the datasheet to realize > we could simply put the device in a different mode and blindly assumed > the mode was inherit to the unbalanced link config. Link balancing > turned out to be more complicated than I cared or had time to pursue. > > Also related to this device, there is a kernel bz for this issue that > might provide more details[2], Bjorn will probably want to add that > reference to the commit log. > > I'll see if I can find the card from that bz to test here. Thanks, Tested against the troublesome dual-port NIC mentioned in the bz below[2]. With VT-d enabled the NIC reports netdev timeouts, fails to DHCP an IP for some time. With this patch I see the info message in dmesg reporting store-forward mode is enabled and both ports appear to work normally with no netdev timeouts. Tested-by: Alex Williamson <alex.williamson@redhat.com> Thanks! Alex > [1]https://lore.kernel.org/all/20161026175156.23495.12980.stgit@gimli.home/ > [2]https://bugzilla.kernel.org/show_bug.cgi?id=177471 > > > --- > > Changes in v2: > > - Added DECLARE_PCI_FIXUP_RESUME to handle applying fixup upon resume as > > switch operation may have been reset or ACS configuration may have > > changed > > --- > > drivers/pci/quirks.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 48 insertions(+) > > > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c > > index e5089af8ad..5849b7046b 100644 > > --- a/drivers/pci/quirks.c > > +++ b/drivers/pci/quirks.c > > @@ -5790,3 +5790,51 @@ static void apex_pci_fixup_class(struct pci_dev *pdev) > > } > > DECLARE_PCI_FIXUP_CLASS_HEADER(0x1ac1, 0x089a, > > PCI_CLASS_NOT_DEFINED, 8, apex_pci_fixup_class); > > + > > +/* > > + * Pericom PI7C9X2G404 switch errata E5 - ACS P2P Request Redirect is not > > + * functional > > + * > > + * When ACS P2P Request Redirect is enabled and bandwidth is not balanced > > + * between upstream and downstream ports, packets are queued in an internal > > + * buffer until CPLD packet. The workaround is to use the switch in store and > > + * forward mode. > > + */ > > +#define PI7C9X2G404_MODE_REG 0x74 > > +#define PI7C9X2G404_STORE_FORWARD_MODE BIT(0) > > +static void pci_fixup_pericom_acs_store_forward(struct pci_dev *pdev) > > +{ > > + struct pci_dev *upstream; > > + u16 val; > > + > > + /* Downstream ports only */ > > + if (pci_pcie_type(pdev) != PCI_EXP_TYPE_DOWNSTREAM) > > + return; > > + > > + /* Check for ACS P2P Request Redirect use */ > > + if (!pdev->acs_cap) > > + return; > > + pci_read_config_word(pdev, pdev->acs_cap + PCI_ACS_CTRL, &val); > > + if (!(val & PCI_ACS_RR)) > > + return; > > + > > + upstream = pci_upstream_bridge(pdev); > > + if (!upstream) > > + return; > > + > > + pci_read_config_word(upstream, PI7C9X2G404_MODE_REG, &val); > > + if (!(val & PI7C9X2G404_STORE_FORWARD_MODE)) { > > + pci_info(upstream, "Setting PI7C9X2G404 store-forward mode\n"); > > + /* Enable store-foward mode */ > > + pci_write_config_word(upstream, PI7C9X2G404_MODE_REG, val | > > + PI7C9X2G404_STORE_FORWARD_MODE); > > + } > > +} > > +/* > > + * Apply fixup on enable and on resume, in order to apply the fix up whenever > > + * ACS configuration changes or switch mode is reset > > + */ > > +DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_PERICOM, 0x2404, > > + pci_fixup_pericom_acs_store_forward); > > +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_PERICOM, 0x2404, > > + pci_fixup_pericom_acs_store_forward); > > --- > > 2.33.0 > > >
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index e5089af8ad..5849b7046b 100644 --- a/drivers/pci/quirks.c +++ b/drivers/pci/quirks.c @@ -5790,3 +5790,51 @@ static void apex_pci_fixup_class(struct pci_dev *pdev) } DECLARE_PCI_FIXUP_CLASS_HEADER(0x1ac1, 0x089a, PCI_CLASS_NOT_DEFINED, 8, apex_pci_fixup_class); + +/* + * Pericom PI7C9X2G404 switch errata E5 - ACS P2P Request Redirect is not + * functional + * + * When ACS P2P Request Redirect is enabled and bandwidth is not balanced + * between upstream and downstream ports, packets are queued in an internal + * buffer until CPLD packet. The workaround is to use the switch in store and + * forward mode. + */ +#define PI7C9X2G404_MODE_REG 0x74 +#define PI7C9X2G404_STORE_FORWARD_MODE BIT(0) +static void pci_fixup_pericom_acs_store_forward(struct pci_dev *pdev) +{ + struct pci_dev *upstream; + u16 val; + + /* Downstream ports only */ + if (pci_pcie_type(pdev) != PCI_EXP_TYPE_DOWNSTREAM) + return; + + /* Check for ACS P2P Request Redirect use */ + if (!pdev->acs_cap) + return; + pci_read_config_word(pdev, pdev->acs_cap + PCI_ACS_CTRL, &val); + if (!(val & PCI_ACS_RR)) + return; + + upstream = pci_upstream_bridge(pdev); + if (!upstream) + return; + + pci_read_config_word(upstream, PI7C9X2G404_MODE_REG, &val); + if (!(val & PI7C9X2G404_STORE_FORWARD_MODE)) { + pci_info(upstream, "Setting PI7C9X2G404 store-forward mode\n"); + /* Enable store-foward mode */ + pci_write_config_word(upstream, PI7C9X2G404_MODE_REG, val | + PI7C9X2G404_STORE_FORWARD_MODE); + } +} +/* + * Apply fixup on enable and on resume, in order to apply the fix up whenever + * ACS configuration changes or switch mode is reset + */ +DECLARE_PCI_FIXUP_ENABLE(PCI_VENDOR_ID_PERICOM, 0x2404, + pci_fixup_pericom_acs_store_forward); +DECLARE_PCI_FIXUP_RESUME(PCI_VENDOR_ID_PERICOM, 0x2404, + pci_fixup_pericom_acs_store_forward);