[PATCH V2] PCI: Do not enable extended tags on pre-dated (v1.x) systems

[PATCH V2] PCI: Do not enable extended tags on pre-dated (v1.x) systems

[Sinan Kaya]

According to extended tags ECN document, all PCIe receivers are expected
to support extended tags support. It should be safe to enable extended
tags on endpoints without checking compatibility.

This assumption seems to be working fine except for the legacy systems.
The ECN has been written against PCIE spec version 2.0. Therefore, we need
to exclude all version 1.0 devices from this change as there is HW out
there that can't handle extended tags.

Note that the default value of Extended Tags Enable bit is implementation
specific. Therefore, we are clearing the bit by default when incompatible
HW is found without assuming that value is zero.

Reported-by: Wim ten Have <wim.ten.have@oracle.com>
Link: https://pcisig.com/sites/default/files/specification_documents/ECN_Extended_Tag_Enable_Default_05Sept2008_final.pdf
Link: https://bugzilla.redhat.com/show_bug.cgi?id=1467674
Fixes: 60db3a4d8cc9 ("PCI: Enable PCIe Extended Tags if supported")
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
 drivers/pci/probe.c | 45 +++++++++++++++++++++++++++++++++++++++------
 1 file changed, 39 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 19c8950..e3b3c18 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1684,21 +1684,51 @@ static void program_hpp_type2(struct pci_dev *dev, struct hpp_type2 *hpp)
 	 */
 }
 
-static void pci_configure_extended_tags(struct pci_dev *dev)
+static int pcie_bus_discover_legacy(struct pci_dev *dev, void *data)
 {
+	bool *found = data;
+	int rc;
+	u16 flags;
+
+	if (!pci_is_pcie(dev))
+		return 0;
+
+	rc = pcie_capability_read_word(dev, PCI_EXP_FLAGS, &flags);
+	if (!rc && ((flags & PCI_EXP_FLAGS_VERS) < 2))
+		*found = true;
+
+	return 0;
+}
+
+static int pcie_bus_configure_exttags(struct pci_dev *dev, void *legacy)
+{
+	bool supported = !*(bool *)legacy;
 	u32 dev_cap;
+	u16 flags;
 	int ret;
 
 	if (!pci_is_pcie(dev))
-		return;
+		return 0;
+
+	ret = pcie_capability_read_word(dev, PCI_EXP_FLAGS, &flags);
+	if (ret || ((flags & PCI_EXP_FLAGS_VERS) < 2))
+		return 0;
 
 	ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &dev_cap);
-	if (ret)
-		return;
+	if (ret || (!(dev_cap & PCI_EXP_DEVCAP_EXT_TAG)))
+		return 0;
 
-	if (dev_cap & PCI_EXP_DEVCAP_EXT_TAG)
+	if (supported) {
+		dev_dbg(&dev->dev, "setting extended tags capability\n");
 		pcie_capability_set_word(dev, PCI_EXP_DEVCTL,
 					 PCI_EXP_DEVCTL_EXT_TAG);
+	} else {
+		dev_dbg(&dev->dev, "clearing extended tags capability\n");
+		pcie_capability_clear_word(dev, PCI_EXP_DEVCTL,
+					   PCI_EXP_DEVCTL_EXT_TAG);
+	}
+
+	return 0;
 }
 
 static void pci_configure_device(struct pci_dev *dev)
@@ -1707,7 +1737,6 @@ static void pci_configure_device(struct pci_dev *dev)
 	int ret;
 
 	pci_configure_mps(dev);
-	pci_configure_extended_tags(dev);
 
 	memset(&hpp, 0, sizeof(hpp));
 	ret = pci_get_hp_params(dev, &hpp);
@@ -2201,6 +2230,7 @@ static int pcie_bus_configure_set(struct pci_dev *dev, void *data)
 void pcie_bus_configure_settings(struct pci_bus *bus)
 {
 	u8 smpss = 0;
+	bool legacy_found = false;
 
 	if (!bus->self)
 		return;
@@ -2224,6 +2254,9 @@ void pcie_bus_configure_settings(struct pci_bus *bus)
 
 	pcie_bus_configure_set(bus->self, &smpss);
 	pci_walk_bus(bus, pcie_bus_configure_set, &smpss);
+
+	pci_walk_bus(bus, pcie_bus_discover_legacy, &legacy_found);
+	pci_walk_bus(bus, pcie_bus_configure_exttags, &legacy_found);
 }
 EXPORT_SYMBOL_GPL(pcie_bus_configure_settings);
 
-- 
1.9.1

[PATCH V2] PCI: Do not enable extended tags on pre-dated (v1.x) systems

[Sinan Kaya]

According to extended tags ECN document, all PCIe receivers are expected
to support extended tags support. It should be safe to enable extended
tags on endpoints without checking compatibility.

This assumption seems to be working fine except for the legacy systems.
The ECN has been written against PCIE spec version 2.0. Therefore, we need
to exclude all version 1.0 devices from this change as there is HW out
there that can't handle extended tags.

Note that the default value of Extended Tags Enable bit is implementation
specific. Therefore, we are clearing the bit by default when incompatible
HW is found without assuming that value is zero.

Reported-by: Wim ten Have <wim.ten.have@oracle.com>
Link: https://pcisig.com/sites/default/files/specification_documents/ECN_Extended_Tag_Enable_Default_05Sept2008_final.pdf
Link: https://bugzilla.redhat.com/show_bug.cgi?id=1467674
Fixes: 60db3a4d8cc9 ("PCI: Enable PCIe Extended Tags if supported")
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
 drivers/pci/probe.c | 45 +++++++++++++++++++++++++++++++++++++++------
 1 file changed, 39 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 19c8950..e3b3c18 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1684,21 +1684,51 @@ static void program_hpp_type2(struct pci_dev *dev, struct hpp_type2 *hpp)
 	 */
 }
 
