From patchwork Fri Jun 24 03:54:53 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jon Mason X-Patchwork-Id: 915272 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter2.kernel.org (8.14.4/8.14.4) with ESMTP id p5O3sxQJ005958 for ; Fri, 24 Jun 2011 03:54:59 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751971Ab1FXDy6 (ORCPT ); Thu, 23 Jun 2011 23:54:58 -0400 Received: from mail-pw0-f46.google.com ([209.85.160.46]:40366 "EHLO mail-pw0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751603Ab1FXDy6 (ORCPT ); Thu, 23 Jun 2011 23:54:58 -0400 Received: by pwj7 with SMTP id 7so1589726pwj.19 for ; Thu, 23 Jun 2011 20:54:57 -0700 (PDT) Received: by 10.142.250.42 with SMTP id x42mr221594wfh.412.1308887697175; Thu, 23 Jun 2011 20:54:57 -0700 (PDT) Received: from myri.com (cpe-70-113-48-102.austin.res.rr.com [70.113.48.102]) by mx.google.com with ESMTPS id w24sm1622483wfd.17.2011.06.23.20.54.55 (version=SSLv3 cipher=OTHER); Thu, 23 Jun 2011 20:54:56 -0700 (PDT) Date: Thu, 23 Jun 2011 22:54:53 -0500 From: Jon Mason To: Jesse Barnes Cc: linux-pci@vger.kernel.org, Benjamin Herrenschmidt , Pratyush Anand , Jordan_Hargrave@dell.com, Andrew Gallatin Subject: [PATCH v5 RESEND] PCI: Set PCI-E Max Payload Size on fabric Message-ID: <20110624035452.GA12806@myri.com> MIME-Version: 1.0 Content-Disposition: inline User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.6 (demeter2.kernel.org [140.211.167.43]); Fri, 24 Jun 2011 03:54:59 +0000 (UTC) There is a sizable performance boost for having the largest possible maximum payload size on each PCI-E device. However, the maximum payload size must be uniform on a given PCI-E fabric, and each device, bridge, and root port can have a different max size. To find and configure the optimal MPS settings, one must walk the fabric and determine the largest MPS available on all the devices on the given fabric. However, on some architectures (notably PPC) it is known that the downstream communication will never be larger than 128B. With this known, the MPS can be configured to be the largest value supported by the PCI-E switches and root port. In most cases this means that the devices on the fabric are not limited to the lowest common denominator. Use with caution, if this is not 100% true then there is the possibility of a bus error. To choose between the two available options, two kernel boot args have been added to the PCI calls. "mpsmin" will provide the former behavior, while "mpsmax" will perform the latter behavior. By default, the former behavior is used. Along with the potential performance improvement, this works around an issue with buggy BIOS revisions found on various whitebox motherboards which do not configure MPS beyond one level below the root port. There is one issue with the "mpsmin" design. For PCI-E hotplug enabled slots not connected directly to a PCI-E root port, there can be problems when hotplugging devices. This is due to the possibility of hotplugging a device into the fabric with a smaller MPS that the devices currently running have configured. Modifying the MPS on the running devices could cause a fatal bus error due to an incoming frame being larger than the newly configured MPS. To work around this, the MPS for the entire fabric must be set to the minimum size. Any devices hotplugged into this fabric will have the minimum MPS set. If the PCI hotplug slot is directly connected to the root port and there are not other devices on the fabric (which seems to be the most common case), then this is not an issue and MPS discovery will occur as normal. Signed-off-by: Jon Mason --- drivers/pci/bus.c | 17 ++++++- drivers/pci/hotplug/pcihp_slot.c | 58 +++++---------------- drivers/pci/pci.c | 6 ++ drivers/pci/probe.c | 102 ++++++++++++++++++++++++++++++++++++++ include/linux/pci.h | 5 ++- 5 files changed, 141 insertions(+), 47 deletions(-) diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c index 1e2ad92..1daeb22 100644 --- a/drivers/pci/bus.c +++ b/drivers/pci/bus.c @@ -256,8 +256,8 @@ void pci_enable_bridges(struct pci_bus *bus) * other than 0, we break out. * */ -void pci_walk_bus(struct pci_bus *top, int (*cb)(struct pci_dev *, void *), - void *userdata) +void pci_walk_bus_self(struct pci_bus *top, int (*cb)(struct pci_dev *, void *), + void *userdata, bool self) { struct pci_dev *dev; struct pci_bus *bus; @@ -276,6 +276,13 @@ void pci_walk_bus(struct pci_bus *top, int (*cb)(struct pci_dev *, void *), bus = bus->self->bus; continue; } + + if (self) { + retval = cb(bus->self, userdata); + if (retval) + break; + } + dev = list_entry(next, struct pci_dev, bus_list); if (dev->subordinate) { /* this is a pci-pci bridge, do its devices next */ @@ -293,6 +300,12 @@ void pci_walk_bus(struct pci_bus *top, int (*cb)(struct pci_dev *, void *), } up_read(&pci_bus_sem); } + +void pci_walk_bus(struct pci_bus *top, int (*cb)(struct pci_dev *, void *), + void *userdata) +{ + pci_walk_bus_self(top, cb, userdata, false); +} EXPORT_SYMBOL_GPL(pci_walk_bus); EXPORT_SYMBOL(pci_bus_alloc_resource); diff --git a/drivers/pci/hotplug/pcihp_slot.c b/drivers/pci/hotplug/pcihp_slot.c index 749fdf0..ee4ff14 100644 --- a/drivers/pci/hotplug/pcihp_slot.c +++ b/drivers/pci/hotplug/pcihp_slot.c @@ -26,6 +26,19 @@ #include #include +extern void pcie_set_maxpayloadsize(struct pci_bus *bus); + +static void pcie_determine_mps(struct pci_dev *dev) +{ + struct pci_bus *bus = dev->bus; + + while (bus->self->pcie_type != PCI_EXP_TYPE_ROOT_PORT && + !pci_is_root_bus(bus)) + bus = bus->parent; + + pcie_set_maxpayloadsize(bus); +} + static struct hpp_type0 pci_default_type0 = { .revision = 1, .cache_line_size = 8, @@ -158,47 +171,6 @@ static void program_hpp_type2(struct pci_dev *dev, struct hpp_type2 *hpp) */ } -/* Program PCIE MaxPayload setting on device: ensure parent maxpayload <= device */ -static int pci_set_payload(struct pci_dev *dev) -{ - int pos, ppos; - u16 pctl, psz; - u16 dctl, dsz, dcap, dmax; - struct pci_dev *parent; - - parent = dev->bus->self; - pos = pci_find_capability(dev, PCI_CAP_ID_EXP); - if (!pos) - return 0; - - /* Read Device MaxPayload capability and setting */ - pci_read_config_word(dev, pos + PCI_EXP_DEVCTL, &dctl); - pci_read_config_word(dev, pos + PCI_EXP_DEVCAP, &dcap); - dsz = (dctl & PCI_EXP_DEVCTL_PAYLOAD) >> 5; - dmax = (dcap & PCI_EXP_DEVCAP_PAYLOAD); - - /* Read Parent MaxPayload setting */ - ppos = pci_find_capability(parent, PCI_CAP_ID_EXP); - if (!ppos) - return 0; - pci_read_config_word(parent, ppos + PCI_EXP_DEVCTL, &pctl); - psz = (pctl & PCI_EXP_DEVCTL_PAYLOAD) >> 5; - - /* If parent payload > device max payload -> error - * If parent payload > device payload -> set speed - * If parent payload <= device payload -> do nothing - */ - if (psz > dmax) - return -1; - else if (psz > dsz) { - dev_info(&dev->dev, "Setting MaxPayload to %d\n", 128 << psz); - pci_write_config_word(dev, pos + PCI_EXP_DEVCTL, - (dctl & ~PCI_EXP_DEVCTL_PAYLOAD) + - (psz << 5)); - } - return 0; -} - void pci_configure_slot(struct pci_dev *dev) { struct pci_dev *cdev; @@ -210,9 +182,7 @@ void pci_configure_slot(struct pci_dev *dev) (dev->class >> 8) == PCI_CLASS_BRIDGE_PCI))) return; - ret = pci_set_payload(dev); - if (ret) - dev_warn(&dev->dev, "could not set device max payload\n"); + pcie_determine_mps(dev); memset(&hpp, 0, sizeof(hpp)); ret = pci_get_hp_params(dev, &hpp); diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 56098b3..bcc5d31 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -77,6 +77,8 @@ unsigned long pci_cardbus_mem_size = DEFAULT_CARDBUS_MEM_SIZE; unsigned long pci_hotplug_io_size = DEFAULT_HOTPLUG_IO_SIZE; unsigned long pci_hotplug_mem_size = DEFAULT_HOTPLUG_MEM_SIZE; +bool pcie_mps_forcemax = false; + /* * The default CLS is used if arch didn't set CLS explicitly and not * all pci devices agree on the same value. Arch can override either @@ -3498,6 +3500,10 @@ static int __init pci_setup(char *str) pci_hotplug_io_size = memparse(str + 9, &str); } else if (!strncmp(str, "hpmemsize=", 10)) { pci_hotplug_mem_size = memparse(str + 10, &str); + } else if (!strncmp(str, "mpsmin", 6)) { + pcie_mps_forcemax = false; + } else if (!strncmp(str, "mpsmax", 6)) { + pcie_mps_forcemax = true; } else { printk(KERN_ERR "PCI: Unknown option `%s'\n", str); diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c index 48849ff..f6bbab1 100644 --- a/drivers/pci/probe.c +++ b/drivers/pci/probe.c @@ -860,6 +860,8 @@ void set_pcie_port_type(struct pci_dev *pdev) pdev->pcie_cap = pos; pci_read_config_word(pdev, pos + PCI_EXP_FLAGS, ®16); pdev->pcie_type = (reg16 & PCI_EXP_FLAGS_TYPE) >> 4; + pci_read_config_word(pdev, pos + PCI_EXP_DEVCAP, ®16); + pdev->pcie_mpss = reg16 & PCI_EXP_DEVCAP_PAYLOAD; } void set_pcie_hotplug_bridge(struct pci_dev *pdev) @@ -1327,6 +1329,101 @@ int pci_scan_slot(struct pci_bus *bus, int devfn) return nr; } +static int pcie_find_smpss(struct pci_dev *dev, void *data) +{ + u8 *smpss = data; + + if (!pci_is_pcie(dev)) + return 0; + + /* For PCIE hotplug enabled slots not connected directly to a + * PCI-E root port, there can be problems when hotplugging + * devices. This is due to the possibility of hotplugging a + * device into the fabric with a smaller MPS that the devices + * currently running have configured. Modifying the MPS on the + * running devices could cause a fatal bus error due to an + * incoming frame being larger than the newly configured MPS. + * To work around this, the MPS for the entire fabric must be + * set to the minimum size. Any devices hotpulled into this + * fabric will have the minimum MPS set. If the PCI hotplug + * slot is directly connected to the root port and there are not + * other devices on the fabric (which seems to be the most + * common case), then this is not an issue and MPS discovery + * will occur as normal. + */ + if (dev->is_hotplug_bridge && dev->pcie_type != PCI_EXP_TYPE_ROOT_PORT) + *smpss = 0; + + if (*smpss > dev->pcie_mpss) + *smpss = dev->pcie_mpss; + + return 0; +} + +static int pcie_write_mps(struct pci_dev *dev, void *data) +{ + u16 dctl, mps; + u8 smpss = *(u8 *)data; + + if (!pci_is_pcie(dev)) + return 0; + + if (pcie_mps_forcemax && dev->bus->self) { + dev_dbg(&dev->bus->dev, "Bus MPSS %x\n", + dev->bus->self->pcie_mpss); + /* For "MPS Force Max", the assumption is made that downstream + * communication will never be larger than 128 bytes. So, the + * MPS only needs to be configured for the upstream + * communication. This being the case, walk from the top down + * and set the MPS of the child to that of the parent bus. + */ + smpss = dev->bus->self->pcie_mpss; + if (dev->pcie_mpss < smpss) + dev_warn(&dev->dev, "MPS configured higher than Max " + "supported. This can cause a bus issue.\n"); + } + + pci_read_config_word(dev, dev->pcie_cap + PCI_EXP_DEVCTL, &dctl); + mps = (dctl & PCI_EXP_DEVCTL_PAYLOAD) >> 5; + + dev_dbg(&dev->dev, "Dev MPS %x MPSS %x\n", mps, dev->pcie_mpss); + + if (smpss != mps) { + dev_info(&dev->dev, "Setting MaxPayload to %d\n", 128 << smpss); + + pci_write_config_word(dev, dev->pcie_cap + PCI_EXP_DEVCTL, + (dctl & ~PCI_EXP_DEVCTL_PAYLOAD) | + (smpss << 5)); + } + + dev->pcie_mpss = smpss; + return 0; +} + +extern void pci_walk_bus_self(struct pci_bus *top, int (*cb)(struct pci_dev *, void *), + void *userdata, bool self); + +void pcie_set_maxpayloadsize(struct pci_bus *bus) +{ + u8 smpss; + + if (!bus->self) + return; + + if (!pci_is_pcie(bus->self)) + return; + + if (bus->self->pcie_type != PCI_EXP_TYPE_ROOT_PORT) + return; + + smpss = bus->self->pcie_mpss; + + if (!pcie_mps_forcemax) + pci_walk_bus_self(bus, pcie_find_smpss, &smpss, true); + + pci_walk_bus_self(bus, pcie_write_mps, &smpss, true); +} + unsigned int __devinit pci_scan_child_bus(struct pci_bus *bus) { unsigned int devfn, pass, max = bus->secondary; @@ -1359,6 +1456,11 @@ unsigned int __devinit pci_scan_child_bus(struct pci_bus *bus) max = pci_scan_bridge(bus, dev, max, pass); } + /* After the bus has been walked and all devices discovered, determine + * the MPS of the fabric and set it. + */ + pcie_set_maxpayloadsize(bus); + /* * We've scanned the bus and so we know all about what's on * the other side of any bridges that may be on this bus plus diff --git a/include/linux/pci.h b/include/linux/pci.h index c446b5c..2a60e635 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -251,7 +251,8 @@ struct pci_dev { u8 revision; /* PCI revision, low byte of class word */ u8 hdr_type; /* PCI header type (`multi' flag masked out) */ u8 pcie_cap; /* PCI-E capability offset */ - u8 pcie_type; /* PCI-E device/port type */ + u8 pcie_type:4; /* PCI-E device/port type */ + u8 pcie_mpss:3; /* PCI-E Max Payload Size Supported */ u8 rom_base_reg; /* which config register controls the ROM */ u8 pin; /* which interrupt pin this device uses */ @@ -617,6 +618,8 @@ struct pci_driver { /* these external functions are only available when PCI support is enabled */ #ifdef CONFIG_PCI +extern bool pcie_mps_forcemax; + extern struct bus_type pci_bus_type; /* Do NOT directly access these two variables, unless you are arch specific pci