[PATCH 13/15] qla4xxx: Added bsg support

[PATCH 13/15] qla4xxx: Added bsg support

[Vikas Chaudhary]

Added BSG support to enable application support to configure
ISP40XX/ISP82XX adapter.

This patch is on top of: http://marc.info/?l=linux-scsi&m=126999297630764&w=2

Signed-off-by: Vikas Chaudhary <vikas.chaudhary@qlogic.com>
Signed-off-by: Ravi Anand <ravi.anand@qlogic.com>
---
 drivers/scsi/qla4xxx/ql4_def.h  |   45 +++++++
 drivers/scsi/qla4xxx/ql4_glbl.h |   18 +++
 drivers/scsi/qla4xxx/ql4_mbx.c  |  262 ++++++++++++++++++++++++++++++++++++++-
 drivers/scsi/qla4xxx/ql4_os.c   |  218 ++++++++++++++++++++++++++++++++
 4 files changed, 541 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/qla4xxx/ql4_def.h b/drivers/scsi/qla4xxx/ql4_def.h
index f28e822..b227bbc 100644
--- a/drivers/scsi/qla4xxx/ql4_def.h
+++ b/drivers/scsi/qla4xxx/ql4_def.h
@@ -584,6 +584,51 @@ struct scsi_qla_host {
 	struct completion mbx_intr_comp;
 };
 
+struct qla4xxx_dma_mem {
+	void *va;
+	dma_addr_t dma;
+	uint32_t size;
+	uint32_t req_sg_cnt;
+	uint32_t rsp_sg_cnt;
+};
+
+struct qla4xxx_bsg_cmd {
+	uint32_t opcode;
+	uint32_t subcode;
+	uint32_t instance;
+	uint32_t data_size;
+	uint32_t offset;
+	uint32_t option0;
+	uint32_t option1;
+	uint32_t option2;
+	uint32_t option3;
+};
+
+enum ql4_vendor_priv_cmd {
+	QL4_GET_DATA,
+	QL4_SET_DATA,
+	QL4_RESTORE_FACTORY_DEF,
+	QL4_DISABLE_ACB,
+};
+
+enum ql4_get_data_cmd {
+	QL4_GET_FLASH,
+	QL4_GET_DDB_DEF,
+	QL4_GET_DDB,
+	QL4_GET_IFCB_DEF,
+	QL4_GET_IFCB,
+	QL4_GET_ACB,
+	QL4_GET_ISCSI_STAT,
+};
+
+enum ql4_set_data_cmd {
+	QL4_SET_FLASH,
+	QL4_SET_DDB,
+	QL4_SET_IFCB,
+	QL4_SET_ACB,
+	QL4_RESET_ISCSI_STAT,
+};
+
 static inline int is_ipv4_enabled(struct scsi_qla_host *ha)
 {
 	return ((ha->ipv4_options & IPOPT_IPv4_PROTOCOL_ENABLE) != 0);
diff --git a/drivers/scsi/qla4xxx/ql4_glbl.h b/drivers/scsi/qla4xxx/ql4_glbl.h
index 9aecfdf..4140dfd 100644
--- a/drivers/scsi/qla4xxx/ql4_glbl.h
+++ b/drivers/scsi/qla4xxx/ql4_glbl.h
@@ -38,6 +38,24 @@ int qla4xxx_get_firmware_state(struct scsi_qla_host *ha, uint32_t *status);
 int qla4xxx_initialize_fw_cb(struct scsi_qla_host *ha);
 int qla4xxx_get_ip_state(struct scsi_qla_host *ha, uint32_t *mbox_status);
 int qla4xxx_get_ddb_info(struct scsi_qla_host *ha, uint32_t *mbox_status);
+int qla4xxx_get_default_ddb(struct scsi_qla_host *ha,
+		dma_addr_t dma_addr);
+int qla4xxx_bsg_get_ifcb(struct scsi_qla_host *ha, dma_addr_t data_dma,
+		uint32_t sub_cmd);
+int qla4xxx_get_acb(struct scsi_qla_host *ha, dma_addr_t data_dma,
+		uint32_t instance, uint32_t data_len);
+int qla4xxx_get_iscsi_stat(struct scsi_qla_host *ha, uint32_t device_index,
+		dma_addr_t data_dma);
+int qla4xxx_set_flash(struct scsi_qla_host *ha, dma_addr_t dma_addr,
+		uint32_t offset, uint32_t option, uint32_t data_len);
+int qla4xxx_bsg_set_ifcb(struct scsi_qla_host *ha, dma_addr_t data_dma);
+int qla4xxx_set_acb(struct scsi_qla_host *ha, dma_addr_t data_dma,
+		uint32_t instance, uint32_t acb_len);
+int qla4xxx_reset_iscsi_stat(struct scsi_qla_host *ha, uint32_t device_index);
+int qla4xxx_restore_factory_defaults(struct iscsi_bsg_job *job,
+		struct qla4xxx_bsg_cmd *qla4xxx_cmd);
+int qla4xxx_disable_acb(struct iscsi_bsg_job *bsg_job,
+		struct qla4xxx_bsg_cmd *qla4xxx_cmd);
 
 /* FIXME: Goodness!  this really wants a small struct to hold the
  * parameters. On x86 the args will get passed on the stack! */
diff --git a/drivers/scsi/qla4xxx/ql4_mbx.c b/drivers/scsi/qla4xxx/ql4_mbx.c
index 7dd28d5..524d03e 100644
--- a/drivers/scsi/qla4xxx/ql4_mbx.c
+++ b/drivers/scsi/qla4xxx/ql4_mbx.c
@@ -1040,8 +1040,8 @@ int qla4xxx_get_fw_version(struct scsi_qla_host * ha)
 	return QLA_SUCCESS;
 }
 
-static int qla4xxx_get_default_ddb(struct scsi_qla_host *ha,
-				   dma_addr_t dma_addr)
+int qla4xxx_get_default_ddb(struct scsi_qla_host *ha,
+		dma_addr_t dma_addr)
 {
 	uint32_t mbox_cmd[MBOX_REG_COUNT];
 	uint32_t mbox_sts[MBOX_REG_COUNT];
@@ -1209,3 +1209,261 @@ exit_ddb_info:
 	memcpy(mbox_status, mbox_sts, sizeof(mbox_sts));
 	return status;
 }
