Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1308307 > unrolled thread

[PATCHv2 0/4] PCI VPD access fixes

Back to article view | Back to linux.kernel

Contents

  [PATCHv2 0/4] PCI VPD access fixes Hannes Reinecke <hare@suse.de> - 2016-01-13 12:30 +0100
    [PATCHv2 2/4] pci: allow access to VPD attributes with size '0' Hannes Reinecke <hare@suse.de> - 2016-01-13 12:30 +0100
    [PATCHv2 1/4] pci: Update VPD definitions Hannes Reinecke <hare@suse.de> - 2016-01-13 12:30 +0100
    RE: [PATCHv2 0/4] PCI VPD access fixes "Seymour, Shane M" <shane.seymour@hpe.com> - 2016-01-15 02:10 +0100
      Re: [PATCHv2 0/4] PCI VPD access fixes Babu Moger <babu.moger@oracle.com> - 2016-01-15 15:20 +0100
        Re: [PATCHv2 0/4] PCI VPD access fixes Hannes Reinecke <hare@suse.de> - 2016-01-15 15:20 +0100
    Re: [PATCHv2 0/4] PCI VPD access fixes Babu Moger <babu.moger@oracle.com> - 2016-01-19 22:00 +0100
    [PATCH v4 4/4] pci: Blacklist vpd access for buggy devices Babu Moger <babu.moger@oracle.com> - 2016-01-21 19:40 +0100

#1308307 — [PATCHv2 0/4] PCI VPD access fixes

Hi all,

the current PCI VPD page access assumes that the entire possible VPD
data is readable. However, the spec only guarantees a VPD data up to
the 'end' marker, with everything beyond that being undefined.
This causes a system lockup on certain devices.

With this patch we always set the VPD sysfs attribute size to '0', and
calculate the available VPD size on the first access.
If no valid data can be read an I/O error is returned.

I've also included the patch from Babu to blacklists devices which
are known to lockup when accessing the VPD data.

Babu Moger (1):
  pci: Blacklist vpd access for buggy devices

Hannes Reinecke (3):
  pci: Update VPD definitions
  pci: allow access to VPD attributes with size '0'
  pci: Determine actual VPD size on first access

 drivers/pci/access.c    | 79 ++++++++++++++++++++++++++++++++++++++++++++++++-
 drivers/pci/pci-sysfs.c | 22 ++++++++------
 drivers/pci/quirks.c    | 41 +++++++++++++++++++++++++
 include/linux/pci.h     | 27 +++++++++++++++--
 4 files changed, 157 insertions(+), 12 deletions(-)

-- 
1.8.5.6

[toc] | [next] | [standalone]

#1308308 — [PATCHv2 2/4] pci: allow access to VPD attributes with size '0'

It is not always possible to determine the actual size of the VPD
data, so allow access to them if the size is set to '0'.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 drivers/pci/pci-sysfs.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index eead54c..de327c3 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -772,10 +772,12 @@ static ssize_t read_vpd_attr(struct file *filp, struct kobject *kobj,
 	struct pci_dev *dev =
 		to_pci_dev(container_of(kobj, struct device, kobj));
 
-	if (off > bin_attr->size)
-		count = 0;
-	else if (count > bin_attr->size - off)
-		count = bin_attr->size - off;
+	if (bin_attr->size > 0) {
+		if (off > bin_attr->size)
+			count = 0;
+		else if (count > bin_attr->size - off)
+			count = bin_attr->size - off;
+	}
 
 	return pci_read_vpd(dev, off, count, buf);
 }
@@ -787,10 +789,12 @@ static ssize_t write_vpd_attr(struct file *filp, struct kobject *kobj,
 	struct pci_dev *dev =
 		to_pci_dev(container_of(kobj, struct device, kobj));
 
-	if (off > bin_attr->size)
-		count = 0;
-	else if (count > bin_attr->size - off)
-		count = bin_attr->size - off;
+	if (bin_attr->size > 0) {
+		if (off > bin_attr->size)
+			count = 0;
+		else if (count > bin_attr->size - off)
+			count = bin_attr->size - off;
+	}
 
 	return pci_write_vpd(dev, off, count, buf);
 }
-- 
1.8.5.6

[toc] | [prev] | [next] | [standalone]

#1308311 — [PATCHv2 1/4] pci: Update VPD definitions

The 'end' tag is actually 0x0f, it's the representation as a
small resource data type tag that's 0x78 (ie shifted by 3).
This patch also adds helper functions to extract the resource
data type tags for both large and small resource data types.

Cc: Alexander Duyck <alexander.duyck@gmail.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Signed-off-by: Hannes Reinecke <hare@suse.com>
---
 include/linux/pci.h | 27 +++++++++++++++++++++++++--
 1 file changed, 25 insertions(+), 2 deletions(-)

