On Thu, Jun 26, 2008 at 09:09:25PM -0700, Benjamin Li wrote:
> 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
> ('/sys/bus/pci/devices/<id>/vpd'). A read to this sysfs entry
> will dump 32k of data. Reading a full 32k will cause an access
> beyond the VPD end tag causing the device to hang. Once the device
> is hung, the bnx2 driver will not be able to reset the device.
> We believe that it is legal to read beyond the end tag and
> therefore the solution is to disable the VPD capability and
> not expose this data.
Different scholars have differing opinions on whether it's legal to read
beyond the end tag. Regardless of the law, there's devices in the field
that have this problem, so let's not throw the baby out with the
bathwater.
I think with the below patch you can now do something like this:
static void __devinit quirk_brcm_570x_shorten_vpd(struct pci_dev *dev)
{
dev->vpd->base.len = 8192;
}
We might also want to consider parsing the top level of the VPD data
structures to find out how long they are.
diff --git a/drivers/pci/access.c b/drivers/pci/access.c
index ec8f700..39bb96b 100644
--- a/drivers/pci/access.c
+++ b/drivers/pci/access.c
@@ -178,8 +178,7 @@ static int pci_vpd_pci22_read(struct pci_dev *dev, int pos, int size,
int ret;
int begin, end, i;
- if (pos < 0 || pos > PCI_VPD_PCI22_SIZE ||
- size > PCI_VPD_PCI22_SIZE - pos)
+ if (pos < 0 || pos > vpd->base.len || size > vpd->base.len - pos)
return -EINVAL;
if (size == 0)
return 0;
@@ -223,8 +222,8 @@ static int pci_vpd_pci22_write(struct pci_dev *dev, int pos, int size,
u32 val;
int ret;
- if (pos < 0 || pos > PCI_VPD_PCI22_SIZE || pos & 3 ||
- size > PCI_VPD_PCI22_SIZE - pos || size < 4)
+ if (pos < 0 || pos > vpd->base.len || pos & 3 ||
+ size > vpd->base.len - pos || size < 4)
return -EINVAL;
val = (u8) *buf++;
@@ -255,11 +254,6 @@ out:
return 4;
}
-static int pci_vpd_pci22_get_size(struct pci_dev *dev)
-{
- return PCI_VPD_PCI22_SIZE;
-}
-
static void pci_vpd_pci22_release(struct pci_dev *dev)
{
kfree(container_of(dev->vpd, struct pci_vpd_pci22, base));
@@ -268,7 +262,6 @@ static void pci_vpd_pci22_release(struct pci_dev *dev)
static struct pci_vpd_ops pci_vpd_pci22_ops = {
.read = pci_vpd_pci22_read,
.write = pci_vpd_pci22_write,
- .get_size = pci_vpd_pci22_get_size,
.release = pci_vpd_pci22_release,
};
@@ -284,6 +277,7 @@ int pci_vpd_pci22_init(struct pci_dev *dev)
if (!vpd)
return -ENOMEM;
+ vpd->base.len = PCI_VPD_PCI22_SIZE;
vpd->base.ops = &pci_vpd_pci22_ops;
spin_lock_init(&vpd->lock);
vpd->cap = cap;
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 271d41c..fce3537 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -697,7 +697,7 @@ int __must_check pci_create_sysfs_dev_files (struct pci_dev *pdev)
attr = kzalloc(sizeof(*attr), GFP_ATOMIC);
if (attr) {
pdev->vpd->attr = attr;
- attr->size = pdev->vpd->ops->get_size(pdev);
+ attr->size = pdev->vpd->len;
attr->attr.name = "vpd";
attr->attr.mode = S_IRUGO | S_IWUSR;
attr->read = pci_read_vpd;
diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
index 0a497c1..00408c9 100644
--- a/drivers/pci/pci.h
+++ b/drivers/pci/pci.h
@@ -21,11 +21,11 @@ extern int pci_user_write_config_dword(struct pci_dev *dev, int where, u32 val);
struct pci_vpd_ops {
int (*read)(struct pci_dev *dev, int pos, int size, char *buf);
int (*write)(struct pci_dev *dev, int pos, int size, const char *buf);
- int (*get_size)(struct pci_dev *dev);
void (*release)(struct pci_dev *dev);
};
struct pci_vpd {
+ unsigned int len;
struct pci_vpd_ops *ops;
struct bin_attribute *attr; /* descriptor for sysfs VPD entry */
};
> Signed-off-by: Benjamin Li <benli@xxxxxxxxxxxx>
> Signed-off-by: Michael Chan <mchan@xxxxxxxxxxxx>
> ---
> drivers/pci/quirks.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 files changed, 68 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index dabb563..46b9f2f 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -1670,6 +1670,74 @@ static void __devinit quirk_via_cx700_pci_parking_caching(struct pci_dev *dev)
> }
> DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_VIA, 0x324e, quirk_via_cx700_pci_parking_caching);
>
> +
> +/*
> + * 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
> + * ('/sys/bus/pci/devices/<id>/vpd'). A read to this sysfs entry
> + * will dump 32k of data. Reading a full 32k will cause an access
> + * beyond the VPD end tag causing the device to hang. Once the device
> + * is hung, the bnx2 driver will not be able to reset the device.
> + * We believe that it is legal to read beyond the end tag and
> + * therefore the solution is to disable the VPD capability and
> + * not expose this data.
> + */
> +static void __devinit quirk_brcm_570x_disable_vpd(struct pci_dev *dev)
> +{
> + u8 rev;
> + u16 device_id;
> +
> + /* Get PCI device ID and revision number */
> + pci_read_config_word(dev, PCI_DEVICE_ID, &device_id);
> + pci_read_config_byte(dev, PCI_REVISION_ID, &rev);
> +
> + /* Configure byte swap and enable write to the reg_window registers */
> + pci_write_config_dword(dev, 0x68, (1L<<7) /* enable reg_window reg */ |
> + (1L<<3) /* configure byte swap */);
> +
> + /* Only disable the VPD capability for 5706, 5708, and 5709 rev. A */
> + if ((device_id == PCI_DEVICE_ID_NX2_5706) ||
> + (device_id == PCI_DEVICE_ID_NX2_5708) ||
> + ((device_id == PCI_DEVICE_ID_NX2_5709) &&
> + (rev & 0xf0) == 0x0)) {
> + u32 cap_ena;
> +
> + /* set proper window to id_val4 */
> + pci_write_config_dword(dev, 0x78, 0x440);
> +
> + /* Only enable PM, MSI, MSIX, PCIE */
> + pci_read_config_dword(dev, 0x80, &cap_ena);
> +
> + if ((device_id == PCI_DEVICE_ID_NX2_5706) ||
> + (device_id == PCI_DEVICE_ID_NX2_5708))
> + cap_ena = (cap_ena & 0xffffffff0) | 0x0e;
> + else
> + cap_ena = (cap_ena & 0xffffffff0) | 0x0b;
> +
> + pci_write_config_dword(dev, 0x80, cap_ena);
> + }
> +}
> +
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_BROADCOM,
> + PCI_DEVICE_ID_NX2_5706,
> + quirk_brcm_570x_disable_vpd);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_BROADCOM,
> + PCI_DEVICE_ID_NX2_5706S,
> + quirk_brcm_570x_disable_vpd);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_BROADCOM,
> + PCI_DEVICE_ID_NX2_5708,
> + quirk_brcm_570x_disable_vpd);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_BROADCOM,
> + PCI_DEVICE_ID_NX2_5708S,
> + quirk_brcm_570x_disable_vpd);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_BROADCOM,
> + PCI_DEVICE_ID_NX2_5709,
> + quirk_brcm_570x_disable_vpd);
> +DECLARE_PCI_FIXUP_EARLY(PCI_VENDOR_ID_BROADCOM,
> + PCI_DEVICE_ID_NX2_5709S,
> + quirk_brcm_570x_disable_vpd);
> +
> #ifdef CONFIG_PCI_MSI
> /* Some chipsets do not support MSI. We cannot easily rely on setting
> * PCI_BUS_FLAGS_NO_MSI in its bus flags because there are actually
> --
> 1.5.5.1
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
