[PATCH] pci: aspm: Add segment support

Jeffrey Hugo jhugo at codeaurora.org
Tue Sep 6 19:42:30 UTC 2016


PCI segement groups are the first level of relationship between PCI devices
in a system.  PCI segments allow bus numbers to be reused as a particular
bus ID must be unique only within the segment upon which it resides.

Segment support is important as a system may have multiple root ports and
attached devices across multiple segements, with different ASPM settings
for each port/device set.  If the test is not aware of segments, it may
incorrectly match a root port from segment A with a device from segment B
and cause a test failure if that port and device happen to not have
matching ASPM settings.

Change-Id: I2aea72d12ca1dc945a4f73cf5cc652c6ffc62654
Signed-off-by: Jeffrey Hugo <jhugo at codeaurora.org>
---
 src/pci/aspm/aspm.c | 36 ++++++++++++++++++++----------------
 1 file changed, 20 insertions(+), 16 deletions(-)

diff --git a/src/pci/aspm/aspm.c b/src/pci/aspm/aspm.c
index 9c3c143..72bee44 100644
--- a/src/pci/aspm/aspm.c
+++ b/src/pci/aspm/aspm.c
@@ -108,29 +108,29 @@ static int pcie_compare_rp_dev_aspm_registers(fwts_framework *fw,
 
 	if (((rp_cap->link_cap & FWTS_PCIE_ASPM_SUPPORT_L0_FIELD) >> 10) !=
 		(rp_cap->link_contrl & FWTS_PCIE_ASPM_CONTROL_L0_FIELD)) {
-		fwts_warning(fw, "RP %02Xh:%02Xh.%02Xh L0s not enabled.",
-			 rp->bus, rp->dev, rp->func);
+		fwts_warning(fw, "RP %04Xh:%02Xh:%02Xh.%02Xh L0s not enabled.",
+			 rp->segment, rp->bus, rp->dev, rp->func);
 		l0s_disabled = true;
 	}
 
 	if (((rp_cap->link_cap & FWTS_PCIE_ASPM_SUPPORT_L1_FIELD) >> 10) !=
 		(rp_cap->link_contrl & FWTS_PCIE_ASPM_CONTROL_L1_FIELD)) {
-		fwts_warning(fw, "RP %02Xh:%02Xh.%02Xh L1 not enabled.",
-			rp->bus, rp->dev, rp->func);
+		fwts_warning(fw, "RP %04Xh:%02Xh:%02Xh.%02Xh L1 not enabled.",
+			rp->segment, rp->bus, rp->dev, rp->func);
 		l1_disabled = true;
 	}
 
 	if (((device_cap->link_cap & FWTS_PCIE_ASPM_SUPPORT_L0_FIELD) >> 10) !=
 		(device_cap->link_contrl & FWTS_PCIE_ASPM_CONTROL_L0_FIELD)) {
-		fwts_warning(fw, "Device %02Xh:%02Xh.%02Xh L0s not enabled.",
-			dev->bus, dev->dev, dev->func);
+		fwts_warning(fw, "Device %04Xh:%02Xh:%02Xh.%02Xh L0s not enabled.",
+			dev->segment, dev->bus, dev->dev, dev->func);
 		l0s_disabled = true;
 	}
 
 	if (((device_cap->link_cap & FWTS_PCIE_ASPM_SUPPORT_L1_FIELD) >> 10) !=
 		(device_cap->link_contrl & FWTS_PCIE_ASPM_CONTROL_L1_FIELD)) {
-		fwts_warning(fw, "Device %02Xh:%02Xh.%02Xh L1 not enabled.",
-			dev->bus, dev->dev, dev->func);
+		fwts_warning(fw, "Device %04Xh:%02Xh:%02Xh.%02Xh L1 not enabled.",
+			dev->segment, dev->bus, dev->dev, dev->func);
 		l1_disabled = true;
 	}
 
@@ -157,10 +157,10 @@ static int pcie_compare_rp_dev_aspm_registers(fwts_framework *fw,
 	if (rp_aspm_cntrl != device_aspm_cntrl) {
 		fwts_failed(fw, LOG_LEVEL_MEDIUM, "PCIEASPM_Unmatched",
 			"PCIe ASPM setting was not matched.");
-		fwts_log_error(fw, "RP %02Xh:%02Xh.%02Xh has ASPM = %02Xh."
-			, rp->bus, rp->dev, rp->func, rp_aspm_cntrl);
-		fwts_log_error(fw, "Device %02Xh:%02Xh.%02Xh has ASPM = %02Xh.",
-			dev->bus, dev->dev, dev->func, device_aspm_cntrl);
+		fwts_log_error(fw, "RP %04Xh:%02Xh:%02Xh.%02Xh has ASPM = %02Xh."
+			, rp->segment, rp->bus, rp->dev, rp->func, rp_aspm_cntrl);
+		fwts_log_error(fw, "Device %04Xh:%02Xh:%02Xh.%02Xh has ASPM = %02Xh.",
+			dev->segment, dev->bus, dev->dev, dev->func, device_aspm_cntrl);
 		fwts_advice(fw,
 			"ASPM control registers between root port and device "
 			"must match in order for ASPM to be active. Unmatched "
@@ -189,11 +189,12 @@ static int pcie_check_aspm_registers(fwts_framework *fw)
 	}
 	while ((entry = readdir(dirp)) != NULL) {
 		uint8_t bus, dev, func;
+		uint16_t segment;
 
 		if (entry->d_name[0] == '.')
 			continue;
 
-		if (sscanf(entry->d_name, "%*x:%" SCNx8 ":%" SCNx8 ".%" SCNx8, &bus, &dev, &func) == 3) {
+		if (sscanf(entry->d_name, "%" SCNx16 ":%" SCNx8 ":%" SCNx8 ".%" SCNx8, &segment, &bus, &dev, &func) == 4) {
 			int fd;
 			char path[PATH_MAX];
 			struct pci_device *device;
@@ -204,6 +205,7 @@ static int pcie_check_aspm_registers(fwts_framework *fw)
 				closedir(dirp);
 				return FWTS_ERROR;
 			}
+			device->segment = segment;
 			device->bus = bus;
 			device->dev = dev;
 			device->func = func;
@@ -235,9 +237,11 @@ static int pcie_check_aspm_registers(fwts_framework *fw)
 		if (cur->config[FWTS_PCI_CONFIG_HEADER_TYPE] & 0x01) {
 			for (ltarget = dev_list.head; ltarget; ltarget = ltarget->next) {
 				struct pci_device *target = (struct pci_device *)ltarget->data;
-				if (target->bus == cur->config[FWTS_PCI_CONFIG_TYPE1_SECONDARY_BUS_NUMBER]) {
-					pcie_compare_rp_dev_aspm_registers(fw, cur, target);
-					break;
+				if (target->segment == cur->segment) {
+					if (target->bus == cur->config[FWTS_PCI_CONFIG_TYPE1_SECONDARY_BUS_NUMBER]) {
+						pcie_compare_rp_dev_aspm_registers(fw, cur, target);
+						break;
+					}
 				}
 			}
 		}
-- 
Qualcomm Datacenter Technologies as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the
Code Aurora Forum, a Linux Foundation Collaborative Project.




More information about the fwts-devel mailing list