diff --git a/include/linux/pci.h b/include/linux/pci.h
index d86378c..5d61c36 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1836,12 +1836,13 @@ bool pci_acs_path_enabled(struct pci_dev *start,
 #define PCI_VPD_LRDT_RW_DATA		PCI_VPD_LRDT_ID(PCI_VPD_LTIN_RW_DATA)
 
 /* Small Resource Data Type Tag Item Names */
-#define PCI_VPD_STIN_END		0x78	/* End */
+#define PCI_VPD_STIN_END		0x0f	/* End */
 
-#define PCI_VPD_SRDT_END		PCI_VPD_STIN_END
+#define PCI_VPD_SRDT_END		(PCI_VPD_STIN_END << 3)
 
 #define PCI_VPD_SRDT_TIN_MASK		0x78
 #define PCI_VPD_SRDT_LEN_MASK		0x07
+#define PCI_VPD_LRDT_TIN_MASK		0x7f
 
 #define PCI_VPD_LRDT_TAG_SIZE		3
 #define PCI_VPD_SRDT_TAG_SIZE		1
@@ -1865,6 +1866,17 @@ static inline u16 pci_vpd_lrdt_size(const u8 *lrdt)
 }
 
 /**
+ * pci_vpd_lrdt_tag - Extracts the Large Resource Data Type Tag Item
+ * @lrdt: Pointer to the beginning of the Large Resource Data Type tag
+ *
+ * Returns the extracted Large Resource Data Type Tag item.
+ */
+static inline u16 pci_vpd_lrdt_tag(const u8 *lrdt)
+{
+    return (u16)(lrdt[0] & PCI_VPD_LRDT_TIN_MASK);
+}
+
+/**
  * pci_vpd_srdt_size - Extracts the Small Resource Data Type length
  * @lrdt: Pointer to the beginning of the Small Resource Data Type tag
  *
@@ -1876,6 +1888,17 @@ static inline u8 pci_vpd_srdt_size(const u8 *srdt)
 }
 
 /**
+ * pci_vpd_srdt_tag - Extracts the Small Resource Data Type Tag Item
+ * @lrdt: Pointer to the beginning of the Small Resource Data Type tag
+ *
+ * Returns the extracted Small Resource Data Type Tag Item.
+ */
+static inline u8 pci_vpd_srdt_tag(const u8 *srdt)
+{
+	return ((*srdt) & PCI_VPD_SRDT_TIN_MASK) >> 3;
+}
+
+/**
  * pci_vpd_info_field_size - Extracts the information field length
  * @lrdt: Pointer to the beginning of an information field header
  *
-- 
1.8.5.6

[toc] | [prev] | [next] | [standalone]

#1309794

For the series. Tested with AE311 PCI-Express 4Gb Fibre Channel HBA.
Truncation of the vpd data returned works (154 bytes) and lspci -vvv
prints out the VPD tags the same way with and without the changes.
This card did not require any quirks it was capable of returning 32k
of data (although it repeated every 4k).
---
Tested-by: Shane Seymour <shane.seymour@hpe.com>

[toc] | [prev] | [next] | [standalone]

#1310149

Shane, Thank You very much..

On 1/14/2016 7:07 PM, Seymour, Shane M wrote:
> For the series. Tested with AE311 PCI-Express 4Gb Fibre Channel HBA.
> Truncation of the vpd data returned works (154 bytes) and lspci -vvv
> prints out the VPD tags the same way with and without the changes.
> This card did not require any quirks it was capable of returning 32k
> of data (although it repeated every 4k).
> ---
> Tested-by: Shane Seymour <shane.seymour@hpe.com>
> 

[toc] | [prev] | [next] | [standalone]

#1310153

On 01/15/2016 03:10 PM, Babu Moger wrote:
> Shane, Thank You very much..
>
> On 1/14/2016 7:07 PM, Seymour, Shane M wrote:
>> For the series. Tested with AE311 PCI-Express 4Gb Fibre Channel HBA.
>> Truncation of the vpd data returned works (154 bytes) and lspci -vvv
>> prints out the VPD tags the same way with and without the changes.
>> This card did not require any quirks it was capable of returning 32k
>> of data (although it repeated every 4k).
>> ---
>> Tested-by: Shane Seymour <shane.seymour@hpe.com>
>>
So, Alexander, are you happy with this series?
It looks as if the technical issues are resolved now ...

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		   Teamlead Storage & Networking
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[toc] | [prev] | [next] | [standalone]

#1312510

Hi,

On 1/13/2016 5:25 AM, Hannes Reinecke wrote:
> Hi all,
> 
> the current PCI VPD page access assumes that the entire possible VPD
> data is readable. However, the spec only guarantees a VPD data up to
> the 'end' marker, with everything beyond that being undefined.
> This causes a system lockup on certain devices.
> 
> With this patch we always set the VPD sysfs attribute size to '0', and
> calculate the available VPD size on the first access.
> If no valid data can be read an I/O error is returned.
> 
> I've also included the patch from Babu to blacklists devices which
> are known to lockup when accessing the VPD data.
> 
> Babu Moger (1):
>   pci: Blacklist vpd access for buggy devices
> 
> Hannes Reinecke (3):
>   pci: Update VPD definitions
>   pci: allow access to VPD attributes with size '0'
>   pci: Determine actual VPD size on first access
> 
>  drivers/pci/access.c    | 79 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  drivers/pci/pci-sysfs.c | 22 ++++++++------
>  drivers/pci/quirks.c    | 41 +++++++++++++++++++++++++
>  include/linux/pci.h     | 27 +++++++++++++++--
>  4 files changed, 157 insertions(+), 12 deletions(-)
> 

Resending the patch 4/4. Added Atheros controller(0x1969:0x1026) in blacklist.
Jordan confirmed the Vendor and Device id(0x1969:0x1026). Here is the device.
09:00.0 Ethernet controller: Atheros Communications AR8121/AR8113/AR8114 Gigabit or Fast Ethernet (rev b0)

[toc] | [prev] | [next] | [standalone]

#1314403 — [PATCH v4 4/4] pci: Blacklist vpd access for buggy devices

Reading or Writing of PCI VPD data causes system panic.
We saw this problem by running "lspci -vvv" in the beginning.
However this can be easily reproduced by running
 cat /sys/bus/devices/XX../vpd

As even a simple read on any VPD data triggers a system
lockup on certain cards this patch implements a PCI quirk
to disabling VPD acces altogether by setting the vpd length
to '0'.

Added all the PCI_VENDOR_ID_ATTANSIC varients.

Signed-off-by: Babu Moger <babu.moger@oracle.com>
Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Jordan Hargrave <Jordan_Hargrave@dell.com>
---
 drivers/pci/access.c |    5 ++++-
 drivers/pci/quirks.c |   43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 1 deletions(-)

diff --git a/drivers/pci/access.c b/drivers/pci/access.c
index 914e023..82f41a8 100644
--- a/drivers/pci/access.c
+++ b/drivers/pci/access.c
@@ -396,7 +396,7 @@ static ssize_t pci_vpd_pci22_read(struct pci_dev *dev, loff_t pos, size_t count,
 	if (pos < 0)
 		return -EINVAL;
 
-	if (!vpd->valid) {
+	if (!vpd->valid && vpd->base.len > 0) {
 		vpd->valid = true;
 		vpd->base.len = pci_vpd_pci22_size(dev);
 	}
@@ -459,6 +459,9 @@ static ssize_t pci_vpd_pci22_write(struct pci_dev *dev, loff_t pos, size_t count
 	loff_t end = pos + count;
 	int ret = 0;
 
+	if (vpd->base.len == 0)
+		return -EIO;
+
 	if (!vpd->valid)
 		return -EAGAIN;
 
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index b03373f..f0007e9 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -2123,6 +2123,49 @@ static void quirk_via_cx700_pci_parking_caching(struct pci_dev *dev)
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_VIA, 0x324e, quirk_via_cx700_pci_parking_caching);
 
 /*
+ * A read/write to sysfs entry ('/sys/bus/pci/devices/<id>/vpd')
+ * will dump 32k of data. The default length is set as 32768.
+ * Reading a full 32k will cause an access beyond the VPD end tag.
+ * The system behaviour at that point is mostly unpredictable.
+ * Apparently, some vendors have not implemented this VPD headers properly.
+ * Adding a generic function disable vpd data for these buggy adapters
+ * Add the DECLARE_PCI_FIXUP_FINAL line below with the specific with
+ * vendor and device of interest to use this quirk.
+ */
+static void quirk_blacklist_vpd(struct pci_dev *dev)
+{
+	if (dev->vpd) {
+		dev->vpd->len = 0;
+		dev_warn(&dev->dev, "PCI vpd access has been disabled due to firmware bug\n");
+	}
+}
+
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LSI_LOGIC, 0x0060,
+		quirk_blacklist_vpd);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LSI_LOGIC, 0x007c,
+		quirk_blacklist_vpd);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LSI_LOGIC, 0x0413,
+		quirk_blacklist_vpd);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LSI_LOGIC, 0x0078,
+		quirk_blacklist_vpd);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LSI_LOGIC, 0x0079,
+		quirk_blacklist_vpd);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LSI_LOGIC, 0x0073,
+		quirk_blacklist_vpd);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LSI_LOGIC, 0x0071,
+		quirk_blacklist_vpd);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LSI_LOGIC, 0x005b,
+		quirk_blacklist_vpd);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LSI_LOGIC, 0x002f,
+		quirk_blacklist_vpd);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LSI_LOGIC, 0x005d,
+		quirk_blacklist_vpd);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LSI_LOGIC, 0x005f,
+		quirk_blacklist_vpd);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATTANSIC, PCI_ANY_ID,
+		quirk_blacklist_vpd);
+
+/*
  * For Broadcom 5706, 5708, 5709 rev. A nics, any read beyond the
  * VPD end tag will hang the device.  This problem was initially
  * observed when a vpd entry was created in sysfs
-- 
1.7.1

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web