-static void pci_configure_extended_tags(struct pci_dev *dev)
+static int pcie_bus_discover_legacy(struct pci_dev *dev, void *data)
 {
+	bool *found = data;
+	int rc;
+	u16 flags;
+
+	if (!pci_is_pcie(dev))
+		return 0;
+
+	rc = pcie_capability_read_word(dev, PCI_EXP_FLAGS, &flags);
+	if (!rc && ((flags & PCI_EXP_FLAGS_VERS) < 2))
+		*found = true;
+
+	return 0;
+}
+
+static int pcie_bus_configure_exttags(struct pci_dev *dev, void *legacy)
+{
+	bool supported = !*(bool *)legacy;
 	u32 dev_cap;
+	u16 flags;
 	int ret;
 
 	if (!pci_is_pcie(dev))
-		return;
+		return 0;
+
+	ret = pcie_capability_read_word(dev, PCI_EXP_FLAGS, &flags);
+	if (ret || ((flags & PCI_EXP_FLAGS_VERS) < 2))
+		return 0;
 
 	ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &dev_cap);
-	if (ret)
-		return;
+	if (ret || (!(dev_cap & PCI_EXP_DEVCAP_EXT_TAG)))
+		return 0;
 
-	if (dev_cap & PCI_EXP_DEVCAP_EXT_TAG)
+	if (supported) {
+		dev_dbg(&dev->dev, "setting extended tags capability\n");
 		pcie_capability_set_word(dev, PCI_EXP_DEVCTL,
 					 PCI_EXP_DEVCTL_EXT_TAG);
+	} else {
+		dev_dbg(&dev->dev, "clearing extended tags capability\n");
+		pcie_capability_clear_word(dev, PCI_EXP_DEVCTL,
+					   PCI_EXP_DEVCTL_EXT_TAG);
+	}
+
+	return 0;
 }
 
 static void pci_configure_device(struct pci_dev *dev)
@@ -1707,7 +1737,6 @@ static void pci_configure_device(struct pci_dev *dev)
 	int ret;
 
 	pci_configure_mps(dev);
-	pci_configure_extended_tags(dev);
 
 	memset(&hpp, 0, sizeof(hpp));
 	ret = pci_get_hp_params(dev, &hpp);
@@ -2201,6 +2230,7 @@ static int pcie_bus_configure_set(struct pci_dev *dev, void *data)
 void pcie_bus_configure_settings(struct pci_bus *bus)
 {
 	u8 smpss = 0;
+	bool legacy_found = false;
 
 	if (!bus->self)
 		return;
@@ -2224,6 +2254,9 @@ void pcie_bus_configure_settings(struct pci_bus *bus)
 
 	pcie_bus_configure_set(bus->self, &smpss);
 	pci_walk_bus(bus, pcie_bus_configure_set, &smpss);
+
+	pci_walk_bus(bus, pcie_bus_discover_legacy, &legacy_found);
+	pci_walk_bus(bus, pcie_bus_configure_exttags, &legacy_found);
 }
 EXPORT_SYMBOL_GPL(pcie_bus_configure_settings);
 
-- 
1.9.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V2] PCI: Do not enable extended tags on pre-dated (v1.x) systems

[Sinan Kaya]

According to extended tags ECN document, all PCIe receivers are expected
to support extended tags support. It should be safe to enable extended
tags on endpoints without checking compatibility.

This assumption seems to be working fine except for the legacy systems.
The ECN has been written against PCIE spec version 2.0. Therefore, we need
to exclude all version 1.0 devices from this change as there is HW out
there that can't handle extended tags.

Note that the default value of Extended Tags Enable bit is implementation
specific. Therefore, we are clearing the bit by default when incompatible
HW is found without assuming that value is zero.

Reported-by: Wim ten Have <wim.ten.have@oracle.com>
Link: https://pcisig.com/sites/default/files/specification_documents/ECN_Extended_Tag_Enable_Default_05Sept2008_final.pdf
Link: https://bugzilla.redhat.com/show_bug.cgi?id=1467674
Fixes: 60db3a4d8cc9 ("PCI: Enable PCIe Extended Tags if supported")
Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
---
 drivers/pci/probe.c | 45 +++++++++++++++++++++++++++++++++++++++------
 1 file changed, 39 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 19c8950..e3b3c18 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1684,21 +1684,51 @@ static void program_hpp_type2(struct pci_dev *dev, struct hpp_type2 *hpp)
 	 */
 }
 
-static void pci_configure_extended_tags(struct pci_dev *dev)
+static int pcie_bus_discover_legacy(struct pci_dev *dev, void *data)
 {
+	bool *found = data;
+	int rc;
+	u16 flags;
+
+	if (!pci_is_pcie(dev))
+		return 0;
+
+	rc = pcie_capability_read_word(dev, PCI_EXP_FLAGS, &flags);
+	if (!rc && ((flags & PCI_EXP_FLAGS_VERS) < 2))
+		*found = true;
+
+	return 0;
+}
+
+static int pcie_bus_configure_exttags(struct pci_dev *dev, void *legacy)
+{
+	bool supported = !*(bool *)legacy;
 	u32 dev_cap;
+	u16 flags;
 	int ret;
 
 	if (!pci_is_pcie(dev))
-		return;
+		return 0;
+
+	ret = pcie_capability_read_word(dev, PCI_EXP_FLAGS, &flags);
+	if (ret || ((flags & PCI_EXP_FLAGS_VERS) < 2))
+		return 0;
 
 	ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &dev_cap);
-	if (ret)
-		return;
+	if (ret || (!(dev_cap & PCI_EXP_DEVCAP_EXT_TAG)))
+		return 0;
 
-	if (dev_cap & PCI_EXP_DEVCAP_EXT_TAG)
+	if (supported) {
+		dev_dbg(&dev->dev, "setting extended tags capability\n");
 		pcie_capability_set_word(dev, PCI_EXP_DEVCTL,
 					 PCI_EXP_DEVCTL_EXT_TAG);
+	} else {
+		dev_dbg(&dev->dev, "clearing extended tags capability\n");
+		pcie_capability_clear_word(dev, PCI_EXP_DEVCTL,
+					   PCI_EXP_DEVCTL_EXT_TAG);
+	}
+
+	return 0;
 }
 
 static void pci_configure_device(struct pci_dev *dev)
