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>
git-mirror · Jul 3, 2011 · 2d70de5 · 2d70de5
1 parent f22be5d
commit 2d70de5
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 38 deletions.
diff --git 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
@@ -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
@@ -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);