lpfc : add module parameter that allows adapter instances to avoid attachment

This patch adds a module parameter that supplies a text string containing a list of PCI <bus>:<slot>.<func> values to identify adapter instances that should *not* be attached to by the driver.

-- james s

Signed-off-by James Smart <[email protected]>

Quoted 8 lines · James Smart

This patch adds a module parameter that supplies a text string containing a list of PCI <bus>:<slot>.<func> values to identify adapter instances that should *not* be attached to by the driver.

-- james s

Signed-off-by James Smart <[email protected]>

Is there a reason why you can't just use the generic unbind interface?

James

The oem requirement we have explicitly states not to use the unbind interface
(we proposed unbind at first as well).

The issue is what happens on the link while we are bound for that short amount of time. It confuses the things on the other side of the link. There's a secondary driver that ends up binding to the adapters we exclude, and the things on the other side only expected to see the second driver.

-- james s

Quoted 6 lines · James Bottomley

Quoted 5 lines · James Smart

This patch adds a module parameter that supplies a text string containing a
list of PCI <bus>:<slot>.<func> values to identify adapter instances that
should *not* be attached to by the driver.

Quoted 8 lines

Quoted 4 lines

-- james s

Signed-off-by James Smart <[email protected]>

Is there a reason why you can't just use the generic unbind interface?

James

The oem requirement we have explicitly states not to use the unbind interface (we proposed unbind at first as well).

The issue is what happens on the link while we are bound for that short amount of time. It confuses the things on the other side of the link. There's a secondary driver that ends up binding to the adapters we exclude, and the things on the other side only expected to see the second driver.

Quoted 7 lines · James Smart

The oem requirement we have explicitly states not to use the unbind interface (we proposed unbind at first as well).

The issue is what happens on the link while we are bound for that short amount of time. It confuses the things on the other side of the link. There's a secondary driver that ends up binding to the adapters we exclude, and the things on the other side only expected to see the second driver.

OK, so it sounds like a proposed extension to the generic unbind interfaces where you specify a no_bind list on the kernel or module command line.

Also (as Randy pointed out) you don't account for the domain. However, you could just by not bothering to convert the string to numbers for the compare, just compare on dev->bus_id ... which is how a generic one would work.

James

Also (as Randy pointed out) you don't account for the domain. However, you could just by not bothering to convert the string to numbers for the compare, just compare on dev->bus_id ... which is how a generic one would work.

Quoted 4 lines · James Bottomley

Also (as Randy pointed out) you don't account for the domain. However, you could just by not bothering to convert the string to numbers for the compare, just compare on dev->bus_id ... which is how a generic one would work.

I think it needs to be PCI and not device core. bus_id isn't guaranteed to be unique between different bus types. I don't know of any that conflict, but we don't want to inadvertently disable, say, an SBUS device while trying to disable a PCI device.

Actually they better be ... we'll get into real trouble if they're not because of the way we flatten the space for multiple bus binding drivers.

Even if I accepted your argument, I still can't see why we'd only implement this for PCI, and thus why it shouldn't be in the generic device part (except possibly with a bus type name qualifier).

Quoted 3 lines · James Bottomley

Actually they better be ... we'll get into real trouble if they're not because of the way we flatten the space for multiple bus binding drivers.

Er ... where do we do that? As far as I'm aware, to bind to multiple busses, you register multiple foo_driver. They're separate namespaces.

Quoted 3 lines

Even if I accepted your argument, I still can't see why we'd only implement this for PCI, and thus why it shouldn't be in the generic device part (except possibly with a bus type name qualifier).

Adding the bus name qualifier would work. It seems like much more typing, in order to get what advantage over just implementing it for PCI?

The oem requirement we have explicitly states not to use the unbind interface
(we proposed unbind at first as well).

The issue is what happens on the link while we are bound for that short amount of time. It confuses the things on the other side of the link. There's a secondary driver that ends up binding to the adapters we exclude, and the things on the other side only expected to see the second driver.

This patch adds a module parameter that supplies a text string containing a list of PCI <bus>:<slot>.<func> values to identify adapter instances that should *not* be attached to by the driver.

-- james s

Signed-off-by James Smart <[email protected]>

---

 lpfc_attr.c |   21 ++++++++++++++++++++
 lpfc_crtn.h |    1 
 lpfc_init.c |   61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 83 insertions(+)

diff -upNr a/drivers/scsi/lpfc/lpfc_attr.c b/drivers/scsi/lpfc/lpfc_attr.c

drivers/scsi/lpfc/lpfc_attr.c +21 -0

@@ -1451,6 +1451,26 @@ lpfc_##attr##_store(struct device *dev, 
 		return -EINVAL;\
 }
 