@@ -1707,7 +1737,6 @@ static void pci_configure_device(struct pci_dev *dev)
 	int ret;
 
 	pci_configure_mps(dev);
-	pci_configure_extended_tags(dev);
 
 	memset(&hpp, 0, sizeof(hpp));
 	ret = pci_get_hp_params(dev, &hpp);
@@ -2201,6 +2230,7 @@ static int pcie_bus_configure_set(struct pci_dev *dev, void *data)
 void pcie_bus_configure_settings(struct pci_bus *bus)
 {
 	u8 smpss = 0;
+	bool legacy_found = false;
 
 	if (!bus->self)
 		return;
@@ -2224,6 +2254,9 @@ void pcie_bus_configure_settings(struct pci_bus *bus)
 
 	pcie_bus_configure_set(bus->self, &smpss);
 	pci_walk_bus(bus, pcie_bus_configure_set, &smpss);
+
+	pci_walk_bus(bus, pcie_bus_discover_legacy, &legacy_found);
+	pci_walk_bus(bus, pcie_bus_configure_exttags, &legacy_found);
 }
 EXPORT_SYMBOL_GPL(pcie_bus_configure_settings);
 
-- 
1.9.1

Re: [PATCH V2] PCI: Do not enable extended tags on pre-dated (v1.x) systems

[Sinan Kaya]

Hi Wim,

On 7/7/2017 10:53 AM, Sinan Kaya wrote:
> According to extended tags ECN document, all PCIe receivers are expected
> to support extended tags support. It should be safe to enable extended
> tags on endpoints without checking compatibility.
> 
> This assumption seems to be working fine except for the legacy systems.
> The ECN has been written against PCIE spec version 2.0. Therefore, we need
> to exclude all version 1.0 devices from this change as there is HW out
> there that can't handle extended tags.
> 
> Note that the default value of Extended Tags Enable bit is implementation
> specific. Therefore, we are clearing the bit by default when incompatible
> HW is found without assuming that value is zero.
> 
> Reported-by: Wim ten Have <wim.ten.have@oracle.com>
> Link: https://pcisig.com/sites/default/files/specification_documents/ECN_Extended_Tag_Enable_Default_05Sept2008_final.pdf
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=1467674
> Fixes: 60db3a4d8cc9 ("PCI: Enable PCIe Extended Tags if supported")
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---

Can you also give this a spin? I don't have a system with v1 PCIe bridges.
I only tested v2 and later code path.

I tried to address Jike Song concerns on this version and removed your tested-by
since the code changed.

Sinan

-- 
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.

Re: [PATCH V2] PCI: Do not enable extended tags on pre-dated (v1.x) systems

[Sinan Kaya]

Hi Wim,

On 7/7/2017 10:53 AM, Sinan Kaya wrote:
> According to extended tags ECN document, all PCIe receivers are expected
> to support extended tags support. It should be safe to enable extended
> tags on endpoints without checking compatibility.
> 
> This assumption seems to be working fine except for the legacy systems.
> The ECN has been written against PCIE spec version 2.0. Therefore, we need
> to exclude all version 1.0 devices from this change as there is HW out
> there that can't handle extended tags.
> 
> Note that the default value of Extended Tags Enable bit is implementation
> specific. Therefore, we are clearing the bit by default when incompatible
> HW is found without assuming that value is zero.
> 
> Reported-by: Wim ten Have <wim.ten.have@oracle.com>
> Link: https://pcisig.com/sites/default/files/specification_documents/ECN_Extended_Tag_Enable_Default_05Sept2008_final.pdf
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=1467674
> Fixes: 60db3a4d8cc9 ("PCI: Enable PCIe Extended Tags if supported")
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---

Can you also give this a spin? I don't have a system with v1 PCIe bridges.
I only tested v2 and later code path.

I tried to address Jike Song concerns on this version and removed your tested-by
since the code changed.

Sinan

-- 
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.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V2] PCI: Do not enable extended tags on pre-dated (v1.x) systems

[Sinan Kaya]

Hi Wim,

On 7/7/2017 10:53 AM, Sinan Kaya wrote:
> According to extended tags ECN document, all PCIe receivers are expected
> to support extended tags support. It should be safe to enable extended
> tags on endpoints without checking compatibility.
> 
> This assumption seems to be working fine except for the legacy systems.
> The ECN has been written against PCIE spec version 2.0. Therefore, we need
> to exclude all version 1.0 devices from this change as there is HW out
> there that can't handle extended tags.
> 
> Note that the default value of Extended Tags Enable bit is implementation
> specific. Therefore, we are clearing the bit by default when incompatible
> HW is found without assuming that value is zero.
> 
> Reported-by: Wim ten Have <wim.ten.have@oracle.com>
> Link: https://pcisig.com/sites/default/files/specification_documents/ECN_Extended_Tag_Enable_Default_05Sept2008_final.pdf
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=1467674
> Fixes: 60db3a4d8cc9 ("PCI: Enable PCIe Extended Tags if supported")
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---

Can you also give this a spin? I don't have a system with v1 PCIe bridges.
I only tested v2 and later code path.

I tried to address Jike Song concerns on this version and removed your tested-by
since the code changed.

Sinan

-- 
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.

Re: [PATCH V2] PCI: Do not enable extended tags on pre-dated (v1.x) systems

[Sinan Kaya]

On 7/7/2017 10:53 AM, Sinan Kaya wrote:
> +	ret = pcie_capability_read_word(dev, PCI_EXP_FLAGS, &flags);
> +	if (ret || ((flags & PCI_EXP_FLAGS_VERS) < 2))
> +		return 0;
>  

Never mind, there is a problem here. I shouldn't have added it here.
I'll remove these and post again.

-- 
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.

Re: [PATCH V2] PCI: Do not enable extended tags on pre-dated (v1.x) systems

[Sinan Kaya]

On 7/7/2017 10:53 AM, Sinan Kaya wrote:
> +	ret = pcie_capability_read_word(dev, PCI_EXP_FLAGS, &flags);
> +	if (ret || ((flags & PCI_EXP_FLAGS_VERS) < 2))
> +		return 0;
>  