+
+int qla4xxx_bsg_get_ifcb(struct scsi_qla_host *ha, dma_addr_t data_dma,
+    uint32_t sub_cmd)
+{
+	uint32_t mbox_cmd[MBOX_REG_COUNT];
+	uint32_t mbox_sts[MBOX_REG_COUNT];
+
+	memset(&mbox_cmd, 0, sizeof(mbox_cmd));
+	memset(&mbox_sts, 0, sizeof(mbox_sts));
+
+	switch (sub_cmd) {
+	case QL4_GET_IFCB_DEF:
+		mbox_cmd[0] = MBOX_CMD_GET_INIT_FW_CTRL_BLOCK_DEFAULTS;
+		break;
+	case QL4_GET_IFCB:
+		mbox_cmd[0] = MBOX_CMD_GET_INIT_FW_CTRL_BLOCK;
+		break;
+	}
+
+	mbox_cmd[2] = LSDW(data_dma);
+	mbox_cmd[3] = MSDW(data_dma);
+	mbox_cmd[4] = sizeof(struct init_fw_ctrl_blk);
+
+	if (qla4xxx_mailbox_command(ha, MBOX_REG_COUNT, 1, &mbox_cmd[0],
+	    &mbox_sts[0]) != QLA_SUCCESS) {
+		printk("scsi%ld: %s: "
+		    "Failed to get init_fw_ctrl_blk, staus: %04X\n",
+		    ha->host_no, __func__, mbox_sts[0]);
+		return QLA_ERROR;
+	}
+	return QLA_SUCCESS;
+}
+
+int qla4xxx_get_acb(struct scsi_qla_host *ha, dma_addr_t data_dma,
+    uint32_t instance, uint32_t data_len)
+{
+	uint32_t mbox_cmd[MBOX_REG_COUNT];
+	uint32_t mbox_sts[MBOX_REG_COUNT];
+
+	memset(&mbox_cmd, 0, sizeof(mbox_cmd));
+	memset(&mbox_sts, 0, sizeof(mbox_sts));
+
+	mbox_cmd[0] = MBOX_CMD_GET_ACB;
+	mbox_cmd[1] = instance;  /* Primary/Secondary */
+	mbox_cmd[2] = LSDW(data_dma);
+	mbox_cmd[3] = MSDW(data_dma);
+	mbox_cmd[4] = data_len;
+
+	if (qla4xxx_mailbox_command(ha, MBOX_REG_COUNT, 1, &mbox_cmd[0],
+	    &mbox_sts[0]) != QLA_SUCCESS) {
+		printk("scsi%ld: %s: "
+		    "Failed to get acb, staus: %04X\n",
+		    ha->host_no, __func__, mbox_sts[0]);
+		return QLA_ERROR;
+	}
+	return QLA_SUCCESS;
+}
+
+int qla4xxx_get_iscsi_stat(struct scsi_qla_host *ha, uint32_t device_index,
+    dma_addr_t data_dma)
+{
+	uint32_t mbox_cmd[MBOX_REG_COUNT];
+	uint32_t mbox_sts[MBOX_REG_COUNT];
+
+	memset(&mbox_cmd, 0, sizeof(mbox_cmd));
+	memset(&mbox_sts, 0, sizeof(mbox_sts));
+
+	mbox_cmd[0] = MBOX_CMD_GET_MANAGEMENT_DATA;
+	mbox_cmd[1] = device_index;
+	mbox_cmd[2] = LSDW(data_dma);
+	mbox_cmd[3] = MSDW(data_dma);
+
+	if (qla4xxx_mailbox_command(ha, MBOX_REG_COUNT, 1, &mbox_cmd[0],
+	    &mbox_sts[0]) != QLA_SUCCESS) {
+		DEBUG2(printk("scsi%ld: %s: MBOX_CMD_GET_MANAGEMENT_DATA,"
+		    " failed w/ status %04X\n",
+		    ha->host_no, __func__, mbox_sts[0]));
+		return QLA_ERROR;
+	}
+	return QLA_SUCCESS;
+}
+
+int qla4xxx_set_flash(struct scsi_qla_host *ha, dma_addr_t dma_addr,
+    uint32_t offset, uint32_t option, uint32_t data_len)
+{
+	uint32_t mbox_cmd[MBOX_REG_COUNT];
+	uint32_t mbox_sts[MBOX_REG_COUNT];
+
+	memset(&mbox_cmd, 0, sizeof(mbox_cmd));
+	memset(&mbox_sts, 0, sizeof(mbox_sts));
+
+	mbox_cmd[0] = MBOX_CMD_WRITE_FLASH;
+	mbox_cmd[1] = LSDW(dma_addr);
+	mbox_cmd[2] = MSDW(dma_addr);
+	mbox_cmd[3] = offset;
+	mbox_cmd[4] = data_len;
+	mbox_cmd[5] = option;
+
+	if (qla4xxx_mailbox_command(ha, MBOX_REG_COUNT, 1, &mbox_cmd[0],
+	    &mbox_sts[0]) != QLA_SUCCESS) {
+		printk("scsi%ld: %s: "
+		    "MBOX_CMD_WRITE_FLASH failed w/ status %04X\n",
+		    ha->host_no, __func__, mbox_sts[0]);
+		return QLA_ERROR;
+	}
+	return QLA_SUCCESS;
+}
+
+int qla4xxx_bsg_set_ifcb(struct scsi_qla_host *ha, dma_addr_t data_dma)
+{
+	uint32_t mbox_cmd[MBOX_REG_COUNT];
+	uint32_t mbox_sts[MBOX_REG_COUNT];
+
+	memset(&mbox_cmd, 0, sizeof(mbox_cmd));
+	memset(&mbox_sts, 0, sizeof(mbox_sts));
+
+	mbox_cmd[0] = MBOX_CMD_INITIALIZE_FIRMWARE;
+	mbox_cmd[1] = 0;
+	mbox_cmd[2] = LSDW(data_dma);
+	mbox_cmd[3] = MSDW(data_dma);
+	mbox_cmd[4] = sizeof(struct init_fw_ctrl_blk);
+
+	if (qla4xxx_mailbox_command(ha, MBOX_REG_COUNT, 2, &mbox_cmd[0],
+	    &mbox_sts[0]) != QLA_SUCCESS) {
+		DEBUG2(printk("scsi%ld: %s: MBOX_CMD_INITIALIZE_FIRMWARE,"
+		    " failed w/ status %04X\n",
+		    ha->host_no, __func__, mbox_sts[0]));
+		return QLA_ERROR;
+	}
+	return QLA_SUCCESS;
+}
+
+int qla4xxx_set_acb(struct scsi_qla_host *ha, dma_addr_t data_dma,
+		uint32_t instance, uint32_t acb_len)
+{
+	uint32_t mbox_cmd[MBOX_REG_COUNT];
+	uint32_t mbox_sts[MBOX_REG_COUNT];
+
+	memset(&mbox_cmd, 0, sizeof(mbox_cmd));
+	memset(&mbox_sts, 0, sizeof(mbox_sts));
+
+	mbox_cmd[0] = MBOX_CMD_SET_ACB;
+	mbox_cmd[1] = instance;
+	mbox_cmd[2] = LSDW(data_dma);
+	mbox_cmd[3] = MSDW(data_dma);
+	mbox_cmd[4] = acb_len;
+
+	if (qla4xxx_mailbox_command(ha, MBOX_REG_COUNT, 2, &mbox_cmd[0],
+	    &mbox_sts[0]) != QLA_SUCCESS) {
+		DEBUG2(printk("scsi%ld: %s: MBOX_CMD_SET_ACB, failed w/ "
+		    "status %04X\n", ha->host_no, __func__, mbox_sts[0]));
+		return QLA_ERROR;
+	}
+
+	return QLA_SUCCESS;
+}
+
+int qla4xxx_reset_iscsi_stat(struct scsi_qla_host *ha, uint32_t device_index)
+{
+	uint32_t mbox_cmd[MBOX_REG_COUNT];
+	uint32_t mbox_sts[MBOX_REG_COUNT];
+
+	memset(&mbox_cmd, 0, sizeof(mbox_cmd));
+	memset(&mbox_sts, 0, sizeof(mbox_sts));
+
+	mbox_cmd[0] = MBOX_CMD_GET_MANAGEMENT_DATA;
+	mbox_cmd[1] = device_index;
+	mbox_cmd[2] = 0;
+	mbox_cmd[3] = 0;
+
+	if (qla4xxx_mailbox_command(ha, MBOX_REG_COUNT, 1, &mbox_cmd[0],
+	    &mbox_sts[0]) != QLA_SUCCESS) {
+		DEBUG2(printk("scsi%ld: %s: MBOX_CMD_GET_MANAGEMENT_DATA,"
+		    " failed w/ status %04X\n",
+		    ha->host_no, __func__, mbox_sts[0]));
+		    return QLA_ERROR;
+	}
+
+	return QLA_SUCCESS;
+}
+
+/**
+ * qla4xxx_restore_factory_defaults - restore hba factory defaults
+ * @job: iscsi_bsg_job to handle
+ **/
+int qla4xxx_restore_factory_defaults(struct iscsi_bsg_job *bsg_job,
+    struct qla4xxx_bsg_cmd *qla4xxx_cmd)
+{
+	uint32_t mbox_cmd[MBOX_REG_COUNT];
+	uint32_t mbox_sts[MBOX_REG_COUNT];
+	struct scsi_qla_host *ha = to_qla_host(bsg_job->shost);
+
+	memset(&mbox_cmd, 0, sizeof(mbox_cmd));
+	memset(&mbox_sts, 0, sizeof(mbox_sts));
+
+	mbox_cmd[0] = MBOX_CMD_RESTORE_FACTORY_DEFAULTS;
+	mbox_cmd[3] = qla4xxx_cmd->option0;
+	mbox_cmd[4] = qla4xxx_cmd->option1;
+	mbox_cmd[5] = qla4xxx_cmd->option2;
+
+	if (qla4xxx_mailbox_command(ha, MBOX_REG_COUNT, 1, &mbox_cmd[0],
+	    &mbox_sts[0]) != QLA_SUCCESS) {
+		DEBUG2(printk("scsi%ld: %s: MBOX_CMD_RESTORE_FACTORY_DEFAULTS,"
+		    " failed w/ status %04X\n",
+		    ha->host_no, __func__, mbox_sts[0]));
+		return QLA_ERROR;
+	}
+	return QLA_SUCCESS;
+}
+
+int qla4xxx_disable_acb(struct iscsi_bsg_job *bsg_job,
+    struct qla4xxx_bsg_cmd *qla4xxx_cmd)
+{
+	uint32_t mbox_cmd[MBOX_REG_COUNT];
+	uint32_t mbox_sts[MBOX_REG_COUNT];
+	struct scsi_qla_host *ha = to_qla_host(bsg_job->shost);
+	int ret = QLA_SUCCESS;
+
+	memset(&mbox_cmd, 0, sizeof(mbox_cmd));
+	memset(&mbox_sts, 0, sizeof(mbox_sts));
+
+	mbox_cmd[0] = MBOX_CMD_DISABLE_ACB;
+	mbox_cmd[1] = qla4xxx_cmd->instance;
+
+	if (qla4xxx_mailbox_command(ha, MBOX_REG_COUNT, 2, &mbox_cmd[0],
+	    &mbox_sts[0]) != QLA_SUCCESS) {
+		if (mbox_sts[0] == MBOX_STS_INTERMEDIATE_COMPLETION) {
+			unsigned long  wait_cnt;
+			wait_cnt = jiffies + (WAIT_CMD_TOV * HZ);
+
+			while (time_before(jiffies, wait_cnt)) {
+				set_current_state(TASK_INTERRUPTIBLE);
+				schedule_timeout(HZ);
+
+				memset(&mbox_cmd, 0, sizeof(mbox_cmd));
+				memset(&mbox_sts, 0, sizeof(mbox_sts));
+
+				mbox_cmd[0] = MBOX_CMD_GET_IP_ADDR_STATE;
+				mbox_cmd[1] = qla4xxx_cmd->instance;
+
+				if (qla4xxx_mailbox_command(ha, MBOX_REG_COUNT,
+				    2, &mbox_cmd[0], &mbox_sts[0]) ==
+				    QLA_SUCCESS) {
+					if (!(mbox_sts[1] & 0xF0000000))
+						break;
+				} else {
+					DEBUG2(printk("scsi%ld: %s: Command "
+					    "failed\n", ha->host_no,
+					    __func__));
+					ret = QLA_ERROR;
+					break;
+				}
+			}
+		} else
+			ret = QLA_ERROR;
+	}
+	return ret;
+}
diff --git a/drivers/scsi/qla4xxx/ql4_os.c b/drivers/scsi/qla4xxx/ql4_os.c
index 56d22b8..c59e832 100644
--- a/drivers/scsi/qla4xxx/ql4_os.c
+++ b/drivers/scsi/qla4xxx/ql4_os.c
@@ -9,6 +9,8 @@
 
 #include <scsi/scsi_tcq.h>
 #include <scsi/scsicam.h>
