[PATCH V2] PCI: enable extended tags support for PCIe devices

[PATCH V2] PCI: enable extended tags support for PCIe devices

[Sinan Kaya]

Each PCIe device can issue up to 32 transactions at a time by default.
This limitation is historically coming from PCI. Each transaction is
tracked by a tag number on the bus. 2.2.6.2. Transaction Descriptor
– Transaction ID Field section of the PCIe 3.1 specification describes
extended tags.

PCI supports 32 outstanding non-posted requests at a given time. This
number has been extended to 256 on PCI Express. According to the
specification, all PCIe devices are required to support receiving
8-bit Tags (Tag completer). The PCIe-PCI bridges handle the translation
of 8-bit tags to 5-bit tags.

However, the generation of 8-bit tags is left optional to a particular HW
implementation. The code needs to check HW support before attempting
to enable extended tags producer capability.

32 outstanding transactions is not enough for some performance critical
applications especially when a lot of small sized frames are transmitted.

Extended tags support increases this number to 256. Devices not
supporting extended tags tie-off this field to 0. According to ECN, it
is safe to enable this feature for all PCIe devices.

Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
 drivers/pci/probe.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index e164b5c..1192475 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1650,12 +1650,28 @@ static void program_hpp_type2(struct pci_dev *dev, struct hpp_type2 *hpp)
 	 */
 }
 
+static void pci_configure_extended_tags(struct pci_dev *pdev)
+{
+	u32 dev_cap;
+	int ret;
+
+	ret = pcie_capability_read_dword(pdev, PCI_EXP_DEVCAP, &dev_cap);
+
+	if (ret)
+		return;
+
+	if (dev_cap & PCI_EXP_DEVCAP_EXT_TAG)
+		pcie_capability_set_word(pdev, PCI_EXP_DEVCTL,
+					 PCI_EXP_DEVCTL_EXT_TAG);
+}
+
 static void pci_configure_device(struct pci_dev *dev)
 {
 	struct hotplug_params hpp;
 	int ret;
 
 	pci_configure_mps(dev);
+	pci_configure_extended_tags(dev);
 
 	memset(&hpp, 0, sizeof(hpp));
 	ret = pci_get_hp_params(dev, &hpp);
-- 
1.9.1

Re: [PATCH V2] PCI: enable extended tags support for PCIe devices

[Sinan Kaya]

On 1/20/2017 9:16 AM, Sinan Kaya wrote:
> Each PCIe device can issue up to 32 transactions at a time by default.
> This limitation is historically coming from PCI. Each transaction is
> tracked by a tag number on the bus. 2.2.6.2. Transaction Descriptor
> – Transaction ID Field section of the PCIe 3.1 specification describes
> extended tags.
> 

Apologies for the spam, no idea what happened. I need to review my publish scripts.

Re: [PATCH V2] PCI: enable extended tags support for PCIe devices

[Bjorn Helgaas]

On Fri, Jan 20, 2017 at 09:16:51AM -0500, Sinan Kaya wrote:
> Each PCIe device can issue up to 32 transactions at a time by default.
> This limitation is historically coming from PCI. Each transaction is
> tracked by a tag number on the bus. 2.2.6.2. Transaction Descriptor
> – Transaction ID Field section of the PCIe 3.1 specification describes
> extended tags.
> 
> PCI supports 32 outstanding non-posted requests at a given time. This
> number has been extended to 256 on PCI Express. According to the
> specification, all PCIe devices are required to support receiving
> 8-bit Tags (Tag completer). The PCIe-PCI bridges handle the translation
> of 8-bit tags to 5-bit tags.
> 
> However, the generation of 8-bit tags is left optional to a particular HW
> implementation. The code needs to check HW support before attempting
> to enable extended tags producer capability.
> 
> 32 outstanding transactions is not enough for some performance critical
> applications especially when a lot of small sized frames are transmitted.
> 
> Extended tags support increases this number to 256. Devices not
> supporting extended tags tie-off this field to 0. According to ECN, it
> is safe to enable this feature for all PCIe devices.
> 
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>

Applied to pci/enumeration for v4.11 as follows, thanks!

I added a check for PCIe, so we don't have to rely on
pcie_capability_read_dword() doing the right thing for PCI and PCI-X
devices.  I think pcie_capability_read_dword() would set dev_cap to zero
and return zero, which would be safe, but I don't the reader should have to
do that much analysis to figure it out.

commit 8de9dcb0d1e0c92c525a826dd69ce61a21264050
Author: Sinan Kaya <okaya@codeaurora.org>
Date:   Fri Jan 20 09:16:51 2017 -0500

    PCI: Enable PCIe Extended Tags if supported
    
    Every PCIe device can generate 5-bit transaction Tags, which allow up to 32
    concurrent requests.  Some devices can generate 8-bit Extended Tags, which
    allow up to 256 concurrent requests.
    
    Per the ECN mentioned below, all PCIe Receivers are expected to support
    Extended Tags, so devices are allowed (but not required) to enable them by
    default.
    
    If a device supports Extended Tags but does not enable them by default,
    enable them.  This allows the device to have up to 256 outstanding
    transactions at a time, which may improve performance.
    
    [bhelgaas: changelog, check for PCIe device]
    Link: https://pcisig.com/sites/default/files/specification_documents/ECN_Extended_Tag_Enable_Default_05Sept2008_final.pdf
    Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
    Signed-off-by: Bjorn Helgaas <helgaas@kernel.org>

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index aca5b2466adb..3abc94212197 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1661,12 +1661,30 @@ static void program_hpp_type2(struct pci_dev *dev, struct hpp_type2 *hpp)
 	 */
 }
 
