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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).