+#include <scsi/scsi_bsg_iscsi.h>
+#include <scsi/scsi_netlink.h>
 
 #include "ql4_def.h"
 #include "ql4_version.h"
@@ -75,6 +77,7 @@ static int qla4xxx_host_get_param(struct Scsi_Host *shost,
 				  enum iscsi_host_param param, char *buf);
 static void qla4xxx_recovery_timedout(struct iscsi_cls_session *session);
 static enum blk_eh_timer_return qla4xxx_eh_cmd_timed_out(struct scsi_cmnd *sc);
+static int qla4xxx_bsg_request(struct iscsi_bsg_job *job);
 
 /*
  * SCSI host template entry points
@@ -118,6 +121,7 @@ static struct scsi_host_template qla4xxx_driver_template = {
 	.sg_tablesize		= SG_ALL,
 
 	.max_sectors		= 0xFFFF,
+	.vendor_id		= SCSI_NL_VID_TYPE_PCI | PCI_VENDOR_ID_QLOGIC,
 	.shost_attrs            = qla4xxx_host_attrs,
 };
 
@@ -142,10 +146,224 @@ static struct iscsi_transport qla4xxx_iscsi_transport = {
 	.get_session_param	= qla4xxx_sess_get_param,
 	.get_host_param		= qla4xxx_host_get_param,
 	.session_recovery_timedout = qla4xxx_recovery_timedout,
+	.bsg_request		= qla4xxx_bsg_request,
 };
 
 static struct scsi_transport_template *qla4xxx_scsi_transport;
 
+/**
+ * qla4xxx_get_data - data from driver to application
+ * @job: iscsi_bsg_job to handle
+ **/
+static int qla4xxx_get_data(struct iscsi_bsg_job *bsg_job,
+    struct qla4xxx_bsg_cmd *qla4xxx_cmd,
+    struct qla4xxx_dma_mem mem)
+{
+	int rval = QLA_SUCCESS;
+	struct scsi_qla_host *ha = to_qla_host(bsg_job->shost);
+
+	switch (qla4xxx_cmd->subcode) {
+	case QL4_GET_FLASH:
+		rval = qla4xxx_get_flash(ha, mem.dma, qla4xxx_cmd->offset,
+		    mem.size);
+		break;
+	case QL4_GET_DDB_DEF:
+		rval = qla4xxx_get_default_ddb(ha, mem.dma);
+		break;
+	case QL4_GET_DDB:
+		rval = qla4xxx_get_fwddb_entry(ha, qla4xxx_cmd->instance,
+		    NULL, mem.dma, NULL, NULL,
+		    NULL, NULL, NULL, NULL);
+		break;
+	case QL4_GET_IFCB_DEF:
+	case QL4_GET_IFCB:
+		rval = qla4xxx_bsg_get_ifcb(ha, mem.dma,
+		    qla4xxx_cmd->subcode);
+		break;
+	case QL4_GET_ACB:
+		rval = qla4xxx_get_acb(ha, mem.dma, qla4xxx_cmd->instance,
+		    mem.size);
+		break;
+	case QL4_GET_ISCSI_STAT:
+		rval = qla4xxx_get_iscsi_stat(ha, qla4xxx_cmd->instance,
+		    mem.dma);
+		break;
+	default:
+		DEBUG2(printk("scsi %ld: %s: invalid sub command code: 0x%x\n",
+		    ha->host_no, __func__, qla4xxx_cmd->opcode));
+		rval = -EINVAL;
+	}
+
+	bsg_job->reply->reply_payload_rcv_len = mem.size;
+	return rval;
+}
+
+/**
+ * qla4xxx_set_data - data from application to driver
+ * @ha: Pointer to host adapter structure.
+ * @job: iscsi_bsg_job to handle
+ * @vendor_cmd: bsg vendor specific command structure
+ * @data: data buffer
+ * @data_dma: DMA address of data buffer
+ **/
+static int qla4xxx_set_data(struct iscsi_bsg_job *bsg_job,
+    struct qla4xxx_bsg_cmd *qla4xxx_cmd,
+    struct qla4xxx_dma_mem mem)
+{
+	int rval = QLA_SUCCESS;
+	struct scsi_qla_host *ha = to_qla_host(bsg_job->shost);
+
+	/* Copy the request buffer in req_data now */
+	sg_copy_to_buffer(bsg_job->request_payload.sg_list,
+	    bsg_job->request_payload.sg_cnt, mem.va,
+	    mem.size);
+
+	switch (qla4xxx_cmd->subcode) {
+	case QL4_SET_FLASH:
+		rval = qla4xxx_set_flash(ha, mem.dma, qla4xxx_cmd->offset,
+		    qla4xxx_cmd->option0, mem.size);
+		break;
+	case QL4_SET_DDB:
+		rval = qla4xxx_set_ddb_entry(ha, qla4xxx_cmd->instance,
+		    mem.dma);
+		break;
+	case QL4_SET_IFCB:
+		rval = qla4xxx_bsg_set_ifcb(ha, mem.dma);
+		break;
+	case QL4_SET_ACB:
+		rval = qla4xxx_set_acb(ha, mem.dma, qla4xxx_cmd->instance,
+		    mem.size);
+		break;
+	case QL4_RESET_ISCSI_STAT:
+		rval = qla4xxx_reset_iscsi_stat(ha, qla4xxx_cmd->instance);
+		break;
+	default:
+		DEBUG2(printk("scsi %ld: %s: invalid sub command code: 0x%x\n",
+		    ha->host_no, __func__, qla4xxx_cmd->opcode));
+		rval = -EINVAL;
+	}
+	bsg_job->reply->reply_payload_rcv_len = 0;
+	return rval;
+}
+
+/**
+ * qla4xxx_process_vendor_specific - handle vendor specific bsg request
+ * @job: iscsi_bsg_job to handle
+ **/
+static int qla4xxx_process_vendor_specific(struct iscsi_bsg_job *bsg_job)
+{
+	struct scsi_qla_host *ha = to_qla_host(bsg_job->shost);
+	struct qla4xxx_bsg_cmd *qla4xxx_cmd;
+	struct qla4xxx_dma_mem mem;
+	int rval = QLA_SUCCESS;
+
+	qla4xxx_cmd = (struct qla4xxx_bsg_cmd *) ((char *)bsg_job->request +
+	    sizeof(struct iscsi_bsg_request));
+
+	mem.req_sg_cnt =
+		dma_map_sg(&ha->pdev->dev, bsg_job->request_payload.sg_list,
+		    bsg_job->request_payload.sg_cnt, DMA_TO_DEVICE);
+
+	if (!mem.req_sg_cnt) {
+		rval = -ENOMEM;
+		goto done;
+	}
+
+	mem.rsp_sg_cnt =
+		dma_map_sg(&ha->pdev->dev, bsg_job->reply_payload.sg_list,
+		    bsg_job->reply_payload.sg_cnt, DMA_FROM_DEVICE);
+
+	if (!mem.rsp_sg_cnt) {
+		rval = -ENOMEM;
+		goto done;
+	}
+
+	if ((mem.req_sg_cnt != bsg_job->request_payload.sg_cnt) ||
+	    (mem.rsp_sg_cnt != bsg_job->reply_payload.sg_cnt)) {
+		DEBUG2(printk(KERN_INFO
+		    "dma mapping resulted in different sg counts \
+		    [request_sg_cnt: %x dma_request_sg_cnt: %x\
+		    reply_sg_cnt: %x dma_reply_sg_cnt: %x]\n",
+		    bsg_job->request_payload.sg_cnt, mem.req_sg_cnt,
+		    bsg_job->reply_payload.sg_cnt, mem.rsp_sg_cnt));
+
+		rval = -EAGAIN;
+		goto done_unmap_sg;
+	}
+
+	mem.size = qla4xxx_cmd->data_size;
+	mem.va = dma_alloc_coherent(&ha->pdev->dev, mem.size,
+	    &mem.dma, GFP_KERNEL);
+	if (mem.va == NULL) {
+		DEBUG2(printk("scsi %ld: %s: fail to allocate dma memory "
+		    "for data", ha->host_no, __func__));
+		rval = -EIO;
+		goto done_free_dma;
+	}
+
+	switch (qla4xxx_cmd->opcode) {
+	case QL4_GET_DATA:
+		rval = qla4xxx_get_data(bsg_job, qla4xxx_cmd, mem);
+		break;
+	case QL4_SET_DATA:
+		rval = qla4xxx_set_data(bsg_job, qla4xxx_cmd, mem);
+		break;
+	case QL4_RESTORE_FACTORY_DEF:
+		rval = qla4xxx_restore_factory_defaults(bsg_job, qla4xxx_cmd);
+		break;
+	case QL4_DISABLE_ACB:
+		rval = qla4xxx_disable_acb(bsg_job, qla4xxx_cmd);
+		break;
+	default:
+		DEBUG2(printk("scsi %ld: %s: invalid command code: 0x%x\n",
+		    ha->host_no, __func__, qla4xxx_cmd->opcode));
+		rval = -EINVAL;
+	}
+	bsg_job->reply->result = rval;
+	bsg_job->job_done(bsg_job);
+
+done_free_dma:
+	dma_free_coherent(&ha->pdev->dev, mem.size, mem.va, mem.dma);
+done_unmap_sg:
+	dma_unmap_sg(&ha->pdev->dev,
+	    bsg_job->request_payload.sg_list,
+	    bsg_job->request_payload.sg_cnt, DMA_TO_DEVICE);
+
+	dma_unmap_sg(&ha->pdev->dev,
+	    bsg_job->reply_payload.sg_list,
+	    bsg_job->reply_payload.sg_cnt, DMA_FROM_DEVICE);
+done:
+	return rval;
+}
+
+/**
+ * qla4xxx_bsg_request - handle bsg request from ISCSI transport
+ * @job: iscsi_bsg_job to handle
+ **/
+static int qla4xxx_bsg_request(struct iscsi_bsg_job *job)
+{
+	uint32_t msgcode;
+	int rval = -EINVAL;
+
+	msgcode = job->request->msgcode;
+
+	switch (msgcode) {
+	case ISCSI_BSG_HST_VENDOR:
+		rval = qla4xxx_process_vendor_specific(job);
+		break;
+	case ISCSI_BSG_HST_NET_CONFIG:
+		printk(KERN_ERR "ISCSI_BSG_HST_NET_CONFIG Commands "
+		    "NOT Supported\n");
+		rval = -ENOSYS;
+		break;
+	default:
+		rval = -EINVAL;
+		break;
+	}
+
+	return rval;
+}
+
 static enum blk_eh_timer_return qla4xxx_eh_cmd_timed_out(struct scsi_cmnd *sc)
 {
 	struct iscsi_cls_session *session;
-- 
1.7.0.5

Re: [PATCH 13/15] qla4xxx: Added bsg support

[Mike Christie]

On 06/11/2010 02:49 AM, Vikas Chaudhary wrote:
> Added BSG support to enable application support to configure
> ISP40XX/ISP82XX adapter.
>
> This patch is on top of: http://marc.info/?l=linux-scsi&m=126999297630764&w=2
>

Did James say that using vendor specific commands was ok? I did not see 
anything on the list. If he did, then instead of the patch above from 
Jay, you want to use iscsi bsg patches that I sent that handle Tomos's 
comments about not duplicating fc code. I sent the bsg lib to the list 
and then in my tree on kernel.org I ported the fc drivers (those might 
need a update for some patches James Smart recently sent). For those bsg 
patches you will want to then also add the vendor specific command back.

RE: [PATCH 13/15] qla4xxx: Added bsg support

[Vikas Chaudhary]

>-----Original Message-----
>From: Mike Christie [mailto:michaelc@cs.wisc.edu]
>Sent: Friday, June 11, 2010 5:24 PM
>To: Vikas Chaudhary
>Cc: james.bottomley@suse.de; linux-scsi@vger.kernel.org; Ravi Anand
>Subject: Re: [PATCH 13/15] qla4xxx: Added bsg support
>
>On 06/11/2010 02:49 AM, Vikas Chaudhary wrote:
>> Added BSG support to enable application support to configure
>> ISP40XX/ISP82XX adapter.
>>
>> This patch is on top of: http://marc.info/?l=linux-
>scsi&m=126999297630764&w=2
>>
>
>Did James say that using vendor specific commands was ok? I did not see
>anything on the list. If he did, then instead of the patch above from
>Jay, you want to use iscsi bsg patches that I sent that handle Tomos's
>comments about not duplicating fc code. I sent the bsg lib to the list
>and then in my tree on kernel.org I ported the fc drivers (those might
>need a update for some patches James Smart recently sent). For those bsg
>patches you will want to then also add the vendor specific command back.

We will rebase qla4xxx bsg patch on top of your patch to use common bsg lib.

We pinged James.B couple of times  but he never replied back to our email.
So we are assuming he is OK with this approach and requesting for formal
inclusion of iscsi bsg support along with our driver support for it.

Let us know if you see any issues.

Thanks,
Vikas.

Re: [PATCH 13/15] qla4xxx: Added bsg support

[James Bottomley]

On Fri, 2010-06-11 at 19:23 -0500, Mike Christie wrote:
> On 06/11/2010 02:49 AM, Vikas Chaudhary wrote:
> > Added BSG support to enable application support to configure
> > ISP40XX/ISP82XX adapter.
> >
> > This patch is on top of: http://marc.info/?l=linux-scsi&m=126999297630764&w=2
> >
> 
> Did James say that using vendor specific commands was ok? I did not see 
> anything on the list.

In principle, the point about vendor specific host commands has already
been conceded ... lpfc and qla2xxx already use them.

I think the real rule is that for host specific BSG commands, it's only
for stuff that's specific to the host ... so not stuff which should be
done generically.

There are no hard and fast rules for applying the test above.  In
theory, the management interfaces exposed by FC_BSG_HST_VENDOR could be
used to send commands ... but it tends to get tolerated as long as the
drivers support the standardised rport commands.

If I look at what the qla4xxx interface would do

     1. it only supports vendor specific commands ... this is a bit of a
        red flag since it's proposing to do nothing in a vendor neutral
        way.  This one, I punt back to you: what should an iscsi device
        implementing BSG support of the standard commands (which have
        only very recently been proposed for definition)
     2. QL4_SET_FLASH looks a bit suspicious ... the only thing I think
        you could really set from that is the mac addresses ... is this
        so?
     3. the DDB commands seem to be for manipulating the end point
        table ... which is basically the iscsi connection table ... does
        manipulating that from BSG make sense?
     4. What is ACB and IFCB?  It's undocumented
     5. GET/SET_ISCSI_STAT seems to be statistics related by the name;
        is it? if so, why not use the more standard statistics interface
        in the iscsi transport class?


>  If he did, then instead of the patch above from 
> Jay, you want to use iscsi bsg patches that I sent that handle Tomos's 
> comments about not duplicating fc code. I sent the bsg lib to the list 
> and then in my tree on kernel.org I ported the fc drivers (those might 
> need a update for some patches James Smart recently sent). For those bsg 
> patches you will want to then also add the vendor specific command back.

Agreed, having a standard way of doing bsg helps.

James

Re: [PATCH 13/15] qla4xxx: Added bsg support

[Mike Christie]

On 06/12/2010 05:37 PM, James Bottomley wrote:
> On Fri, 2010-06-11 at 19:23 -0500, Mike Christie wrote:
>> On 06/11/2010 02:49 AM, Vikas Chaudhary wrote:
>>> Added BSG support to enable application support to configure
>>> ISP40XX/ISP82XX adapter.
>>>
>>> This patch is on top of: http://marc.info/?l=linux-scsi&m=126999297630764&w=2
>>>
>>
>> Did James say that using vendor specific commands was ok? I did not see
>> anything on the list.
>
> In principle, the point about vendor specific host commands has already
> been conceded ... lpfc and qla2xxx already use them.
>
> I think the real rule is that for host specific BSG commands, it's only
> for stuff that's specific to the host ... so not stuff which should be
> done generically.
>
> There are no hard and fast rules for applying the test above.  In
> theory, the management interfaces exposed by FC_BSG_HST_VENDOR could be
> used to send commands ... but it tends to get tolerated as long as the
> drivers support the standardised rport commands.
>
> If I look at what the qla4xxx interface would do
>
>       1. it only supports vendor specific commands ... this is a bit of a
>          red flag since it's proposing to do nothing in a vendor neutral
>          way.  This one, I punt back to you: what should an iscsi device
>          implementing BSG support of the standard commands

I am not 100% sure what we need yet, because qlogic has been pushing the 
vendor specific route and I cannot see exactly what it needs with some 
of its commands.

Based on other drivers and from looking at the qlogic tools and their 
qlogic.com driver which seems to convert some management operations to 
flash commands, I have the list below:

- Operation to set net settings like ip/dhcp, gateway, dns, subnet, 
vlan, tcp/ip settings, speed/auto-negotiate, etc.

Besides supporting a IPv4 and IPv6 address, some cards can support lots 
of IPs and/or vlans per same physical port, so this operation cannot be 
tied to just the host.

- Operation to get port state.

- Operation to set iscsi initiator port settings like initiator 
name/alias/isid, iscsi IO settings like FirstBurstLength or Digests, 
auth settings like chap or radius, etc.

This should not be tied to the physical port either since we can have 
multiple initiator ports per physical port.

- Operation to set iscsi settings for specific sessions like iSCSI IO 
settings and auth settings mentioned above (some drivers want global 
defaults settable at the host/initiator-port level then be able to 
specify specific values for each session).

- Operation to discover targets and export targets that were found and:
	- Operation to control if we want to log into specific targets that 
were found.

- Operation to notify userpsace of network link state changes.

- Operation to notify userpsace of iscsi session/connection state changes.

Re: [PATCH 13/15] qla4xxx: Added bsg support

[FUJITA Tomonori]

On Sun, 13 Jun 2010 15:10:39 -0500
Mike Christie <michaelc@cs.wisc.edu> wrote:

> On 06/12/2010 05:37 PM, James Bottomley wrote:
> > On Fri, 2010-06-11 at 19:23 -0500, Mike Christie wrote:
> >> On 06/11/2010 02:49 AM, Vikas Chaudhary wrote:
> >>> Added BSG support to enable application support to configure
> >>> ISP40XX/ISP82XX adapter.
> >>>
> >>> This patch is on top of: http://marc.info/?l=linux-scsi&m=126999297630764&w=2
> >>>
> >>
> >> Did James say that using vendor specific commands was ok? I did not see
> >> anything on the list.
> >
> > In principle, the point about vendor specific host commands has already
> > been conceded ... lpfc and qla2xxx already use them.
> >
> > I think the real rule is that for host specific BSG commands, it's only
> > for stuff that's specific to the host ... so not stuff which should be
> > done generically.
> >
> > There are no hard and fast rules for applying the test above.  In
> > theory, the management interfaces exposed by FC_BSG_HST_VENDOR could be
> > used to send commands ... but it tends to get tolerated as long as the
> > drivers support the standardised rport commands.
> >
> > If I look at what the qla4xxx interface would do
> >
> >       1. it only supports vendor specific commands ... this is a bit of a
> >          red flag since it's proposing to do nothing in a vendor neutral
> >          way.  This one, I punt back to you: what should an iscsi device
> >          implementing BSG support of the standard commands
> 
> I am not 100% sure what we need yet, because qlogic has been pushing the 
> vendor specific route and I cannot see exactly what it needs with some 
> of its commands.
> 
> Based on other drivers and from looking at the qlogic tools and their 
> qlogic.com driver which seems to convert some management operations to 
> flash commands, I have the list below:
> 
> - Operation to set net settings like ip/dhcp, gateway, dns, subnet, 
> vlan, tcp/ip settings, speed/auto-negotiate, etc.
> 
> Besides supporting a IPv4 and IPv6 address, some cards can support lots 
> of IPs and/or vlans per same physical port, so this operation cannot be 
> tied to just the host.
> 
> - Operation to get port state.
> 
> - Operation to set iscsi initiator port settings like initiator 
> name/alias/isid, iscsi IO settings like FirstBurstLength or Digests, 
> auth settings like chap or radius, etc.
> 
> This should not be tied to the physical port either since we can have 
> multiple initiator ports per physical port.
> 
> - Operation to set iscsi settings for specific sessions like iSCSI IO 
> settings and auth settings mentioned above (some drivers want global 
> defaults settable at the host/initiator-port level then be able to 
> specify specific values for each session).

/sys/class/iscsi_* might work better for the above?


> - Operation to discover targets and export targets that were found and:
> 	- Operation to control if we want to log into specific targets that 
> were found.
> 
> - Operation to notify userpsace of network link state changes.
> 
> - Operation to notify userpsace of iscsi session/connection state changes.

bsg supports only the request/response interface. The event
notification from kernel space isn't fit for bsg.

Sounds like all the features are useful to any iSCSI HBAs?

Re: [PATCH 13/15] qla4xxx: Added bsg support

[Mike Christie]

On 06/13/2010 07:55 PM, FUJITA Tomonori wrote:
> On Sun, 13 Jun 2010 15:10:39 -0500
> Mike Christie<michaelc@cs.wisc.edu>  wrote:
>
>> On 06/12/2010 05:37 PM, James Bottomley wrote:
>>> On Fri, 2010-06-11 at 19:23 -0500, Mike Christie wrote:
>>>> On 06/11/2010 02:49 AM, Vikas Chaudhary wrote:
>>>>> Added BSG support to enable application support to configure
>>>>> ISP40XX/ISP82XX adapter.
>>>>>
>>>>> This patch is on top of: http://marc.info/?l=linux-scsi&m=126999297630764&w=2
>>>>>
>>>>
>>>> Did James say that using vendor specific commands was ok? I did not see
>>>> anything on the list.
>>>
>>> In principle, the point about vendor specific host commands has already
>>> been conceded ... lpfc and qla2xxx already use them.
>>>
>>> I think the real rule is that for host specific BSG commands, it's only
>>> for stuff that's specific to the host ... so not stuff which should be
>>> done generically.
>>>
>>> There are no hard and fast rules for applying the test above.  In
>>> theory, the management interfaces exposed by FC_BSG_HST_VENDOR could be
>>> used to send commands ... but it tends to get tolerated as long as the
>>> drivers support the standardised rport commands.
>>>
>>> If I look at what the qla4xxx interface would do
>>>
>>>        1. it only supports vendor specific commands ... this is a bit of a
>>>           red flag since it's proposing to do nothing in a vendor neutral
>>>           way.  This one, I punt back to you: what should an iscsi device
>>>           implementing BSG support of the standard commands
>>
>> I am not 100% sure what we need yet, because qlogic has been pushing the
>> vendor specific route and I cannot see exactly what it needs with some
>> of its commands.
>>
>> Based on other drivers and from looking at the qlogic tools and their
>> qlogic.com driver which seems to convert some management operations to
>> flash commands, I have the list below:
>>
>> - Operation to set net settings like ip/dhcp, gateway, dns, subnet,
>> vlan, tcp/ip settings, speed/auto-negotiate, etc.
>>
>> Besides supporting a IPv4 and IPv6 address, some cards can support lots
>> of IPs and/or vlans per same physical port, so this operation cannot be
>> tied to just the host.
>>
>> - Operation to get port state.
>>
>> - Operation to set iscsi initiator port settings like initiator
>> name/alias/isid, iscsi IO settings like FirstBurstLength or Digests,
>> auth settings like chap or radius, etc.
>>
>> This should not be tied to the physical port either since we can have
>> multiple initiator ports per physical port.
>>
>> - Operation to set iscsi settings for specific sessions like iSCSI IO
>> settings and auth settings mentioned above (some drivers want global
>> defaults settable at the host/initiator-port level then be able to
>> specify specific values for each session).
>
> /sys/class/iscsi_* might work better for the above?

Yes and no. It works well when we just have to set 1 value, but if we 
have to set multiple ones then we need to batch them up because it can 
be disruptive to all sessions. When we set a setting in qla4xxx and 
be2iscsi, then I think the card is basically not accessible while the 
value is being set. So if we did use sysfs then we would need some other 
files like maybe one to indicate that we are done setting params and the 
card can write them to the flash now.

>
>
>> - Operation to discover targets and export targets that were found and:
>> 	- Operation to control if we want to log into specific targets that
>> were found.
>>
>> - Operation to notify userpsace of network link state changes.
>>
>> - Operation to notify userpsace of iscsi session/connection state changes.
>
> bsg supports only the request/response interface. The event
> notification from kernel space isn't fit for bsg.

Ah yeah, I just included it for completeness because in their previous 
patchset they were using the scsi netlink code to add vendor specific 
events for them.

>
> Sounds like all the features are useful to any iSCSI HBAs?


Yeah, the iscsi ones are useful to any iscsi HBA that does 
session/connection management in the card. The net ones are useful to 
cards that do net management in the card.

RE: [PATCH 13/15] qla4xxx: Added bsg support

[Vikas Chaudhary]

>-----Original Message-----
>From: James Bottomley [mailto:James.Bottomley@suse.de]
>Sent: Saturday, June 12, 2010 3:38 PM
>To: Mike Christie
>Cc: Vikas Chaudhary; linux-scsi@vger.kernel.org; Ravi Anand
>Subject: Re: [PATCH 13/15] qla4xxx: Added bsg support
>
>On Fri, 2010-06-11 at 19:23 -0500, Mike Christie wrote:
>> On 06/11/2010 02:49 AM, Vikas Chaudhary wrote:
>> > Added BSG support to enable application support to configure
>> > ISP40XX/ISP82XX adapter.
>> >
>> > This patch is on top of: http://marc.info/?l=linux-
>scsi&m=126999297630764&w=2
>> >
>>
>> Did James say that using vendor specific commands was ok? I did not see
>> anything on the list.
>
>In principle, the point about vendor specific host commands has already
>been conceded ... lpfc and qla2xxx already use them.
>
>I think the real rule is that for host specific BSG commands, it's only
>for stuff that's specific to the host ... so not stuff which should be
>done generically.
>
>There are no hard and fast rules for applying the test above.  In
>theory, the management interfaces exposed by FC_BSG_HST_VENDOR could be
>used to send commands ... but it tends to get tolerated as long as the
>drivers support the standardised rport commands.
>
>If I look at what the qla4xxx interface would do
>
>     1. it only supports vendor specific commands ... this is a bit of a
>        red flag since it's proposing to do nothing in a vendor neutral
>        way.  This one, I punt back to you: what should an iscsi device
>        implementing BSG support of the standard commands (which have
>        only very recently been proposed for definition)

That's not true. For ex : Look at the ping support we have added which other
iSCSI HBA vendors can use as well. Here's the thread : 
http://article.gmane.org/gmane.linux.scsi/59535

Also we are adding support in iSCSI transport for other stuff which we think
is the right interface to use. Here's the thread :
http://article.gmane.org/gmane.linux.scsi/59532

But the reason we are trying to implement most of the commands using vendor
specific way is because *setting up the card/configuration* related stuff
are communicated using mailbox command interface with our firmware which
do blob data transfer back and forth. Driver is basically funneling info 
back and forth between F/W and API in a blob format. So we need this to allow
end user's to configure the card using QLogic Application or *iscsiadm* in
future as we are migrating from ioctl interface to bsg/netlink/sysfs route
which will require API changes.

Additionally we are working on to add support in  libiscsi user space to
support qla4xxx which will manipulate this blob and hookup with Linux Native
*iscsiadm*  tool. Mike was OK with this approach as long as user space
iscsiadm support will be enabled. This is something we are working on
parallely as well. Here's the email thread where Mike responded and we pretty
much agreed to it :
http://article.gmane.org/gmane.linux.scsi/58849/match=qla4xxx+bsg


>     2. QL4_SET_FLASH looks a bit suspicious ... the only thing I think
>        you could really set from that is the mac addresses ... is this
>        so?

It's used for multiple purpose, for ex :-
	1. write firmware image to flash
	2. write target configuration to flash and make target persistent
	3. write firmware control block to flash
	4. write chap information to flash, etc..

>     3. the DDB commands seem to be for manipulating the end point
>        table ... which is basically the iscsi connection table ... does
>        manipulating that from BSG make sense?

For qla4xxx firmware establishes  Connection/Session and qla4xxx driver
works in session mode. We are using ddb commands here to establish
session with target.
Let us know if you have any suggestions over here.

>     4. What is ACB and IFCB?  It's undocumented

It is control block containing qla4xxx firmware related settings.

>     5. GET/SET_ISCSI_STAT seems to be statistics related by the name;
>        is it? if so, why not use the more standard statistics interface
>        in the iscsi transport class?
>

we added it via BSG using vendor command as it blob data transfer.
API interprets the data and displays the statistics. Driver is just a passthru.

>>  If he did, then instead of the patch above from
>> Jay, you want to use iscsi bsg patches that I sent that handle Tomos's
>> comments about not duplicating fc code. I sent the bsg lib to the list
>> and then in my tree on kernel.org I ported the fc drivers (those might
>> need a update for some patches James Smart recently sent). For those bsg
>> patches you will want to then also add the vendor specific command back.
>
>Agreed, having a standard way of doing bsg helps.

We will do patches again on top of bsg lib patches.

>
>James
>

RE: [PATCH 13/15] qla4xxx: Added bsg support

[Vikas Chaudhary]

>-----Original Message-----
>From: Mike Christie [mailto:michaelc@cs.wisc.edu]
>Sent: Sunday, June 13, 2010 1:11 PM
>To: James Bottomley
>Cc: Vikas Chaudhary; linux-scsi@vger.kernel.org; Ravi Anand
>Subject: Re: [PATCH 13/15] qla4xxx: Added bsg support
>
>On 06/12/2010 05:37 PM, James Bottomley wrote:
>> On Fri, 2010-06-11 at 19:23 -0500, Mike Christie wrote:
>>> On 06/11/2010 02:49 AM, Vikas Chaudhary wrote:
>>>> Added BSG support to enable application support to configure
>>>> ISP40XX/ISP82XX adapter.
>>>>
>>>> This patch is on top of: http://marc.info/?l=linux-
>scsi&m=126999297630764&w=2
>>>>
>>>
>>> Did James say that using vendor specific commands was ok? I did not see
>>> anything on the list.
>>
>> In principle, the point about vendor specific host commands has already
>> been conceded ... lpfc and qla2xxx already use them.
>>
>> I think the real rule is that for host specific BSG commands, it's only
>> for stuff that's specific to the host ... so not stuff which should be
>> done generically.
>>
>> There are no hard and fast rules for applying the test above.  In
>> theory, the management interfaces exposed by FC_BSG_HST_VENDOR could be
>> used to send commands ... but it tends to get tolerated as long as the
>> drivers support the standardised rport commands.
>>
>> If I look at what the qla4xxx interface would do
>>
>>       1. it only supports vendor specific commands ... this is a bit of a
>>          red flag since it's proposing to do nothing in a vendor neutral
>>          way.  This one, I punt back to you: what should an iscsi device
>>          implementing BSG support of the standard commands
>
>I am not 100% sure what we need yet, because qlogic has been pushing the
>vendor specific route and I cannot see exactly what it needs with some
>of its commands.
>
>Based on other drivers and from looking at the qlogic tools and their
>qlogic.com driver which seems to convert some management operations to
>flash commands, I have the list below:
>
>- Operation to set net settings like ip/dhcp, gateway, dns, subnet,
>vlan, tcp/ip settings, speed/auto-negotiate, etc.
>
>Besides supporting a IPv4 and IPv6 address, some cards can support lots
>of IPs and/or vlans per same physical port, so this operation cannot be
>tied to just the host.

These operations are tied to per pci function in qla4xxx driver.

>
>- Operation to get port state.
>
>- Operation to set iscsi initiator port settings like initiator
>name/alias/isid, iscsi IO settings like FirstBurstLength or Digests,
>auth settings like chap or radius, etc.
>
>This should not be tied to the physical port either since we can have
>multiple initiator ports per physical port.
>
>- Operation to set iscsi settings for specific sessions like iSCSI IO
>settings and auth settings mentioned above (some drivers want global
>defaults settable at the host/initiator-port level then be able to
>specify specific values for each session).
>
>- Operation to discover targets and export targets that were found and:
>	- Operation to control if we want to log into specific targets that
>were found.
>
>- Operation to notify userpsace of network link state changes.
>
>- Operation to notify userpsace of iscsi session/connection state changes.

Here's the email thread info which outlines the initial patch we submitted
for link states changes : http://article.gmane.org/gmane.linux.scsi/58844

and based on Mike.C comment we are working on to make it more like FC where
it's based on LINK DOWN/UP etc rather than one generic AEN.

We are planning to resubmit the updated patch as part of our next patch set.

RE: [PATCH 13/15] qla4xxx: Added bsg support

[Vikas Chaudhary]

>-----Original Message-----
>From: FUJITA Tomonori [mailto:fujita.tomonori@lab.ntt.co.jp]
>Sent: Sunday, June 13, 2010 5:56 PM
>To: michaelc@cs.wisc.edu
>Cc: James.Bottomley@suse.de; Vikas Chaudhary; linux-scsi@vger.kernel.org;
>Ravi Anand
>Subject: Re: [PATCH 13/15] qla4xxx: Added bsg support
>
>On Sun, 13 Jun 2010 15:10:39 -0500
>Mike Christie <michaelc@cs.wisc.edu> wrote:
>
>> On 06/12/2010 05:37 PM, James Bottomley wrote:
>> > On Fri, 2010-06-11 at 19:23 -0500, Mike Christie wrote:
>> >> On 06/11/2010 02:49 AM, Vikas Chaudhary wrote:
>> >>> Added BSG support to enable application support to configure
>> >>> ISP40XX/ISP82XX adapter.
>> >>>
>> >>> This patch is on top of: http://marc.info/?l=linux-
>scsi&m=126999297630764&w=2
>> >>>
>> >>
>> >> Did James say that using vendor specific commands was ok? I did not
>see
>> >> anything on the list.
>> >
>> > In principle, the point about vendor specific host commands has already
>> > been conceded ... lpfc and qla2xxx already use them.
>> >
>> > I think the real rule is that for host specific BSG commands, it's only
>> > for stuff that's specific to the host ... so not stuff which should be
>> > done generically.
>> >
>> > There are no hard and fast rules for applying the test above.  In
>> > theory, the management interfaces exposed by FC_BSG_HST_VENDOR could be
>> > used to send commands ... but it tends to get tolerated as long as the
>> > drivers support the standardised rport commands.
>> >
>> > If I look at what the qla4xxx interface would do
>> >
>> >       1. it only supports vendor specific commands ... this is a bit of
>a
>> >          red flag since it's proposing to do nothing in a vendor
>neutral
>> >          way.  This one, I punt back to you: what should an iscsi
>device
>> >          implementing BSG support of the standard commands
>>
>> I am not 100% sure what we need yet, because qlogic has been pushing the
>> vendor specific route and I cannot see exactly what it needs with some
>> of its commands.
>>
>> Based on other drivers and from looking at the qlogic tools and their
>> qlogic.com driver which seems to convert some management operations to
>> flash commands, I have the list below:
>>
>> - Operation to set net settings like ip/dhcp, gateway, dns, subnet,
>> vlan, tcp/ip settings, speed/auto-negotiate, etc.
>>
>> Besides supporting a IPv4 and IPv6 address, some cards can support lots
>> of IPs and/or vlans per same physical port, so this operation cannot be
>> tied to just the host.
>>
>> - Operation to get port state.
>>
>> - Operation to set iscsi initiator port settings like initiator
>> name/alias/isid, iscsi IO settings like FirstBurstLength or Digests,
>> auth settings like chap or radius, etc.
>>
>> This should not be tied to the physical port either since we can have
>> multiple initiator ports per physical port.
>>
>> - Operation to set iscsi settings for specific sessions like iSCSI IO
>> settings and auth settings mentioned above (some drivers want global
>> defaults settable at the host/initiator-port level then be able to
>> specify specific values for each session).
>
>/sys/class/iscsi_* might work better for the above?

For above setting qla4xxx firmware need data in blob format so that's
why we are trying to do it using BSG.

>
>
>> - Operation to discover targets and export targets that were found and:
>> 	- Operation to control if we want to log into specific targets that
>> were found.
>>
>> - Operation to notify userpsace of network link state changes.
>>
>> - Operation to notify userpsace of iscsi session/connection state
>changes.
>
>bsg supports only the request/response interface. The event
>notification from kernel space isn't fit for bsg.

We are working to add netlink support for same in iscsi transport.
We submitted the initial one as RFC patches and working to incorporate 
Mike.C comment to make it like FC as alluded in the other email thread
and resubmit as part of next patch set.

>
>Sounds like all the features are useful to any iSCSI HBAs?

Re: [PATCH 13/15] qla4xxx: Added bsg support

[Mike Christie]

On 06/15/2010 09:43 PM, Vikas Chaudhary wrote:
>> -----Original Message-----
>> From: Mike Christie [mailto:michaelc@cs.wisc.edu]
>> Sent: Sunday, June 13, 2010 1:11 PM
>> To: James Bottomley
>> Cc: Vikas Chaudhary; linux-scsi@vger.kernel.org; Ravi Anand
>> Subject: Re: [PATCH 13/15] qla4xxx: Added bsg support
>>
>> On 06/12/2010 05:37 PM, James Bottomley wrote:
>>> On Fri, 2010-06-11 at 19:23 -0500, Mike Christie wrote:
>>>> On 06/11/2010 02:49 AM, Vikas Chaudhary wrote:
>>>>> Added BSG support to enable application support to configure
>>>>> ISP40XX/ISP82XX adapter.
>>>>>
>>>>> This patch is on top of: http://marc.info/?l=linux-
>> scsi&m=126999297630764&w=2
>>>>>
>>>>
>>>> Did James say that using vendor specific commands was ok? I did not see
>>>> anything on the list.
>>>
>>> In principle, the point about vendor specific host commands has already
>>> been conceded ... lpfc and qla2xxx already use them.
>>>
>>> I think the real rule is that for host specific BSG commands, it's only
>>> for stuff that's specific to the host ... so not stuff which should be
>>> done generically.
>>>
>>> There are no hard and fast rules for applying the test above.  In
>>> theory, the management interfaces exposed by FC_BSG_HST_VENDOR could be
>>> used to send commands ... but it tends to get tolerated as long as the
>>> drivers support the standardised rport commands.
>>>
>>> If I look at what the qla4xxx interface would do
>>>
>>>        1. it only supports vendor specific commands ... this is a bit of a
>>>           red flag since it's proposing to do nothing in a vendor neutral
>>>           way.  This one, I punt back to you: what should an iscsi device
>>>           implementing BSG support of the standard commands
>>
>> I am not 100% sure what we need yet, because qlogic has been pushing the
>> vendor specific route and I cannot see exactly what it needs with some
>> of its commands.
>>
>> Based on other drivers and from looking at the qlogic tools and their
>> qlogic.com driver which seems to convert some management operations to
>> flash commands, I have the list below:
>>
>> - Operation to set net settings like ip/dhcp, gateway, dns, subnet,
>> vlan, tcp/ip settings, speed/auto-negotiate, etc.
>>
>> Besides supporting a IPv4 and IPv6 address, some cards can support lots
>> of IPs and/or vlans per same physical port, so this operation cannot be
>> tied to just the host.
>
> These operations are tied to per pci function in qla4xxx driver.


ServerEgnines supports multiple ips per pci function, so you need 
something that works for everyone like I was talking about in that other 
mail you guys sent about how to export the ipv4 and ipv6 addr.

Re: [PATCH 13/15] qla4xxx: Added bsg support

[Mike Christie]

On 06/15/2010 09:40 PM, Vikas Chaudhary wrote:
>
> But the reason we are trying to implement most of the commands using vendor
> specific way is because *setting up the card/configuration* related stuff
> are communicated using mailbox command interface with our firmware which
> do blob data transfer back and forth. Driver is basically funneling info
> back and forth between F/W and API in a blob format. So we need this to allow
> end user's to configure the card using QLogic Application or *iscsiadm* in
> future as we are migrating from ioctl interface to bsg/netlink/sysfs route
> which will require API changes.

We all know we are adding a interface so some tools can config the card. 
The question is why does it have to be a binary driver specific blob 
though? Why can't you do something like  qla4extioctl_set_init_fw_iscsi 
in the qlogic.com driver, but instead of a qlogic specific struct 
pinit_fw use a common struct?


>
> Additionally we are working on to add support in  libiscsi user space to
> support qla4xxx which will manipulate this blob and hookup with Linux Native
> *iscsiadm*  tool. Mike was OK with this approach as long as user space
> iscsiadm support will be enabled. This is something we are working on
> parallely as well. Here's the email thread where Mike responded and we pretty
> much agreed to it :
> http://article.gmane.org/gmane.linux.scsi/58849/match=qla4xxx+bsg


I think if we go this route, I think it is best to see the userspace 
code finished before the kernel code is merged, so my concern in that 
thread is nullified. At least could we come up with a rough outline? 
Something to give users a little hope that it will be done one day :)



