On Sat, May 28, 2011 at 3:09 AM, Jon Mason <jon.mason@xxxxxxxx> wrote:
> This is required to work around buggy BIOS revisions found on various
> whitebox motherboards which do not configure mps beyond one level below
> the root port. Ensure that the max payload size of children bridges and
> devices are the same as the parent bus.
>
> Signed-off-by: Jon Mason <mason@xxxxxxxx>
> ---
> drivers/pci/bus.c | 63 ++++++++++++++++++++++++++++++++++++++
> drivers/pci/hotplug/pcihp_slot.c | 45 ++-------------------------
> 2 files changed, 66 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c
> index 1e2ad92..7dd3d24 100644
> --- a/drivers/pci/bus.c
> +++ b/drivers/pci/bus.c
> @@ -169,6 +169,59 @@ int pci_bus_add_child(struct pci_bus *bus)
> return retval;
> }
>
> +/* Program PCIE MaxPayload setting on device:
> + * ensure parent maxpayload <= device
> + */
> +int pci_set_payload(struct pci_dev *dev, bool hotplug)
> +{
> + int pos, ppos;
> + u16 pctl, psz;
> + u16 dctl, dsz, dcap, dmax;
> + struct pci_dev *parent;
> +
> + parent = dev->bus->self;
> + if (!parent)
> + return 0;
> +
> + 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
Consider a Situation : RC->EP
RC's Max_Payload_Size is 256 bytes.
EP's Max_Payload_Size Supported is only 128 bytes.
Should this system not work? It must .
So the soultion should be "to align Max_Payload_Size for
all devices to the minimum of the downstream tree"
> + * If parent payload > device payload -> set speed
> + * If parent payload <= device payload -> do nothing
> + */
> + if (psz > dmax)
> + return -1;
> + else if (psz > dsz) {
> + if (hotplug)
> + dev_info(&dev->dev, "Setting MaxPayload to %d\n",
> + 128 << psz);
> + else
> + dev_warn(&dev->dev, "Child MPS of %d != Parent MPS of "
> + "%d! Most likely caused by bad BIOS. Setting"
> + " Child MPS to %d\n", 128 << dsz, 128 << psz,
> + 128 << psz);
> +
> + pci_write_config_word(dev, pos + PCI_EXP_DEVCTL, (psz << 5) |
> + (dctl & ~PCI_EXP_DEVCTL_PAYLOAD));
> + }
> + return 0;
> +}
> +
> /**
> * pci_bus_add_devices - insert newly discovered PCI devices
> * @bus: bus to check for new devices
> @@ -188,6 +241,16 @@ void pci_bus_add_devices(const struct pci_bus *bus)
> int retval;
>
> list_for_each_entry(dev, &bus->devices, bus_list) {
> +
> + /*
> + * This is required to work around buggy BIOS revisions found on
> + * various whitebox motherboards which do not configure mps
> + * beyond one level below the root port. Ensure that the max
> + * payload size of children bridges and devices are the same as
> + * the parent bus.
> + */
We should also consider about Max_Read_Request_Size.
Max_Read_Request_Size of any of the down stream device
should be less than or equal to Device's Max_Payload_Size.
I mean, if an EP with Max_Read_Request_Size 256 bytes generates an
outbound request to an RC with Max_Payload_Size 128 bytes, then RC will
not be able to respond. In this scenerio, EP's Max_Read_Request_Size should be
set to 128.
Regards
Pratyush
> + pci_set_payload(dev, false);
> +
> /* Skip already-added devices */
> if (dev->is_added)
> continue;
> diff --git a/drivers/pci/hotplug/pcihp_slot.c b/drivers/pci/hotplug/pcihp_slot.c
> index 749fdf0..0038c40 100644
> --- a/drivers/pci/hotplug/pcihp_slot.c
> +++ b/drivers/pci/hotplug/pcihp_slot.c
> @@ -26,6 +26,8 @@
> #include <linux/pci.h>
> #include <linux/pci_hotplug.h>
>
> +extern int pci_set_payload(struct pci_dev *dev, bool hotplug);
> +
> static struct hpp_type0 pci_default_type0 = {
> .revision = 1,
> .cache_line_size = 8,
> @@ -158,47 +160,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,7 +171,7 @@ void pci_configure_slot(struct pci_dev *dev)
> (dev->class >> 8) == PCI_CLASS_BRIDGE_PCI)))
> return;
>
> - ret = pci_set_payload(dev);
> + ret = pci_set_payload(dev, true);
> if (ret)
> dev_warn(&dev->dev, "could not set device max payload\n");
>
> --
> 1.7.4.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
