2009-10-12 19:14:35

by Andrew Patterson

[permalink] [raw]
Subject: [PATCH v2 0/4] PCI: add domain support to aer_inject and other fixups

This patchset adds PCI domain (segment) support to the aer_inject
driver, changes its error return values to provide more meaningful errors
to the user, and fixes a memory leak.

This patchset is intended for 2.6.33.

Changlog:

v2
-- Merged backwards compatibilty patch with the add domain support patch.


Andrew Patterson (4):
PCI: add pci_get_domain_bus_and_slot function
PCI: add support for PCI domains to aer_inject
PCI: use better error return values in aer_inject
PCI: fix memory leak in aer_inject


drivers/pci/pcie/aer/aer_inject.c | 56 ++++++++++++++++++++++++++-----------
drivers/pci/search.c | 34 +++++++++++-----------
include/linux/pci.h | 8 +++++
3 files changed, 63 insertions(+), 35 deletions(-)


--
Andrew Patterson


2009-10-12 19:14:44

by Andrew Patterson

[permalink] [raw]
Subject: [PATCH v2 1/4] PCI: add pci_get_domain_bus_and_slot function

PCI: add pci_get_domain_bus_and_slot function

Added the pci_get_domain_and_slot_function which is analogous to
pci_get_bus_and_slot. It returns a pci_dev given a domain (segment) number,
bus number, and devnr. Like pci_get_bus_and_slot,
pci_get_domain_bus_and_slot holds a reference to the returned pci_dev.

Converted pci_get_bus_and_slot to a wrapper that calls
pci_get_domain_bus_and_slot with the domain hard-coded to 0.

This routine was patterned off code suggested by Bjorn Helgaas.

Signed-off-by: Andrew Patterson <[email protected]>
---

diff --git a/drivers/pci/search.c b/drivers/pci/search.c
index ec41535..7582648 100644
--- a/drivers/pci/search.c
+++ b/drivers/pci/search.c
@@ -149,32 +149,33 @@ struct pci_dev * pci_get_slot(struct pci_bus *bus, unsigned int devfn)
}

/**
- * pci_get_bus_and_slot - locate PCI device from a given PCI bus & slot
- * @bus: number of PCI bus on which desired PCI device resides
- * @devfn: encodes number of PCI slot in which the desired PCI
- * device resides and the logical device number within that slot
- * in case of multi-function devices.
- *
- * Note: the bus/slot search is limited to PCI domain (segment) 0.
+ * pci_get_domain_bus_and_slot - locate PCI device for a given PCI domain (segment), bus, and slot
+ * @domain: PCI domain/segment on which the PCI device resides.
+ * @bus: PCI bus on which desired PCI device resides
+ * @devfn: encodes number of PCI slot in which the desired PCI device
+ * resides and the logical device number within that slot in case of
+ * multi-function devices.
*
- * Given a PCI bus and slot/function number, the desired PCI device
- * is located in system global list of PCI devices. If the device
- * is found, a pointer to its data structure is returned. If no
- * device is found, %NULL is returned. The returned device has its
- * reference count bumped by one.
+ * Given a PCI domain, bus, and slot/function number, the desired PCI
+ * device is located in the list of PCI devices. If the device is
+ * found, its reference count is increased and this function returns a
+ * pointer to its data structure. The caller must decrement the
+ * reference count by calling pci_dev_put(). If no device is found,
+ * %NULL is returned.
*/
-
-struct pci_dev * pci_get_bus_and_slot(unsigned int bus, unsigned int devfn)
+struct pci_dev *pci_get_domain_bus_and_slot(int domain, unsigned int bus,
+ unsigned int devfn)
{
struct pci_dev *dev = NULL;

while ((dev = pci_get_device(PCI_ANY_ID, PCI_ANY_ID, dev)) != NULL) {
- if (pci_domain_nr(dev->bus) == 0 &&
- (dev->bus->number == bus && dev->devfn == devfn))
+ if (pci_domain_nr(dev->bus) == domain &&
+ (dev->bus->number == bus && dev->devfn == devfn))
return dev;
}
return NULL;
}
+EXPORT_SYMBOL(pci_get_domain_bus_and_slot);

