linux-kernel-mentees.lists.linuxfoundation.org archive mirror
 help / color / mirror / Atom feed
* [Linux-kernel-mentees] [PATCH 0/14 v4] PCI: Remove '*val = 0' from pcie_capability_read_*()
@ 2020-07-14 11:04 Saheed Olayemi Bolarinwa
  2020-07-14 11:04 ` [Linux-kernel-mentees] [PATCH 1/14 v4] IB/hfi1: Check the return value of pcie_capability_read_*() Saheed Olayemi Bolarinwa
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Saheed Olayemi Bolarinwa @ 2020-07-14 11:04 UTC (permalink / raw)
  To: helgaas
  Cc: Mike Marciniszyn, linux-rdma, Bolarinwa Olayemi Saheed,
	Dennis Dalessandro, linux-kernel, Jason Gunthorpe, Doug Ledford,
	linux-pci, linux-kernel-mentees

From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>

v4 CHANGES:
- Remove unnecessary boolean conversion
- fix bugs introduced by previous version in PATCH 11/14

v3 CHANGES:
- Split previous PATCH 6/13 into two : PATCH 6/14 and PATCH 7/14
- Fix commit message of PATCH 5/14
- Update Patch numbering and Commit messages
- Add 'Acked by Greg KH' to PATCH 2/14
- Add PATCH version

v2 CHANGES:
- Fix missing comma, causing the email cc error
- Fix typos and numbering errors in commit messages
- Add commit message to 13/13
- Add two more patches: PATCH 3/13 and PATCH 4/13

MERGING:
Patch 7/14 depends on Patch 6/14. However Patch 6/14 has no dependency.
Please, merge PATCH 7/14 only after Patch 6/14.
Patch 14/14 depend on all preceeding patchs. Except for Patch 6/14 and
Patch 7/14, all other patches are independent of one another. Hence,
please merge Patch 14/14 only after other patches in this series have
been merged.


PATCH 6/14:
Make the function set status to "Power On" by default and only set to
Set "Power Off" only if pcie_capability_read_word() is successful and
(slot_ctrl & PCI_EXP_SLTCTL_PCC) == PCI_EXP_SLTCTL_PWR_OFF. 

PATCH 1/14 to 13/14:
Check the return value of pcie_capability_read_*() to ensure success or
confirm failure. While maintaining these functions, this ensures that the
changes in PATCH 14/14 does not introduce any bug. 

PATCH 14/14:
There are several reasons why a PCI capability read may fail whether the
device is present or not. If this happens, pcie_capability_read_*() will
return -EINVAL/PCIBIOS_BAD_REGISTER_NUMBER or PCIBIOS_DEVICE_NOT_FOUND
and *val is set to 0.

This behaviour if further ensured by this code inside
pcie_capability_read_*()

 ret = pci_read_config_dword(dev, pci_pcie_cap(dev) + pos, val);
 /*
  * Reset *val to 0 if pci_read_config_dword() fails, it may
  * have been written as 0xFFFFFFFF if hardware error happens
  * during pci_read_config_dword().
  */
 if (ret)
	 *val = 0;
 return ret;

a) Since all pci_generic_config_read() does is read a register value,
it may return success after reading a ~0 which *may* have been fabricated
by the PCI host bridge due to a read timeout. Hence pci_read_config_*() 
will return success with a fabricated ~0 in *val, indicating a problem.
In this case, the assumed behaviour of  pcie_capability_read_*() will be
wrong. To avoid error slipping through, more checks are necessary.

b) pci_read_config_*() will return PCIBIOS_DEVICE_NOT_FOUND only if 
dev->error_state = pci_channel_io_perm_failure (i.e. 
pci_dev_is_disconnected()) or if pci_generic_config_read() can't find the
device. In both cases *val is initially set to ~0 but as shown in the code
above pcie_capability_read_*() resets it back to 0. Even with this effort,
drivers still have to perform validation checks more so if 0 is a valid
value.

Most drivers only consider the case (b) and in some cases, there is the 
expectation that on timeout *val has a fabricated value of ~0, which *may*
not always be true as explained in (a).

In any case, checks need to be done to validate the value read and maybe
confirm which error has occurred. It is better left to the drivers to do.

Check the return value of pcie_capability_read_dword() to ensure success
and avoid bug as a result of Patch 14/14.
Remove the reset of *val to 0 when pci_read_config_*() fails.


Bolarinwa Olayemi Saheed (14):
  IB/hfi1: Check the return value of pcie_capability_read_*()
  misc: rtsx: Check the return value of pcie_capability_read_*()
  ath9k: Check the return value of pcie_capability_read_*()
  iwlegacy: Check the return value of pcie_capability_read_*()
  PCI: pciehp: Check the return value of pcie_capability_read_*()
  PCI: pciehp: Make "Power On" the default 
  PCI: pciehp: Check the return value of pcie_capability_read_*()
  PCI/ACPI: Check the return value of pcie_capability_read_*()
  PCI: pciehp: Check the return value of pcie_capability_read_*()
  PCI: Check the return value of pcie_capability_read_*()
  PCI/PM: Check return value of pcie_capability_read_*()
  PCI/AER: Check the return value of pcie_capability_read_*()
  PCI/ASPM: Check the return value of pcie_capability_read_*()
  PCI: Remove '*val = 0' from pcie_capability_read_*()

 drivers/net/wireless/ath/ath9k/pci.c         | 5 +++--
 drivers/net/wireless/intel/iwlegacy/common.c | 4 ++--
 drivers/infiniband/hw/hfi1/aspm.c | 7 ++++---
 drivers/misc/cardreader/rts5227.c | 5 +++--
 drivers/misc/cardreader/rts5249.c | 5 +++--
 drivers/misc/cardreader/rts5260.c | 5 +++--
 drivers/misc/cardreader/rts5261.c | 5 +++--
 drivers/pci/pcie/aer.c  |  5 +++--
 drivers/pci/pcie/aspm.c | 33 +++++++++++++++++----------------
 drivers/pci/hotplug/pciehp_hpc.c | 47 ++++++++++++++++----------------
 drivers/pci/pci-acpi.c           | 10 ++++---
 drivers/pci/probe.c              | 29 ++++++++++++--------
 drivers/pci/access.c | 14 --------------
 13 files changed, 87 insertions(+), 87 deletions(-)

-- 
2.18.2

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* [Linux-kernel-mentees] [PATCH 1/14 v4] IB/hfi1: Check the return value of pcie_capability_read_*()
  2020-07-14 11:04 [Linux-kernel-mentees] [PATCH 0/14 v4] PCI: Remove '*val = 0' from pcie_capability_read_*() Saheed Olayemi Bolarinwa
@ 2020-07-14 11:04 ` Saheed Olayemi Bolarinwa
  2020-07-14 11:04 ` [Linux-kernel-mentees] [PATCH 10/14 v4] PCI: Check " Saheed Olayemi Bolarinwa
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Saheed Olayemi Bolarinwa @ 2020-07-14 11:04 UTC (permalink / raw)
  To: helgaas, Mike Marciniszyn, Dennis Dalessandro, Doug Ledford,
	Jason Gunthorpe
  Cc: linux-rdma, Bolarinwa Olayemi Saheed, linux-kernel, linux-kernel-mentees

From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>

On failure pcie_capability_read_dword() sets it's last parameter,
val to 0. In this case dn and up will be 0, so aspm_hw_l1_supported()
will return false.
However, with Patch 14/14, it is possible that val is set to ~0 on
failure. This would introduce a bug because (x & x) == (~0 & x). So with
dn and up being 0x02, a true value is return when the read has actually
failed.

This bug can be avoided if the return value of pcie_capability_read_dword
is checked to confirm success. The behaviour of the function remains
intact.

Check the return value of pcie_capability_read_dword() to ensure success.

Suggested-by: Bjorn Helgaas <bjorn@helgaas.com>
Signed-off-by: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
---
v4 changes:
Remove unnecessary boolean conversion.

 drivers/infiniband/hw/hfi1/aspm.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/aspm.c b/drivers/infiniband/hw/hfi1/aspm.c
index a3c53be4072c..80d0b3edd983 100644
--- a/drivers/infiniband/hw/hfi1/aspm.c
+++ b/drivers/infiniband/hw/hfi1/aspm.c
@@ -24,6 +24,7 @@ static bool aspm_hw_l1_supported(struct hfi1_devdata *dd)
 {
 	struct pci_dev *parent = dd->pcidev->bus->self;
 	u32 up, dn;
+	int ret_up, ret_dn;
 
 	/*
 	 * If the driver does not have access to the upstream component,
@@ -32,14 +33,14 @@ static bool aspm_hw_l1_supported(struct hfi1_devdata *dd)
 	if (!parent)
 		return false;
 
-	pcie_capability_read_dword(dd->pcidev, PCI_EXP_LNKCAP, &dn);
+	ret_dn = pcie_capability_read_dword(dd->pcidev, PCI_EXP_LNKCAP, &dn);
 	dn = ASPM_L1_SUPPORTED(dn);
 
-	pcie_capability_read_dword(parent, PCI_EXP_LNKCAP, &up);
+	ret_up = pcie_capability_read_dword(parent, PCI_EXP_LNKCAP, &up);
 	up = ASPM_L1_SUPPORTED(up);
 
 	/* ASPM works on A-step but is reported as not supported */
-	return (!!dn || is_ax(dd)) && !!up;
+	return ret_dn && ret_up && (dn || is_ax(dd)) && up;
 }
 
 /* Set L1 entrance latency for slower entry to L1 */
-- 
2.18.2

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* [Linux-kernel-mentees] [PATCH 10/14 v4] PCI: Check return value of pcie_capability_read_*()
  2020-07-14 11:04 [Linux-kernel-mentees] [PATCH 0/14 v4] PCI: Remove '*val = 0' from pcie_capability_read_*() Saheed Olayemi Bolarinwa
  2020-07-14 11:04 ` [Linux-kernel-mentees] [PATCH 1/14 v4] IB/hfi1: Check the return value of pcie_capability_read_*() Saheed Olayemi Bolarinwa
