[PATCH v2 0/3] PCIe Capability accessor fixes [stable backports]

[PATCH v2 0/3] PCIe Capability accessor fixes [stable backports]

[Bjorn Helgaas]

The following patches fix issues reported by Yuval Mintz
<yuvalmin@broadcom.com> and Myron Stowe <myron.stowe@redhat.com>.

These fix pcie_capability_read_dword() and related accessors, which first
appeared in v3.7.  I backported these to v3.10.19 because that appears to
be the oldest maintained stable kernel that is affected.

These patches replace the ones I posted yesterday at [1], because Myron
pointed out that an additional patch, "Remove PCIe Capability version
checks," is needed to fix the issue he found [2].

[1] http://lkml.kernel.org/r/20131118191723.26774.53479.stgit@bhelgaas-glaptop.roam.corp.google.com
[2] https://bugzilla.kernel.org/show_bug.cgi?id=65211


---

Bjorn Helgaas (3):
      PCI: Allow PCIe Capability link-related register access for switches
      PCI: Remove PCIe Capability version checks
      PCI: Support PCIe Capability Slot registers only for ports with slots


 drivers/pci/access.c |   19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

[PATCH v2 1/3] PCI: Allow PCIe Capability link-related register access for switches

[Bjorn Helgaas]

Commit d3694d4fa3f44f6a295f8ab064937c8a1549d174 upstream.

Every PCIe device has a link, except Root Complex Integrated Endpoints
and Root Complex Event Collectors.  Previously we didn't give access
to PCIe capability link-related registers for Upstream Ports, Downstream
Ports, and Bridges, so attempts to read PCI_EXP_LNKCTL incorrectly
returned zero.  See PCIe spec r3.0, sec 7.8 and 1.3.2.3.

Reference: http://lkml.kernel.org/r/979A8436335E3744ADCD3A9F2A2B68A52AD136BE@SJEXCHMB10.corp.ad.broadcom.com
Reported-by: Yuval Mintz <yuvalmin@broadcom.com>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-By: Jiang Liu <jiang.liu@huawei.com>
CC: stable@vger.kernel.org	# v3.7+
---
 drivers/pci/access.c |    8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/access.c b/drivers/pci/access.c
index 1cc23661f79b..e26c3bd9aca4 100644
--- a/drivers/pci/access.c
+++ b/drivers/pci/access.c
@@ -485,9 +485,13 @@ static inline bool pcie_cap_has_lnkctl(const struct pci_dev *dev)
 	int type = pci_pcie_type(dev);
 
 	return pcie_cap_version(dev) > 1 ||
-	       type == PCI_EXP_TYPE_ROOT_PORT ||
 	       type == PCI_EXP_TYPE_ENDPOINT ||
-	       type == PCI_EXP_TYPE_LEG_END;
+	       type == PCI_EXP_TYPE_LEG_END ||
+	       type == PCI_EXP_TYPE_ROOT_PORT ||
+	       type == PCI_EXP_TYPE_UPSTREAM ||
+	       type == PCI_EXP_TYPE_DOWNSTREAM ||
+	       type == PCI_EXP_TYPE_PCI_BRIDGE ||
+	       type == PCI_EXP_TYPE_PCIE_BRIDGE;
 }
 
 static inline bool pcie_cap_has_sltctl(const struct pci_dev *dev)

[PATCH v2 2/3] PCI: Remove PCIe Capability version checks

[Bjorn Helgaas]

Commit c8b303d0206b28c4ff3aecada47108d1655ae00f upstream.  This is part
of the fix for https://bugzilla.kernel.org/show_bug.cgi?id=65211

Previously we relied on the PCIe r3.0, sec 7.8, spec language that says
"For Functions that do not implement the [Link, Slot, Root] registers,
these spaces must be hardwired to 0b," which means that for v2 PCIe
capabilities, we don't need to check the device type at all.

But it's simpler if we don't need to check the capability version at all,
and I think the spec is explicit enough about which registers are required
for which types that we can remove the version checks.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-By: Jiang Liu <jiang.liu@huawei.com>
CC: stable@vger.kernel.org	# v3.7+
---
 drivers/pci/access.c |    9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/access.c b/drivers/pci/access.c