static int match_pci_dev_by_id(struct device *dev, void *data)
{
@@ -354,5 +355,4 @@ EXPORT_SYMBOL(pci_find_next_bus);
EXPORT_SYMBOL(pci_get_device);
EXPORT_SYMBOL(pci_get_subsys);
EXPORT_SYMBOL(pci_get_slot);
-EXPORT_SYMBOL(pci_get_bus_and_slot);
EXPORT_SYMBOL(pci_get_class);
diff --git a/include/linux/pci.h b/include/linux/pci.h
index f5c7cd3..b9b5cd5 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -635,7 +635,13 @@ struct pci_dev *pci_get_subsys(unsigned int vendor, unsigned int device,
unsigned int ss_vendor, unsigned int ss_device,
struct pci_dev *from);
struct pci_dev *pci_get_slot(struct pci_bus *bus, unsigned int devfn);
-struct pci_dev *pci_get_bus_and_slot(unsigned int bus, unsigned int devfn);
+struct pci_dev *pci_get_domain_bus_and_slot(int domain, unsigned int bus,
+ unsigned int devfn);
+static inline struct pci_dev *pci_get_bus_and_slot(unsigned int bus,
+ unsigned int devfn)
+{
+ return pci_get_domain_bus_and_slot(0, bus, devfn);
+}
struct pci_dev *pci_get_class(unsigned int class, struct pci_dev *from);
int pci_dev_present(const struct pci_device_id *ids);

2009-10-12 19:14:50

by Andrew Patterson

[permalink] [raw]
Subject: [PATCH v2 2/4] PCI: add support for PCI domains to aer_inject

PCI: add support for PCI domains to aer_inject

Add support for PCI domains (segments) to aer_inject.

Signed-off-by: Andrew Patterson <[email protected]>
---

diff --git a/drivers/pci/pcie/aer/aer_inject.c b/drivers/pci/pcie/aer/aer_inject.c
index 62d15f6..ac0b5e7 100644
--- a/drivers/pci/pcie/aer/aer_inject.c
+++ b/drivers/pci/pcie/aer/aer_inject.c
@@ -23,6 +23,7 @@
#include <linux/pci.h>
#include <linux/fs.h>
#include <linux/uaccess.h>
+#include <linux/stddef.h>
#include "aerdrv.h"

struct aer_error_inj {
@@ -35,10 +36,12 @@ struct aer_error_inj {
u32 header_log1;
u32 header_log2;
u32 header_log3;
+ u16 domain;
};

struct aer_error {
struct list_head list;
+ u16 domain;
unsigned int bus;
unsigned int devfn;
int pos_cap_err;
@@ -66,22 +69,27 @@ static LIST_HEAD(pci_bus_ops_list);
/* Protect einjected and pci_bus_ops_list */
static DEFINE_SPINLOCK(inject_lock);

-static void aer_error_init(struct aer_error *err, unsigned int bus,
- unsigned int devfn, int pos_cap_err)
+static void aer_error_init(struct aer_error *err, u16 domain,
+ unsigned int bus, unsigned int devfn,
+ int pos_cap_err)
{
INIT_LIST_HEAD(&err->list);
+ err->domain = domain;
err->bus = bus;
err->devfn = devfn;
err->pos_cap_err = pos_cap_err;
}

/* inject_lock must be held before calling */
-static struct aer_error *__find_aer_error(unsigned int bus, unsigned int devfn)
+static struct aer_error *__find_aer_error(u16 domain, unsigned int bus,
+ unsigned int devfn)
{
struct aer_error *err;

list_for_each_entry(err, &einjected, list) {
- if (bus == err->bus && devfn == err->devfn)
+ if (domain == err->domain &&
+ bus == err->bus &&
+ devfn == err->devfn)
return err;
}
return NULL;
@@ -90,7 +98,10 @@ static struct aer_error *__find_aer_error(unsigned int bus, unsigned int devfn)
/* inject_lock must be held before calling */
static struct aer_error *__find_aer_error_by_dev(struct pci_dev *dev)
{
- return __find_aer_error(dev->bus->number, dev->devfn);
+ int domain = pci_domain_nr(dev->bus);
+ if (domain < 0)
+ return NULL;
+ return __find_aer_error((u16)domain, dev->bus->number, dev->devfn);
}

/* inject_lock must be held before calling */
@@ -172,11 +183,15 @@ static int pci_read_aer(struct pci_bus *bus, unsigned int devfn, int where,
struct aer_error *err;
unsigned long flags;
struct pci_ops *ops;
+ int domain;

spin_lock_irqsave(&inject_lock, flags);
if (size != sizeof(u32))
goto out;
- err = __find_aer_error(bus->number, devfn);
+ domain = pci_domain_nr(bus);
+ if (domain < 0)
+ goto out;
+ err = __find_aer_error((u16)domain, bus->number, devfn);
if (!err)
goto out;

@@ -200,11 +215,15 @@ int pci_write_aer(struct pci_bus *bus, unsigned int devfn, int where, int size,
unsigned long flags;
int rw1cs;
struct pci_ops *ops;
+ int domain;

spin_lock_irqsave(&inject_lock, flags);
if (size != sizeof(u32))
goto out;
- err = __find_aer_error(bus->number, devfn);
+ domain = pci_domain_nr(bus);
+ if (domain < 0)
+ goto out;
+ err = __find_aer_error((u16)domain, bus->number, devfn);
if (!err)
goto out;

@@ -305,7 +324,7 @@ static int aer_inject(struct aer_error_inj *einj)
u32 sever;
int ret = 0;

- dev = pci_get_bus_and_slot(einj->bus, devfn);
+ dev = pci_get_domain_bus_and_slot((int)einj->domain, einj->bus, devfn);
if (!dev)
return -EINVAL;
rpdev = pcie_find_root_port(dev);
@@ -344,7 +363,8 @@ static int aer_inject(struct aer_error_inj *einj)
if (!err) {
err = err_alloc;
err_alloc = NULL;
- aer_error_init(err, einj->bus, devfn, pos_cap_err);
+ aer_error_init(err, einj->domain, einj->bus, devfn,
+ pos_cap_err);
list_add(&err->list, &einjected);
}
err->uncor_status |= einj->uncor_status;
@@ -358,7 +378,8 @@ static int aer_inject(struct aer_error_inj *einj)
if (!rperr) {
rperr = rperr_alloc;
rperr_alloc = NULL;
- aer_error_init(rperr, rpdev->bus->number, rpdev->devfn,
+ aer_error_init(rperr, pci_domain_nr(rpdev->bus),
+ rpdev->bus->number, rpdev->devfn,
rp_pos_cap_err);
list_add(&rperr->list, &einjected);
}
@@ -411,10 +432,11 @@ static ssize_t aer_inject_write(struct file *filp, const char __user *ubuf,

if (!capable(CAP_SYS_ADMIN))
return -EPERM;
-
- if (usize != sizeof(struct aer_error_inj))
+ if (usize < offsetof(struct aer_error_inj, domain) ||
+ usize > sizeof(einj))
return -EINVAL;

+ memset(&einj, 0, sizeof(einj));
if (copy_from_user(&einj, ubuf, usize))
return -EFAULT;

2009-10-12 19:14:58

by Andrew Patterson

[permalink] [raw]
Subject: [PATCH v2 3/4] PCI: use better error return values in aer_inject

PCI: use better error return values in aer_inject

Replaced some error return values in aer_inject. Use -ENODEV when we
can't find a device and -ENOTTY when the device does not support PCIe AER.

Signed-off-by: Andrew Patterson <[email protected]>
---

diff --git a/drivers/pci/pcie/aer/aer_inject.c b/drivers/pci/pcie/aer/aer_inject.c
index ac0b5e7..da2ad6e 100644
--- a/drivers/pci/pcie/aer/aer_inject.c
+++ b/drivers/pci/pcie/aer/aer_inject.c
@@ -326,23 +326,23 @@ static int aer_inject(struct aer_error_inj *einj)

dev = pci_get_domain_bus_and_slot((int)einj->domain, einj->bus, devfn);
if (!dev)
- return -EINVAL;
+ return -ENODEV;
rpdev = pcie_find_root_port(dev);
if (!rpdev) {
- ret = -EINVAL;
+ ret = -ENOTTY;
goto out_put;
}

pos_cap_err = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ERR);
if (!pos_cap_err) {
- ret = -EIO;
+ ret = -ENOTTY;
goto out_put;
}
pci_read_config_dword(dev, pos_cap_err + PCI_ERR_UNCOR_SEVER, &sever);

rp_pos_cap_err = pci_find_ext_capability(rpdev, PCI_EXT_CAP_ID_ERR);
if (!rp_pos_cap_err) {
- ret = -EIO;
+ ret = -ENOTTY;
goto out_put;
}

2009-10-12 19:15:14

by Andrew Patterson

[permalink] [raw]
Subject: [PATCH v2 4/4] PCI: fix memory leak in aer_inject

PCI: fix memory leak in aer_inject

Fixed probable typo in aer_inject cleanup code resulting in a memory
leak.

Signed-off-by: Andrew Patterson <[email protected]>
---

diff --git a/drivers/pci/pcie/aer/aer_inject.c b/drivers/pci/pcie/aer/aer_inject.c
index da2ad6e..2246bf7 100644
--- a/drivers/pci/pcie/aer/aer_inject.c
+++ b/drivers/pci/pcie/aer/aer_inject.c
@@ -474,7 +474,7 @@ static void __exit aer_inject_exit(void)
}

spin_lock_irqsave(&inject_lock, flags);
- list_for_each_entry_safe(err, err_next, &pci_bus_ops_list, list) {
+ list_for_each_entry_safe(err, err_next, &einjected, list) {
list_del(&err->list);
kfree(err);
}

2009-11-04 17:25:57

by Jesse Barnes

[permalink] [raw]
Subject: Re: [PATCH v2 1/4] PCI: add pci_get_domain_bus_and_slot function

On Mon, 12 Oct 2009 13:14:00 -0600
Andrew Patterson <[email protected]> wrote:

> PCI: add pci_get_domain_bus_and_slot function
>
> Added the pci_get_domain_and_slot_function which is analogous to
> pci_get_bus_and_slot. It returns a pci_dev given a domain (segment)
> number, bus number, and devnr. Like pci_get_bus_and_slot,
> pci_get_domain_bus_and_slot holds a reference to the returned pci_dev.
>
> Converted pci_get_bus_and_slot to a wrapper that calls
> pci_get_domain_bus_and_slot with the domain hard-coded to 0.
>
> This routine was patterned off code suggested by Bjorn Helgaas.

Applied this series, thanks Andrew.

--
Jesse Barnes, Intel Open Source Technology Center