RE: [PATCH] IB/qib: detour pcie_caps for certain chip sets
> It doesn't seem to me like this has any place in the quirks thing > either. Things seem to be working properly, the MaxPayLoad of 128 is > clearly the highest the system will support correctly. > > Jason Probably the best thing to do unwind the module parameter default in 8d4548f2b which would change the initial value back to 0. That's the way the file has always been and that won't change the rc. Mike This message and any attached documents contain information from QLogic Corporation or its wholly-owned subsidiaries that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] IB/qib: detour pcie_caps for certain chip sets
On Thu, Jan 12, 2012 at 02:14:12PM -0800, Mike Marciniszyn wrote: > Actually, Jason is right. The vendor check you reference here is in > qib_tune_pcie_coalesce() and not the routine being patched. > > A bit of background here is that the issue was noted with the > indicated Harpertown root complex chip sets as follows: > - The BIOS set the root complex MaxPayLoad to 128, but rc capabilities > indicate 256 is possible > - To get the best performance we tried going to 256 on the rc and > our card and noted the Poisoned TLP I don't think it is appropriate for a driver to modify the pci configuration of the root complex.. What if other drivers also try and modify this configuration? Chaos. It doesn't seem to me like this has any place in the quirks thing either. Things seem to be working properly, the MaxPayLoad of 128 is clearly the highest the system will support correctly. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] IB/qib: detour pcie_caps for certain chip sets
> >if (parent->vendor != 0x8086) > return 1; > > so I guess you don't need another vendor check. Actually, Jason is right. The vendor check you reference here is in qib_tune_pcie_coalesce() and not the routine being patched. A bit of background here is that the issue was noted with the indicated Harpertown root complex chip sets as follows: - The BIOS set the root complex MaxPayLoad to 128, but rc capabilities indicate 256 is possible - To get the best performance we tried going to 256 on the rc and our card and noted the Poisoned TLP - The patch is an effort to avoid having to use set pcie_caps at all as well as avoiding issues with the problematic chip sets - The module parameter can still be used to experiment We have never the issue with AMD or other Intel chipsets. The problematic device ids are not in fixup.c in lib. I can reissue a v2 with: - the vendor check - define use when available We probably need to do something, since the current 3.2 rc has the above risk. Mike This message and any attached documents contain information from QLogic Corporation or its wholly-owned subsidiaries that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] IB/qib: detour pcie_caps for certain chip sets
On Thu, Jan 12, 2012 at 9:17 AM, Mike Marciniszyn wrote: >> Does this work on systems where the broken chipset might >> not be the immediate parent of the qib device (ie there are >> some PCIe switches in between)? > The code figures this out at the top of routine and returns, changing nothing. OIC. Also I see if (parent->vendor != 0x8086) return 1; so I guess you don't need another vendor check. Although this might be better written as PCI_VENDOR_ID_INTEL instead of 0x8086. I guess this is OK, although as Jason said it would be much better if the PCI core knew about these chipset errata. - R. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] IB/qib: detour pcie_caps for certain chip sets
On Thu, Jan 12, 2012 at 08:02:52AM -0800, Mike Marciniszyn wrote: > > Should whatever this issue is be a general PCI fixup? Like broken MSI, > > etc. > > > Can you point me to some details on this? I can explain the broken MSI stuff, as an example. As I noted I'm not sure what you are working around here, but if there are limits imposed on otherwise correct values in the PCI capabilities block then I think it is broadly applicable to handle this in core code... There are flags in pci.h like: PCI_BUS_FLAGS_NO_MSI = (__force pci_bus_flags_t) 1, Which are quirk things.. Look in drivers/pci/quirks.c to see how it is set. So broadly you'd make a new appropriate bus flag to control whatever you are working around and then test and set it in quirks, and provide core code to traverse the bus path from a device to ensure nothing in the path sets that quirk. Really depends what the problem actually is. Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] IB/qib: detour pcie_caps for certain chip sets
> Does this work on systems where the broken chipset might > not be the immediate parent of the qib device (ie there are > some PCIe switches in between)? > The code figures this out at the top of routine and returns, changing nothing. This message and any attached documents contain information from QLogic Corporation or its wholly-owned subsidiaries that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] IB/qib: detour pcie_caps for certain chip sets
> Should whatever this issue is be a general PCI fixup? Like broken MSI, > etc. > Can you point me to some details on this? > Might be nice to include what 0x51 tunes in the commit to aide other > peoole with the broken chipset :) > > Isn't it necesary to check the PCI vendor as well as the devid? > Will do both of these in a V2. Mike This message and any attached documents contain information from QLogic Corporation or its wholly-owned subsidiaries that may be confidential. If you are not the intended recipient, you may not read, copy, distribute, or use this information. If you have received this transmission in error, please notify the sender immediately by reply e-mail and then delete this message. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] IB/qib: detour pcie_caps for certain chip sets
On Wed, Jan 11, 2012 at 9:48 PM, Jason Gunthorpe wrote: > Isn't it necesary to check the PCI vendor as well as the devid? That definitely seems like a good idea. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] IB/qib: detour pcie_caps for certain chip sets
On Wed, Jan 11, 2012 at 7:00 PM, Mike Marciniszyn wrote: > + if (!qib_pcie_caps) { > + u16 devid = parent->device; > + if ((devid >= 0x25e2 && devid <= 0x25fa) || > + (devid >= 0x65e2 && devid <= 0x65fa) || > + (devid >= 0x4021 && devid <= 0x402e)) > + caps = 0; > + } else Does this work on systems where the broken chipset might not be the immediate parent of the qib device (ie there are some PCIe switches in between)? Pushing knowledge of completely unrelated devices out into every device driver looks it will be impossible to maintain. If you need to know about the whole PCIe fabric to do this type of tuning, the only way I can see to make this work is to add some helper functions to set these values for a given device, and then add all the workaround handling to the core PCI driver. - R. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] IB/qib: detour pcie_caps for certain chip sets
On Wed, Jan 11, 2012 at 10:00:49PM -0500, Mike Marciniszyn wrote: > Commit 8d4548f2b ("IB/qib: Default some module parameters optimally") > introduced an issue with older root complexes. They cannot handle > the pcie_caps of 0x51. Should whatever this issue is be a general PCI fixup? Like broken MSI, etc. Might be nice to include what 0x51 tunes in the commit to aide other peoole with the broken chipset :) Isn't it necesary to check the PCI vendor as well as the devid? Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html