+static void pci_configure_extended_tags(struct pci_dev *dev)
+{
+	u32 dev_cap;
+	int ret;
+
+	if (!pci_is_pcie(dev))
+		return;
+
+	ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &dev_cap);
+	if (ret)
+		return;
+
+	if (dev_cap & PCI_EXP_DEVCAP_EXT_TAG)
+		pcie_capability_set_word(dev, PCI_EXP_DEVCTL,
+					 PCI_EXP_DEVCTL_EXT_TAG);
+}
+
 static void pci_configure_device(struct pci_dev *dev)
 {
 	struct hotplug_params hpp;
 	int ret;
 
 	pci_configure_mps(dev);
+	pci_configure_extended_tags(dev);
 
 	memset(&hpp, 0, sizeof(hpp));
 	ret = pci_get_hp_params(dev, &hpp);

> ---
>  drivers/pci/probe.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index e164b5c..1192475 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1650,12 +1650,28 @@ static void program_hpp_type2(struct pci_dev *dev, struct hpp_type2 *hpp)
>  	 */
>  }
>  
> +static void pci_configure_extended_tags(struct pci_dev *pdev)
> +{
> +	u32 dev_cap;
> +	int ret;
> +
> +	ret = pcie_capability_read_dword(pdev, PCI_EXP_DEVCAP, &dev_cap);
> +
> +	if (ret)
> +		return;
> +
> +	if (dev_cap & PCI_EXP_DEVCAP_EXT_TAG)
> +		pcie_capability_set_word(pdev, PCI_EXP_DEVCTL,
> +					 PCI_EXP_DEVCTL_EXT_TAG);
> +}
> +
>  static void pci_configure_device(struct pci_dev *dev)
>  {
>  	struct hotplug_params hpp;
>  	int ret;
>  
>  	pci_configure_mps(dev);
> +	pci_configure_extended_tags(dev);
>  
>  	memset(&hpp, 0, sizeof(hpp));
>  	ret = pci_get_hp_params(dev, &hpp);
> -- 
> 1.9.1
> 

Re: [PATCH V2] PCI: enable extended tags support for PCIe devices

[Sinan Kaya]

On 2/9/2017 6:33 PM, Bjorn Helgaas wrote:
> Applied to pci/enumeration for v4.11 as follows, thanks!
> 
> I added a check for PCIe, so we don't have to rely on
> pcie_capability_read_dword() doing the right thing for PCI and PCI-X
> devices.  I think pcie_capability_read_dword() would set dev_cap to zero
> and return zero, which would be safe, but I don't the reader should have to
> do that much analysis to figure it out.

Thanks for cleaning up the commit message and adding the pci_is_pcie check.

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

[PATCH V2] PCI: enable extended tags support for PCIe devices

[Sinan Kaya]

RWFjaCBQQ0llIGRldmljZSBjYW4gaXNzdWUgdXAgdG8gMzIgdHJhbnNhY3Rpb25zIGF0IGEgdGlt
ZSBieSBkZWZhdWx0LgpFYWNoIHRyYW5zYWN0aW9uIGlzIHRyYWNrZWQgYnkgYSB0YWcgbnVtYmVy
IG9uIHRoZSBidXMuIDIuMi42LjIuClRyYW5zYWN0aW9uIERlc2NyaXB0b3Ig4oCTIFRyYW5zYWN0
aW9uIElEIEZpZWxkIHNlY3Rpb24gb2YgdGhlIFBDSWUgMy4xCnNwZWNpZmljYXRpb24gZGVzY3Jp
YmVzIGV4dGVuZGVkIHRhZ3MuCgozMiB0cmFuc2FjdGlvbiBsaW1pdCBoYXMgYmVlbiBleHRlbmRl
ZCB0byAyNTYgb24gUENJIEV4cHJlc3MuIEFjY29yZGluZwp0byB0aGUgc3BlY2lmaWNhdGlvbiwg
YWxsIFBDSWUgZGV2aWNlcyBhcmUgcmVxdWlyZWQgdG8gc3VwcG9ydCByZWNlaXZpbmcKOC1iaXQg
VGFncyAoVGFnIGNvbXBsZXRlcikuIFRoZSBQQ0llLVBDSSBicmlkZ2VzIGhhbmRsZSB0aGUgdHJh
bnNsYXRpb24Kb2YgOC1iaXQgdGFncyB0byA1LWJpdCB0YWdzLgoKSG93ZXZlciwgdGhlIGdlbmVy
YXRpb24gb2YgOC1iaXQgdGFncyBpcyBsZWZ0IG9wdGlvbmFsIHRvIGEgcGFydGljdWxhciBIVwpp
bXBsZW1lbnRhdGlvbi4gVGhlIGNvZGUgbmVlZHMgdG8gY2hlY2sgSFcgc3VwcG9ydCBiZWZvcmUg
YXR0ZW1wdGluZwp0byBlbmFibGUgZXh0ZW5kZWQgdGFncyBwcm9kdWNlciBjYXBhYmlsaXR5LgoK
MzIgb3V0c3RhbmRpbmcgdHJhbnNhY3Rpb25zIGlzIG5vdCBlbm91Z2ggZm9yIHNvbWUgcGVyZm9y
bWFuY2UgY3JpdGljYWwKYXBwbGljYXRpb25zIGVzcGVjaWFsbHkgd2hlbiBhIGxvdCBvZiBzbWFs
bCBzaXplZCBmcmFtZXMgYXJlIHRyYW5zbWl0dGVkLgoKRXh0ZW5kZWQgdGFncyBzdXBwb3J0IGlu
Y3JlYXNlcyB0aGlzIG51bWJlciB0byAyNTYuIERldmljZXMgbm90CnN1cHBvcnRpbmcgZXh0ZW5k
ZWQgdGFncyB0aWUtb2ZmIHRoaXMgZmllbGQgdG8gMC4gQWNjb3JkaW5nIHRvIEVDTiwgaXQKaXMg
c2FmZSB0byBlbmFibGUgdGhpcyBmZWF0dXJlIGZvciBhbGwgUENJZSBkZXZpY2VzLgoKU2lnbmVk
LW9mZi1ieTogU2luYW4gS2F5YSA8b2theWFAY29kZWF1cm9yYS5vcmc+Ci0tLQogZHJpdmVycy9w
Y2kvcHJvYmUuYyB8IDE2ICsrKysrKysrKysrKysrKysKIDEgZmlsZSBjaGFuZ2VkLCAxNiBpbnNl
cnRpb25zKCspCgpkaWZmIC0tZ2l0IGEvZHJpdmVycy9wY2kvcHJvYmUuYyBiL2RyaXZlcnMvcGNp
L3Byb2JlLmMKaW5kZXggZTE2NGI1Yy4uMTE5MjQ3NSAxMDA2NDQKLS0tIGEvZHJpdmVycy9wY2kv
cHJvYmUuYworKysgYi9kcml2ZXJzL3BjaS9wcm9iZS5jCkBAIC0xNjUwLDEyICsxNjUwLDI4IEBA
IHN0YXRpYyB2b2lkIHByb2dyYW1faHBwX3R5cGUyKHN0cnVjdCBwY2lfZGV2ICpkZXYsIHN0cnVj
dCBocHBfdHlwZTIgKmhwcCkKIAkgKi8KIH0KIAorc3RhdGljIHZvaWQgcGNpX2NvbmZpZ3VyZV9l
eHRlbmRlZF90YWdzKHN0cnVjdCBwY2lfZGV2ICpwZGV2KQoreworCXUzMiBkZXZfY2FwOworCWlu
dCByZXQ7CisKKwlyZXQgPSBwY2llX2NhcGFiaWxpdHlfcmVhZF9kd29yZChwZGV2LCBQQ0lfRVhQ
X0RFVkNBUCwgJmRldl9jYXApOworCisJaWYgKHJldCkKKwkJcmV0dXJuOworCisJaWYgKGRldl9j
YXAgJiBQQ0lfRVhQX0RFVkNBUF9FWFRfVEFHKQorCQlwY2llX2NhcGFiaWxpdHlfc2V0X3dvcmQo
cGRldiwgUENJX0VYUF9ERVZDVEwsCisJCQkJCSBQQ0lfRVhQX0RFVkNUTF9FWFRfVEFHKTsKK30K
Kwogc3RhdGljIHZvaWQgcGNpX2NvbmZpZ3VyZV9kZXZpY2Uoc3RydWN0IHBjaV9kZXYgKmRldikK
IHsKIAlzdHJ1Y3QgaG90cGx1Z19wYXJhbXMgaHBwOwogCWludCByZXQ7CiAKIAlwY2lfY29uZmln
dXJlX21wcyhkZXYpOworCXBjaV9jb25maWd1cmVfZXh0ZW5kZWRfdGFncyhkZXYpOwogCiAJbWVt
c2V0KCZocHAsIDAsIHNpemVvZihocHApKTsKIAlyZXQgPSBwY2lfZ2V0X2hwX3BhcmFtcyhkZXYs
ICZocHApOwotLSAKMS45LjEKCgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f
X19fX19fX19fXwpsaW51eC1hcm0ta2VybmVsIG1haWxpbmcgbGlzdApsaW51eC1hcm0ta2VybmVs
QGxpc3RzLmluZnJhZGVhZC5vcmcKaHR0cDovL2xpc3RzLmluZnJhZGVhZC5vcmcvbWFpbG1hbi9s
aXN0aW5mby9saW51eC1hcm0ta2VybmVsCg==

[PATCH V2] PCI: enable extended tags support for PCIe devices

[Sinan Kaya]

Each PCIe device can issue up to 32 transactions at a time by default.
Each transaction is tracked by a tag number on the bus. 2.2.6.2.
Transaction Descriptor – Transaction ID Field section of the PCIe 3.1
specification describes extended tags.

32 transaction limit has been extended to 256 on PCI Express. According
to the specification, all PCIe devices are required to support receiving
8-bit Tags (Tag completer). The PCIe-PCI bridges handle the translation
of 8-bit tags to 5-bit tags.

However, the generation of 8-bit tags is left optional to a particular HW
implementation. The code needs to check HW support before attempting
to enable extended tags producer capability.

32 outstanding transactions is not enough for some performance critical
applications especially when a lot of small sized frames are transmitted.

Extended tags support increases this number to 256. Devices not
supporting extended tags tie-off this field to 0. According to ECN, it
is safe to enable this feature for all PCIe devices.

Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
 drivers/pci/probe.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index e164b5c..1192475 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1650,12 +1650,28 @@ static void program_hpp_type2(struct pci_dev *dev, struct hpp_type2 *hpp)
 	 */
 }
 
+static void pci_configure_extended_tags(struct pci_dev *pdev)
+{
+	u32 dev_cap;
+	int ret;
+
+	ret = pcie_capability_read_dword(pdev, PCI_EXP_DEVCAP, &dev_cap);
+
+	if (ret)
+		return;
+
+	if (dev_cap & PCI_EXP_DEVCAP_EXT_TAG)
+		pcie_capability_set_word(pdev, PCI_EXP_DEVCTL,
+					 PCI_EXP_DEVCTL_EXT_TAG);
+}
+
 static void pci_configure_device(struct pci_dev *dev)
 {
 	struct hotplug_params hpp;
 	int ret;
 
 	pci_configure_mps(dev);
+	pci_configure_extended_tags(dev);
 
 	memset(&hpp, 0, sizeof(hpp));
 	ret = pci_get_hp_params(dev, &hpp);
-- 
1.9.1