@ 2020-07-14 11:04 ` Saheed Olayemi Bolarinwa
  2020-07-14 11:04 ` [Linux-kernel-mentees] [PATCH 11/14 v4] PCI/PM: " Saheed Olayemi Bolarinwa
  2020-07-14 15:49 ` [Linux-kernel-mentees] [PATCH 0/14 v4] PCI: Remove '*val = 0' from pcie_capability_read_*() Bjorn Helgaas
  3 siblings, 0 replies; 5+ messages in thread
From: Saheed Olayemi Bolarinwa @ 2020-07-14 11:04 UTC (permalink / raw)
  To: helgaas
  Cc: Bolarinwa Olayemi Saheed, linux-kernel, linux-pci, linux-kernel-mentees

From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>

On failure pcie_capability_read_dword() sets it's last parameter,
val to 0.
However, with Patch 14/14, it is possible that val is set to ~0 on
failure. This would introduce a bug because (x & x) == (~0 & x).

This bug can be avoided if the return value of pcie_capability_read_word
is checked to confirm success.

Check the return value of pcie_capability_read_word() to ensure success.

Suggested-by: Bjorn Helgaas <bjorn@helgaas.com>
Signed-off-by: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
---
v4 changes:
Remove unnecessary boolean conversion.

 drivers/pci/probe.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 2f66988cea25..3c87a8a1d4b5 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -1121,10 +1121,11 @@ EXPORT_SYMBOL(pci_add_new_bus);
 static void pci_enable_crs(struct pci_dev *pdev)
 {
 	u16 root_cap = 0;
+	int ret;
 
 	/* Enable CRS Software Visibility if supported */
-	pcie_capability_read_word(pdev, PCI_EXP_RTCAP, &root_cap);
-	if (root_cap & PCI_EXP_RTCAP_CRSVIS)
+	ret = pcie_capability_read_word(pdev, PCI_EXP_RTCAP, &root_cap);
+	if (!ret && (root_cap & PCI_EXP_RTCAP_CRSVIS))
 		pcie_capability_set_word(pdev, PCI_EXP_RTCTL,
 					 PCI_EXP_RTCTL_CRSSVE);
 }
@@ -1519,9 +1520,10 @@ void set_pcie_port_type(struct pci_dev *pdev)
 void set_pcie_hotplug_bridge(struct pci_dev *pdev)
 {
 	u32 reg32;
+	int ret;
 
-	pcie_capability_read_dword(pdev, PCI_EXP_SLTCAP, &reg32);
-	if (reg32 & PCI_EXP_SLTCAP_HPC)
+	ret = pcie_capability_read_dword(pdev, PCI_EXP_SLTCAP, &reg32);
+	if (!ret && (reg32 & PCI_EXP_SLTCAP_HPC))
 		pdev->is_hotplug_bridge = 1;
 }
 
@@ -2057,10 +2059,11 @@ int pci_configure_extended_tags(struct pci_dev *dev, void *ign)
 bool pcie_relaxed_ordering_enabled(struct pci_dev *dev)
 {
 	u16 v;
+	int ret;
 
-	pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &v);
+	ret = pcie_capability_read_word(dev, PCI_EXP_DEVCTL, &v);
 
-	return !!(v & PCI_EXP_DEVCTL_RELAX_EN);
+	return (!ret && (v & PCI_EXP_DEVCTL_RELAX_EN));
 }
 EXPORT_SYMBOL(pcie_relaxed_ordering_enabled);
 
@@ -2096,16 +2099,17 @@ static void pci_configure_ltr(struct pci_dev *dev)
 	struct pci_host_bridge *host = pci_find_host_bridge(dev->bus);
 	struct pci_dev *bridge;
 	u32 cap, ctl;
+	int ret;
 
 	if (!pci_is_pcie(dev))
 		return;
 
-	pcie_capability_read_dword(dev, PCI_EXP_DEVCAP2, &cap);
-	if (!(cap & PCI_EXP_DEVCAP2_LTR))
+	ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP2, &cap);
+	if (ret || !(cap & PCI_EXP_DEVCAP2_LTR))
 		return;
 
-	pcie_capability_read_dword(dev, PCI_EXP_DEVCTL2, &ctl);
-	if (ctl & PCI_EXP_DEVCTL2_LTR_EN) {
+	ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCTL2, &ctl);
+	if (!ret && (ctl & PCI_EXP_DEVCTL2_LTR_EN)) {
 		if (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT) {
 			dev->ltr_path = 1;
 			return;
@@ -2142,12 +2146,13 @@ static void pci_configure_eetlp_prefix(struct pci_dev *dev)
 	struct pci_dev *bridge;
 	int pcie_type;
 	u32 cap;
+	int ret;
 
 	if (!pci_is_pcie(dev))
 		return;
 
-	pcie_capability_read_dword(dev, PCI_EXP_DEVCAP2, &cap);
-	if (!(cap & PCI_EXP_DEVCAP2_EE_PREFIX))
+	ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP2, &cap);
+	if (ret || !(cap & PCI_EXP_DEVCAP2_EE_PREFIX))
 		return;
 
 	pcie_type = pci_pcie_type(dev);
-- 
2.18.2

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* [Linux-kernel-mentees] [PATCH 11/14 v4] PCI/PM: Check return value of pcie_capability_read_*()
  2020-07-14 11:04 [Linux-kernel-mentees] [PATCH 0/14 v4] PCI: Remove '*val = 0' from pcie_capability_read_*() Saheed Olayemi Bolarinwa
  2020-07-14 11:04 ` [Linux-kernel-mentees] [PATCH 1/14 v4] IB/hfi1: Check the return value of pcie_capability_read_*() Saheed Olayemi Bolarinwa
  2020-07-14 11:04 ` [Linux-kernel-mentees] [PATCH 10/14 v4] PCI: Check " Saheed Olayemi Bolarinwa
@ 2020-07-14 11:04 ` Saheed Olayemi Bolarinwa
  2020-07-14 15:49 ` [Linux-kernel-mentees] [PATCH 0/14 v4] PCI: Remove '*val = 0' from pcie_capability_read_*() Bjorn Helgaas
  3 siblings, 0 replies; 5+ messages in thread
From: Saheed Olayemi Bolarinwa @ 2020-07-14 11:04 UTC (permalink / raw)
  To: helgaas
  Cc: Bolarinwa Olayemi Saheed, linux-kernel, linux-pci, linux-kernel-mentees

From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>

On failure pcie_capability_read_dword() sets it's last parameter,
val to 0.
However, with Patch 14/14, it is possible that val is set to ~0 on
failure. This would introduce a bug because (x & x) == (~0 & x).

This bug can be avoided if the return value of pcie_capability_read_dword
is checked to confirm success/failure. This check helps ensure that
functions exit only when their post-condition is true. For functions which
walks through device heirarchy like pci_enable_atomic_ops_to_root(), this
is important.

Check the return value of pcie_capability_read_dword() to ensure success.

Suggested-by: Bjorn Helgaas <bjorn@helgaas.com>
Signed-off-by: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
---
v4 changes:
Remove unnecessary boolean conversion.
Fix bugs introduced by previous changes

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 4b2a348576cb..d82b79291742 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -3207,6 +3207,7 @@ void pci_configure_ari(struct pci_dev *dev)
 {
 	u32 cap;
 	struct pci_dev *bridge;
+	int ret;
 
 	if (pcie_ari_disabled || !pci_is_pcie(dev) || dev->devfn)
 		return;
@@ -3215,8 +3216,8 @@ void pci_configure_ari(struct pci_dev *dev)
 	if (!bridge)
 		return;
 
-	pcie_capability_read_dword(bridge, PCI_EXP_DEVCAP2, &cap);
-	if (!(cap & PCI_EXP_DEVCAP2_ARI))
+	ret = pcie_capability_read_dword(bridge, PCI_EXP_DEVCAP2, &cap);
+	if (ret || !(cap & PCI_EXP_DEVCAP2_ARI))
 		return;
 
 	if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ARI)) {
@@ -3606,6 +3607,7 @@ int pci_enable_atomic_ops_to_root(struct pci_dev *dev, u32 cap_mask)
 	struct pci_bus *bus = dev->bus;
 	struct pci_dev *bridge;
 	u32 cap, ctl2;
+	int ret;
 
 	if (!pci_is_pcie(dev))
 		return -EINVAL;
@@ -3629,28 +3631,29 @@ int pci_enable_atomic_ops_to_root(struct pci_dev *dev, u32 cap_mask)
 	while (bus->parent) {
 		bridge = bus->self;
 
-		pcie_capability_read_dword(bridge, PCI_EXP_DEVCAP2, &cap);
+		ret = pcie_capability_read_dword(bridge, PCI_EXP_DEVCAP2,
+								&cap);
 
 		switch (pci_pcie_type(bridge)) {
 		/* Ensure switch ports support AtomicOp routing */
 		case PCI_EXP_TYPE_UPSTREAM:
 		case PCI_EXP_TYPE_DOWNSTREAM:
-			if (!(cap & PCI_EXP_DEVCAP2_ATOMIC_ROUTE))
+			if (ret || !(cap & PCI_EXP_DEVCAP2_ATOMIC_ROUTE))
 				return -EINVAL;
 			break;
 
 		/* Ensure root port supports all the sizes we care about */
 		case PCI_EXP_TYPE_ROOT_PORT:
-			if ((cap & cap_mask) != cap_mask)
+			if (ret || ((cap & cap_mask) != cap_mask))
 				return -EINVAL;
 			break;
 		}
 
 		/* Ensure upstream ports don't block AtomicOps on egress */
 		if (pci_pcie_type(bridge) == PCI_EXP_TYPE_UPSTREAM) {
-			pcie_capability_read_dword(bridge, PCI_EXP_DEVCTL2,
-						   &ctl2);
-			if (ctl2 & PCI_EXP_DEVCTL2_ATOMIC_EGRESS_BLOCK)
+			ret = pcie_capability_read_dword(bridge,
+						PCI_EXP_DEVCTL2, &ctl2);
+			if (!ret && (ctl2 & PCI_EXP_DEVCTL2_ATOMIC_EGRESS_BLOCK)
 				return -EINVAL;
 		}
 
@@ -4507,12 +4510,13 @@ EXPORT_SYMBOL(pci_wait_for_pending_transaction);
 bool pcie_has_flr(struct pci_dev *dev)
 {
 	u32 cap;
+	int ret;
 
 	if (dev->dev_flags & PCI_DEV_FLAGS_NO_FLR_RESET)
 		return false;
 
-	pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &cap);
-	return cap & PCI_EXP_DEVCAP_FLR;
+	ret = pcie_capability_read_dword(dev, PCI_EXP_DEVCAP, &cap);
+	return (!ret && (cap & PCI_EXP_DEVCAP_FLR));
 }
 EXPORT_SYMBOL_GPL(pcie_has_flr);
 
@@ -4646,7 +4650,7 @@ static int pci_pm_reset(struct pci_dev *dev, int probe)
 static bool pcie_wait_for_link_delay(struct pci_dev *pdev, bool active,
 				     int delay)
 {
-	int timeout = 1000;
+	int rc, timeout = 1000;
 	bool ret;
 	u16 lnk_status;
 
@@ -4671,9 +4675,9 @@ static bool pcie_wait_for_link_delay(struct pci_dev *pdev, bool active,
 	if (active)
 		msleep(20);
 	for (;;) {
-		pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnk_status);
-		ret = !!(lnk_status & PCI_EXP_LNKSTA_DLLLA);
-		if (ret == active)
+		rc = pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnk_status);
+		ret = !!(lnk_status & PCI_EXP_LNKSTA_DLLLA);	
+		if (!rc && (ret == active))
 			break;
 		if (timeout <= 0)
 			break;
@@ -4682,10 +4686,10 @@ static bool pcie_wait_for_link_delay(struct pci_dev *pdev, bool active,
 	}
 	if (active && ret && delay)
 		msleep(delay);
-	else if (ret != active)
+	else if (rc || (ret != active))
 		pci_info(pdev, "Data Link Layer Link Active not %s in 1000 msec\n",
 			active ? "set" : "cleared");
-	return ret == active;
+	return (!rc && (ret == active));
 }
 
 /**
@@ -5774,6 +5778,7 @@ u32 pcie_bandwidth_available(struct pci_dev *dev, struct pci_dev **limiting_dev,
 	enum pci_bus_speed next_speed;
 	enum pcie_link_width next_width;
 	u32 bw, next_bw;
+	int ret;
 
 	if (speed)
 		*speed = PCI_SPEED_UNKNOWN;
@@ -5783,7 +5788,12 @@ u32 pcie_bandwidth_available(struct pci_dev *dev, struct pci_dev **limiting_dev,
 	bw = 0;
 
 	while (dev) {
-		pcie_capability_read_word(dev, PCI_EXP_LNKSTA, &lnksta);
+		ret = pcie_capability_read_word(dev, PCI_EXP_LNKSTA, &lnksta);
+
+		if (ret) {
+			dev = pci_upstream_bridge(dev);
+			continue;
+		}
 
 		next_speed = pcie_link_speed[lnksta & PCI_EXP_LNKSTA_CLS];
 		next_width = (lnksta & PCI_EXP_LNKSTA_NLW) >>
@@ -5820,6 +5830,7 @@ EXPORT_SYMBOL(pcie_bandwidth_available);
 enum pci_bus_speed pcie_get_speed_cap(struct pci_dev *dev)
 {
 	u32 lnkcap2, lnkcap;
+	int ret;
 
 	/*
 	 * Link Capabilities 2 was added in PCIe r3.0, sec 7.8.18.  The
@@ -5830,16 +5841,18 @@ enum pci_bus_speed pcie_get_speed_cap(struct pci_dev *dev)
 	 * should use the Supported Link Speeds field in Link Capabilities,
 	 * where only 2.5 GT/s and 5.0 GT/s speeds were defined.
 	 */
-	pcie_capability_read_dword(dev, PCI_EXP_LNKCAP2, &lnkcap2);
+	ret = pcie_capability_read_dword(dev, PCI_EXP_LNKCAP2, &lnkcap2);
 
 	/* PCIe r3.0-compliant */
-	if (lnkcap2)
+	if (!ret && lnkcap2)
 		return PCIE_LNKCAP2_SLS2SPEED(lnkcap2);
 
-	pcie_capability_read_dword(dev, PCI_EXP_LNKCAP, &lnkcap);
-	if ((lnkcap & PCI_EXP_LNKCAP_SLS) == PCI_EXP_LNKCAP_SLS_5_0GB)
+	ret = pcie_capability_read_dword(dev, PCI_EXP_LNKCAP, &lnkcap);
+	if (!ret &&
+		((lnkcap & PCI_EXP_LNKCAP_SLS) == PCI_EXP_LNKCAP_SLS_5_0GB))
 		return PCIE_SPEED_5_0GT;
-	else if ((lnkcap & PCI_EXP_LNKCAP_SLS) == PCI_EXP_LNKCAP_SLS_2_5GB)
+	else if (!ret &&
+		((lnkcap & PCI_EXP_LNKCAP_SLS) == PCI_EXP_LNKCAP_SLS_2_5GB))
 		return PCIE_SPEED_2_5GT;
 
 	return PCI_SPEED_UNKNOWN;
@@ -5856,9 +5869,10 @@ EXPORT_SYMBOL(pcie_get_speed_cap);
 enum pcie_link_width pcie_get_width_cap(struct pci_dev *dev)
 {
 	u32 lnkcap;
+	int ret;
 
-	pcie_capability_read_dword(dev, PCI_EXP_LNKCAP, &lnkcap);
-	if (lnkcap)
+	ret = pcie_capability_read_dword(dev, PCI_EXP_LNKCAP, &lnkcap);
+	if (!ret && lnkcap)
 		return (lnkcap & PCI_EXP_LNKCAP_MLW) >> 4;
 
 	return PCIE_LNK_WIDTH_UNKNOWN;
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

* Re: [Linux-kernel-mentees] [PATCH 0/14 v4] PCI: Remove '*val = 0' from pcie_capability_read_*()
  2020-07-14 11:04 [Linux-kernel-mentees] [PATCH 0/14 v4] PCI: Remove '*val = 0' from pcie_capability_read_*() Saheed Olayemi Bolarinwa
                   ` (2 preceding siblings ...)
  2020-07-14 11:04 ` [Linux-kernel-mentees] [PATCH 11/14 v4] PCI/PM: " Saheed Olayemi Bolarinwa
@ 2020-07-14 15:49 ` Bjorn Helgaas
  3 siblings, 0 replies; 5+ messages in thread