Never mind, there is a problem here. I shouldn't have added it here.
I'll remove these and post again.

-- 
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.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V2] PCI: Do not enable extended tags on pre-dated (v1.x) systems

[Sinan Kaya]

On 7/7/2017 10:53 AM, Sinan Kaya wrote:
> +	ret = pcie_capability_read_word(dev, PCI_EXP_FLAGS, &flags);
> +	if (ret || ((flags & PCI_EXP_FLAGS_VERS) < 2))
> +		return 0;
>  

Never mind, there is a problem here. I shouldn't have added it here.
I'll remove these and post again.

-- 
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.

Re: [PATCH V2] PCI: Do not enable extended tags on pre-dated (v1.x) systems

[Sinan Kaya]

On 7/7/2017 11:07 AM, Sinan Kaya wrote:
> On 7/7/2017 10:53 AM, Sinan Kaya wrote:
>> +	ret = pcie_capability_read_word(dev, PCI_EXP_FLAGS, &flags);
>> +	if (ret || ((flags & PCI_EXP_FLAGS_VERS) < 2))
>> +		return 0;
>>  
> 
> Never mind, there is a problem here. I shouldn't have added it here.
> I'll remove these and post again.
> 

I guess I'll wait until Bjorn gets a chance to review it. There is a decision
that needs to be made here. 

Under normal circumstances, extended tags capability is a reserved field on
v1 that's expected to be 0.

Code is checking for extended tags capability being non-zero next
before setting/clearing the bit.

It should be safe to rely on capability being 0 on v1. However, we can go
paranoid and add the check above to not even look at the capability like I
did it. 

That's why, I thought this is redundant. 

I'll wait until Bjorn chimes in. It is OK to keep the code as it is. It is
just doing too much validation in my opinion. Somebody can always say
play safe.

-- 
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.

Re: [PATCH V2] PCI: Do not enable extended tags on pre-dated (v1.x) systems

[Sinan Kaya]

On 7/7/2017 11:07 AM, Sinan Kaya wrote:
> On 7/7/2017 10:53 AM, Sinan Kaya wrote:
>> +	ret = pcie_capability_read_word(dev, PCI_EXP_FLAGS, &flags);
>> +	if (ret || ((flags & PCI_EXP_FLAGS_VERS) < 2))
>> +		return 0;
>>  
> 
> Never mind, there is a problem here. I shouldn't have added it here.
> I'll remove these and post again.
> 

I guess I'll wait until Bjorn gets a chance to review it. There is a decision
that needs to be made here. 

Under normal circumstances, extended tags capability is a reserved field on
v1 that's expected to be 0.

Code is checking for extended tags capability being non-zero next
before setting/clearing the bit.

It should be safe to rely on capability being 0 on v1. However, we can go
paranoid and add the check above to not even look at the capability like I
did it. 

That's why, I thought this is redundant. 

I'll wait until Bjorn chimes in. It is OK to keep the code as it is. It is
just doing too much validation in my opinion. Somebody can always say
play safe.

-- 
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.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V2] PCI: Do not enable extended tags on pre-dated (v1.x) systems

[Sinan Kaya]

On 7/7/2017 11:07 AM, Sinan Kaya wrote:
> On 7/7/2017 10:53 AM, Sinan Kaya wrote:
>> +	ret = pcie_capability_read_word(dev, PCI_EXP_FLAGS, &flags);
>> +	if (ret || ((flags & PCI_EXP_FLAGS_VERS) < 2))
>> +		return 0;
>>  
> 
> Never mind, there is a problem here. I shouldn't have added it here.
> I'll remove these and post again.
> 

I guess I'll wait until Bjorn gets a chance to review it. There is a decision
that needs to be made here. 

Under normal circumstances, extended tags capability is a reserved field on
v1 that's expected to be 0.

Code is checking for extended tags capability being non-zero next
before setting/clearing the bit.

It should be safe to rely on capability being 0 on v1. However, we can go
paranoid and add the check above to not even look at the capability like I
did it. 

That's why, I thought this is redundant. 

I'll wait until Bjorn chimes in. It is OK to keep the code as it is. It is
just doing too much validation in my opinion. Somebody can always say
play safe.

-- 
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.

Re: [PATCH V2] PCI: Do not enable extended tags on pre-dated (v1.x) systems

[Wim ten Have]

On Fri, 7 Jul 2017 11:01:16 -0400
Sinan Kaya <okaya@codeaurora.org> wrote:

> Hi Wim,
> 
> On 7/7/2017 10:53 AM, Sinan Kaya wrote:
> > According to extended tags ECN document, all PCIe receivers are expected
> > to support extended tags support. It should be safe to enable extended
> > tags on endpoints without checking compatibility.
> > 
> > This assumption seems to be working fine except for the legacy systems.
> > The ECN has been written against PCIE spec version 2.0. Therefore, we need
> > to exclude all version 1.0 devices from this change as there is HW out
> > there that can't handle extended tags.
> > 
> > Note that the default value of Extended Tags Enable bit is implementation
> > specific. Therefore, we are clearing the bit by default when incompatible
> > HW is found without assuming that value is zero.
> > 
> > Reported-by: Wim ten Have <wim.ten.have@oracle.com>
> > Link: https://pcisig.com/sites/default/files/specification_documents/ECN_Extended_Tag_Enable_Default_05Sept2008_final.pdf
> > Link: https://bugzilla.redhat.com/show_bug.cgi?id=1467674
> > Fixes: 60db3a4d8cc9 ("PCI: Enable PCIe Extended Tags if supported")
> > Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> > ---  
> 
> Can you also give this a spin? I don't have a system with v1 PCIe bridges.
> I only tested v2 and later code path.
> 
> I tried to address Jike Song concerns on this version and removed your tested-by
> since the code changed.
> 
> Sinan
> 

Sure,

-- 
Wim ten Have | Consulting Member of Technical Staff
Oracle Linux and VM Development Engineering
ORACLE Nederland BV | Hertogswetering 163-167 | 3543 AS Utrecht/NL