index e26c3bd9aca4..9a46fa9135d9 100644
--- a/drivers/pci/access.c
+++ b/drivers/pci/access.c
@@ -484,8 +484,7 @@ static inline bool pcie_cap_has_lnkctl(const struct pci_dev *dev)
 {
 	int type = pci_pcie_type(dev);
 
-	return pcie_cap_version(dev) > 1 ||
-	       type == PCI_EXP_TYPE_ENDPOINT ||
+	return type == PCI_EXP_TYPE_ENDPOINT ||
 	       type == PCI_EXP_TYPE_LEG_END ||
 	       type == PCI_EXP_TYPE_ROOT_PORT ||
 	       type == PCI_EXP_TYPE_UPSTREAM ||
@@ -498,8 +497,7 @@ static inline bool pcie_cap_has_sltctl(const struct pci_dev *dev)
 {
 	int type = pci_pcie_type(dev);
 
-	return pcie_cap_version(dev) > 1 ||
-	       type == PCI_EXP_TYPE_ROOT_PORT ||
+	return type == PCI_EXP_TYPE_ROOT_PORT ||
 	       (type == PCI_EXP_TYPE_DOWNSTREAM &&
 		pcie_caps_reg(dev) & PCI_EXP_FLAGS_SLOT);
 }
@@ -508,8 +506,7 @@ static inline bool pcie_cap_has_rtctl(const struct pci_dev *dev)
 {
 	int type = pci_pcie_type(dev);
 
-	return pcie_cap_version(dev) > 1 ||
-	       type == PCI_EXP_TYPE_ROOT_PORT ||
+	return type == PCI_EXP_TYPE_ROOT_PORT ||
 	       type == PCI_EXP_TYPE_RC_EC;
 }
 

[PATCH v2 3/3] PCI: Support PCIe Capability Slot registers only for ports with slots

[Bjorn Helgaas]

Commit 6d3a1741f1e648cfbd5a0cc94477a0d5004c6f5e upstream.  This is part
of the fix for https://bugzilla.kernel.org/show_bug.cgi?id=65211

Previously we allowed callers to access Slot Capabilities, Status, and
Control for Root Ports even if the Root Port did not implement a slot.
This seems dubious because the spec only requires these registers if a
slot is implemented.

It's true that even Root Ports without slots must have *space* for these
slot registers, because the Root Capabilities, Status, and Control
registers are after the slot registers in the capability.  However,
for a v1 PCIe Capability, the *semantics* of the slot registers are
undefined unless a slot is implemented.

Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-By: Jiang Liu <jiang.liu@huawei.com>
CC: stable@vger.kernel.org	# v3.7+
---
 drivers/pci/access.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/access.c b/drivers/pci/access.c
index 9a46fa9135d9..061da8c3ab4b 100644
--- a/drivers/pci/access.c
+++ b/drivers/pci/access.c
@@ -497,9 +497,9 @@ static inline bool pcie_cap_has_sltctl(const struct pci_dev *dev)
 {
 	int type = pci_pcie_type(dev);
 
-	return type == PCI_EXP_TYPE_ROOT_PORT ||
-	       (type == PCI_EXP_TYPE_DOWNSTREAM &&
-		pcie_caps_reg(dev) & PCI_EXP_FLAGS_SLOT);
+	return (type == PCI_EXP_TYPE_ROOT_PORT ||
+		type == PCI_EXP_TYPE_DOWNSTREAM) &&
+	       pcie_caps_reg(dev) & PCI_EXP_FLAGS_SLOT;
 }
 
 static inline bool pcie_cap_has_rtctl(const struct pci_dev *dev)

Re: [PATCH v2 2/3] PCI: Remove PCIe Capability version checks

[Myron Stowe]

On Tue, Nov 19, 2013 at 2:35 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> Commit c8b303d0206b28c4ff3aecada47108d1655ae00f upstream.  This is part
> of the fix for https://bugzilla.kernel.org/show_bug.cgi?id=65211
>
> Previously we relied on the PCIe r3.0, sec 7.8, spec language that says
> "For Functions that do not implement the [Link, Slot, Root] registers,
> these spaces must be hardwired to 0b," which means that for v2 PCIe
> capabilities, we don't need to check the device type at all.
>
> But it's simpler if we don't need to check the capability version at all,
> and I think the spec is explicit enough about which registers are required
> for which types that we can remove the version checks.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> Reviewed-By: Jiang Liu <jiang.liu@huawei.com>
> CC: stable@vger.kernel.org      # v3.7+
> ---
>  drivers/pci/access.c |    9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/access.c b/drivers/pci/access.c
> index e26c3bd9aca4..9a46fa9135d9 100644
> --- a/drivers/pci/access.c
> +++ b/drivers/pci/access.c
> @@ -484,8 +484,7 @@ static inline bool pcie_cap_has_lnkctl(const struct pci_dev *dev)
>  {
>         int type = pci_pcie_type(dev);
>
> -       return pcie_cap_version(dev) > 1 ||
> -              type == PCI_EXP_TYPE_ENDPOINT ||
> +       return type == PCI_EXP_TYPE_ENDPOINT ||
>                type == PCI_EXP_TYPE_LEG_END ||
>                type == PCI_EXP_TYPE_ROOT_PORT ||
>                type == PCI_EXP_TYPE_UPSTREAM ||
> @@ -498,8 +497,7 @@ static inline bool pcie_cap_has_sltctl(const struct pci_dev *dev)
>  {
>         int type = pci_pcie_type(dev);
>
> -       return pcie_cap_version(dev) > 1 ||
> -              type == PCI_EXP_TYPE_ROOT_PORT ||
> +       return type == PCI_EXP_TYPE_ROOT_PORT ||
>                (type == PCI_EXP_TYPE_DOWNSTREAM &&
>                 pcie_caps_reg(dev) & PCI_EXP_FLAGS_SLOT);
>  }
> @@ -508,8 +506,7 @@ static inline bool pcie_cap_has_rtctl(const struct pci_dev *dev)
>  {
>         int type = pci_pcie_type(dev);
>
> -       return pcie_cap_version(dev) > 1 ||
> -              type == PCI_EXP_TYPE_ROOT_PORT ||
> +       return type == PCI_EXP_TYPE_ROOT_PORT ||
>                type == PCI_EXP_TYPE_RC_EC;
>  }
>
>
> --

Acked-by: Myron Stowe <myron.stowe@redhat.com>

> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 3/3] PCI: Support PCIe Capability Slot registers only for ports with slots

[Myron Stowe]

On Tue, Nov 19, 2013 at 2:35 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
> Commit 6d3a1741f1e648cfbd5a0cc94477a0d5004c6f5e upstream.  This is part
> of the fix for https://bugzilla.kernel.org/show_bug.cgi?id=65211
>
> Previously we allowed callers to access Slot Capabilities, Status, and
> Control for Root Ports even if the Root Port did not implement a slot.
> This seems dubious because the spec only requires these registers if a
> slot is implemented.
>
> It's true that even Root Ports without slots must have *space* for these
> slot registers, because the Root Capabilities, Status, and Control
> registers are after the slot registers in the capability.  However,
> for a v1 PCIe Capability, the *semantics* of the slot registers are
> undefined unless a slot is implemented.
>
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> Reviewed-By: Jiang Liu <jiang.liu@huawei.com>
> CC: stable@vger.kernel.org      # v3.7+
> ---
>  drivers/pci/access.c |    6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/pci/access.c b/drivers/pci/access.c
> index 9a46fa9135d9..061da8c3ab4b 100644
> --- a/drivers/pci/access.c
> +++ b/drivers/pci/access.c
> @@ -497,9 +497,9 @@ static inline bool pcie_cap_has_sltctl(const struct pci_dev *dev)
>  {
>         int type = pci_pcie_type(dev);
>
> -       return type == PCI_EXP_TYPE_ROOT_PORT ||
> -              (type == PCI_EXP_TYPE_DOWNSTREAM &&
> -               pcie_caps_reg(dev) & PCI_EXP_FLAGS_SLOT);
> +       return (type == PCI_EXP_TYPE_ROOT_PORT ||
> +               type == PCI_EXP_TYPE_DOWNSTREAM) &&
> +              pcie_caps_reg(dev) & PCI_EXP_FLAGS_SLOT;
>  }
>
>  static inline bool pcie_cap_has_rtctl(const struct pci_dev *dev)
>

Acked-by: Myron Stowe <myron.stowe@redhat.com>

> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2 0/3] PCIe Capability accessor fixes [stable backports]

[Adam Lee]

On Tue, Nov 19, 2013 at 02:35:37PM -0700, Bjorn Helgaas wrote:
> The following patches fix issues reported by Yuval Mintz
> <yuvalmin@broadcom.com> and Myron Stowe <myron.stowe@redhat.com>.
> 
> These fix pcie_capability_read_dword() and related accessors, which first
> appeared in v3.7.  I backported these to v3.10.19 because that appears to
> be the oldest maintained stable kernel that is affected.
> 
> These patches replace the ones I posted yesterday at [1], because Myron
> pointed out that an additional patch, "Remove PCIe Capability version
> checks," is needed to fix the issue he found [2].
> 
> ---
> Bjorn Helgaas (3):
>       PCI: Allow PCIe Capability link-related register access for switches
>       PCI: Remove PCIe Capability version checks
>       PCI: Support PCIe Capability Slot registers only for ports with slots
> 
>  drivers/pci/access.c |   19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)

Fixed a booting hang on some Lenovo laptops.

Tested-by: Adam Lee <adam.lee@canonical.com>

-- 
Adam Lee