[PATCH 0/5] PCI: add domain support to aer_inject and other fixups

[PATCH 0/5] PCI: add domain support to aer_inject and other fixups

[Andrew Patterson]

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

[PATCH 1/5] PCI: add pci_get_domain_bus_and_slot function

[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 <andrew.patterson@hp.com>
---

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);
 

[PATCH 2/5] PCI: add support for PCI domains to aer_inject

[Andrew Patterson]

PCI: add support for PCI domains to aer_inject

Add support for PCI domains (segments) to aer_inject.

Signed-off-by: Andrew Patterson <andrew.patterson@hp.com>
---

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);
 	}

[PATCH 3/5] PCI: maintain backwards compatibility with aer-inject user-land tool

[Andrew Patterson]

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<andrew.patterson@hp.com>
---

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;
 

Re: [PATCH 3/5] PCI: maintain backwards compatibility with aer-inject user-land tool

[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<andrew.patterson@hp.com>
> ---
> 
> 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

[PATCH 4/5] PCI: use better error return values in aer_inject

[Andrew Patterson]

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 <andrew.patterson@hp.com>
---

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;
 	}
 

[PATCH 5/5] PCI: fix memory leak in aer_inject

[Andrew Patterson]

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 <andrew.patterson@hp.com>
---

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);
 	}

Re: [PATCH 5/5] PCI: fix memory leak in aer_inject

[Huang Ying]

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 <andrew.patterson@hp.com>
> ---
> 
> 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 <ying.huang@intel.com>

Best Regards,
Huang Ying

Re: [PATCH 5/5] PCI: fix memory leak in aer_inject

[Jesse Barnes]

On Mon, 12 Oct 2009 15:44:30 +0800
Huang Ying <ying.huang@intel.com> 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 <andrew.patterson@hp.com>
> > ---
> > 
> > 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 <ying.huang@intel.com>
> 

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

Re: [PATCH 5/5] PCI: fix memory leak in aer_inject

[Huang Ying]

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 <ying.huang@intel.com> 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 <andrew.patterson@hp.com>
> > > ---
> > > 
> > > 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 <ying.huang@intel.com>
> > 
> 
> 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 <ying.huang@intel.com>

Best Regards,
Huang Ying