Re: [PATCH V2] PCI: Do not enable extended tags on pre-dated (v1.x) systems

[Wim ten Have]

On Fri, 7 Jul 2017 11:01:16 -0400
Sinan Kaya <okaya@codeaurora.org> wrote:

> Hi Wim,
> 
> On 7/7/2017 10:53 AM, Sinan Kaya wrote:
> > According to extended tags ECN document, all PCIe receivers are expected
> > to support extended tags support. It should be safe to enable extended
> > tags on endpoints without checking compatibility.
> > 
> > This assumption seems to be working fine except for the legacy systems.
> > The ECN has been written against PCIE spec version 2.0. Therefore, we need
> > to exclude all version 1.0 devices from this change as there is HW out
> > there that can't handle extended tags.
> > 
> > Note that the default value of Extended Tags Enable bit is implementation
> > specific. Therefore, we are clearing the bit by default when incompatible
> > HW is found without assuming that value is zero.
> > 
> > Reported-by: Wim ten Have <wim.ten.have@oracle.com>
> > Link: https://pcisig.com/sites/default/files/specification_documents/ECN_Extended_Tag_Enable_Default_05Sept2008_final.pdf
> > Link: https://bugzilla.redhat.com/show_bug.cgi?id=1467674
> > Fixes: 60db3a4d8cc9 ("PCI: Enable PCIe Extended Tags if supported")
> > Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> > ---  
> 
> Can you also give this a spin? I don't have a system with v1 PCIe bridges.
> I only tested v2 and later code path.
> 
> I tried to address Jike Song concerns on this version and removed your tested-by
> since the code changed.
> 
> Sinan
> 

Sure,

-- 
Wim ten Have | Consulting Member of Technical Staff
Oracle Linux and VM Development Engineering
ORACLE Nederland BV | Hertogswetering 163-167 | 3543 AS Utrecht/NL

Re: [PATCH V2] PCI: Do not enable extended tags on pre-dated (v1.x) systems

[Wim ten Have]

On Fri, 7 Jul 2017 11:01:16 -0400
Sinan Kaya <okaya@codeaurora.org> wrote:

> Hi Wim,
> 
> On 7/7/2017 10:53 AM, Sinan Kaya wrote:
> > According to extended tags ECN document, all PCIe receivers are expected
> > to support extended tags support. It should be safe to enable extended
> > tags on endpoints without checking compatibility.
> > 
> > This assumption seems to be working fine except for the legacy systems.
> > The ECN has been written against PCIE spec version 2.0. Therefore, we need
> > to exclude all version 1.0 devices from this change as there is HW out
> > there that can't handle extended tags.
> > 
> > Note that the default value of Extended Tags Enable bit is implementation
> > specific. Therefore, we are clearing the bit by default when incompatible
> > HW is found without assuming that value is zero.
> > 
> > Reported-by: Wim ten Have <wim.ten.have@oracle.com>
> > Link: https://pcisig.com/sites/default/files/specification_documents/ECN_Extended_Tag_Enable_Default_05Sept2008_final.pdf
> > Link: https://bugzilla.redhat.com/show_bug.cgi?id=1467674
> > Fixes: 60db3a4d8cc9 ("PCI: Enable PCIe Extended Tags if supported")
> > Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> > ---  
> 
> Can you also give this a spin? I don't have a system with v1 PCIe bridges.
> I only tested v2 and later code path.
> 
> I tried to address Jike Song concerns on this version and removed your tested-by
> since the code changed.
> 
> Sinan
> 

Sure,

-- 
Wim ten Have | Consulting Member of Technical Staff
Oracle Linux and VM Development Engineering
ORACLE Nederland BV | Hertogswetering 163-167 | 3543 AS Utrecht/NL

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V2] PCI: Do not enable extended tags on pre-dated (v1.x) systems

[Wim ten Have]

On Fri, 7 Jul 2017 11:01:16 -0400
Sinan Kaya <okaya@codeaurora.org> wrote:

> Hi Wim,
> 
> On 7/7/2017 10:53 AM, Sinan Kaya wrote:
> > According to extended tags ECN document, all PCIe receivers are expected
> > to support extended tags support. It should be safe to enable extended
> > tags on endpoints without checking compatibility.
> > 
> > This assumption seems to be working fine except for the legacy systems.
> > The ECN has been written against PCIE spec version 2.0. Therefore, we need
> > to exclude all version 1.0 devices from this change as there is HW out
> > there that can't handle extended tags.
> > 
> > Note that the default value of Extended Tags Enable bit is implementation
> > specific. Therefore, we are clearing the bit by default when incompatible
> > HW is found without assuming that value is zero.
> > 
> > Reported-by: Wim ten Have <wim.ten.have@oracle.com>
> > Link: https://pcisig.com/sites/default/files/specification_documents/ECN_Extended_Tag_Enable_Default_05Sept2008_final.pdf
> > Link: https://bugzilla.redhat.com/show_bug.cgi?id=1467674
> > Fixes: 60db3a4d8cc9 ("PCI: Enable PCIe Extended Tags if supported")
> > Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> > ---  
> 
> Can you also give this a spin? I don't have a system with v1 PCIe bridges.
> I only tested v2 and later code path.
> 
> I tried to address Jike Song concerns on this version and removed your tested-by
> since the code changed.
> 
> Sinan
> 

Sure,

-- 
Wim ten Have | Consulting Member of Technical Staff
Oracle Linux and VM Development Engineering
ORACLE Nederland BV | Hertogswetering 163-167 | 3543 AS Utrecht/NL

Re: [PATCH V2] PCI: Do not enable extended tags on pre-dated (v1.x) systems

[Bjorn Helgaas]

On Fri, Jul 07, 2017 at 10:53:13AM -0400, Sinan Kaya wrote:
> According to extended tags ECN document, all PCIe receivers are expected
> to support extended tags support. It should be safe to enable extended
> tags on endpoints without checking compatibility.
> 
> This assumption seems to be working fine except for the legacy systems.
> The ECN has been written against PCIE spec version 2.0. Therefore, we need
> to exclude all version 1.0 devices from this change as there is HW out
> there that can't handle extended tags.
> 
> Note that the default value of Extended Tags Enable bit is implementation
> specific. Therefore, we are clearing the bit by default when incompatible
> HW is found without assuming that value is zero.

