All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH next V4 0/8] Report PCI device link status
@ 2018-03-30  7:14 Tal Gilboa
  2018-03-30  7:14 ` [PATCH next V4 1/8] PCI: Add a query function for PCI device's speed cap Tal Gilboa
                   ` (9 more replies)
  0 siblings, 10 replies; 11+ messages in thread
From: Tal Gilboa @ 2018-03-30  7:14 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Linux PCI, Tariq Toukan, Saeed Mahameed, Keller Jacob E, Tal Gilboa

These patches introduce the ability to report PCI link width
and speed for all PCI devices in kernel log.
This would reduce code duplication between drivers and unify
the approach for reporting PCI link status.
Implemented and tested for Mellanox devices.

v4: Reverted most of v2, meaning re-introduced bandwidth limitation calculation.
Limiting device is now returned from the available PCIe bandwidth calculation
function and is logged in kernel log (suggested by Bjorn Helgaas and Jacob E.
Keller).

Applied PCIe encoding overhead to PCIe bandwidth calculation (available/capable).
this way upper bandwidth limit of the device is more accurate.

v3: Add Reviewed-by: Tariq Toukan to all commits.

v2: Remove chain BW calculation. This might be a nice feature, but
without the ability to know the exact limiting part, it is more
confusing than useful.
Remove warnings for failed PCI query actions, leaving only the status
and gaps from max capabilities report.
Use pci_warn()/pci_info() instead of dev_warn()/dev_info().
(suggested by Bjorn Helgaas).

v1: Split to multiple patches, calculate BW limitation and remove
MACRO definition for LNKCAP shift (suggested by Bjorn Helgaas).
Multiple fixes - conventions, typos, function naming and functional
(suggested by Tariq Toukan).

Tal Gilboa (8):
  PCI: Add a query function for PCI device's speed cap
  PCI: Add a query function for PCI device's width cap
  PCI: Add device link bandwidth capabilities calculation
  PCI: Calculate available bandwidth for PCI devices
  PCI: Print PCI device link status in kernel log
  net/mlx4_core: Report PCI properties using dedicated function
  net/mlx5: Report device PCI link status and issues
  net/mlx5e: Use generic PCI function for bandwidth calculation

 drivers/net/ethernet/mellanox/mlx4/main.c         |  81 +----------
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c |  17 +--
 drivers/net/ethernet/mellanox/mlx5/core/main.c    |   4 +
 drivers/pci/pci-sysfs.c                           |  28 ++--
 drivers/pci/pci.c                                 | 161 +++++++++++++++++++++-
 include/linux/pci.h                               |  24 ++++
 6 files changed, 194 insertions(+), 121 deletions(-)

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH next V4 1/8] PCI: Add a query function for PCI device's speed cap
  2018-03-30  7:14 [PATCH next V4 0/8] Report PCI device link status Tal Gilboa
@ 2018-03-30  7:14 ` Tal Gilboa
  2018-03-30  7:14 ` [PATCH next V4 2/8] PCI: Add a query function for PCI device's width cap Tal Gilboa
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tal Gilboa @ 2018-03-30  7:14 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Linux PCI, Tariq Toukan, Saeed Mahameed, Keller Jacob E, Tal Gilboa

pcie_get_speed_cap() implements the logic for querying a PCI device's
maximum speed capability. Change max_link_speed_show() function to
use pcie_get_speed_cap().

