[PATCH v2] fsi: occ: Improve response status checking

[Eddie James]

If the driver sequence number coincidentally equals the previous
command response sequence number, the driver may proceed with
fetching the entire buffer before the OCC has processed the current
command. To be sure the correct response is obtained, check the
command type and also retry if any of the response parameters have
changed when the rest of the buffer is fetched. Also initialize the
driver with a random sequence number in order to reduce the chances
of this happening.

Signed-off-by: Eddie James <eajames at linux.ibm.com>
---
Changes since v1:
 - Refactor the retry into one loop
 - Add a comment about the pseudo-random number

 drivers/fsi/fsi-occ.c | 87 ++++++++++++++++++++++++++++---------------
 1 file changed, 56 insertions(+), 31 deletions(-)

diff --git a/drivers/fsi/fsi-occ.c b/drivers/fsi/fsi-occ.c
index 7eaab1be0aa4..c9cc75fbdfb9 100644
--- a/drivers/fsi/fsi-occ.c
+++ b/drivers/fsi/fsi-occ.c
@@ -451,6 +451,14 @@ static int occ_trigger_attn(struct occ *occ)
 	return rc;
 }
 
+static bool fsi_occ_response_not_ready(struct occ_response *resp, u8 seq_no,
+				       u8 cmd_type)
+{
+	return resp->return_status == OCC_RESP_CMD_IN_PRG ||
+		resp->return_status == OCC_RESP_CRIT_INIT ||
+		resp->seq_no != seq_no || resp->cmd_type != cmd_type;
+}
+
 int fsi_occ_submit(struct device *dev, const void *request, size_t req_len,
 		   void *response, size_t *resp_len)
 {
@@ -461,10 +469,11 @@ int fsi_occ_submit(struct device *dev, const void *request, size_t req_len,
 	struct occ_response *resp = response;
 	size_t user_resp_len = *resp_len;
 	u8 seq_no;
+	u8 cmd_type;
 	u16 checksum = 0;
 	u16 resp_data_length;
 	const u8 *byte_request = (const u8 *)request;
-	unsigned long start;
+	unsigned long end;
 	int rc;
 	size_t i;
 
@@ -478,6 +487,8 @@ int fsi_occ_submit(struct device *dev, const void *request, size_t req_len,
 		return -EINVAL;
 	}
 
+	cmd_type = byte_request[1];
+
 	/* Checksum the request, ignoring first byte (sequence number). */
 	for (i = 1; i < req_len - 2; ++i)
 		checksum += byte_request[i];
@@ -509,51 +520,61 @@ int fsi_occ_submit(struct device *dev, const void *request, size_t req_len,
 	if (rc)
 		goto done;
 
-	/* Read occ response header */
-	start = jiffies;
-	do {
+	end = jiffies + timeout;
+	while (true) {
+		/* Read occ response header */
 		rc = occ_getsram(occ, 0, resp, 8);
 		if (rc)
 			goto done;
 
-		if (resp->return_status == OCC_RESP_CMD_IN_PRG ||
-		    resp->return_status == OCC_RESP_CRIT_INIT ||
-		    resp->seq_no != seq_no) {
-			rc = -ETIMEDOUT;
-
-			if (time_after(jiffies, start + timeout)) {
-				dev_err(occ->dev, "resp timeout status=%02x "
-					"resp seq_no=%d our seq_no=%d\n",
+		if (fsi_occ_response_not_ready(resp, seq_no, cmd_type)) {
+			if (time_after(jiffies, end)) {
+				dev_err(occ->dev,
+					"resp timeout status=%02x seq=%d cmd=%d, our seq=%d cmd=%d\n",
 					resp->return_status, resp->seq_no,
-					seq_no);
+					resp->cmd_type, seq_no, cmd_type);
+				rc = -ETIMEDOUT;
 				goto done;
 			}
 
 			set_current_state(TASK_UNINTERRUPTIBLE);
 			schedule_timeout(wait_time);
-		}
-	} while (rc);
-
-	/* Extract size of response data */
-	resp_data_length = get_unaligned_be16(&resp->data_length);
+		} else {
+			/* Extract size of response data */
+			resp_data_length =
+				get_unaligned_be16(&resp->data_length);
+
+			/*
+			 * Message size is data length + 5 bytes header + 2
+			 * bytes checksum
+			 */
+			if ((resp_data_length + 7) > user_resp_len) {
+				rc = -EMSGSIZE;
+				goto done;
+			}
 
-	/* Message size is data length + 5 bytes header + 2 bytes checksum */
-	if ((resp_data_length + 7) > user_resp_len) {
-		rc = -EMSGSIZE;
-		goto done;
+			/*
+			 * Get the entire response including the header again,
+			 * in case it changed
+			 */
+			if (resp_data_length > 1) {
+				rc = occ_getsram(occ, 0, resp,
+						 resp_data_length + 7);
+				if (rc)
+					goto done;
+
+				if (!fsi_occ_response_not_ready(resp, seq_no,
+								cmd_type))
+					break;
+			} else {
+				break;
+			}
+		}
 	}
 
 	dev_dbg(dev, "resp_status=%02x resp_data_len=%d\n",
 		resp->return_status, resp_data_length);
 
-	/* Grab the rest */
-	if (resp_data_length > 1) {
-		/* already got 3 bytes resp, also need 2 bytes checksum */
-		rc = occ_getsram(occ, 8, &resp->data[3], resp_data_length - 1);
-		if (rc)
-			goto done;
-	}
-
 	occ->client_response_size = resp_data_length + 7;
 	rc = occ_verify_checksum(occ, resp, resp_data_length);
 
@@ -598,7 +619,11 @@ static int occ_probe(struct platform_device *pdev)
 	occ->version = (uintptr_t)of_device_get_match_data(dev);
 	occ->dev = dev;
 	occ->sbefifo = dev->parent;
-	occ->sequence_number = 1;
+	/*
+	 * Quickly derive a pseudo-random number from jiffies so that
+	 * re-probing the driver doesn't accidentally overlap sequence numbers.
+	 */
+	occ->sequence_number = (u8)((jiffies % 0xff) + 1);
 	mutex_init(&occ->occ_lock);
 
 	if (dev->of_node) {
-- 
2.27.0