From: Eddie James <eajames@linux.ibm.com>
To: linux-kernel@vger.kernel.org
Cc: linux-hwmon@vger.kernel.org, linux-fsi@lists.ozlabs.org,
linux@roeck-us.net, jdelvare@suse.com, jk@ozlabs.org,
joel@jms.id.au, alistair@popple.id.au, openbmc@lists.ozlabs.org,
Eddie James <eajames@linux.ibm.com>
Subject: [PATCH 1/3] fsi: occ: Force sequence numbering per OCC
Date: Fri, 16 Jul 2021 10:18:48 -0500 [thread overview]
Message-ID: <20210716151850.28973-2-eajames@linux.ibm.com> (raw)
In-Reply-To: <20210716151850.28973-1-eajames@linux.ibm.com>
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@linux.ibm.com>
---
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
WARNING: multiple messages have this Message-ID (diff)
From: Eddie James <eajames@linux.ibm.com>
To: linux-kernel@vger.kernel.org
Cc: linux-hwmon@vger.kernel.org, jdelvare@suse.com,
openbmc@lists.ozlabs.org, Eddie James <eajames@linux.ibm.com>,
linux-fsi@lists.ozlabs.org, linux@roeck-us.net
Subject: [PATCH 1/3] fsi: occ: Force sequence numbering per OCC
Date: Fri, 16 Jul 2021 10:18:48 -0500 [thread overview]
Message-ID: <20210716151850.28973-2-eajames@linux.ibm.com> (raw)
In-Reply-To: <20210716151850.28973-1-eajames@linux.ibm.com>
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@linux.ibm.com>
---
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
next prev parent reply other threads:[~2021-07-16 15:19 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-07-16 15:18 [PATCH 0/3] OCC: fsi and hwmon: Set sequence number in submit interface Eddie James
2021-07-16 15:18 ` Eddie James
2021-07-16 15:18 ` Eddie James [this message]
2021-07-16 15:18 ` [PATCH 1/3] fsi: occ: Force sequence numbering per OCC Eddie James
2021-07-21 2:37 ` Joel Stanley
2021-07-21 2:37 ` Joel Stanley
2021-07-21 13:27 ` Eddie James
2021-07-16 15:18 ` [PATCH 2/3] hwmon: (occ) Remove sequence numbering and checksum calculation Eddie James
2021-07-16 15:18 ` Eddie James
2021-07-17 14:04 ` Guenter Roeck
2021-07-17 14:04 ` Guenter Roeck
2021-07-18 20:08 ` kernel test robot
2021-07-18 20:08 ` kernel test robot
2021-07-18 20:08 ` kernel test robot
2021-07-18 20:26 ` kernel test robot
2021-07-18 20:26 ` kernel test robot
2021-07-18 20:26 ` kernel test robot
2021-07-21 2:43 ` Joel Stanley
2021-07-21 2:43 ` Joel Stanley
2021-07-21 13:41 ` Eddie James
2021-07-16 15:18 ` [PATCH 3/3] fsi: occ: Add dynamic debug to dump command and response Eddie James
2021-07-16 15:18 ` Eddie James
2021-07-19 0:26 ` kernel test robot
2021-07-19 0:26 ` kernel test robot
2021-07-19 0:26 ` kernel test robot
2021-07-21 23:28 ` Jeremy Kerr
2021-07-21 23:28 ` Jeremy Kerr
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210716151850.28973-2-eajames@linux.ibm.com \
--to=eajames@linux.ibm.com \
--cc=alistair@popple.id.au \
--cc=jdelvare@suse.com \
--cc=jk@ozlabs.org \
--cc=joel@jms.id.au \
--cc=linux-fsi@lists.ozlabs.org \
--cc=linux-hwmon@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@roeck-us.net \
--cc=openbmc@lists.ozlabs.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.