The PCI_EXP_DEVCTL_EXT_TAG bit controls the behavior of the function as a
Requester.  As far as I can see, it has nothing to do with whether the
function supports 8-bit tags as a *Completer*, so the implicit assumption
of the spec is that all Completers always support 8-bit tags.  My guess is
that's why the ECN thought it would be safe to enable extended tags by
default.

If that's the case, this is just a defect in the device (the Completer),
and we should blacklist it.  Looking at the PCIe Capability version might
happen to correlate with Completer support for 8-bit tags, but that looks
like just a coincidence to me.

> Reported-by: Wim ten Have <wim.ten.have@oracle.com>
> Link: https://pcisig.com/sites/default/files/specification_documents/ECN_Extended_Tag_Enable_Default_05Sept2008_final.pdf
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=1467674
> Fixes: 60db3a4d8cc9 ("PCI: Enable PCIe Extended Tags if supported")
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
>  drivers/pci/probe.c | 45 +++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 39 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 19c8950..e3b3c18 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1684,21 +1684,51 @@ static void program_hpp_type2(struct pci_dev *dev, struct hpp_type2 *hpp)
>  	 */
>  }
>  
> -static void pci_configure_extended_tags(struct pci_dev *dev)
> +static int pcie_bus_discover_legacy(struct pci_dev *dev, void *data)
>  {
> +	bool *found = data;
> +	int rc;
> +	u16 flags;
> +
> +	if (!pci_is_pcie(dev))
> +		return 0;
> +
> +	rc = pcie_capability_read_word(dev, PCI_EXP_FLAGS, &flags);
> +	if (!rc && ((flags & PCI_EXP_FLAGS_VERS) < 2))
> +		*found = true;
> +
> +	return 0;
> +}
> +
> +static int pcie_bus_configure_exttags(struct pci_dev *dev, void *legacy)
> +{
> +	bool supported = !*(bool *)legacy;
>  	u32 dev_cap;
> +	u16 flags;
>  	int ret;
>  
>  	if (!pci_is_pcie(dev))
> -		return;
> +		return 0;
> +
> +	ret = pcie_capability_read_word(dev, PCI_EXP_FLAGS, &flags);
> +	if (ret || ((flags & PCI_EXP_FLAGS_VERS) < 2))
> +		return 0;
>  
>  	ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &dev_cap);
> -	if (ret)
> -		return;
> +	if (ret || (!(dev_cap & PCI_EXP_DEVCAP_EXT_TAG)))
> +		return 0;
>  
> -	if (dev_cap & PCI_EXP_DEVCAP_EXT_TAG)
> +	if (supported) {
> +		dev_dbg(&dev->dev, "setting extended tags capability\n");
>  		pcie_capability_set_word(dev, PCI_EXP_DEVCTL,
>  					 PCI_EXP_DEVCTL_EXT_TAG);
> +	} else {
> +		dev_dbg(&dev->dev, "clearing extended tags capability\n");
> +		pcie_capability_clear_word(dev, PCI_EXP_DEVCTL,
> +					   PCI_EXP_DEVCTL_EXT_TAG);
> +	}
> +
> +	return 0;
>  }
>  
>  static void pci_configure_device(struct pci_dev *dev)
> @@ -1707,7 +1737,6 @@ static void pci_configure_device(struct pci_dev *dev)
>  	int ret;
>  
>  	pci_configure_mps(dev);
> -	pci_configure_extended_tags(dev);
>  
>  	memset(&hpp, 0, sizeof(hpp));
>  	ret = pci_get_hp_params(dev, &hpp);
> @@ -2201,6 +2230,7 @@ static int pcie_bus_configure_set(struct pci_dev *dev, void *data)
>  void pcie_bus_configure_settings(struct pci_bus *bus)
>  {
>  	u8 smpss = 0;
> +	bool legacy_found = false;
>  
>  	if (!bus->self)
>  		return;
> @@ -2224,6 +2254,9 @@ void pcie_bus_configure_settings(struct pci_bus *bus)
>  
>  	pcie_bus_configure_set(bus->self, &smpss);
>  	pci_walk_bus(bus, pcie_bus_configure_set, &smpss);
> +
> +	pci_walk_bus(bus, pcie_bus_discover_legacy, &legacy_found);
> +	pci_walk_bus(bus, pcie_bus_configure_exttags, &legacy_found);
>  }
>  EXPORT_SYMBOL_GPL(pcie_bus_configure_settings);
>  
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V2] PCI: Do not enable extended tags on pre-dated (v1.x) systems

[Bjorn Helgaas]

On Fri, Jul 07, 2017 at 10:53:13AM -0400, Sinan Kaya wrote:
> According to extended tags ECN document, all PCIe receivers are expected
> to support extended tags support. It should be safe to enable extended
> tags on endpoints without checking compatibility.
> 
> This assumption seems to be working fine except for the legacy systems.
> The ECN has been written against PCIE spec version 2.0. Therefore, we need
> to exclude all version 1.0 devices from this change as there is HW out
> there that can't handle extended tags.
> 
> Note that the default value of Extended Tags Enable bit is implementation
> specific. Therefore, we are clearing the bit by default when incompatible
> HW is found without assuming that value is zero.

The PCI_EXP_DEVCTL_EXT_TAG bit controls the behavior of the function as a
Requester.  As far as I can see, it has nothing to do with whether the
function supports 8-bit tags as a *Completer*, so the implicit assumption
of the spec is that all Completers always support 8-bit tags.  My guess is
that's why the ECN thought it would be safe to enable extended tags by
default.

If that's the case, this is just a defect in the device (the Completer),
and we should blacklist it.  Looking at the PCIe Capability version might
happen to correlate with Completer support for 8-bit tags, but that looks
like just a coincidence to me.