From: Bjorn Helgaas @ 2020-07-14 15:49 UTC (permalink / raw)
  To: Saheed Olayemi Bolarinwa
  Cc: Mike Marciniszyn, linux-rdma, linux-pci, Dennis Dalessandro,
	linux-kernel, Jason Gunthorpe, Doug Ledford,
	linux-kernel-mentees

On Tue, Jul 14, 2020 at 01:04:42PM +0200, Saheed Olayemi Bolarinwa wrote:
> From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
> ...

> Bolarinwa Olayemi Saheed (14):
>   IB/hfi1: Check the return value of pcie_capability_read_*()
>   misc: rtsx: Check the return value of pcie_capability_read_*()
>   ath9k: Check the return value of pcie_capability_read_*()
>   iwlegacy: Check the return value of pcie_capability_read_*()
>   PCI: pciehp: Check the return value of pcie_capability_read_*()
>   PCI: pciehp: Make "Power On" the default 
>   PCI: pciehp: Check the return value of pcie_capability_read_*()
>   PCI/ACPI: Check the return value of pcie_capability_read_*()
>   PCI: pciehp: Check the return value of pcie_capability_read_*()
>   PCI: Check the return value of pcie_capability_read_*()
>   PCI/PM: Check return value of pcie_capability_read_*()
>   PCI/AER: Check the return value of pcie_capability_read_*()
>   PCI/ASPM: Check the return value of pcie_capability_read_*()
>   PCI: Remove '*val = 0' from pcie_capability_read_*()

