From: Yijing Wang <wangyijing@huawei.com>
To: Ethan Zhao <ethan.kernel@gmail.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
linux-pci <linux-pci@vger.kernel.org>,
"<Jordan_Hargrave@dell.com>" <Jordan_Hargrave@dell.com>,
"<keith.busch@intel.com>" <keith.busch@intel.com>,
"<jon.mason@intel.com>" <jon.mason@intel.com>,
"Jon Mason" <jdmason@kudzu.us>
Subject: Re: [PATCH] PCI: update device mps when doing pci hotplug
Date: Wed, 30 Jul 2014 12:42:01 +0800 [thread overview]
Message-ID: <53D87799.6030900@huawei.com> (raw)
In-Reply-To: <76A89A35-D0BA-4E83-97BA-3FF490EF0877@gmail.com>
On 2014/7/30 11:58, Ethan Zhao wrote:
>
>
>> 在 2014年7月30日,上午11:42,Yijing Wang <wangyijing@huawei.com> 写道:
>>
>>> On 2014/7/30 11:33, Ethan Zhao wrote:
>>> Yijing,
>>> Seems you want to workaround platform bug in generic PCIe hotplug
>>> code, while not specific to some platforms or devices, that is not
>>> safe/fair to other vendor's platform.
>>> and the updating to MPS of device is out of PCIe specification.
>>> So the best way is to fix within platform, at least, any platform
>>> specific way in Linux code.
>>
>> No, this is not a platform bug, and this is safe, you can refer to my last reply
>> to Alex.
> if is not a platform bug, why Didn't other platforms that can do hotplug report such issue ?
> If it was safe, is it possible that you have all other vendors verify it ?
This issue was triggered by firmware set the device mps to 256B. It's not related to platform
hardware.
I think this is safe, because I only modified the device newly hot-added, this device is not enabled yet.
Thanks!
Yijing.
>>>
>>>> On Tue, Jul 29, 2014 at 4:17 PM, Yijing Wang <wangyijing@huawei.com> 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
>>>> Reported-by: Keith Busch <keith.busch@intel.com>
>>>> Reported-by: Jordan_Hargrave@Dell.com
>>>> Reported-by: Yijing Wang <wangyijing@huawei.com>
>>>> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
>>>> Cc: Jon Mason <jdmason@kudzu.us>
>>>> ---
>>>> 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@vger.kernel.org
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>
>>> .
>>
>>
>> --
>> Thanks!
>> Yijing
>>
>
> .
>
--
Thanks!
Yijing
next prev parent reply other threads:[~2014-07-30 4:42 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-29 8:17 [PATCH] PCI: update device mps when doing pci hotplug Yijing Wang
2014-07-29 16:18 ` Alex Williamson
2014-07-29 16:30 ` Keith Busch
2014-07-29 16:42 ` Alex Williamson
2014-07-29 19:04 ` Keith Busch
2014-07-30 3:35 ` Yijing Wang
2014-07-30 3:27 ` Yijing Wang
2014-07-30 3:33 ` Ethan Zhao
2014-07-30 3:42 ` Yijing Wang
2014-07-30 3:58 ` Ethan Zhao
2014-07-30 4:42 ` Yijing Wang [this message]
2014-07-30 6:26 ` Ethan Zhao
2014-07-30 6:57 ` Yijing Wang
2014-07-30 7:17 ` Ethan Zhao
2014-07-30 8:13 ` Yijing Wang
2014-07-30 8:38 ` Ethan Zhao
2014-07-30 9:17 ` Yijing Wang
2014-07-30 19:41 ` Jordan_Hargrave
2014-09-03 19:20 ` Bjorn Helgaas
2014-09-03 22:42 ` Bjorn Helgaas
2014-09-04 6:12 ` Yijing Wang
2014-09-04 13:16 ` Bjorn Helgaas
2014-09-05 1:27 ` Yijing Wang
2014-09-05 14:37 ` Keith Busch
2014-09-24 22:41 ` Keith Busch
2014-09-24 23:30 ` Bjorn Helgaas
2014-09-25 1:23 ` Yijing Wang
2014-09-25 16:46 ` Keith Busch
2014-09-26 3:22 ` Yijing Wang
2014-10-02 15:31 ` Jordan_Hargrave
-- strict thread matches above, loose matches on Subject: below --
2014-07-29 8:23 Yijing Wang
2013-02-05 3:55 Yijing Wang
2013-05-28 3:15 ` Yijing Wang
2013-07-29 23:33 ` Bjorn Helgaas
2013-07-30 3:20 ` Yijing Wang
2013-07-30 3:42 ` Bjorn Helgaas
2013-07-30 22:29 ` Bjorn Helgaas
2013-07-31 9:15 ` Yijing Wang
2013-07-31 17:53 ` Bjorn Helgaas
2013-07-31 20:42 ` Bjorn Helgaas
2013-08-01 1:23 ` Yijing Wang
2013-08-01 1:21 ` Yijing Wang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=53D87799.6030900@huawei.com \
--to=wangyijing@huawei.com \
--cc=Jordan_Hargrave@dell.com \
--cc=bhelgaas@google.com \
--cc=ethan.kernel@gmail.com \
--cc=jdmason@kudzu.us \
--cc=jon.mason@intel.com \
--cc=keith.busch@intel.com \
--cc=linux-pci@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).