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 <yinghai.lu@oracle.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>

diff --git a/drivers/scsi/isci/core/scic_config_parameters.h b/drivers/scsi/isci/core/scic_config_parameters.h
index cb6abcc7e7cdefc09183f0aa825f2865ae1a7bc6..716abfcd0c23cf8d23b6b84ad1b5bbee8e08679a 100644 (file)
--- 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 8194618b76c6f28baad81d19989cc6d521609e5a..9bb78a2e6ff79047bb7b3553a0fab845808a5513 100644 (file)
--- 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 5da9a6925cd7c2b2c932c7c92d55e8034d54437d..015ce94453bb51db73041999b73f9deaf9451945 100644 (file)
--- 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);