x86/pci/mrst: add extra check in fixed bar detection

PATCH In review Jacob Pan 5 participants 6 messages Jul 16, 2010 – Jul 20, 2010

Subsystems (2)

x86 PCI

lore

Share & tools

lore

Jacob Pan

author

2010-07-16 19:07 UTC #0 lore raw

PATCH 1/1 x86/pci/mrst: add extra check in fixed bar detection

Fixed bar capability structure is searched in PCI extended configuration space. We need to make sure there is a valid capability id to begin with otherwise, the search code may stuck in a infinite loop which results in boot hang.
This patch adds additional check for cap ID 0, which is also invalid.

Signed-off-by Jacob Pan <[email protected]>

---
 arch/x86/pci/mrst.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

arch/x86/pci/mrst.c +4 -3

index c9649d0..8636567 100644
@@ -66,8 +66,9 @@ static int fixed_bar_cap(struct pci_bus *bus, unsigned int devfn)
 					  devfn, pos, 4, &pcie_cap))
 			return 0;
 
-		if (pcie_cap == 0xffffffff)
-			return 0;
+		if (PCI_EXT_CAP_ID(pcie_cap) == 0x0000 ||
+			PCI_EXT_CAP_ID(pcie_cap) == 0xffff)
+			break;
 
 		if (PCI_EXT_CAP_ID(pcie_cap) == PCI_EXT_CAP_ID_VNDR) {
 			raw_pci_ext_ops->read(pci_domain_nr(bus), bus->number,

@@ -76,7 +77,7 @@ static int fixed_bar_cap(struct pci_bus *bus, unsigned int devfn)
 				return pos;
 		}
 
-		pos = pcie_cap >> 20;
+		pos = PCI_EXT_CAP_NEXT(pcie_cap);
 	}
 
 	return 0;

Signature · 1 line

1.7.0.4

Jesse Barnes

Reviewed-by on v1 1/1

2010-07-16 19:12 UTC #1 lore raw

Quoted 30 lines · On Fri, 16 Jul 2010 11:58:26 -0700 Jacob Pan <[email protected]>…

Fixed bar capability structure is searched in PCI extended configuration space. We need to make sure there is a valid capability id to begin with otherwise, the search code may stuck in a infinite loop which results in boot hang.
This patch adds additional check for cap ID 0, which is also invalid.
Suggested-by "H. Peter Anvin" <[email protected]>
Signed-off-by Jacob Pan <[email protected]>
---
 arch/x86/pci/mrst.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)
arch/x86/pci/mrst.c +3 -2
index c9649d0..8636567 100644
@@ -66,8 +66,9 @@ static int fixed_bar_cap(struct pci_bus *bus, unsigned int devfn)
 					  devfn, pos, 4, &pcie_cap))
 			return 0;
 
-		if (pcie_cap == 0xffffffff)
-			return 0;
+		if (PCI_EXT_CAP_ID(pcie_cap) == 0x0000 ||
+			PCI_EXT_CAP_ID(pcie_cap) == 0xffff)
+			break;
 
 		if (PCI_EXT_CAP_ID(pcie_cap) == PCI_EXT_CAP_ID_VNDR) {
 			raw_pci_ext_ops->read(pci_domain_nr(bus), bus->number,

arch/x86/pci/mrst.c static int fixed_bar_cap(struct pci_bus *bus, unsigned int devfn) · line 83

Quoted 8 lines

arch/x86/pci/mrst.c +1 -1

@@ -76,7 +77,7 @@ static int fixed_bar_cap(struct pci_bus *bus, unsigned int devfn)
 				return pos;
 		}
 
-		pos = pcie_cap >> 20;
+		pos = PCI_EXT_CAP_NEXT(pcie_cap);
 	}
 
 	return 0;

Looks good.

Reviewed-by Jesse Barnes <[email protected]>

Signature · 1 line

Jesse Barnes, Intel Open Source Technology Center

tip-bot for Jacob Pan

author

2010-07-17 00:49 UTC #2 lore raw

[tip:x86/urgent] x86, pci, mrst: Add extra sanity check in walking the PCI extended cap chain

In reply to Jacob Pan

Commit-ID:  f82c3d71d6fd2e6a3e3416f09099e29087e39abf
Gitweb:     http://git.kernel.org/tip/f82c3d71d6fd2e6a3e3416f09099e29087e39abf
Author:     Jacob Pan <[email protected]>
AuthorDate: Fri, 16 Jul 2010 11:58:26 -0700
Committer:  H. Peter Anvin <[email protected]>
CommitDate: Fri, 16 Jul 2010 16:52:15 -0700

x86, pci, mrst: Add extra sanity check in walking the PCI extended cap chain

The fixed bar capability structure is searched in PCI extended configuration space. We need to make sure there is a valid capability ID to begin with otherwise, the search code may stuck in a infinite loop which results in boot hang. This patch adds additional check for cap ID 0, which is also invalid, and indicates end of chain.