+/*
+# lpfc_exclude_hba: This parameter contain a list of PCI slots with lpfc HBAs
+# 	which need to be excluded from attachment by the driver.
+#  String Syntax: <bus>:<slot>.<func>[|<bus>:<slot>.<func>...]
+*/
+char *lpfc_exclude_hba;
+module_param(lpfc_exclude_hba, charp, S_IRUGO);
+MODULE_PARM_DESC(lpfc_exclude_hba, "list of lpfc HBA PCI locations"
+	" to be excluded from attachment:  '<bus>:<slot>.<func>' separated by"
+	" | character");
+static ssize_t
+lpfc_exclude_hba_show(struct device *dev, struct device_attribute *attr,
+	char *buf)
+{
+	return snprintf(buf, PAGE_SIZE, "%s\n",
+		(lpfc_exclude_hba == NULL) ? "" : lpfc_exclude_hba);
+}
+
+static DEVICE_ATTR(lpfc_exclude_hba, S_IRUGO, lpfc_exclude_hba_show,
+		NULL);
 
 #define LPFC_ATTR(name, defval, minval, maxval, desc) \
 static int lpfc_##name = defval;\

@@ -2939,6 +2959,7 @@ struct device_attribute *lpfc_hba_attrs[
 	&dev_attr_lpfc_max_scsicmpl_time,
 	&dev_attr_lpfc_stat_data_ctrl,
 	&dev_attr_lpfc_prot_sg_seg_cnt,
+	&dev_attr_lpfc_exclude_hba,
 	NULL,
 };
 

diff -upNr a/drivers/scsi/lpfc/lpfc_crtn.h b/drivers/scsi/lpfc/lpfc_crtn.h

drivers/scsi/lpfc/lpfc_crtn.h +1 -0

@@ -264,6 +264,7 @@ extern struct fc_function_template lpfc_
 extern struct fc_function_template lpfc_vport_transport_functions;
 extern int lpfc_sli_mode;
 extern int lpfc_enable_npiv;
+extern char *lpfc_exclude_hba;
 
 int  lpfc_vport_symbolic_node_name(struct lpfc_vport *, char *, size_t);
 int  lpfc_vport_symbolic_port_name(struct lpfc_vport *, char *,	size_t);

diff -upNr a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c

drivers/scsi/lpfc/lpfc_init.c +61 -1

@@ -33,6 +33,7 @@
 #include <scsi/scsi_device.h>
 #include <scsi/scsi_host.h>
 #include <scsi/scsi_transport_fc.h>
+#include <linux/ctype.h>
 
 #include "lpfc_hw.h"
 #include "lpfc_sli.h"

@@ -54,6 +55,7 @@ spinlock_t _dump_buf_lock;
 static int lpfc_parse_vpd(struct lpfc_hba *, uint8_t *, int);
 static void lpfc_get_hba_model_desc(struct lpfc_hba *, uint8_t *, uint8_t *);
 static int lpfc_post_rcv_buf(struct lpfc_hba *);
+static uint32_t lpfc_is_excluded_hba(struct pci_dev *pdev);
 
 static struct scsi_transport_template *lpfc_transport_template = NULL;
 static struct scsi_transport_template *lpfc_vport_transport_template = NULL;

@@ -2607,6 +2609,17 @@ lpfc_pci_probe_one(struct pci_dev *pdev,
 	int bars = pci_select_bars(pdev, IORESOURCE_MEM);
 	struct lpfc_adapter_event_header adapter_event;
 
+	/*
+	 * Check if FC controller is excluded from binding to lpfc driver.
+	 */
+	if (lpfc_is_excluded_hba(pdev)) {
+		dev_printk(KERN_ERR, &pdev->dev, "%s: controller %02x:%02x.%x"
+			" is excluded from binding.\n",
+			LPFC_DRIVER_NAME, pdev->bus->number,
+			PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
+		return -EPERM;
+	}
+
 	if (pci_enable_device_mem(pdev))
 		goto out;
 	if (pci_request_selected_regions(pdev, bars, LPFC_DRIVER_NAME))

@@ -3471,6 +3484,54 @@ lpfc_init(void)
 	return error;
 }
 
+/*
+ * lpfc_is_excluded_hba - Check if the PCI device is excluded by user.
+ * @pdev: pointer to pci device.
+ *
+ * Return 1 if the pci device is excluded by the lpfc_exclude_hba
+ * parameter else return 0.
+ */
+static uint32_t
+lpfc_is_excluded_hba(struct pci_dev *pdev)
+{
+	u_int	bus, slot, func;
+	char	*cp, pci_buf[9];
+	char	*conf_p = lpfc_exclude_hba;
+
+	if (!conf_p)
+		return 0;
+
+	while (*conf_p) {
+		if (*conf_p == '|' || isspace((int)*conf_p)) {
+			conf_p++;
+			continue;
+		}
+		cp = pci_buf;
+
+		do {
+			*cp = *conf_p;
+			cp++;
+			conf_p++;
+			if (!(*conf_p) || (*conf_p == '-') || (*conf_p == '|')
+				|| isspace((int)*conf_p))
+				break;
+		} while (cp < pci_buf + sizeof(pci_buf));
+
+		*cp = '\0';
+
+		if (sscanf(pci_buf, "%x:%x.%x", &bus, &slot, &func) != 3 ||
+			(bus | slot | func) > 0xff)
+			continue;
+
+		if ((u_char)bus == pdev->bus->number &&
+		    (u_char)slot == PCI_SLOT(pdev->devfn) &&
+		    (u_char)func == PCI_FUNC(pdev->devfn)) {
+			return 1;
+		}
+	}
+	return 0;
+}
+
 /**
  * lpfc_exit: lpfc module removal routine.
  *
 
 
--

To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html