[PATCH v2 1/3] fsi: occ: Force sequence numbering per OCC

Eddie James eajames at linux.ibm.com
Thu Jul 22 05:02:29 AEST 2021


Set and increment the sequence number during the submit operation.
This prevents sequence number conflicts between different users of
the interface. A sequence number conflict may result in a user
getting an OCC response meant for a different command. Since the
sequence number is now modified, the checksum must be calculated and
set before submitting the command.

Signed-off-by: Eddie James <eajames at linux.ibm.com>
Reviewed-by: Joel Stanley <joel at jms.id.au>
---
 drivers/fsi/fsi-occ.c | 54 +++++++++++++++++++++++++++++--------------
 1 file changed, 37 insertions(+), 17 deletions(-)

diff --git a/drivers/fsi/fsi-occ.c b/drivers/fsi/fsi-occ.c
index b223f0ef337b..ecf738411fe2 100644
--- a/drivers/fsi/fsi-occ.c
+++ b/drivers/fsi/fsi-occ.c
@@ -50,6 +50,7 @@ struct occ {
 	struct device *sbefifo;
 	char name[32];
 	int idx;
+	u8 sequence_number;
 	enum versions version;
 	struct miscdevice mdev;
 	struct mutex occ_lock;
@@ -141,8 +142,7 @@ static ssize_t occ_write(struct file *file, const char __user *buf,
 {
 	struct occ_client *client = file->private_data;
 	size_t rlen, data_length;
-	u16 checksum = 0;
-	ssize_t rc, i;
+	ssize_t rc;
 	u8 *cmd;
 
 	if (!client)
@@ -156,9 +156,6 @@ static ssize_t occ_write(struct file *file, const char __user *buf,
 	/* Construct the command */
 	cmd = client->buffer;
 
-	/* Sequence number (we could increment and compare with response) */
-	cmd[0] = 1;
-
 	/*
 	 * Copy the user command (assume user data follows the occ command
 	 * format)
@@ -178,14 +175,7 @@ static ssize_t occ_write(struct file *file, const char __user *buf,
 		goto done;
 	}
 
-	/* Calculate checksum */
-	for (i = 0; i < data_length + 4; ++i)
-		checksum += cmd[i];
-
-	cmd[data_length + 4] = checksum >> 8;
-	cmd[data_length + 5] = checksum & 0xFF;
-
-	/* Submit command */
+	/* Submit command; 4 bytes before the data and 2 bytes after */
 	rlen = PAGE_SIZE;
 	rc = fsi_occ_submit(client->occ->dev, cmd, data_length + 6, cmd,
 			    &rlen);
@@ -314,11 +304,13 @@ static int occ_getsram(struct occ *occ, u32 offset, void *data, ssize_t len)
 	return rc;
 }
 
-static int occ_putsram(struct occ *occ, const void *data, ssize_t len)
+static int occ_putsram(struct occ *occ, const void *data, ssize_t len,
+		       u8 seq_no, u16 checksum)
 {
 	size_t cmd_len, buf_len, resp_len, resp_data_len;
 	u32 data_len = ((len + 7) / 8) * 8;	/* must be multiples of 8 B */
 	__be32 *buf;
+	u8 *byte_buf;
 	int idx = 0, rc;
 
 	cmd_len = (occ->version == occ_p10) ? 6 : 5;
@@ -358,6 +350,15 @@ static int occ_putsram(struct occ *occ, const void *data, ssize_t len)
 	buf[4 + idx] = cpu_to_be32(data_len);
 	memcpy(&buf[5 + idx], data, len);
 
+	byte_buf = (u8 *)&buf[5 + idx];
+	/*
+	 * Overwrite the first byte with our sequence number and the last two
+	 * bytes with the checksum.
+	 */
+	byte_buf[0] = seq_no;
+	byte_buf[len - 2] = checksum >> 8;
+	byte_buf[len - 1] = checksum & 0xff;
+
 	rc = sbefifo_submit(occ->sbefifo, buf, cmd_len, buf, &resp_len);
 	if (rc)
 		goto free;
@@ -467,9 +468,12 @@ int fsi_occ_submit(struct device *dev, const void *request, size_t req_len,
 	struct occ *occ = dev_get_drvdata(dev);
 	struct occ_response *resp = response;
 	u8 seq_no;
+	u16 checksum = 0;
 	u16 resp_data_length;
+	const u8 *byte_request = (const u8 *)request;
 	unsigned long start;
 	int rc;
+	size_t i;
 
 	if (!occ)
 		return -ENODEV;
@@ -479,11 +483,26 @@ int fsi_occ_submit(struct device *dev, const void *request, size_t req_len,
 		return -EINVAL;
 	}
 
+	/* Checksum the request, ignoring first byte (sequence number). */
+	for (i = 1; i < req_len - 2; ++i)
+		checksum += byte_request[i];
+
 	mutex_lock(&occ->occ_lock);
 
-	/* Extract the seq_no from the command (first byte) */
-	seq_no = *(const u8 *)request;
-	rc = occ_putsram(occ, request, req_len);
+	/*
+	 * Get a sequence number and update the counter. Avoid a sequence
+	 * number of 0 which would pass the response check below even if the
+	 * OCC response is uninitialized. Any sequence number the user is
+	 * trying to send is overwritten since this function is the only common
+	 * interface to the OCC and therefore the only place we can guarantee
+	 * unique sequence numbers.
+	 */
+	seq_no = occ->sequence_number++;
+	if (!occ->sequence_number)
+		occ->sequence_number = 1;
+	checksum += seq_no;
+
+	rc = occ_putsram(occ, request, req_len, seq_no, checksum);
 	if (rc)
 		goto done;
 
@@ -574,6 +593,7 @@ 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;
 	mutex_init(&occ->occ_lock);
 
 	if (dev->of_node) {
-- 
2.27.0



More information about the openbmc mailing list