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.
Andrew Patterson (5):
PCI: add pci_get_domain_bus_and_slot function
PCI: add support for PCI domains to aer_inject
PCI: maintain backwards compatibility with aer-inject user-land tool
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
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);
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..5fc5b3e 100644
--- a/drivers/pci/pcie/aer/aer_inject.c
+++ b/drivers/pci/pcie/aer/aer_inject.c
@@ -26,6 +26,7 @@
#include "aerdrv.h"
struct aer_error_inj {
+ u16 domain;
u8 bus;
u8 dev;
u8 fn;
@@ -39,6 +40,7 @@ struct aer_error_inj {
struct aer_error {
struct list_head list;
+ u16 domain;
unsigned int bus;
unsigned int devfn;
int pos_cap_err;
@@ -66,22 +68,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 +97,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 +182,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 +214,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 +323,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 +362,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 +377,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);
}
PCI: maintain backwards compatibility with aer-inject user-land tool
Maintain backwards compatibility with the 0.1 version of the aer-inject
user-land tool.
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 5fc5b3e..ac0b5e7 100644
--- a/drivers/pci/pcie/aer/aer_inject.c
+++ b/drivers/pci/pcie/aer/aer_inject.c
@@ -23,10 +23,10 @@
#include <linux/pci.h>
#include <linux/fs.h>
#include <linux/uaccess.h>
+#include <linux/stddef.h>
#include "aerdrv.h"
struct aer_error_inj {
- u16 domain;
u8 bus;
u8 dev;
u8 fn;
@@ -36,6 +36,7 @@ struct aer_error_inj {
u32 header_log1;
u32 header_log2;
u32 header_log3;
+ u16 domain;
};
struct aer_error {
@@ -431,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;
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;
}
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);
}
On Mon, 2009-10-12 at 15:03 +0800, Andrew Patterson wrote:
> 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);
> }
Thanks for the patch.
Acked-by: Huang Ying <[email protected]>
Best Regards,
Huang Ying
On Mon, 2009-10-12 at 15:03 +0800, Andrew Patterson wrote:
> PCI: maintain backwards compatibility with aer-inject user-land tool
>
> Maintain backwards compatibility with the 0.1 version of the aer-inject
> user-land tool.
>
> 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 5fc5b3e..ac0b5e7 100644
> --- a/drivers/pci/pcie/aer/aer_inject.c
> +++ b/drivers/pci/pcie/aer/aer_inject.c
> @@ -23,10 +23,10 @@
> #include <linux/pci.h>
> #include <linux/fs.h>
> #include <linux/uaccess.h>
> +#include <linux/stddef.h>
> #include "aerdrv.h"
>
> struct aer_error_inj {
> - u16 domain;
> u8 bus;
> u8 dev;
> u8 fn;
> @@ -36,6 +36,7 @@ struct aer_error_inj {
> u32 header_log1;
> u32 header_log2;
> u32 header_log3;
> + u16 domain;
> };
>
> struct aer_error {
> @@ -431,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;
>
Why not merge this patch with [2/5]?
Best Regards,
Huang Ying
On Mon, 12 Oct 2009 15:44:30 +0800
Huang Ying <[email protected]> wrote:
> On Mon, 2009-10-12 at 15:03 +0800, Andrew Patterson wrote:
> > 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);
> > }
>
> Thanks for the patch.
>
> Acked-by: Huang Ying <[email protected]>
>
How do the rest of the patches look, Ying? Should I add your acked-by
on Andrew's latest set?
Thanks,
--
Jesse Barnes, Intel Open Source Technology Center
Hi, Jesse,
On Tue, 2009-10-27 at 05:21 +0800, Jesse Barnes wrote:
> On Mon, 12 Oct 2009 15:44:30 +0800
> Huang Ying <[email protected]> wrote:
>
> > On Mon, 2009-10-12 at 15:03 +0800, Andrew Patterson wrote:
> > > 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);
> > > }
> >
> > Thanks for the patch.
> >
> > Acked-by: Huang Ying <[email protected]>
> >
>
> How do the rest of the patches look, Ying? Should I add your acked-by
> on Andrew's latest set?
The whole latest patchset looks good to me.
Acked-by: Huang Ying <[email protected]>
Best Regards,
Huang Ying