>
>
>>      2. QL4_SET_FLASH looks a bit suspicious ... the only thing I think
>>         you could really set from that is the mac addresses ... is this
>>         so?
>
> It's used for multiple purpose, for ex :-
> 	1. write firmware image to flash
> 	2. write target configuration to flash and make target persistent
> 	3. write firmware control block to flash
> 	4. write chap information to flash, etc..
>
>>      3. the DDB commands seem to be for manipulating the end point
>>         table ... which is basically the iscsi connection table ... does
>>         manipulating that from BSG make sense?
>
> For qla4xxx firmware establishes  Connection/Session and qla4xxx driver
> works in session mode. We are using ddb commands here to establish
> session with target.
> Let us know if you have any suggestions over here.

Do you have an idea of how to manage this stuff in the userspace lib, so 
it gives a better picture of why your approach is better? When I asked 
for you guys to come up with a way to add a kernel interface that does 
this in a standard way, could you not figure out a nice way to do it, 
but figured out a nice way to do it in userspace instead (if so what is it)?

RE: [PATCH 13/15] qla4xxx: Added bsg support

[James Bottomley]

On Tue, 2010-06-15 at 19:40 -0700, Vikas Chaudhary wrote:
> >-----Original Message-----
> >From: James Bottomley [mailto:James.Bottomley@suse.de]
> >Sent: Saturday, June 12, 2010 3:38 PM
> >To: Mike Christie
> >Cc: Vikas Chaudhary; linux-scsi@vger.kernel.org; Ravi Anand
> >Subject: Re: [PATCH 13/15] qla4xxx: Added bsg support
> >
> >On Fri, 2010-06-11 at 19:23 -0500, Mike Christie wrote:
> >> On 06/11/2010 02:49 AM, Vikas Chaudhary wrote:
> >> > Added BSG support to enable application support to configure
> >> > ISP40XX/ISP82XX adapter.
> >> >
> >> > This patch is on top of: http://marc.info/?l=linux-
> >scsi&m=126999297630764&w=2
> >> >
> >>
> >> Did James say that using vendor specific commands was ok? I did not see
> >> anything on the list.
> >
> >In principle, the point about vendor specific host commands has already
> >been conceded ... lpfc and qla2xxx already use them.
> >
> >I think the real rule is that for host specific BSG commands, it's only
> >for stuff that's specific to the host ... so not stuff which should be
> >done generically.
> >
> >There are no hard and fast rules for applying the test above.  In
> >theory, the management interfaces exposed by FC_BSG_HST_VENDOR could be
> >used to send commands ... but it tends to get tolerated as long as the
> >drivers support the standardised rport commands.
> >
> >If I look at what the qla4xxx interface would do
> >
> >     1. it only supports vendor specific commands ... this is a bit of a
> >        red flag since it's proposing to do nothing in a vendor neutral
> >        way.  This one, I punt back to you: what should an iscsi device
> >        implementing BSG support of the standard commands (which have
> >        only very recently been proposed for definition)
> 
> That's not true. For ex : Look at the ping support we have added which other
> iSCSI HBA vendors can use as well. Here's the thread : 
> http://article.gmane.org/gmane.linux.scsi/59535