End of chain is supposed to have all fields zero, but that doesn't seem to always be the case in the field.

Signed-off-by Jacob Pan <[email protected]>

Reviewed-by Jesse Barnes <[email protected]>

LKML-Reference: <[email protected]>

Signed-off-by H. Peter Anvin <[email protected]>

---
 arch/x86/pci/mrst.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

arch/x86/pci/mrst.c +4 -3

index 7ef3a27..cb29191 100644
@@ -66,8 +66,9 @@ static int fixed_bar_cap(struct pci_bus *bus, unsigned int devfn)
 					  devfn, pos, 4, &pcie_cap))
 			return 0;
 
-		if (pcie_cap == 0xffffffff)
-			return 0;
+		if (PCI_EXT_CAP_ID(pcie_cap) == 0x0000 ||
+			PCI_EXT_CAP_ID(pcie_cap) == 0xffff)
+			break;
 
 		if (PCI_EXT_CAP_ID(pcie_cap) == PCI_EXT_CAP_ID_VNDR) {
 			raw_pci_ext_ops->read(pci_domain_nr(bus), bus->number,

@@ -76,7 +77,7 @@ static int fixed_bar_cap(struct pci_bus *bus, unsigned int devfn)
 				return pos;
 		}
 
-		pos = pcie_cap >> 20;
+		pos = PCI_EXT_CAP_NEXT(pcie_cap);
 	}
 
 	return 0;

Ben Greear

2010-07-20 17:19 UTC #3 lore raw

In reply to Jacob Pan

Quoted 5 lines · Jacob Pan

Fixed bar capability structure is searched in PCI extended configuration space. We need to make sure there is a valid capability id to begin with otherwise, the search code may stuck in a infinite loop which results in boot hang.
This patch adds additional check for cap ID 0, which is also invalid.

I think this should be added to the stable tree for 2.6.34 as well...

Thanks,
Ben

Quoted 33 lines

Signed-off-by Jacob Pan<[email protected]>

---
  arch/x86/pci/mrst.c |    7 ++++---
  1 files changed, 4 insertions(+), 3 deletions(-)

arch/x86/pci/mrst.c +4 -3

index c9649d0..8636567 100644
@@ -66,8 +66,9 @@ static int fixed_bar_cap(struct pci_bus *bus, unsigned int devfn)
  					  devfn, pos, 4,&pcie_cap))
  			return 0;
 
-		if (pcie_cap == 0xffffffff)
-			return 0;
+		if (PCI_EXT_CAP_ID(pcie_cap) == 0x0000 ||
+			PCI_EXT_CAP_ID(pcie_cap) == 0xffff)
+			break;
 
  		if (PCI_EXT_CAP_ID(pcie_cap) == PCI_EXT_CAP_ID_VNDR) {
  			raw_pci_ext_ops->read(pci_domain_nr(bus), bus->number,

@@ -76,7 +77,7 @@ static int fixed_bar_cap(struct pci_bus *bus, unsigned int devfn)
  				return pos;
  		}
 
-		pos = pcie_cap>>  20;
+		pos = PCI_EXT_CAP_NEXT(pcie_cap);
  	}
 
  	return 0;

Signature · 2 lines

Ben Greear <[email protected]>
Candela Technologies Inc  http://www.candelatech.com

H. Peter Anvin

reviewer

2010-07-20 17:26 UTC #4 lore raw

Quoted 10 lines · Ben Greear

Quoted 5 lines · Jacob Pan
Fixed bar capability structure is searched in PCI extended configuration space. We need to make sure there is a valid capability id to begin with otherwise, the search code may stuck in a infinite loop which results in boot hang.
This patch adds additional check for cap ID 0, which is also invalid.
I think this should be added to the stable tree for 2.6.34 as well...

Agreed. Greg, could you consider upstream f82c3d71d6fd2e6a3e3416f09099e29087e39abf for -stable 2.6.34?

-hpa

Signature · 2 lines

H. Peter Anvin, Intel Open Source Technology Center
I work for Intel.  I don't speak on their behalf.

Greg KH

2010-07-20 19:08 UTC #5 lore raw

Quoted 8 lines · H. Peter Anvin

Quoted 7 lines · Ben Greear
Quoted 5 lines · Jacob Pan
Fixed bar capability structure is searched in PCI extended configuration space. We need to make sure there is a valid capability id to begin with otherwise, the search code may stuck in a infinite loop which results in boot hang.
This patch adds additional check for cap ID 0, which is also invalid.

Quoted 6 lines

Quoted 3 lines
I think this should be added to the stable tree for 2.6.34 as well...
Agreed. Greg, could you consider upstream f82c3d71d6fd2e6a3e3416f09099e29087e39abf for -stable 2.6.34?

Will do.

thanks,

greg k-h