From 2d70de5a0f03072289015917b059c155936c894d Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Wed, 20 Apr 2011 12:57:08 -0700 Subject: [PATCH] isci: validate oem parameters early, and fallback If the platform specifies invalid parameters warn the user and fallback to internal defaults rather than fail the driver load altogether. Reported-by: Yinghai Lu Signed-off-by: Dan Williams --- .../scsi/isci/core/scic_config_parameters.h | 2 + drivers/scsi/isci/core/scic_sds_controller.c | 75 +++++++++---------- drivers/scsi/isci/init.c | 10 +++ 3 files changed, 49 insertions(+), 38 deletions(-) diff --git a/drivers/scsi/isci/core/scic_config_parameters.h b/drivers/scsi/isci/core/scic_config_parameters.h index cb6abcc7e7c..716abfcd0c2 100644 --- a/drivers/scsi/isci/core/scic_config_parameters.h +++ b/drivers/scsi/isci/core/scic_config_parameters.h @@ -279,6 +279,8 @@ enum sci_status scic_oem_parameters_set( struct scic_sds_controller *controller, union scic_oem_parameters *oem_parameters); +int scic_oem_parameters_validate(struct scic_sds_oem_params *oem); + /** * scic_oem_parameters_get() - This method allows the user to retreive the OEM * parameters utilized by the controller. diff --git a/drivers/scsi/isci/core/scic_sds_controller.c b/drivers/scsi/isci/core/scic_sds_controller.c index 8194618b76c..9bb78a2e6ff 100644 --- a/drivers/scsi/isci/core/scic_sds_controller.c +++ b/drivers/scsi/isci/core/scic_sds_controller.c @@ -2455,52 +2455,51 @@ enum sci_status scic_user_parameters_set( return SCI_FAILURE_INVALID_STATE; } -enum sci_status scic_oem_parameters_set( - struct scic_sds_controller *scic, - union scic_oem_parameters *scic_parms) +int scic_oem_parameters_validate(struct scic_sds_oem_params *oem) { - u32 state = scic->state_machine.current_state_id; + int i; - if (state == SCI_BASE_CONTROLLER_STATE_RESET || - state == SCI_BASE_CONTROLLER_STATE_INITIALIZING || - state == SCI_BASE_CONTROLLER_STATE_INITIALIZED) { - u16 index; - u8 combined_phy_mask = 0; + for (i = 0; i < SCI_MAX_PORTS; i++) + if (oem->ports[i].phy_mask > SCIC_SDS_PARM_PHY_MASK_MAX) + return -EINVAL; + + for (i = 0; i < SCI_MAX_PHYS; i++) + if (oem->phys[i].sas_address.high == 0 && + oem->phys[i].sas_address.low == 0) + return -EINVAL; + + if (oem->controller.mode_type == SCIC_PORT_AUTOMATIC_CONFIGURATION_MODE) { + for (i = 0; i < SCI_MAX_PHYS; i++) + if (oem->ports[i].phy_mask != 0) + return -EINVAL; + } else if (oem->controller.mode_type == SCIC_PORT_MANUAL_CONFIGURATION_MODE) { + u8 phy_mask = 0; + + for (i = 0; i < SCI_MAX_PHYS; i++) + phy_mask |= oem->ports[i].phy_mask; + + if (phy_mask == 0) + return -EINVAL; + } else + return -EINVAL; - /* - * Validate the oem parameters. If they are not legal, then - * return a failure. */ - for (index = 0; index < SCI_MAX_PORTS; index++) { - if (scic_parms->sds1.ports[index].phy_mask > SCIC_SDS_PARM_PHY_MASK_MAX) - return SCI_FAILURE_INVALID_PARAMETER_VALUE; - } + if (oem->controller.max_concurrent_dev_spin_up > MAX_CONCURRENT_DEVICE_SPIN_UP_COUNT) + return -EINVAL; - for (index = 0; index < SCI_MAX_PHYS; index++) { - if ((scic_parms->sds1.phys[index].sas_address.high == 0) && - (scic_parms->sds1.phys[index].sas_address.low == 0)) - return SCI_FAILURE_INVALID_PARAMETER_VALUE; - } + return 0; +} - if (scic_parms->sds1.controller.mode_type == - SCIC_PORT_AUTOMATIC_CONFIGURATION_MODE) { - for (index = 0; index < SCI_MAX_PHYS; index++) { - if (scic_parms->sds1.ports[index].phy_mask != 0) - return SCI_FAILURE_INVALID_PARAMETER_VALUE; - } - } else if (scic_parms->sds1.controller.mode_type == - SCIC_PORT_MANUAL_CONFIGURATION_MODE) { - for (index = 0; index < SCI_MAX_PHYS; index++) - combined_phy_mask |= scic_parms->sds1.ports[index].phy_mask; +enum sci_status scic_oem_parameters_set(struct scic_sds_controller *scic, + union scic_oem_parameters *scic_parms) +{ + u32 state = scic->state_machine.current_state_id; - if (combined_phy_mask == 0) - return SCI_FAILURE_INVALID_PARAMETER_VALUE; - } else - return SCI_FAILURE_INVALID_PARAMETER_VALUE; + if (state == SCI_BASE_CONTROLLER_STATE_RESET || + state == SCI_BASE_CONTROLLER_STATE_INITIALIZING || + state == SCI_BASE_CONTROLLER_STATE_INITIALIZED) { - if (scic_parms->sds1.controller.max_concurrent_dev_spin_up > - MAX_CONCURRENT_DEVICE_SPIN_UP_COUNT) + if (scic_oem_parameters_validate(&scic_parms->sds1)) return SCI_FAILURE_INVALID_PARAMETER_VALUE; - scic->oem_parameters.sds1 = scic_parms->sds1; return SCI_SUCCESS; diff --git a/drivers/scsi/isci/init.c b/drivers/scsi/isci/init.c index 5da9a6925cd..015ce94453b 100644 --- a/drivers/scsi/isci/init.c +++ b/drivers/scsi/isci/init.c @@ -489,6 +489,16 @@ static int __devinit isci_pci_probe(struct pci_dev *pdev, const struct pci_devic else orom = isci_request_oprom(pdev); + for (i = 0; orom && i < ARRAY_SIZE(orom->ctrl); i++) { + if (scic_oem_parameters_validate(&orom->ctrl[i])) { + dev_warn(&pdev->dev, + "[%d]: invalid oem parameters detected, falling back to firmware\n", i); + devm_kfree(&pdev->dev, orom); + orom = NULL; + break; + } + } + if (!orom) { source = "(firmware)"; orom = isci_request_firmware(pdev, fw); -- 2.39.5