From: Bolarinwa Olayemi Saheed <[email protected]>

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 <[email protected]>
Signed-off-by: Bolarinwa Olayemi Saheed <[email protected]>
---
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;

Reply via email to