Ilpo Järvinen wrote: > When bifurcated to x2, Xeon 6 Root Port performance is sensitive to the > configuration of Extended Tags, Max Read Request Size (MRRS), and 10-Bit > Tag Requester (note: there is currently no 10-Bit Tag support in the > kernel). While those can be configured to the recommended values by FW, > kernel may decide to overwrite the initial values. > > Add a quirk that disallows enabling Extended Tags and setting MRRS > larger than 128B for devices under Xeon 6 Root Ports if the Root Port > is bifurcated to x2. Use the host bridge's enable_device hook to > overwrite MRRS if it's set to >128B for the device to be enabled. > > The earlier attempts to implement this quirk polluted PCI core code > with the checks necessary to support this quirk. Using the > enable_device hook keeps the quirk well-contained, away from the PCI > core code. > > Suggested-by: Lukas Wunner <lukas@xxxxxxxxx> > Link: https://cdrdv2.intel.com/v1/dl/getContent/837176 > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx> > --- > > Ingo gave his Ack on v2 but since the used approach has once again > changed, I didn't add his Ack. > > Mani did express his concern on using enable_device hook but suggested > I send the patch anyway. > > We're also looking into using _HPX Type 3 (suggested by Bjorn) eventually > for this class of problems where FW settings get overwritten by the > kernel (but it will take time to make it the sanctioned solution). In > the meantime, this is a real problem for Xeon 6 out there so it warrants > adding the quirk (which is now pretty well-contained). > > v3: > - Use enable_device hook to overwrite MRRS to 128B if needed. (Lukas) > - Typo fix to comment (Ingo) > > v2: > - Explain in changelog why FW cannot solve this on its own > - Moved the quirk under arch/x86/pci/ > - Don't NULL check value from pci_find_host_bridge() > - Added comment above the quirk about the performance degradation > - Removed all setup chain 128B quirk overrides expect for MRRS write > itself (assumes a sane initial value is set by FW) > > > arch/x86/pci/fixup.c | 40 ++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 40 insertions(+) Thanks for the work on this Ilpo, and thanks for the cleanup suggestions Lukas. This version looks reasonable to carry the kernel until _HPX can be relied upon to help the kernel apply quirks like this. You can add: Reviewed-by: Dan Williams <dan.j.williams@xxxxxxxxx>