The fact remains that none of the non-vendor-specific bsg commands is
implemented in this patch set.

> Also we are adding support in iSCSI transport for other stuff which we think
> is the right interface to use. Here's the thread :
> http://article.gmane.org/gmane.linux.scsi/59532

That's good ... and part of the problem is defining the standard set of
BSG packets for iSCSI

> But the reason we are trying to implement most of the commands using vendor
> specific way is because *setting up the card/configuration* related stuff
> are communicated using mailbox command interface with our firmware which
> do blob data transfer back and forth. Driver is basically funneling info 
> back and forth between F/W and API in a blob format. So we need this to allow
> end user's to configure the card using QLogic Application or *iscsiadm* in
> future as we are migrating from ioctl interface to bsg/netlink/sysfs route
> which will require API changes.

That's not really a relevant concern though ... at some level,
programming any HBA to do work becomes the exchange of HBA specific
"binary blob" information.  It's the job of a driver to provide a
standardised interface to the operating system which hides as much of
the binary blob transformations as possible.

As I said, I think the general rule of thumb is that anything that would
be something you'd expect all iSCSI initiators to do (like setting mac
addresses or listing endpoints) should be done via standardised,
non-vendor specific commands.

> Additionally we are working on to add support in  libiscsi user space to
> support qla4xxx which will manipulate this blob and hookup with Linux Native
> *iscsiadm*  tool. Mike was OK with this approach as long as user space
> iscsiadm support will be enabled. This is something we are working on
> parallely as well. Here's the email thread where Mike responded and we pretty
> much agreed to it :
> http://article.gmane.org/gmane.linux.scsi/58849/match=qla4xxx+bsg