> Reported-by: Wim ten Have <wim.ten.have@oracle.com>
> Link: https://pcisig.com/sites/default/files/specification_documents/ECN_Extended_Tag_Enable_Default_05Sept2008_final.pdf
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=1467674
> Fixes: 60db3a4d8cc9 ("PCI: Enable PCIe Extended Tags if supported")
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
>  drivers/pci/probe.c | 45 +++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 39 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 19c8950..e3b3c18 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1684,21 +1684,51 @@ static void program_hpp_type2(struct pci_dev *dev, struct hpp_type2 *hpp)
>  	 */
>  }
>  
> -static void pci_configure_extended_tags(struct pci_dev *dev)
> +static int pcie_bus_discover_legacy(struct pci_dev *dev, void *data)
>  {
> +	bool *found = data;
> +	int rc;
> +	u16 flags;
> +
> +	if (!pci_is_pcie(dev))
> +		return 0;
> +
> +	rc = pcie_capability_read_word(dev, PCI_EXP_FLAGS, &flags);
> +	if (!rc && ((flags & PCI_EXP_FLAGS_VERS) < 2))
> +		*found = true;
> +
> +	return 0;
> +}
> +
> +static int pcie_bus_configure_exttags(struct pci_dev *dev, void *legacy)
> +{
> +	bool supported = !*(bool *)legacy;
>  	u32 dev_cap;
> +	u16 flags;
>  	int ret;
>  
>  	if (!pci_is_pcie(dev))
> -		return;
> +		return 0;
> +
> +	ret = pcie_capability_read_word(dev, PCI_EXP_FLAGS, &flags);
> +	if (ret || ((flags & PCI_EXP_FLAGS_VERS) < 2))
> +		return 0;
>  
>  	ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &dev_cap);
> -	if (ret)
> -		return;
> +	if (ret || (!(dev_cap & PCI_EXP_DEVCAP_EXT_TAG)))
> +		return 0;
>  
> -	if (dev_cap & PCI_EXP_DEVCAP_EXT_TAG)
> +	if (supported) {
> +		dev_dbg(&dev->dev, "setting extended tags capability\n");
>  		pcie_capability_set_word(dev, PCI_EXP_DEVCTL,
>  					 PCI_EXP_DEVCTL_EXT_TAG);
> +	} else {
> +		dev_dbg(&dev->dev, "clearing extended tags capability\n");
> +		pcie_capability_clear_word(dev, PCI_EXP_DEVCTL,
> +					   PCI_EXP_DEVCTL_EXT_TAG);
> +	}
> +
> +	return 0;
>  }
>  
>  static void pci_configure_device(struct pci_dev *dev)
> @@ -1707,7 +1737,6 @@ static void pci_configure_device(struct pci_dev *dev)
>  	int ret;
>  
>  	pci_configure_mps(dev);
> -	pci_configure_extended_tags(dev);
>  
>  	memset(&hpp, 0, sizeof(hpp));
>  	ret = pci_get_hp_params(dev, &hpp);
> @@ -2201,6 +2230,7 @@ static int pcie_bus_configure_set(struct pci_dev *dev, void *data)
>  void pcie_bus_configure_settings(struct pci_bus *bus)
>  {
>  	u8 smpss = 0;
> +	bool legacy_found = false;
>  
>  	if (!bus->self)
>  		return;
> @@ -2224,6 +2254,9 @@ void pcie_bus_configure_settings(struct pci_bus *bus)
>  
>  	pcie_bus_configure_set(bus->self, &smpss);
>  	pci_walk_bus(bus, pcie_bus_configure_set, &smpss);
> +
> +	pci_walk_bus(bus, pcie_bus_discover_legacy, &legacy_found);
> +	pci_walk_bus(bus, pcie_bus_configure_exttags, &legacy_found);
>  }
>  EXPORT_SYMBOL_GPL(pcie_bus_configure_settings);
>  
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH V2] PCI: Do not enable extended tags on pre-dated (v1.x) systems

[Bjorn Helgaas]

On Fri, Jul 07, 2017 at 10:53:13AM -0400, Sinan Kaya wrote:
> According to extended tags ECN document, all PCIe receivers are expected
> to support extended tags support. It should be safe to enable extended
> tags on endpoints without checking compatibility.
> 
> This assumption seems to be working fine except for the legacy systems.
> The ECN has been written against PCIE spec version 2.0. Therefore, we need
> to exclude all version 1.0 devices from this change as there is HW out
> there that can't handle extended tags.
> 
> Note that the default value of Extended Tags Enable bit is implementation
> specific. Therefore, we are clearing the bit by default when incompatible
> HW is found without assuming that value is zero.

The PCI_EXP_DEVCTL_EXT_TAG bit controls the behavior of the function as a
Requester.  As far as I can see, it has nothing to do with whether the
function supports 8-bit tags as a *Completer*, so the implicit assumption
of the spec is that all Completers always support 8-bit tags.  My guess is
that's why the ECN thought it would be safe to enable extended tags by
default.

If that's the case, this is just a defect in the device (the Completer),
and we should blacklist it.  Looking at the PCIe Capability version might
happen to correlate with Completer support for 8-bit tags, but that looks
like just a coincidence to me.