Signed-off-by: Tal Gilboa <talgi@mellanox.com>
Reviewed-by: Tariq Toukan <tariqt@mellanox.com>
---
 drivers/pci/pci-sysfs.c | 22 +++++-----------------
 drivers/pci/pci.c       | 42 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/pci.h     |  7 +++++++
 3 files changed, 54 insertions(+), 17 deletions(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index eb6bee8..69624d3 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -159,29 +159,17 @@ static ssize_t max_link_speed_show(struct device *dev,
 				   struct device_attribute *attr, char *buf)
 {
 	struct pci_dev *pci_dev = to_pci_dev(dev);
-	u32 linkcap;
+	enum pci_bus_speed speed;
+	const char *speed_str;
 	int err;
-	const char *speed;
 
-	err = pcie_capability_read_dword(pci_dev, PCI_EXP_LNKCAP, &linkcap);
+	err = pcie_get_speed_cap(pci_dev, &speed);
 	if (err)
 		return -EINVAL;
 
-	switch (linkcap & PCI_EXP_LNKCAP_SLS) {
-	case PCI_EXP_LNKCAP_SLS_8_0GB:
-		speed = "8 GT/s";
-		break;
-	case PCI_EXP_LNKCAP_SLS_5_0GB:
-		speed = "5 GT/s";
-		break;
-	case PCI_EXP_LNKCAP_SLS_2_5GB:
-		speed = "2.5 GT/s";
-		break;
-	default:
-		speed = "Unknown speed";
-	}
+	speed_str = PCIE_SPEED2STR(speed);
 
-	return sprintf(buf, "%s\n", speed);
+	return sprintf(buf, "%s\n", speed_str);
 }
 static DEVICE_ATTR_RO(max_link_speed);
 
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index f6a4dd1..6fbf170 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5147,6 +5147,48 @@ int pcie_get_minimum_link(struct pci_dev *dev, enum pci_bus_speed *speed,
 EXPORT_SYMBOL(pcie_get_minimum_link);
 
 /**
+ * pcie_get_speed_cap - queries for the PCI device's link speed capability
+ * @dev: PCI device to query
+ * @speed: storage for link speed
+ *
+ * This function queries the PCI device speed capability.
+ */
+int pcie_get_speed_cap(struct pci_dev *dev, enum pci_bus_speed *speed)
+{
+	u32 lnkcap;
+	int err1, err2;
+
+	*speed = PCI_SPEED_UNKNOWN;
+
+	err1 = pcie_capability_read_dword(dev, PCI_EXP_LNKCAP,
+					  &lnkcap);
+	if (!err1 && lnkcap) {
+		if (lnkcap & PCI_EXP_LNKCAP_SLS_8_0GB)
+			*speed = PCIE_SPEED_8_0GT;
+		else if (lnkcap & PCI_EXP_LNKCAP_SLS_5_0GB)
+			*speed = PCIE_SPEED_5_0GT;
+		else if (lnkcap & PCI_EXP_LNKCAP_SLS_2_5GB)
+			*speed = PCIE_SPEED_2_5GT;
+		return 0;
+	}
+
+	err2 = pcie_capability_read_dword(dev, PCI_EXP_LNKCAP2,
+					  &lnkcap);
+	if (!err2 && lnkcap) { /* PCIe r3.0-compliant */
+		if (lnkcap & PCI_EXP_LNKCAP2_SLS_8_0GB)
+			*speed = PCIE_SPEED_8_0GT;
+		else if (lnkcap & PCI_EXP_LNKCAP2_SLS_5_0GB)
+			*speed = PCIE_SPEED_5_0GT;
+		else if (lnkcap & PCI_EXP_LNKCAP2_SLS_2_5GB)
+			*speed = PCIE_SPEED_2_5GT;
+		return 0;
+	}
+
+	return err1 ? err1 : err2;
+}
+EXPORT_SYMBOL(pcie_get_speed_cap);
+
+/**
  * pci_select_bars - Make BAR mask from the type of resource
  * @dev: the PCI device for which BAR mask is made
  * @flags: resource type mask to be selected
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 024a1be..fcc6eb3 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -259,6 +259,12 @@ enum pci_bus_speed {
 	PCI_SPEED_UNKNOWN		= 0xff,
 };
 
+#define PCIE_SPEED2STR(speed) \
+	((speed) == PCIE_SPEED_8_0GT ? "8 GT/s" : \
+	 (speed) == PCIE_SPEED_5_0GT ? "5 GT/s" : \
+	 (speed) == PCIE_SPEED_2_5GT ? "2.5 GT/s" : \
+	 "Unknown speed")
+
 struct pci_cap_saved_data {
 	u16		cap_nr;
 	bool		cap_extended;
@@ -1082,6 +1088,7 @@ static inline int pci_is_managed(struct pci_dev *pdev)
 int pcie_set_mps(struct pci_dev *dev, int mps);
 int pcie_get_minimum_link(struct pci_dev *dev, enum pci_bus_speed *speed,
 			  enum pcie_link_width *width);
+int pcie_get_speed_cap(struct pci_dev *dev, enum pci_bus_speed *speed);
 void pcie_flr(struct pci_dev *dev);
 int __pci_reset_function_locked(struct pci_dev *dev);
 int pci_reset_function(struct pci_dev *dev);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH next V4 2/8] PCI: Add a query function for PCI device's width cap
  2018-03-30  7:14 [PATCH next V4 0/8] Report PCI device link status Tal Gilboa
  2018-03-30  7:14 ` [PATCH next V4 1/8] PCI: Add a query function for PCI device's speed cap Tal Gilboa
@ 2018-03-30  7:14 ` Tal Gilboa
  2018-03-30  7:14 ` [PATCH next V4 3/8] PCI: Add device link bandwidth capabilities calculation Tal Gilboa
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tal Gilboa @ 2018-03-30  7:14 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Linux PCI, Tariq Toukan, Saeed Mahameed, Keller Jacob E, Tal Gilboa

pcie_get_width_cap() implements the logic for querying a PCI device's
maximum width capability. Change max_link_width_show() function to
use pcie_get_width_cap().

Signed-off-by: Tal Gilboa <talgi@mellanox.com>
Reviewed-by: Tariq Toukan <tariqt@mellanox.com>
---
 drivers/pci/pci-sysfs.c |  6 +++---
 drivers/pci/pci.c       | 23 +++++++++++++++++++++++
 include/linux/pci.h     |  1 +
 3 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 69624d3..f4b8867 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -177,14 +177,14 @@ static ssize_t max_link_width_show(struct device *dev,
 				   struct device_attribute *attr, char *buf)
 {
 	struct pci_dev *pci_dev = to_pci_dev(dev);
-	u32 linkcap;
+	enum pcie_link_width width;
 	int err;
 
-	err = pcie_capability_read_dword(pci_dev, PCI_EXP_LNKCAP, &linkcap);
+	err = pcie_get_width_cap(pci_dev, &width);
 	if (err)
 		return -EINVAL;
 
-	return sprintf(buf, "%u\n", (linkcap & PCI_EXP_LNKCAP_MLW) >> 4);
+	return sprintf(buf, "%u\n", width);
 }
 static DEVICE_ATTR_RO(max_link_width);
 
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 6fbf170..90a6cf2 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5189,6 +5189,29 @@ int pcie_get_speed_cap(struct pci_dev *dev, enum pci_bus_speed *speed)
 EXPORT_SYMBOL(pcie_get_speed_cap);
 
 /**
+ * pcie_get_width_cap - queries for the PCI device's link width capability
+ * @dev: PCI device to query
+ * @width: storage for link width
+ *
+ * This function queries the PCI device width capability.
+ */
+int pcie_get_width_cap(struct pci_dev *dev, enum pcie_link_width *width)
+{
+	u32 lnkcap;
+	int err;
+
+	*width = PCIE_LNK_WIDTH_UNKNOWN;
+
+	err = pcie_capability_read_dword(dev, PCI_EXP_LNKCAP, &lnkcap);
+	if (!err && lnkcap)
+		/* Shift start of width mask by 4 to get actual speed cap */
+		*width = (lnkcap & PCI_EXP_LNKCAP_MLW) >> 4;
+
+	return err;
+}
+EXPORT_SYMBOL(pcie_get_width_cap);
+
+/**
  * pci_select_bars - Make BAR mask from the type of resource
  * @dev: the PCI device for which BAR mask is made
  * @flags: resource type mask to be selected
diff --git a/include/linux/pci.h b/include/linux/pci.h
index fcc6eb3..1e3d05f 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1089,6 +1089,7 @@ static inline int pci_is_managed(struct pci_dev *pdev)
 int pcie_get_minimum_link(struct pci_dev *dev, enum pci_bus_speed *speed,
 			  enum pcie_link_width *width);
 int pcie_get_speed_cap(struct pci_dev *dev, enum pci_bus_speed *speed);
+int pcie_get_width_cap(struct pci_dev *dev, enum pcie_link_width *width);
 void pcie_flr(struct pci_dev *dev);
 int __pci_reset_function_locked(struct pci_dev *dev);
 int pci_reset_function(struct pci_dev *dev);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH next V4 3/8] PCI: Add device link bandwidth capabilities calculation
  2018-03-30  7:14 [PATCH next V4 0/8] Report PCI device link status Tal Gilboa
  2018-03-30  7:14 ` [PATCH next V4 1/8] PCI: Add a query function for PCI device's speed cap Tal Gilboa
  2018-03-30  7:14 ` [PATCH next V4 2/8] PCI: Add a query function for PCI device's width cap Tal Gilboa
@ 2018-03-30  7:14 ` Tal Gilboa
  2018-03-30  7:14 ` [PATCH next V4 4/8] PCI: Calculate available bandwidth for PCI devices Tal Gilboa
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tal Gilboa @ 2018-03-30  7:14 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Linux PCI, Tariq Toukan, Saeed Mahameed, Keller Jacob E, Tal Gilboa

Added a function for calculating a PCI device's total link bandwidth
capabilities. This function queries for the device link speed and
width, multiplies them and applies encoding overhead for the different
PCIe generations.

Signed-off-by: Tal Gilboa <talgi@mellanox.com>
Reviewed-by: Tariq Toukan <tariqt@mellanox.com>
---
 drivers/pci/pci.c   | 22 ++++++++++++++++++++++
 include/linux/pci.h | 12 ++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 90a6cf2..553d8f3 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5212,6 +5212,28 @@ int pcie_get_width_cap(struct pci_dev *dev, enum pcie_link_width *width)
 EXPORT_SYMBOL(pcie_get_width_cap);
 
 /**
+ * pcie_bandwidth_capable - Calculates a PCI device's link bandwidth capability
+ * @dev: PCI device
+ * @speed: storage for link speed
+ * @width: storage for link width
+ *
+ * This function caculates a PCI device's link bandwidth by querying for its
+ * link speed and width, multiplying them, and applying encoding overhead.
+ */
+int pcie_bandwidth_capable(struct pci_dev *dev, enum pci_bus_speed *speed,
+			   enum pcie_link_width *width)
+{
+	pcie_get_speed_cap(dev, speed);
+	pcie_get_width_cap(dev, width);
+
+	if (*speed == PCI_SPEED_UNKNOWN || *width == PCIE_LNK_WIDTH_UNKNOWN)
+		return 0;
+
+	return (*width) * PCIE_SPEED2MBS_ENC(*speed);
+}
+EXPORT_SYMBOL(pcie_bandwidth_capable);
+
+/**
  * pci_select_bars - Make BAR mask from the type of resource
  * @dev: the PCI device for which BAR mask is made
  * @flags: resource type mask to be selected
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 1e3d05f..9f57c45 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -265,6 +265,16 @@ enum pci_bus_speed {
 	 (speed) == PCIE_SPEED_2_5GT ? "2.5 GT/s" : \
 	 "Unknown speed")
 
+/**
+ * PCIe speed to Mb/s with encoding overhead:
+ * 20% for gen2, ~1.5% for gen3
+ */
+#define PCIE_SPEED2MBS_ENC(speed) \
+	((speed) == PCIE_SPEED_8_0GT ? 7877 : \
+	 (speed) == PCIE_SPEED_5_0GT ? 4000 : \
+	 (speed) == PCIE_SPEED_2_5GT ? 2000 : \
+	 0)
+
 struct pci_cap_saved_data {
 	u16		cap_nr;
 	bool		cap_extended;
@@ -1090,6 +1100,8 @@ int pcie_get_minimum_link(struct pci_dev *dev, enum pci_bus_speed *speed,
 			  enum pcie_link_width *width);
 int pcie_get_speed_cap(struct pci_dev *dev, enum pci_bus_speed *speed);
 int pcie_get_width_cap(struct pci_dev *dev, enum pcie_link_width *width);
+int pcie_bandwidth_capable(struct pci_dev *dev, enum pci_bus_speed *speed,
+			   enum pcie_link_width *width);
 void pcie_flr(struct pci_dev *dev);
 int __pci_reset_function_locked(struct pci_dev *dev);
 int pci_reset_function(struct pci_dev *dev);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH next V4 4/8] PCI: Calculate available bandwidth for PCI devices
  2018-03-30  7:14 [PATCH next V4 0/8] Report PCI device link status Tal Gilboa
                   ` (2 preceding siblings ...)
  2018-03-30  7:14 ` [PATCH next V4 3/8] PCI: Add device link bandwidth capabilities calculation Tal Gilboa
@ 2018-03-30  7:14 ` Tal Gilboa
  2018-03-30  7:14 ` [PATCH next V4 5/8] PCI: Print PCI device link status in kernel log Tal Gilboa
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tal Gilboa @ 2018-03-30  7:14 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Linux PCI, Tariq Toukan, Saeed Mahameed, Keller Jacob E, Tal Gilboa

pcie_bandwidth_available() function, which is based on
pcie_get_minimum_link(), iterates over the PCI chain and calculates
available bandwidth in addition to minimum speed and width. The
bandwidth calculation at each level is encoding * speed * width, so
even if, for instance, a level has lower width than the device max
capabilities, it still might not cause a bandwidth limitation if it
has a higher speed.
The function also returns the device causing the limitation.
pcie_get_minimum_link() is kept for compatibility.

Signed-off-by: Tal Gilboa <talgi@mellanox.com>
Reviewed-by: Tariq Toukan <tariqt@mellanox.com>
---
 drivers/pci/pci.c   | 45 ++++++++++++++++++++++++++++++++++++++-------
 include/linux/pci.h |  3 +++
 2 files changed, 41 insertions(+), 7 deletions(-)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 553d8f3..fb79ff6 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5109,25 +5109,48 @@ int pcie_set_mps(struct pci_dev *dev, int mps)
  * @speed: storage for minimum speed
  * @width: storage for minimum width
  *
- * This function will walk up the PCI device chain and determine the minimum
- * link width and speed of the device.
+ * This function use pcie_bandwidth_available() for determining the minimum
+ * link width and speed of the device. Legacy code is kept for compatibility.
  */
 int pcie_get_minimum_link(struct pci_dev *dev, enum pci_bus_speed *speed,
 			  enum pcie_link_width *width)
 {
-	int ret;
+	int bw;
+
+	return pcie_bandwidth_available(dev, speed, width, &bw, NULL);
+}
+EXPORT_SYMBOL(pcie_get_minimum_link);
+
+/**
+ * pcie_bandwidth_available - determine minimum link settings of a PCIe
+			      device and its bandwidth limitation
+ * @dev: PCI device to query
+ * @speed: storage for minimum speed
+ * @width: storage for minimum width
+ * @bw: storage for link bandwidth
+ * @limiting_dev: storage for device causing the bandwidth limitation
+ *
+ * This function walks up the PCI device chain and determines the minimum width,
+ * minimum speed and available bandwidth of the device.
+ */
+int pcie_bandwidth_available(struct pci_dev *dev, enum pci_bus_speed *speed,
+			     enum pcie_link_width *width, int *bw,
+			     struct pci_dev **limiting_dev)
+{
+	int err;
 
 	*speed = PCI_SPEED_UNKNOWN;
 	*width = PCIE_LNK_WIDTH_UNKNOWN;
+	*bw = 0;
 
 	while (dev) {
 		u16 lnksta;
 		enum pci_bus_speed next_speed;
 		enum pcie_link_width next_width;
 
-		ret = pcie_capability_read_word(dev, PCI_EXP_LNKSTA, &lnksta);
-		if (ret)
-			return ret;
+		err = pcie_capability_read_word(dev, PCI_EXP_LNKSTA, &lnksta);
+		if (err)
+			return err;
 
 		next_speed = pcie_link_speed[lnksta & PCI_EXP_LNKSTA_CLS];
 		next_width = (lnksta & PCI_EXP_LNKSTA_NLW) >>
@@ -5139,12 +5162,20 @@ int pcie_get_minimum_link(struct pci_dev *dev, enum pci_bus_speed *speed,
 		if (next_width < *width)
 			*width = next_width;
 
+		/* Check if current device limits the total bandwidth */
+		if (!(*bw) ||
+		    (*bw > next_width * PCIE_SPEED2MBS_ENC(next_speed))) {
+			if (limiting_dev)
+				*limiting_dev = dev;
+			*bw = next_width * PCIE_SPEED2MBS_ENC(next_speed);
+		}
+
 		dev = dev->bus->self;
 	}
 
 	return 0;
 }
-EXPORT_SYMBOL(pcie_get_minimum_link);
+EXPORT_SYMBOL(pcie_bandwidth_available);
 
 /**
  * pcie_get_speed_cap - queries for the PCI device's link speed capability
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 9f57c45..585cea1 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1098,6 +1098,9 @@ static inline int pci_is_managed(struct pci_dev *pdev)
 int pcie_set_mps(struct pci_dev *dev, int mps);
 int pcie_get_minimum_link(struct pci_dev *dev, enum pci_bus_speed *speed,
 			  enum pcie_link_width *width);
+int pcie_bandwidth_available(struct pci_dev *dev, enum pci_bus_speed *speed,
+			     enum pcie_link_width *width, int *bw,
+			     struct pci_dev **limiting_dev);
 int pcie_get_speed_cap(struct pci_dev *dev, enum pci_bus_speed *speed);
 int pcie_get_width_cap(struct pci_dev *dev, enum pcie_link_width *width);
 int pcie_bandwidth_capable(struct pci_dev *dev, enum pci_bus_speed *speed,
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH next V4 5/8] PCI: Print PCI device link status in kernel log
  2018-03-30  7:14 [PATCH next V4 0/8] Report PCI device link status Tal Gilboa
                   ` (3 preceding siblings ...)
  2018-03-30  7:14 ` [PATCH next V4 4/8] PCI: Calculate available bandwidth for PCI devices Tal Gilboa
@ 2018-03-30  7:14 ` Tal Gilboa
  2018-03-30  7:14 ` [PATCH next V4 6/8] net/mlx4_core: Report PCI properties using dedicated function Tal Gilboa
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tal Gilboa @ 2018-03-30  7:14 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Linux PCI, Tariq Toukan, Saeed Mahameed, Keller Jacob E, Tal Gilboa

Add pcie_print_link_status() function for querying and verifying
a PCI device link status. The PCI speed and width are reported
in kernel log. If there's a bandwidth limitation caused by a device
in the PCIe chain, a relevant entry would be added to kernel log.

This provides a unified method for all PCI devices to
report status and issues, instead of each device reporting in a
different way, using different code.

Signed-off-by: Tal Gilboa <talgi@mellanox.com>
Reviewed-by: Tariq Toukan <tariqt@mellanox.com>
---
 drivers/pci/pci.c   | 29 +++++++++++++++++++++++++++++
 include/linux/pci.h |  1 +
 2 files changed, 30 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index fb79ff6..bd8aa64 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -5265,6 +5265,35 @@ int pcie_bandwidth_capable(struct pci_dev *dev, enum pci_bus_speed *speed,
 EXPORT_SYMBOL(pcie_bandwidth_capable);
 
 /**
+ * pcie_print_link_status - Reports the PCI device's link speed and width.
+ * @dev: PCI device to query
+ *
+ * This function checks whether the PCI device current speed and width are equal
+ * to the maximum PCI device capabilities.
+ */
+void pcie_print_link_status(struct pci_dev *dev)
+{
+	enum pcie_link_width width, width_cap;
+	struct pci_dev *limiting_dev = NULL;
+	enum pci_bus_speed speed, speed_cap;
+	int bw, bw_cap;
+
+	bw_cap = pcie_bandwidth_capable(dev, &speed_cap, &width_cap);
+	pcie_bandwidth_available(dev, &speed, &width, &bw, &limiting_dev);
+
+	if (bw >= bw_cap)
+		pci_info(dev, "%d Mb/s available bandwidth (%s x%d link)\n",
+			 bw, PCIE_SPEED2STR(speed), width);
+	else
+		pci_info(dev, "%d Mb/s available bandwidth (capable of %d Mb/s, %s x%d link)\n",
+			 bw, bw_cap, PCIE_SPEED2STR(speed_cap), width_cap);
+	if (limiting_dev && strcmp(pci_name(limiting_dev), pci_name(dev)))
+		pci_info(dev, "Bandwidth limited by device at %s\n",
+			 pci_name(limiting_dev));
+}
+EXPORT_SYMBOL(pcie_print_link_status);
+
+/**
  * pci_select_bars - Make BAR mask from the type of resource
  * @dev: the PCI device for which BAR mask is made
  * @flags: resource type mask to be selected
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 585cea1..1a672c9 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1105,6 +1105,7 @@ int pcie_bandwidth_available(struct pci_dev *dev, enum pci_bus_speed *speed,
 int pcie_get_width_cap(struct pci_dev *dev, enum pcie_link_width *width);
 int pcie_bandwidth_capable(struct pci_dev *dev, enum pci_bus_speed *speed,
 			   enum pcie_link_width *width);
+void pcie_print_link_status(struct pci_dev *dev);
 void pcie_flr(struct pci_dev *dev);
 int __pci_reset_function_locked(struct pci_dev *dev);
 int pci_reset_function(struct pci_dev *dev);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH next V4 6/8] net/mlx4_core: Report PCI properties using dedicated function
  2018-03-30  7:14 [PATCH next V4 0/8] Report PCI device link status Tal Gilboa
                   ` (4 preceding siblings ...)
  2018-03-30  7:14 ` [PATCH next V4 5/8] PCI: Print PCI device link status in kernel log Tal Gilboa
@ 2018-03-30  7:14 ` Tal Gilboa
  2018-03-30  7:14 ` [PATCH next V4 7/8] net/mlx5: Report device PCI link status and issues Tal Gilboa
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tal Gilboa @ 2018-03-30  7:14 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Linux PCI, Tariq Toukan, Saeed Mahameed, Keller Jacob E, Tal Gilboa

Change mlx4 method of checking and reporting PCI status and
maximum capabilities to use the pci driver functions instead
of implementing them in the driver code.

Signed-off-by: Tal Gilboa <talgi@mellanox.com>
Signed-off-by: Tariq Toukan <tariqt@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx4/main.c | 81 +------------------------------
 1 file changed, 1 insertion(+), 80 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index 5a26851..e8c6707 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -623,85 +623,6 @@ static int mlx4_dev_cap(struct mlx4_dev *dev, struct mlx4_dev_cap *dev_cap)
 	return 0;
 }
 
-static int mlx4_get_pcie_dev_link_caps(struct mlx4_dev *dev,
-				       enum pci_bus_speed *speed,
-				       enum pcie_link_width *width)
-{
-	u32 lnkcap1, lnkcap2;
-	int err1, err2;
-
-#define  PCIE_MLW_CAP_SHIFT 4	/* start of MLW mask in link capabilities */
-
-	*speed = PCI_SPEED_UNKNOWN;
-	*width = PCIE_LNK_WIDTH_UNKNOWN;
-
-	err1 = pcie_capability_read_dword(dev->persist->pdev, PCI_EXP_LNKCAP,
-					  &lnkcap1);
-	err2 = pcie_capability_read_dword(dev->persist->pdev, PCI_EXP_LNKCAP2,
-					  &lnkcap2);
-	if (!err2 && lnkcap2) { /* PCIe r3.0-compliant */
-		if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_8_0GB)
-			*speed = PCIE_SPEED_8_0GT;
-		else if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_5_0GB)
-			*speed = PCIE_SPEED_5_0GT;
-		else if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_2_5GB)
-			*speed = PCIE_SPEED_2_5GT;
-	}
-	if (!err1) {
-		*width = (lnkcap1 & PCI_EXP_LNKCAP_MLW) >> PCIE_MLW_CAP_SHIFT;
-		if (!lnkcap2) { /* pre-r3.0 */
-			if (lnkcap1 & PCI_EXP_LNKCAP_SLS_5_0GB)
-				*speed = PCIE_SPEED_5_0GT;
-			else if (lnkcap1 & PCI_EXP_LNKCAP_SLS_2_5GB)
-				*speed = PCIE_SPEED_2_5GT;
-		}
-	}
-
-	if (*speed == PCI_SPEED_UNKNOWN || *width == PCIE_LNK_WIDTH_UNKNOWN) {
-		return err1 ? err1 :
-			err2 ? err2 : -EINVAL;
-	}
-	return 0;
-}
-
-static void mlx4_check_pcie_caps(struct mlx4_dev *dev)
-{
-	enum pcie_link_width width, width_cap;
-	enum pci_bus_speed speed, speed_cap;
-	int err;
-
-#define PCIE_SPEED_STR(speed) \
-	(speed == PCIE_SPEED_8_0GT ? "8.0GT/s" : \
-	 speed == PCIE_SPEED_5_0GT ? "5.0GT/s" : \
-	 speed == PCIE_SPEED_2_5GT ? "2.5GT/s" : \
-	 "Unknown")
-
-	err = mlx4_get_pcie_dev_link_caps(dev, &speed_cap, &width_cap);
-	if (err) {
-		mlx4_warn(dev,
-			  "Unable to determine PCIe device BW capabilities\n");
-		return;
-	}
-
-	err = pcie_get_minimum_link(dev->persist->pdev, &speed, &width);
-	if (err || speed == PCI_SPEED_UNKNOWN ||
-	    width == PCIE_LNK_WIDTH_UNKNOWN) {
-		mlx4_warn(dev,
-			  "Unable to determine PCI device chain minimum BW\n");
-		return;
-	}
-
-	if (width != width_cap || speed != speed_cap)
-		mlx4_warn(dev,
-			  "PCIe BW is different than device's capability\n");
-
-	mlx4_info(dev, "PCIe link speed is %s, device supports %s\n",
-		  PCIE_SPEED_STR(speed), PCIE_SPEED_STR(speed_cap));
-	mlx4_info(dev, "PCIe link width is x%d, device supports x%d\n",
-		  width, width_cap);
-	return;
-}
-
 /*The function checks if there are live vf, return the num of them*/
 static int mlx4_how_many_lives_vf(struct mlx4_dev *dev)
 {
@@ -3475,7 +3396,7 @@ static int mlx4_load_one(struct pci_dev *pdev, int pci_dev_data,
 	 * express device capabilities are under-satisfied by the bus.
 	 */
 	if (!mlx4_is_slave(dev))
-		mlx4_check_pcie_caps(dev);
+		pcie_print_link_status(dev->persist->pdev);
 
 	/* In master functions, the communication channel must be initialized
 	 * after obtaining its address from fw */
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH next V4 7/8] net/mlx5: Report device PCI link status and issues
  2018-03-30  7:14 [PATCH next V4 0/8] Report PCI device link status Tal Gilboa
                   ` (5 preceding siblings ...)
  2018-03-30  7:14 ` [PATCH next V4 6/8] net/mlx4_core: Report PCI properties using dedicated function Tal Gilboa
@ 2018-03-30  7:14 ` Tal Gilboa
  2018-03-30  7:14 ` [PATCH next V4 8/8] net/mlx5e: Use generic PCI function for bandwidth calculation Tal Gilboa
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 11+ messages in thread
From: Tal Gilboa @ 2018-03-30  7:14 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Linux PCI, Tariq Toukan, Saeed Mahameed, Keller Jacob E, Tal Gilboa

Add a kernel log print for mlx5 PCI device's link status.

Signed-off-by: Tal Gilboa <talgi@mellanox.com>
Reviewed-by: Tariq Toukan <tariqt@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index 808091d..b865597 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -1045,6 +1045,10 @@ static int mlx5_load_one(struct mlx5_core_dev *dev, struct mlx5_priv *priv,
 	dev_info(&pdev->dev, "firmware version: %d.%d.%d\n", fw_rev_maj(dev),
 		 fw_rev_min(dev), fw_rev_sub(dev));
 
+	/* Only PFs hold the relevant PCIe information for this query */
+	if (mlx5_core_is_pf(dev))
+		pcie_print_link_status(dev->pdev);
+
 	/* on load removing any previous indication of internal error, device is
 	 * up
 	 */
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH next V4 8/8] net/mlx5e: Use generic PCI function for bandwidth calculation
  2018-03-30  7:14 [PATCH next V4 0/8] Report PCI device link status Tal Gilboa
                   ` (6 preceding siblings ...)
  2018-03-30  7:14 ` [PATCH next V4 7/8] net/mlx5: Report device PCI link status and issues Tal Gilboa
@ 2018-03-30  7:14 ` Tal Gilboa
  2018-03-30 16:29 ` [PATCH next V4 0/8] Report PCI device link status Keller, Jacob E
  2018-03-30 20:36 ` Bjorn Helgaas
  9 siblings, 0 replies; 11+ messages in thread
From: Tal Gilboa @ 2018-03-30  7:14 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Linux PCI, Tariq Toukan, Saeed Mahameed, Keller Jacob E, Tal Gilboa

Newly introduced pci_bandwidth_available() function calculates
maximum available bandwidth through the PCI chain. We can use this
value for mlx5e_get_pci_bw() instead of calculating ourselves.
This is mainly used for detecting cases on which PCIe bandwidth
can't support current link speed and the driver need to act accordingly.
By taking PCIe encoding into account, this calculation is even more
accurate.

Signed-off-by: Tal Gilboa <talgi@mellanox.com>
Reviewed-by: Tariq Toukan <tariqt@mellanox.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 4a675f1..fc51a73 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -4011,27 +4011,16 @@ static int mlx5e_get_pci_bw(struct mlx5_core_dev *mdev, u32 *pci_bw)
 	enum pcie_link_width width;
 	enum pci_bus_speed speed;
 	int err = 0;
+	int bw;
 
-	err = pcie_get_minimum_link(mdev->pdev, &speed, &width);
+	err = pcie_bandwidth_available(mdev->pdev, &speed, &width, &bw, NULL);
 	if (err)
 		return err;
 
 	if (speed == PCI_SPEED_UNKNOWN || width == PCIE_LNK_WIDTH_UNKNOWN)
 		return -EINVAL;
 
-	switch (speed) {
-	case PCIE_SPEED_2_5GT:
-		*pci_bw = 2500 * width;
-		break;
-	case PCIE_SPEED_5_0GT:
-		*pci_bw = 5000 * width;
-		break;
-	case PCIE_SPEED_8_0GT:
-		*pci_bw = 8000 * width;
-		break;
-	default:
-		return -EINVAL;
-	}
+	*pci_bw = bw;
 
 	return 0;
 }
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* RE: [PATCH next V4 0/8] Report PCI device link status
  2018-03-30  7:14 [PATCH next V4 0/8] Report PCI device link status Tal Gilboa
                   ` (7 preceding siblings ...)
  2018-03-30  7:14 ` [PATCH next V4 8/8] net/mlx5e: Use generic PCI function for bandwidth calculation Tal Gilboa
@ 2018-03-30 16:29 ` Keller, Jacob E
  2018-03-30 20:36 ` Bjorn Helgaas
  9 siblings, 0 replies; 11+ messages in thread
From: Keller, Jacob E @ 2018-03-30 16:29 UTC (permalink / raw)
  To: Tal Gilboa, Bjorn Helgaas; +Cc: Linux PCI, Tariq Toukan, Saeed Mahameed



> -----Original Message-----
> From: Tal Gilboa [mailto:talgi@mellanox.com]
> Sent: Friday, March 30, 2018 12:15 AM
> To: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Linux PCI <linux-pci@vger.kernel.org>; Tariq Toukan <tariqt@mellanox.com>;
> Saeed Mahameed <saeedm@mellanox.com>; Keller, Jacob E
> <jacob.e.keller@intel.com>; Tal Gilboa <talgi@mellanox.com>
> Subject: [PATCH next V4 0/8] Report PCI device link status
> 
> These patches introduce the ability to report PCI link width
> and speed for all PCI devices in kernel log.
> This would reduce code duplication between drivers and unify
> the approach for reporting PCI link status.
> Implemented and tested for Mellanox devices.
> 
> v4: Reverted most of v2, meaning re-introduced bandwidth limitation calculation.
> Limiting device is now returned from the available PCIe bandwidth calculation
> function and is logged in kernel log (suggested by Bjorn Helgaas and Jacob E.
> Keller).
> 
> Applied PCIe encoding overhead to PCIe bandwidth calculation
> (available/capable).
> this way upper bandwidth limit of the device is more accurate.
> 

I didn't find any objections to the series. We can probably remove the old function in the future once no one uses it anymore.

Acked-by: Jacob Keller <jacob.e.keller@intel.com>

> v3: Add Reviewed-by: Tariq Toukan to all commits.
> 
> v2: Remove chain BW calculation. This might be a nice feature, but
> without the ability to know the exact limiting part, it is more
> confusing than useful.
> Remove warnings for failed PCI query actions, leaving only the status
> and gaps from max capabilities report.
> Use pci_warn()/pci_info() instead of dev_warn()/dev_info().
> (suggested by Bjorn Helgaas).
> 
> v1: Split to multiple patches, calculate BW limitation and remove
> MACRO definition for LNKCAP shift (suggested by Bjorn Helgaas).
> Multiple fixes - conventions, typos, function naming and functional
> (suggested by Tariq Toukan).
> 
> Tal Gilboa (8):
>   PCI: Add a query function for PCI device's speed cap
>   PCI: Add a query function for PCI device's width cap
>   PCI: Add device link bandwidth capabilities calculation
>   PCI: Calculate available bandwidth for PCI devices
>   PCI: Print PCI device link status in kernel log
>   net/mlx4_core: Report PCI properties using dedicated function
>   net/mlx5: Report device PCI link status and issues
>   net/mlx5e: Use generic PCI function for bandwidth calculation
> 
>  drivers/net/ethernet/mellanox/mlx4/main.c         |  81 +----------
>  drivers/net/ethernet/mellanox/mlx5/core/en_main.c |  17 +--
>  drivers/net/ethernet/mellanox/mlx5/core/main.c    |   4 +
>  drivers/pci/pci-sysfs.c                           |  28 ++--
>  drivers/pci/pci.c                                 | 161 +++++++++++++++++++++-
>  include/linux/pci.h                               |  24 ++++
>  6 files changed, 194 insertions(+), 121 deletions(-)
> 
> --
> 1.8.3.1

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH next V4 0/8] Report PCI device link status
  2018-03-30  7:14 [PATCH next V4 0/8] Report PCI device link status Tal Gilboa
                   ` (8 preceding siblings ...)
  2018-03-30 16:29 ` [PATCH next V4 0/8] Report PCI device link status Keller, Jacob E
@ 2018-03-30 20:36 ` Bjorn Helgaas
  9 siblings, 0 replies; 11+ messages in thread
From: Bjorn Helgaas @ 2018-03-30 20:36 UTC (permalink / raw)
  To: Tal Gilboa
  Cc: Bjorn Helgaas, Linux PCI, Tariq Toukan, Saeed Mahameed, Keller Jacob E

On Fri, Mar 30, 2018 at 10:14:38AM +0300, Tal Gilboa wrote:
> These patches introduce the ability to report PCI link width
> and speed for all PCI devices in kernel log.
> This would reduce code duplication between drivers and unify
> the approach for reporting PCI link status.
> Implemented and tested for Mellanox devices.

This is awesome.

I reworked a few things and converted the remaining users of
pcie_get_minimum_link().  I'll post the result as a V5 of this series and
you can see what you think.  Hopefully I didn't break too many things.

Bjorn

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-03-30 20:36 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-30  7:14 [PATCH next V4 0/8] Report PCI device link status Tal Gilboa
2018-03-30  7:14 ` [PATCH next V4 1/8] PCI: Add a query function for PCI device's speed cap Tal Gilboa
2018-03-30  7:14 ` [PATCH next V4 2/8] PCI: Add a query function for PCI device's width cap Tal Gilboa
2018-03-30  7:14 ` [PATCH next V4 3/8] PCI: Add device link bandwidth capabilities calculation Tal Gilboa
2018-03-30  7:14 ` [PATCH next V4 4/8] PCI: Calculate available bandwidth for PCI devices Tal Gilboa
2018-03-30  7:14 ` [PATCH next V4 5/8] PCI: Print PCI device link status in kernel log Tal Gilboa
2018-03-30  7:14 ` [PATCH next V4 6/8] net/mlx4_core: Report PCI properties using dedicated function Tal Gilboa
2018-03-30  7:14 ` [PATCH next V4 7/8] net/mlx5: Report device PCI link status and issues Tal Gilboa
2018-03-30  7:14 ` [PATCH next V4 8/8] net/mlx5e: Use generic PCI function for bandwidth calculation Tal Gilboa
2018-03-30 16:29 ` [PATCH next V4 0/8] Report PCI device link status Keller, Jacob E
2018-03-30 20:36 ` Bjorn Helgaas

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.