Right, so I'm pretty much endorsing this approach.  With the proviso
that for anything that we already provide interfaces for (or which we
will provide interfaces for in the immediate future) you should be
using, so it's not a licence to take the existing solaris tool and hack
BSG up to use the blob data format unmodified. 

> 
> >     2. QL4_SET_FLASH looks a bit suspicious ... the only thing I think
> >        you could really set from that is the mac addresses ... is this
> >        so?
> 
> It's used for multiple purpose, for ex :-
> 	1. write firmware image to flash
> 	2. write target configuration to flash and make target persistent
> 	3. write firmware control block to flash
> 	4. write chap information to flash, etc..

So isn't the existing request_firmware() interface more appropriate for
this?

> >     3. the DDB commands seem to be for manipulating the end point
> >        table ... which is basically the iscsi connection table ... does
> >        manipulating that from BSG make sense?
> 
> For qla4xxx firmware establishes  Connection/Session and qla4xxx driver
> works in session mode. We are using ddb commands here to establish
> session with target.
> Let us know if you have any suggestions over here.

Tell us what you want to know ... getting session information out looks
to be a fairly universally useful thing rather than qlogic specific.

> >     4. What is ACB and IFCB?  It's undocumented
> 
> It is control block containing qla4xxx firmware related settings.

discuss ...

> >     5. GET/SET_ISCSI_STAT seems to be statistics related by the name;
> >        is it? if so, why not use the more standard statistics interface
> >        in the iscsi transport class?
> >
> 
> we added it via BSG using vendor command as it blob data transfer.
> API interprets the data and displays the statistics. Driver is just a passthru.

OK, so what statistics?  We already use the sysfs statistics group for
fibre channel, should we also be using that for iscsi?

> >>  If he did, then instead of the patch above from
> >> Jay, you want to use iscsi bsg patches that I sent that handle Tomos's
> >> comments about not duplicating fc code. I sent the bsg lib to the list
> >> and then in my tree on kernel.org I ported the fc drivers (those might
> >> need a update for some patches James Smart recently sent). For those bsg
> >> patches you will want to then also add the vendor specific command back.
> >
> >Agreed, having a standard way of doing bsg helps.
> 
> We will do patches again on top of bsg lib patches.

Thanks,

James