1) Let's slow down on posting patches.  We need time to think and have
a conversation about where we're going, and waking up to dozens of new
patches every day doesn't help.

2) This series claims to have 14 patches, but only 3 made it to the
list.  I don't know if the others were rejected for too many folks in
the cc: list or what.  If you only updated these 3, we will still want
the full set of 14 posted because it's too hard to collect 11 things
from v3 and 3 things from v4, etc.

  $ b4 am -om/ 20200714110445.32605-1-refactormyself@gmail.com
  Looking up https://lore.kernel.org/r/20200714110445.32605-1-refactormyself%40gmail.com
  Grabbing thread from lore.kernel.org/linux-kernel-mentees
  Analyzing 4 messages in the thread
  ---
  Thread incomplete, attempting to backfill
  Grabbing thread from lore.kernel.org/linux-rdma
  Grabbing thread from lore.kernel.org/lkml
  Grabbing thread from lore.kernel.org/linux-pci
  ---
  Writing m/v4_20200714_refactormyself_pci_remove_val_0_from_pcie_capability_read.mbx
    [PATCH 1/14 v4] IB/hfi1: Check the return value of pcie_capability_read_*()
    ERROR: missing [2/14]!
    ERROR: missing [3/14]!
    ERROR: missing [4/14]!
    ERROR: missing [5/14]!
    ERROR: missing [6/14]!
    ERROR: missing [7/14]!
    ERROR: missing [8/14]!
    ERROR: missing [9/14]!
    [PATCH 10/14 v4] PCI: Check return value of pcie_capability_read_*()
    [PATCH 11/14 v4] PCI/PM: Check return value of pcie_capability_read_*()
    ERROR: missing [12/14]!
    ERROR: missing [13/14]!
    ERROR: missing [14/14]!
  ---
  Total patches: 3
  ---
  WARNING: Thread incomplete!
  Cover: m/v4_20200714_refactormyself_pci_remove_val_0_from_pcie_capability_read.cover
   Link: https://lore.kernel.org/r/20200714110445.32605-1-refactormyself@gmail.com
   Base: not found
	 git am m/v4_20200714_refactormyself_pci_remove_val_0_from_pcie_capability_read.mbx

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

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

end of thread, other threads:[~2020-07-14 15:49 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-14 11:04 [Linux-kernel-mentees] [PATCH 0/14 v4] PCI: Remove '*val = 0' from pcie_capability_read_*() Saheed Olayemi Bolarinwa
2020-07-14 11:04 ` [Linux-kernel-mentees] [PATCH 1/14 v4] IB/hfi1: Check the return value of pcie_capability_read_*() Saheed Olayemi Bolarinwa
2020-07-14 11:04 ` [Linux-kernel-mentees] [PATCH 10/14 v4] PCI: Check " Saheed Olayemi Bolarinwa
2020-07-14 11:04 ` [Linux-kernel-mentees] [PATCH 11/14 v4] PCI/PM: " Saheed Olayemi Bolarinwa
2020-07-14 15:49 ` [Linux-kernel-mentees] [PATCH 0/14 v4] PCI: Remove '*val = 0' from pcie_capability_read_*() Bjorn Helgaas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).