From: Hannes Reinecke <hare@suse.de>
To: Jitendra Bhivare <jitendra.bhivare@avagotech.com>,
linux-scsi@vger.kernel.org, michaelc@cs.wisc.edu
Subject: Re: [PATCH 10/15] be2iscsi: Add FW config validation
Date: Fri, 18 Dec 2015 10:03:52 +0100 [thread overview]
Message-ID: <5673CBF8.1040401@suse.de> (raw)
In-Reply-To: <1450194906-12925-11-git-send-email-jitendra.bhivare@avagotech.com>
On 12/15/2015 04:55 PM, Jitendra Bhivare wrote:
> From: Jitendra <jitendra.bhivare@avagotech.com>
>
> System crash in I+T card personality
>
> Fix to add validation for ULP in initiator mode, physical port number,
> and supported queue, icd, cid counts.
>
> Signed-off-by: Jitendra <jitendra.bhivare@avagotech.com>
> ---
> drivers/scsi/be2iscsi/be_main.c | 2 +-
> drivers/scsi/be2iscsi/be_main.h | 2 +
> drivers/scsi/be2iscsi/be_mgmt.c | 188 +++++++++++++++++++++++++--------------
> 3 files changed, 123 insertions(+), 69 deletions(-)
>
> diff --git a/drivers/scsi/be2iscsi/be_main.c b/drivers/scsi/be2iscsi/be_main.c
> index 8967e05..a665e6a 100644
> --- a/drivers/scsi/be2iscsi/be_main.c
> +++ b/drivers/scsi/be2iscsi/be_main.c
> @@ -5670,6 +5670,7 @@ static int beiscsi_dev_probe(struct pci_dev *pcidev,
> goto free_port;
> }
> mgmt_get_port_name(&phba->ctrl, phba);
> + beiscsi_get_params(phba);
>
> if (enable_msix)
> find_num_cpus(phba);
> @@ -5687,7 +5688,6 @@ static int beiscsi_dev_probe(struct pci_dev *pcidev,
> }
>
> phba->shost->max_id = phba->params.cxns_per_ctrl;
> - beiscsi_get_params(phba);
> phba->shost->can_queue = phba->params.ios_per_ctrl;
> ret = beiscsi_init_port(phba);
> if (ret < 0) {
> diff --git a/drivers/scsi/be2iscsi/be_main.h b/drivers/scsi/be2iscsi/be_main.h
> index c09082a..f89861b 100644
> --- a/drivers/scsi/be2iscsi/be_main.h
> +++ b/drivers/scsi/be2iscsi/be_main.h
> @@ -397,7 +397,9 @@ struct beiscsi_hba {
> * group together since they are used most frequently
> * for cid to cri conversion
> */
> +#define BEISCSI_PHYS_PORT_MAX 4
> unsigned int phys_port;
> + /* valid values of phys_port id are 0, 1, 2, 3 */
> unsigned int eqid_count;
> unsigned int cqid_count;
> unsigned int iscsi_cid_start[BEISCSI_ULP_COUNT];
> diff --git a/drivers/scsi/be2iscsi/be_mgmt.c b/drivers/scsi/be2iscsi/be_mgmt.c
> index ad7aa75..15f7ad7 100644
> --- a/drivers/scsi/be2iscsi/be_mgmt.c
> +++ b/drivers/scsi/be2iscsi/be_mgmt.c
> @@ -373,90 +373,142 @@ int mgmt_get_fw_config(struct be_ctrl_info *ctrl,
> struct beiscsi_hba *phba)
> {
> struct be_mcc_wrb *wrb = wrb_from_mbox(&ctrl->mbox_mem);
> - struct be_fw_cfg *req = embedded_payload(wrb);
> - int status = 0;
> + struct be_fw_cfg *pfw_cfg = embedded_payload(wrb);
> + uint32_t cid_count, icd_count;
> + int status = -EINVAL;
> + uint8_t ulp_num = 0;
>
> mutex_lock(&ctrl->mbox_lock);
> memset(wrb, 0, sizeof(*wrb));
> + be_wrb_hdr_prepare(wrb, sizeof(*pfw_cfg), true, 0);
>
> - be_wrb_hdr_prepare(wrb, sizeof(*req), true, 0);
> -
> - be_cmd_hdr_prepare(&req->hdr, CMD_SUBSYSTEM_COMMON,
> + be_cmd_hdr_prepare(&pfw_cfg->hdr, CMD_SUBSYSTEM_COMMON,
> OPCODE_COMMON_QUERY_FIRMWARE_CONFIG,
> EMBED_MBX_MAX_PAYLOAD_SIZE);
> - status = be_mbox_notify(ctrl);
> - if (!status) {
> - uint8_t ulp_num = 0;
> - struct be_fw_cfg *pfw_cfg;
> - pfw_cfg = req;
>
> - if (!is_chip_be2_be3r(phba)) {
> - phba->fw_config.eqid_count = pfw_cfg->eqid_count;
> - phba->fw_config.cqid_count = pfw_cfg->cqid_count;
> + if (be_mbox_notify(ctrl)) {
> + beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
> + "BG_%d : Failed in mgmt_get_fw_config\n");
> + goto fail_init;
> + }
>
> - beiscsi_log(phba, KERN_INFO,
> - BEISCSI_LOG_INIT,
> - "BG_%d : EQ_Count : %d CQ_Count : %d\n",
> - phba->fw_config.eqid_count,
> + /* FW response formats depend on port id */
> + phba->fw_config.phys_port = pfw_cfg->phys_port;
> + if (phba->fw_config.phys_port >= BEISCSI_PHYS_PORT_MAX) {
> + beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
> + "BG_%d : invalid physical port id %d\n",
> + phba->fw_config.phys_port);
> + goto fail_init;
> + }
> +
> + /* populate and check FW config against min and max values */
> + if (!is_chip_be2_be3r(phba)) {
> + phba->fw_config.eqid_count = pfw_cfg->eqid_count;
> + phba->fw_config.cqid_count = pfw_cfg->cqid_count;
> + if (phba->fw_config.eqid_count == 0 ||
> + phba->fw_config.eqid_count > 2048) {
> + beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
> + "BG_%d : invalid EQ count %d\n",
> + phba->fw_config.eqid_count);
> + goto fail_init;
> + }
> + if (phba->fw_config.cqid_count == 0 ||
> + phba->fw_config.cqid_count > 4096) {
> + beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
> + "BG_%d : invalid CQ count %d\n",
> phba->fw_config.cqid_count);
> + goto fail_init;
> }
> + beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_INIT,
> + "BG_%d : EQ_Count : %d CQ_Count : %d\n",
> + phba->fw_config.eqid_count,
> + phba->fw_config.cqid_count);
> + }
>
> - for (ulp_num = 0; ulp_num < BEISCSI_ULP_COUNT; ulp_num++)
> - if (pfw_cfg->ulp[ulp_num].ulp_mode &
> - BEISCSI_ULP_ISCSI_INI_MODE)
> - set_bit(ulp_num,
> - &phba->fw_config.ulp_supported);
> -
> - phba->fw_config.phys_port = pfw_cfg->phys_port;
> - for (ulp_num = 0; ulp_num < BEISCSI_ULP_COUNT; ulp_num++) {
> - if (test_bit(ulp_num, &phba->fw_config.ulp_supported)) {
> -
> - phba->fw_config.iscsi_cid_start[ulp_num] =
> - pfw_cfg->ulp[ulp_num].sq_base;
> - phba->fw_config.iscsi_cid_count[ulp_num] =
> - pfw_cfg->ulp[ulp_num].sq_count;
> -
> - phba->fw_config.iscsi_icd_start[ulp_num] =
> - pfw_cfg->ulp[ulp_num].icd_base;
> - phba->fw_config.iscsi_icd_count[ulp_num] =
> - pfw_cfg->ulp[ulp_num].icd_count;
> -
> - phba->fw_config.iscsi_chain_start[ulp_num] =
> - pfw_cfg->chain_icd[ulp_num].chain_base;
> - phba->fw_config.iscsi_chain_count[ulp_num] =
> - pfw_cfg->chain_icd[ulp_num].chain_count;
> -
> - beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_INIT,
> - "BG_%d : Function loaded on ULP : %d\n"
> - "\tiscsi_cid_count : %d\n"
> - "\tiscsi_cid_start : %d\n"
> - "\t iscsi_icd_count : %d\n"
> - "\t iscsi_icd_start : %d\n",
> - ulp_num,
> - phba->fw_config.
> - iscsi_cid_count[ulp_num],
> - phba->fw_config.
> - iscsi_cid_start[ulp_num],
> - phba->fw_config.
> - iscsi_icd_count[ulp_num],
> - phba->fw_config.
> - iscsi_icd_start[ulp_num]);
> - }
> + /**
> + * Check on which all ULP iSCSI Protocol is loaded.
> + * Set the Bit for those ULP. This set flag is used
> + * at all places in the code to check on which ULP
> + * iSCSi Protocol is loaded
> + **/
> + for (ulp_num = 0; ulp_num < BEISCSI_ULP_COUNT; ulp_num++) {
> + if (pfw_cfg->ulp[ulp_num].ulp_mode &
> + BEISCSI_ULP_ISCSI_INI_MODE) {
> + set_bit(ulp_num, &phba->fw_config.ulp_supported);
> +
> + /* Get the CID, ICD and Chain count for each ULP */
> + phba->fw_config.iscsi_cid_start[ulp_num] =
> + pfw_cfg->ulp[ulp_num].sq_base;
> + phba->fw_config.iscsi_cid_count[ulp_num] =
> + pfw_cfg->ulp[ulp_num].sq_count;
> +
> + phba->fw_config.iscsi_icd_start[ulp_num] =
> + pfw_cfg->ulp[ulp_num].icd_base;
> + phba->fw_config.iscsi_icd_count[ulp_num] =
> + pfw_cfg->ulp[ulp_num].icd_count;
> +
> + phba->fw_config.iscsi_chain_start[ulp_num] =
> + pfw_cfg->chain_icd[ulp_num].chain_base;
> + phba->fw_config.iscsi_chain_count[ulp_num] =
> + pfw_cfg->chain_icd[ulp_num].chain_count;
> +
> + beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_INIT,
> + "BG_%d : Function loaded on ULP : %d\n"
> + "\tiscsi_cid_count : %d\n"
> + "\tiscsi_cid_start : %d\n"
> + "\t iscsi_icd_count : %d\n"
> + "\t iscsi_icd_start : %d\n",
> + ulp_num,
> + phba->fw_config.
> + iscsi_cid_count[ulp_num],
> + phba->fw_config.
> + iscsi_cid_start[ulp_num],
> + phba->fw_config.
> + iscsi_icd_count[ulp_num],
> + phba->fw_config.
> + iscsi_icd_start[ulp_num]);
> }
> + }
>
> - phba->fw_config.dual_ulp_aware = (pfw_cfg->function_mode &
> - BEISCSI_FUNC_DUA_MODE);
> + if (phba->fw_config.ulp_supported == 0) {
> + beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
> + "BG_%d : iSCSI initiator mode not set: ULP0 %x ULP1 %x\n",
> + pfw_cfg->ulp[BEISCSI_ULP0].ulp_mode,
> + pfw_cfg->ulp[BEISCSI_ULP1].ulp_mode);
> + goto fail_init;
> + }
>
> - beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_INIT,
> - "BG_%d : DUA Mode : 0x%x\n",
> - phba->fw_config.dual_ulp_aware);
> + /**
> + * ICD is shared among ULPs. Use icd_count of any one loaded ULP
> + **/
> + for (ulp_num = 0; ulp_num < BEISCSI_ULP_COUNT; ulp_num++)
> + if (test_bit(ulp_num, &phba->fw_config.ulp_supported))
> + break;
> + icd_count = phba->fw_config.iscsi_icd_count[ulp_num];
> + if (icd_count == 0 || icd_count > 65536) {
> + beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
> + "BG_%d: invalid ICD count %d\n", icd_count);
> + goto fail_init;
> + }
>
> - } else {
> + cid_count = BEISCSI_GET_CID_COUNT(phba, BEISCSI_ULP0) +
> + BEISCSI_GET_CID_COUNT(phba, BEISCSI_ULP1);
> + if (cid_count == 0 || cid_count > 4096) {
> beiscsi_log(phba, KERN_ERR, BEISCSI_LOG_INIT,
> - "BG_%d : Failed in mgmt_get_fw_config\n");
> - status = -EINVAL;
> + "BG_%d: invalid CID count %d\n", cid_count);
> + goto fail_init;
> }
>
> + phba->fw_config.dual_ulp_aware = (pfw_cfg->function_mode &
> + BEISCSI_FUNC_DUA_MODE);
> +
> + beiscsi_log(phba, KERN_INFO, BEISCSI_LOG_INIT,
> + "BG_%d : DUA Mode : 0x%x\n",
> + phba->fw_config.dual_ulp_aware);
> +
> + /* all set, continue using this FW config */
> + status = 0;
> +fail_init:
> mutex_unlock(&ctrl->mbox_lock);
> return status;
> }
>
Shouldn't that be 'DUAL mode' ?
But then, even the original had 'DUA'. So no need to redo the patch
for that.
Reviewed-by: Hannes Reinecke <hare@suse.com>
Cheers,
Hannes
--
Dr. Hannes Reinecke zSeries & Storage
hare@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
next prev parent reply other threads:[~2015-12-18 9:03 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-15 15:54 [PATCH 00/15] be2iscsi: driver update 11.0.0.0 Jitendra Bhivare
2015-12-15 15:54 ` [PATCH 01/15] be2iscsi: Fix soft lockup in mgmt_get_all_if_id path using bmbx Jitendra Bhivare
2015-12-18 8:58 ` Hannes Reinecke
2015-12-15 15:54 ` [PATCH 02/15] be2iscsi: Fix mbox synchronization replacing spinlock with mutex Jitendra Bhivare
2015-12-18 8:59 ` Hannes Reinecke
2015-12-20 7:35 ` Mike Christie
2015-12-20 7:47 ` Mike Christie
2015-12-15 15:54 ` [PATCH 03/15] be2iscsi: Fix to use atomic operations for tag_state Jitendra Bhivare
2015-12-18 8:59 ` Hannes Reinecke
2015-12-20 7:44 ` Mike Christie
2015-12-20 9:01 ` Mike Christie
2015-12-20 10:13 ` Mike Christie
2015-12-21 4:47 ` Jitendra Bhivare
2015-12-15 15:54 ` [PATCH 04/15] be2iscsi: Fix to synchronize tag allocation using spin_lock Jitendra Bhivare
2015-12-18 8:59 ` Hannes Reinecke
2015-12-15 15:54 ` [PATCH 05/15] be2iscsi: Set mbox timeout to 30s Jitendra Bhivare
2015-12-18 9:00 ` Hannes Reinecke
2015-12-15 15:54 ` [PATCH 06/15] be2iscsi: Added return value check for mgmt_get_all_if_id Jitendra Bhivare
2015-12-18 9:00 ` Hannes Reinecke
2015-12-15 15:54 ` [PATCH 07/15] be2iscsi: Fix to remove shutdown entry point Jitendra Bhivare
2015-12-18 9:01 ` Hannes Reinecke
2015-12-15 15:54 ` [PATCH 08/15] be2iscsi: Fix VLAN support for IPv6 network Jitendra Bhivare
2015-12-18 9:01 ` Hannes Reinecke
2015-12-15 15:55 ` [PATCH 09/15] be2iscsi: Fix to handle misconfigured optics events Jitendra Bhivare
2015-12-18 9:02 ` Hannes Reinecke
2015-12-15 15:55 ` [PATCH 10/15] be2iscsi: Add FW config validation Jitendra Bhivare
2015-12-18 9:03 ` Hannes Reinecke [this message]
2015-12-21 2:57 ` Jitendra Bhivare
2015-12-21 3:59 ` Jitendra Bhivare
2015-12-15 15:55 ` [PATCH 11/15] be2iscsi: Fix return value for MCC completion Jitendra Bhivare
2015-12-18 9:04 ` Hannes Reinecke
2015-12-15 15:55 ` [PATCH 12/15] be2iscsi: Fix IOPOLL implementation Jitendra Bhivare
2015-12-18 9:04 ` Hannes Reinecke
2015-12-15 15:55 ` [PATCH 13/15] be2iscsi: Fix to process 25G link speed info from FW Jitendra Bhivare
2015-12-18 9:07 ` Hannes Reinecke
[not found] ` <CAOA9gs36+ozeo=ZtqHYMvznwMSG6wKnG_zLOp__=wnKMkKv0CA@mail.gmail.com>
2015-12-21 7:13 ` Hannes Reinecke
2015-12-15 15:55 ` [PATCH 14/15] be2iscsi: Fix WRB leak in login/logout path Jitendra Bhivare
2015-12-18 9:08 ` Hannes Reinecke
2015-12-15 15:55 ` [PATCH 15/15] be2iscsi: Update the driver version Jitendra Bhivare
2015-12-18 9:08 ` Hannes Reinecke
2015-12-18 9:11 ` [PATCH 00/15] be2iscsi: driver update 11.0.0.0 Hannes Reinecke
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=5673CBF8.1040401@suse.de \
--to=hare@suse.de \
--cc=jitendra.bhivare@avagotech.com \
--cc=linux-scsi@vger.kernel.org \
--cc=michaelc@cs.wisc.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.