> Reported-by: Wim ten Have <wim.ten.have@oracle.com>
> Link: https://pcisig.com/sites/default/files/specification_documents/ECN_Extended_Tag_Enable_Default_05Sept2008_final.pdf
> Link: https://bugzilla.redhat.com/show_bug.cgi?id=1467674
> Fixes: 60db3a4d8cc9 ("PCI: Enable PCIe Extended Tags if supported")
> Signed-off-by: Sinan Kaya <okaya@codeaurora.org>
> ---
>  drivers/pci/probe.c | 45 +++++++++++++++++++++++++++++++++++++++------
>  1 file changed, 39 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 19c8950..e3b3c18 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1684,21 +1684,51 @@ static void program_hpp_type2(struct pci_dev *dev, struct hpp_type2 *hpp)
>  	 */
>  }
>  
> -static void pci_configure_extended_tags(struct pci_dev *dev)
> +static int pcie_bus_discover_legacy(struct pci_dev *dev, void *data)
>  {
> +	bool *found = data;
> +	int rc;
> +	u16 flags;
> +
> +	if (!pci_is_pcie(dev))
> +		return 0;
> +
> +	rc = pcie_capability_read_word(dev, PCI_EXP_FLAGS, &flags);
> +	if (!rc && ((flags & PCI_EXP_FLAGS_VERS) < 2))
> +		*found = true;
> +
> +	return 0;
> +}
> +
> +static int pcie_bus_configure_exttags(struct pci_dev *dev, void *legacy)
> +{
> +	bool supported = !*(bool *)legacy;
>  	u32 dev_cap;
> +	u16 flags;
>  	int ret;
>  
>  	if (!pci_is_pcie(dev))
> -		return;
> +		return 0;
> +
> +	ret = pcie_capability_read_word(dev, PCI_EXP_FLAGS, &flags);
> +	if (ret || ((flags & PCI_EXP_FLAGS_VERS) < 2))
> +		return 0;
>  
>  	ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &dev_cap);
> -	if (ret)
> -		return;
> +	if (ret || (!(dev_cap & PCI_EXP_DEVCAP_EXT_TAG)))
> +		return 0;
>  
> -	if (dev_cap & PCI_EXP_DEVCAP_EXT_TAG)
> +	if (supported) {
> +		dev_dbg(&dev->dev, "setting extended tags capability\n");
>  		pcie_capability_set_word(dev, PCI_EXP_DEVCTL,
>  					 PCI_EXP_DEVCTL_EXT_TAG);
> +	} else {
> +		dev_dbg(&dev->dev, "clearing extended tags capability\n");
> +		pcie_capability_clear_word(dev, PCI_EXP_DEVCTL,
> +					   PCI_EXP_DEVCTL_EXT_TAG);
> +	}
> +
> +	return 0;
>  }
>  
>  static void pci_configure_device(struct pci_dev *dev)
> @@ -1707,7 +1737,6 @@ static void pci_configure_device(struct pci_dev *dev)
>  	int ret;
>  
>  	pci_configure_mps(dev);
> -	pci_configure_extended_tags(dev);
>  
>  	memset(&hpp, 0, sizeof(hpp));
>  	ret = pci_get_hp_params(dev, &hpp);
> @@ -2201,6 +2230,7 @@ static int pcie_bus_configure_set(struct pci_dev *dev, void *data)
>  void pcie_bus_configure_settings(struct pci_bus *bus)
>  {
>  	u8 smpss = 0;
> +	bool legacy_found = false;
>  
>  	if (!bus->self)
>  		return;
> @@ -2224,6 +2254,9 @@ void pcie_bus_configure_settings(struct pci_bus *bus)
>  
>  	pcie_bus_configure_set(bus->self, &smpss);
>  	pci_walk_bus(bus, pcie_bus_configure_set, &smpss);
> +
> +	pci_walk_bus(bus, pcie_bus_discover_legacy, &legacy_found);
> +	pci_walk_bus(bus, pcie_bus_configure_exttags, &legacy_found);
>  }
>  EXPORT_SYMBOL_GPL(pcie_bus_configure_settings);
>  
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH V2] PCI: Do not enable extended tags on pre-dated (v1.x) systems

[Sinan Kaya]

Hi Bjorn,

On 7/10/2017 7:09 PM, Bjorn Helgaas wrote:
> The PCI_EXP_DEVCTL_EXT_TAG bit controls the behavior of the function as a
> Requester.  As far as I can see, it has nothing to do with whether the
> function supports 8-bit tags as a *Completer*, so the implicit assumption
> of the spec is that all Completers always support 8-bit tags.  My guess is
> that's why the ECN thought it would be safe to enable extended tags by
> default.
> 
> If that's the case, this is just a defect in the device (the Completer),
> and we should blacklist it.  Looking at the PCIe Capability version might
> happen to correlate with Completer support for 8-bit tags, but that looks
> like just a coincidence to me.

Sure, I can change to blacklist. I'll not enable it unless the device is blacklisted
via quirks.

Sinan
-- 
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: Do not enable extended tags on pre-dated (v1.x) systems

[Sinan Kaya]

Hi Bjorn,

On 7/10/2017 7:09 PM, Bjorn Helgaas wrote:
> The PCI_EXP_DEVCTL_EXT_TAG bit controls the behavior of the function as a
> Requester.  As far as I can see, it has nothing to do with whether the
> function supports 8-bit tags as a *Completer*, so the implicit assumption
> of the spec is that all Completers always support 8-bit tags.  My guess is
> that's why the ECN thought it would be safe to enable extended tags by
> default.
> 
> If that's the case, this is just a defect in the device (the Completer),
> and we should blacklist it.  Looking at the PCIe Capability version might
> happen to correlate with Completer support for 8-bit tags, but that looks
> like just a coincidence to me.

Sure, I can change to blacklist. I'll not enable it unless the device is blacklisted
via quirks.

Sinan
-- 
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.

Re: [PATCH V2] PCI: Do not enable extended tags on pre-dated (v1.x) systems

[Sinan Kaya]

One more question:

On 7/10/2017 8:20 PM, Sinan Kaya wrote:
> Sure, I can change to blacklist. I'll not enable it unless the device is blacklisted
> via quirks.

Since the quirk is at the completer, are you OK with walking the list and clearing the
extended tags across the tree when at least one device with a quirk is found?

Would you look for another solution?

I was told that Linux assumes peer-to-peer is possible by default on another thread.

-- 
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: Do not enable extended tags on pre-dated (v1.x) systems

[Sinan Kaya]

One more question:

On 7/10/2017 8:20 PM, Sinan Kaya wrote:
> Sure, I can change to blacklist. I'll not enable it unless the device is blacklisted
> via quirks.

Since the quirk is at the completer, are you OK with walking the list and clearing the
extended tags across the tree when at least one device with a quirk is found?

Would you look for another solution?

I was told that Linux assumes peer-to-peer is possible by default on another thread.

-- 
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.