On Wed, Jul 30, 2014 at 2:57 PM, Yijing Wang <wangyijing@xxxxxxxxxx> wrote: > On 2014/7/30 14:26, Ethan Zhao wrote: >> On Tue, Jul 29, 2014 at 4:17 PM, Yijing Wang <wangyijing@xxxxxxxxxx> wrote: >>> Currently we don't update device's mps value when doing >>> pci device hot-add. The hot-added device's mps will be set >>> to default value (128B). But the upstream port device's mps >>> may be larger than 128B which was set by firmware during >>> system bootup. In this case the new added device may not >>> work normally. This issue was found in huawei 5885 server >>> and Dell R620 server. And if we run the platform with windows, >>> this problem is gone. This patch try to update the hot added >>> device mps equal to its parent mps, if device mpss < parent mps, >>> print warning. >>> >>> References: https://bugzilla.kernel.org/show_bug.cgi?id=60671 >> >> http://marc.info/?l=e1000-devel&m=134182518500774&w=2 >> >> The bug was reported by Joe.jin@xxxxxxxxxx. the related hardware >> is SUN FIRE X2270 M2, it is not a hotplug sever except SAS HDD, >> and the the issue is definitely a BIOS bug. not related to the hotplug >> code path. was fixed by BIOS. >> > > Yes, that issue is BIOS bug, the mps setting is wrong after system boot up. > But that issue is not same as this one, Keith and Jordan found the issue > after hot-plug. And my patch only modify the hotplug slot connected device. > > In my idea, make the device work is important, because these platforms with windows > can run happy, why linux leave this issue to BIOS. That is a reason to make it works with Linux, but does your platform have _HPX from ACPI for those hot-added back devices ? if it has, maybe windows could apply _HPX to configure those devices and work well. Maybe you should check with _HPX . That is a generic way to import configuration from firmware when hot-add device. Please see also chapter 6.2.8 _HPX (Hot Plug Parameter Extensions) of ACPI spec 4&5 Thanks, Ethan > > Anyway, this is my personal opinion. > > Thanks! > Yijing. > >> >>> Reported-by: Keith Busch <keith.busch@xxxxxxxxx> >>> Reported-by: Jordan_Hargrave@xxxxxxxx >>> Reported-by: Yijing Wang <wangyijing@xxxxxxxxxx> >>> Signed-off-by: Yijing Wang <wangyijing@xxxxxxxxxx> >>> Cc: Jon Mason <jdmason@xxxxxxxx> >>> --- >>> drivers/pci/probe.c | 39 +++++++++++++++++++++++++++++++++++++++ >>> 1 files changed, 39 insertions(+), 0 deletions(-) >>> >>> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c >>> index e3cf8a2..583ca52 100644 >>> --- a/drivers/pci/probe.c >>> +++ b/drivers/pci/probe.c >>> @@ -1613,6 +1613,44 @@ static void pcie_write_mrrs(struct pci_dev *dev) >>> dev_err(&dev->dev, "MRRS was unable to be configured with a safe value. If problems are experienced, try running with pci=pcie_bus_safe\n"); >>> } >>> >>> +/** >>> + * pcie_bus_update_set - update device mps when device doing hot-add >>> + * @dev: PCI device to set >>> + * >>> + * After device hot add, mps will be set to default(128B), But the >>> + * upstream port device's mps may be larger than 128B which was set >>> + * by firmware during system bootup. Then we should update the device >>> + * mps to equal to its parent mps, Or the device can not work normally. >>> + */ >>> +static void pcie_bus_update_set(struct pci_dev *dev) >>> +{ >>> + int mps, p_mps, mpss; >>> + struct pci_dev *parent; >>> + >>> + if (!pci_is_pcie(dev) || !dev->bus->self >>> + || !dev->bus->self->is_hotplug_bridge) >>> + return; >>> + >>> + parent = dev->bus->self; >>> + mps = pcie_get_mps(dev); >>> + p_mps = pcie_get_mps(parent); >>> + >>> + if (mps >= p_mps) >>> + return; >>> + >>> + mpss = 128 << dev->pcie_mpss; >>> + if (mpss < p_mps) { >>> + dev_warn(&dev->dev, "MPSS %d smaller than upstream MPS %d\n" >>> + "If necessary, use \"pci=pcie_bus_safe\" boot parameter to avoid this problem\n", >>> + mpss, p_mps); >>> + return; >>> + } >>> + >>> + pcie_write_mps(dev, p_mps); >>> + dev_info(&dev->dev, "Max Payload Size set to %4d/%4d (was %4d)\n", >>> + pcie_get_mps(dev), 128 << dev->pcie_mpss, mps); >>> +} >>> + >>> static void pcie_bus_detect_mps(struct pci_dev *dev) >>> { >>> struct pci_dev *bridge = dev->bus->self; >>> @@ -1637,6 +1675,7 @@ static int pcie_bus_configure_set(struct pci_dev *dev, void *data) >>> return 0; >>> >>> if (pcie_bus_config == PCIE_BUS_TUNE_OFF) { >>> + pcie_bus_update_set(dev); >>> pcie_bus_detect_mps(dev); >>> return 0; >>> } >>> -- >>> 1.7.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 >> >> . >> > > > -- > Thanks! > Yijing > -- 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
