On 11/12/2014 12:25 PM, Kashyap Desai wrote: >> -----Original Message----- >> From: Tomas Henzl [mailto:thenzl@xxxxxxxxxx] >> Sent: Tuesday, November 11, 2014 7:43 PM >> To: Sumit.Saxena@xxxxxxxxxxxxx; linux-scsi@xxxxxxxxxxxxxxx >> Cc: martin.petersen@xxxxxxxxxx; hch@xxxxxxxxxxxxx; >> jbottomley@xxxxxxxxxxxxx; kashyap.desai@xxxxxxxxxxxxx; >> aradford@xxxxxxxxx >> Subject: Re: [PATCH 4/7] megaraid_sas : Online Firmware upgrade suppport >> for Extended VD feature >> >> On 11/10/2014 01:21 PM, Sumit.Saxena@xxxxxxxxxxxxx wrote: >>> This patch provides driver compatibility for updating firmware online >>> to upgrade legacy(64 VD) firmware to Extended VD firmware and >>> viceversa. Currently, at driver load time only, driver will check >>> whether Firmware is legacy or 240 VD. If legacy Firmware is upgraded > to >> Extended VD firmware without unloading-loading driver, driver will never >> come to know the underlying firmware is changed and it will always acts > as >> firmware type is same, which was queried at driver load time. >>> Below is the descriptions of code changes done- >>> >>> 1)This patch has added functionality to get controller information >>> from OCR(Online Controller Rest) path(which is called after firmware >> upgrade) and update the paramter of firmwware type flashed on > controller- >> Extended VD firmware or Legacy VD firmware. >>> Driver will not change any other parameter except for this one. >>> >>> 2)Also added few code optimizations/fixes[memset after get_free_pages, >>> and reduced redundant parameters in megasas_ctrl_info() function. >>> >>> Signed-off-by: Sumit Saxena <sumit.saxena@xxxxxxxxxxxxx> >>> Signed-off-by: Kashyap Desai <kashyap.desai@xxxxxxxxxxxxx> >>> --- >>> drivers/scsi/megaraid/megaraid_sas.h | 3 +- >>> drivers/scsi/megaraid/megaraid_sas_base.c | 80 >> +++++++++++++++++++++++---- >>> drivers/scsi/megaraid/megaraid_sas_fusion.c | 55 > +++++-------------- >>> 3 files changed, 83 insertions(+), 55 deletions(-) >>> >>> diff --git a/drivers/scsi/megaraid/megaraid_sas.h >>> b/drivers/scsi/megaraid/megaraid_sas.h >>> index a49914d..0408dda 100644 >>> --- a/drivers/scsi/megaraid/megaraid_sas.h >>> +++ b/drivers/scsi/megaraid/megaraid_sas.h >>> @@ -1931,8 +1931,7 @@ u16 get_updated_dev_handle(struct >> megasas_instance *instance, >>> struct LD_LOAD_BALANCE_INFO *lbInfo, struct IO_REQUEST_INFO >>> *in_info); void mr_update_load_balance_params(struct >> MR_DRV_RAID_MAP_ALL *map, >>> struct LD_LOAD_BALANCE_INFO *lbInfo); -int >>> megasas_get_ctrl_info(struct megasas_instance *instance, >>> - struct megasas_ctrl_info *ctrl_info); >>> +int megasas_get_ctrl_info(struct megasas_instance *instance); >>> int megasas_set_crash_dump_params(struct megasas_instance >> *instance, >>> u8 crash_buf_state); >>> void megasas_free_host_crash_buffer(struct megasas_instance >>> *instance); diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c >>> b/drivers/scsi/megaraid/megaraid_sas_base.c >>> index 7754eeb..9d5db5f 100644 >>> --- a/drivers/scsi/megaraid/megaraid_sas_base.c >>> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c >>> @@ -4034,25 +4034,81 @@ megasas_ld_list_query(struct >> megasas_instance *instance, u8 query_type) >>> return ret; >>> } >>> >>> +/* >>> + * megasas_update_ext_vd_details : Update details w.r.t Extended VD >>> + * instance : Controller's instance >>> +*/ >>> +static void megasas_update_ext_vd_details(struct megasas_instance >>> +*instance) { >>> + struct fusion_context *fusion; >>> + >>> + fusion = instance->ctrl_context; >>> + /* For MFI based controllers return dummy success */ >>> + if (!fusion) >>> + return; >>> + >>> + instance->supportmax256vd = >>> + instance->ctrl_info->adapterOperations3.supportMaxExtLDs; >>> + /* Below is additional check to address future FW enhancement */ >>> + if (instance->ctrl_info->max_lds > 64) >>> + instance->supportmax256vd = 1; >>> + >>> + instance->drv_supported_vd_count = >> MEGASAS_MAX_LD_CHANNELS >>> + * >> MEGASAS_MAX_DEV_PER_CHANNEL; >>> + instance->drv_supported_pd_count = >> MEGASAS_MAX_PD_CHANNELS >>> + * >> MEGASAS_MAX_DEV_PER_CHANNEL; >>> + if (instance->supportmax256vd) { >>> + instance->fw_supported_vd_count = >> MAX_LOGICAL_DRIVES_EXT; >>> + instance->fw_supported_pd_count = >> MAX_PHYSICAL_DEVICES; >>> + } else { >>> + instance->fw_supported_vd_count = >> MAX_LOGICAL_DRIVES; >>> + instance->fw_supported_pd_count = >> MAX_PHYSICAL_DEVICES; >>> + } >>> + dev_info(&instance->pdev->dev, "Firmware supports %d VD %d >> PD\n", >>> + instance->fw_supported_vd_count, >>> + instance->fw_supported_pd_count); >>> + dev_info(&instance->pdev->dev, "Driver supports %d VD %d PD\n", >>> + instance->drv_supported_vd_count, >>> + instance->drv_supported_pd_count); >>> + >>> + fusion->old_map_sz = sizeof(struct MR_FW_RAID_MAP) + >>> + (sizeof(struct MR_LD_SPAN_MAP) * >>> + (instance->fw_supported_vd_count - 1)); >>> + fusion->new_map_sz = sizeof(struct MR_FW_RAID_MAP_EXT); >>> + fusion->drv_map_sz = sizeof(struct MR_DRV_RAID_MAP) + >>> + (sizeof(struct MR_LD_SPAN_MAP) * >>> + (instance->drv_supported_vd_count - 1)); >>> + >>> + fusion->max_map_sz = max(fusion->old_map_sz, fusion- >>> new_map_sz); >> old_map_sz + new_map_sz are used only locally, no need to have it stored > in >> fusion_context what is the reason for max_map_sz ? It is used in some >> places outside this function, but why do you not use current_map_sz >> instead? > Agree with your comment. > old_map_sz and new_map_sz is local and we could have used local variable > on stack, instead of storing it on fusion_context. We will make this > change in resend the patch. > > Max_map_sz is used to provide code readability for future enhancement. As > of today new_map_sz is always larger than old map, so we could have use > new_map_sz and completely avoid using max_map_sz..but we may have to be > prepare with another raid map changes in future which need driver to > calculate what is a Max of all possible raid map. > This max map size is what driver allocate for FW raid map and Driver raid > map. The max_map_sz is computed in this function so as is current_map_sz, the name 'current' suggest this is the field/size whatever used from now on. Why can't you allocate based on current_map_sz? In case current_map_sz well ever be smaller than max_map_sz you'll allocate too much. > > ` Kashyap > >> Cheers, Tomas >> >>> + >>> + >>> + if (instance->supportmax256vd) >>> + fusion->current_map_sz = fusion->new_map_sz; >>> + else >>> + fusion->current_map_sz = fusion->old_map_sz; >>> + >>> +} >>> + >>> /** >>> * megasas_get_controller_info - Returns FW's controller structure >>> * @instance: Adapter soft state >>> - * @ctrl_info: Controller information >> structure >>> * >>> * Issues an internal command (DCMD) to get the FW's controller > structure. >>> * This information is mainly used to find out the maximum IO > transfer per >>> * command supported by the FW. >>> */ >>> int >>> -megasas_get_ctrl_info(struct megasas_instance *instance, >>> - struct megasas_ctrl_info *ctrl_info) >>> +megasas_get_ctrl_info(struct megasas_instance *instance) >>> { >>> int ret = 0; >>> struct megasas_cmd *cmd; >>> struct megasas_dcmd_frame *dcmd; >>> struct megasas_ctrl_info *ci; >>> + struct megasas_ctrl_info *ctrl_info; >>> dma_addr_t ci_h = 0; >>> >>> + ctrl_info = instance->ctrl_info; >>> + >>> cmd = megasas_get_cmd(instance); >>> >>> if (!cmd) { >>> @@ -4092,8 +4148,13 @@ megasas_get_ctrl_info(struct megasas_instance >> *instance, >>> else >>> ret = megasas_issue_polled(instance, cmd); >>> >>> - if (!ret) >>> + if (!ret) { >>> memcpy(ctrl_info, ci, sizeof(struct megasas_ctrl_info)); >>> + le32_to_cpus((u32 *)&ctrl_info- >>> properties.OnOffProperties); >>> + le32_to_cpus((u32 *)&ctrl_info->adapterOperations2); >>> + le32_to_cpus((u32 *)&ctrl_info->adapterOperations3); >>> + megasas_update_ext_vd_details(instance); >>> + } >>> >>> pci_free_consistent(instance->pdev, sizeof(struct >> megasas_ctrl_info), >>> ci, ci_h); >>> @@ -4295,7 +4356,7 @@ megasas_init_adapter_mfi(struct >> megasas_instance *instance) >>> if (megasas_issue_init_mfi(instance)) >>> goto fail_fw_init; >>> >>> - if (megasas_get_ctrl_info(instance, instance->ctrl_info)) { >>> + if (megasas_get_ctrl_info(instance)) { >>> dev_err(&instance->pdev->dev, "(%d): Could get controller >> info " >>> "Fail from %s %d\n", instance->unique_id, >>> __func__, __LINE__); >>> @@ -4533,12 +4594,8 @@ static int megasas_init_fw(struct >> megasas_instance *instance) >>> dev_info(&instance->pdev->dev, >>> "Controller type: iMR\n"); >>> } >>> - /* OnOffProperties are converted into CPU arch*/ >>> - le32_to_cpus((u32 *)&ctrl_info->properties.OnOffProperties); >>> instance->disableOnlineCtrlReset = >>> ctrl_info->properties.OnOffProperties.disableOnlineCtrlReset; >>> - /* adapterOperations2 are converted into CPU arch*/ >>> - le32_to_cpus((u32 *)&ctrl_info->adapterOperations2); >>> instance->mpio = ctrl_info->adapterOperations2.mpio; >>> instance->UnevenSpanSupport = >>> ctrl_info->adapterOperations2.supportUnevenSpans; >>> @@ -4568,7 +4625,6 @@ static int megasas_init_fw(struct >> megasas_instance *instance) >>> "requestorId %d\n", instance->requestorId); >>> } >>> >>> - le32_to_cpus((u32 *)&ctrl_info->adapterOperations3); >>> instance->crash_dump_fw_support = >>> ctrl_info->adapterOperations3.supportCrashDump; >>> instance->crash_dump_drv_support = >>> @@ -4593,8 +4649,6 @@ static int megasas_init_fw(struct >> megasas_instance *instance) >>> if (tmp_sectors && (instance->max_sectors_per_req > >> tmp_sectors)) >>> instance->max_sectors_per_req = tmp_sectors; >>> >>> - kfree(ctrl_info); >>> - >>> /* Check for valid throttlequeuedepth module parameter */ >>> if (instance->is_imr) { >>> if (throttlequeuedepth > (instance->max_fw_cmds - @@ - >> 5085,6 >>> +5139,8 @@ static int megasas_probe_one(struct pci_dev *pdev, >>> goto fail_alloc_dma_buf; >>> } >>> fusion = instance->ctrl_context; >>> + memset(fusion, 0, >>> + ((1 << PAGE_SHIFT) << instance- >>> ctrl_context_pages)); >>> INIT_LIST_HEAD(&fusion->cmd_pool); >>> spin_lock_init(&fusion->mpt_pool_lock); >>> memset(fusion->load_balance_info, 0, diff --git >>> a/drivers/scsi/megaraid/megaraid_sas_fusion.c >>> b/drivers/scsi/megaraid/megaraid_sas_fusion.c >>> index f37eed6..b8411a1 100644 >>> --- a/drivers/scsi/megaraid/megaraid_sas_fusion.c >>> +++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c >>> @@ -1065,48 +1065,16 @@ megasas_init_adapter_fusion(struct >> megasas_instance *instance) >>> goto fail_ioc_init; >>> >>> megasas_display_intel_branding(instance); >>> - if (megasas_get_ctrl_info(instance, instance->ctrl_info)) { >>> + if (megasas_get_ctrl_info(instance)) { >>> dev_err(&instance->pdev->dev, >>> "Could not get controller info. Fail from %s > %d\n", >>> __func__, __LINE__); >>> goto fail_ioc_init; >>> } >>> >>> - instance->supportmax256vd = >>> - instance->ctrl_info->adapterOperations3.supportMaxExtLDs; >>> - /* Below is additional check to address future FW enhancement */ >>> - if (instance->ctrl_info->max_lds > 64) >>> - instance->supportmax256vd = 1; >>> - instance->drv_supported_vd_count = >> MEGASAS_MAX_LD_CHANNELS >>> - * >> MEGASAS_MAX_DEV_PER_CHANNEL; >>> - instance->drv_supported_pd_count = >> MEGASAS_MAX_PD_CHANNELS >>> - * >> MEGASAS_MAX_DEV_PER_CHANNEL; >>> - if (instance->supportmax256vd) { >>> - instance->fw_supported_vd_count = >> MAX_LOGICAL_DRIVES_EXT; >>> - instance->fw_supported_pd_count = >> MAX_PHYSICAL_DEVICES; >>> - } else { >>> - instance->fw_supported_vd_count = >> MAX_LOGICAL_DRIVES; >>> - instance->fw_supported_pd_count = >> MAX_PHYSICAL_DEVICES; >>> - } >>> - dev_info(&instance->pdev->dev, "Firmware supports %d VDs %d >> PDs\n" >>> - "Driver supports %d VDs %d PDs\n", >>> - instance->fw_supported_vd_count, >>> - instance->fw_supported_pd_count, >>> - instance->drv_supported_vd_count, >>> - instance->drv_supported_pd_count); >>> - >>> instance->flag_ieee = 1; >>> fusion->fast_path_io = 0; >>> >>> - fusion->old_map_sz = >>> - sizeof(struct MR_FW_RAID_MAP) + (sizeof(struct >> MR_LD_SPAN_MAP) * >>> - (instance->fw_supported_vd_count - 1)); >>> - fusion->new_map_sz = >>> - sizeof(struct MR_FW_RAID_MAP_EXT); >>> - fusion->drv_map_sz = >>> - sizeof(struct MR_DRV_RAID_MAP) + (sizeof(struct >> MR_LD_SPAN_MAP) * >>> - (instance->drv_supported_vd_count - 1)); >>> - >>> fusion->drv_map_pages = get_order(fusion->drv_map_sz); >>> for (i = 0; i < 2; i++) { >>> fusion->ld_map[i] = NULL; >>> @@ -1121,16 +1089,10 @@ megasas_init_adapter_fusion(struct >> megasas_instance *instance) >>> fusion->drv_map_pages); >>> goto fail_ioc_init; >>> } >>> + memset(fusion->ld_drv_map[i], 0, >>> + ((1 << PAGE_SHIFT) << fusion->drv_map_pages)); >>> } >>> >>> - fusion->max_map_sz = max(fusion->old_map_sz, fusion- >>> new_map_sz); >>> - >>> - if (instance->supportmax256vd) >>> - fusion->current_map_sz = fusion->new_map_sz; >>> - else >>> - fusion->current_map_sz = fusion->old_map_sz; >>> - >>> - >>> for (i = 0; i < 2; i++) { >>> fusion->ld_map[i] = dma_alloc_coherent(&instance->pdev- >>> dev, >>> fusion->max_map_sz, >>> @@ -2385,6 +2347,8 @@ megasas_alloc_host_crash_buffer(struct >> megasas_instance *instance) >>> "memory allocation failed at index %d\n", > i); >>> break; >>> } >>> + memset(instance->crash_buf[i], 0, >>> + ((1 << PAGE_SHIFT) << instance->crash_buf_pages)); >>> } >>> instance->drv_buf_alloc = i; >>> } >>> @@ -2842,6 +2806,15 @@ int megasas_reset_fusion(struct Scsi_Host >> *shost, int iotimeout) >>> instance->instancet->enable_intr(instance); >>> instance->adprecovery = >> MEGASAS_HBA_OPERATIONAL; >>> + if (megasas_get_ctrl_info(instance)) { >>> + dev_info(&instance->pdev->dev, >>> + "Failed from %s %d\n", >>> + __func__, __LINE__); >>> + instance->adprecovery = >>> + MEGASAS_HW_CRITICAL_ERROR; >>> + megaraid_sas_kill_hba(instance); >>> + retval = FAILED; >>> + } >>> /* Reset load balance info */ >>> memset(fusion->load_balance_info, 0, >>> sizeof(struct LD_LOAD_BALANCE_INFO) > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
