* [PATCH linux dev-5.15 1/2] hwmon: (occ) Prevent power cap command overwriting poll response
@ 2022-07-20 20:15 Eddie James
2022-07-20 20:15 ` [PATCH linux dev-5.15 2/2] hwmon (occ): Fix response length in checksum retry Eddie James
2022-08-04 2:55 ` [PATCH linux dev-5.15 1/2] hwmon: (occ) Prevent power cap command overwriting poll response Joel Stanley
0 siblings, 2 replies; 3+ messages in thread
From: Eddie James @ 2022-07-20 20:15 UTC (permalink / raw)
To: openbmc; +Cc: joel
Currently, the response to the power cap command overwrites the
first eight bytes of the poll response, since the commands use
the same buffer. This means that user's get the wrong data between
the time of sending the power cap and the next poll response update.
Fix this by specifying a different buffer for the power cap command
response.
Fixes: 5b5513b88002 ("hwmon: Add On-Chip Controller (OCC) hwmon driver")
Signed-off-by: Eddie James <eajames@linux.ibm.com>
Link: https://lore.kernel.org/r/20220628203029.51747-1-eajames@linux.ibm.com
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
drivers/hwmon/occ/common.c | 5 +++--
drivers/hwmon/occ/common.h | 3 ++-
drivers/hwmon/occ/p8_i2c.c | 13 +++++++------
drivers/hwmon/occ/p9_sbe.c | 10 ++++------
4 files changed, 16 insertions(+), 15 deletions(-)
diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c
index f00cd59f1d19..1757f3ab842e 100644
--- a/drivers/hwmon/occ/common.c
+++ b/drivers/hwmon/occ/common.c
@@ -145,7 +145,7 @@ static int occ_poll(struct occ *occ)
cmd[6] = 0; /* checksum lsb */
/* mutex should already be locked if necessary */
- rc = occ->send_cmd(occ, cmd, sizeof(cmd));
+ rc = occ->send_cmd(occ, cmd, sizeof(cmd), &occ->resp, sizeof(occ->resp));
if (rc) {
occ->last_error = rc;
if (occ->error_count++ > OCC_ERROR_COUNT_THRESHOLD)
@@ -182,6 +182,7 @@ static int occ_set_user_power_cap(struct occ *occ, u16 user_power_cap)
{
int rc;
u8 cmd[8];
+ u8 resp[8];
__be16 user_power_cap_be = cpu_to_be16(user_power_cap);
cmd[0] = 0; /* sequence number */
@@ -198,7 +199,7 @@ static int occ_set_user_power_cap(struct occ *occ, u16 user_power_cap)
if (rc)
return rc;
- rc = occ->send_cmd(occ, cmd, sizeof(cmd));
+ rc = occ->send_cmd(occ, cmd, sizeof(cmd), resp, sizeof(resp));
mutex_unlock(&occ->lock);
diff --git a/drivers/hwmon/occ/common.h b/drivers/hwmon/occ/common.h
index 2dd4a4d240c0..726943af9a07 100644
--- a/drivers/hwmon/occ/common.h
+++ b/drivers/hwmon/occ/common.h
@@ -96,7 +96,8 @@ struct occ {
int powr_sample_time_us; /* average power sample time */
u8 poll_cmd_data; /* to perform OCC poll command */
- int (*send_cmd)(struct occ *occ, u8 *cmd, size_t len);
+ int (*send_cmd)(struct occ *occ, u8 *cmd, size_t len, void *resp,
+ size_t resp_len);
unsigned long next_update;
struct mutex lock; /* lock OCC access */
diff --git a/drivers/hwmon/occ/p8_i2c.c b/drivers/hwmon/occ/p8_i2c.c
index 9e61e1fb5142..c35c07964d85 100644
--- a/drivers/hwmon/occ/p8_i2c.c
+++ b/drivers/hwmon/occ/p8_i2c.c
@@ -111,7 +111,8 @@ static int p8_i2c_occ_putscom_be(struct i2c_client *client, u32 address,
be32_to_cpu(data1));
}
-static int p8_i2c_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len)
+static int p8_i2c_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len,
+ void *resp, size_t resp_len)
{
int i, rc;
unsigned long start;
@@ -120,7 +121,7 @@ static int p8_i2c_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len)
const long wait_time = msecs_to_jiffies(OCC_CMD_IN_PRG_WAIT_MS);
struct p8_i2c_occ *ctx = to_p8_i2c_occ(occ);
struct i2c_client *client = ctx->client;
- struct occ_response *resp = &occ->resp;
+ struct occ_response *or = (struct occ_response *)resp;
start = jiffies;
@@ -151,7 +152,7 @@ static int p8_i2c_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len)
return rc;
/* wait for OCC */
- if (resp->return_status == OCC_RESP_CMD_IN_PRG) {
+ if (or->return_status == OCC_RESP_CMD_IN_PRG) {
rc = -EALREADY;
if (time_after(jiffies, start + timeout))
@@ -163,7 +164,7 @@ static int p8_i2c_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len)
} while (rc);
/* check the OCC response */
- switch (resp->return_status) {
+ switch (or->return_status) {
case OCC_RESP_CMD_IN_PRG:
rc = -ETIMEDOUT;
break;
@@ -192,8 +193,8 @@ static int p8_i2c_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len)
if (rc < 0)
return rc;
- data_length = get_unaligned_be16(&resp->data_length);
- if (data_length > OCC_RESP_DATA_BYTES)
+ data_length = get_unaligned_be16(&or->data_length);
+ if ((data_length + 7) > resp_len)
return -EMSGSIZE;
/* fetch the rest of the response data */
diff --git a/drivers/hwmon/occ/p9_sbe.c b/drivers/hwmon/occ/p9_sbe.c
index d34d6a007e8d..ad2fcc31db78 100644
--- a/drivers/hwmon/occ/p9_sbe.c
+++ b/drivers/hwmon/occ/p9_sbe.c
@@ -79,13 +79,11 @@ static bool p9_sbe_occ_save_ffdc(struct p9_sbe_occ *ctx, const void *resp,
return notify;
}
-static int p9_sbe_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len)
+static int p9_sbe_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len,
+ void *resp, size_t resp_len)
{
- struct occ_response *resp = &occ->resp;
struct p9_sbe_occ *ctx = to_p9_sbe_occ(occ);
- size_t resp_len = sizeof(*resp);
- int i;
- int rc;
+ int rc, i;
for (i = 0; i < OCC_CHECKSUM_RETRIES; ++i) {
rc = fsi_occ_submit(ctx->sbe, cmd, len, resp, &resp_len);
@@ -102,7 +100,7 @@ static int p9_sbe_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len)
return rc;
}
- switch (resp->return_status) {
+ switch (((struct occ_response *)resp)->return_status) {
case OCC_RESP_CMD_IN_PRG:
rc = -ETIMEDOUT;
break;
--
2.31.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH linux dev-5.15 2/2] hwmon (occ): Fix response length in checksum retry
2022-07-20 20:15 [PATCH linux dev-5.15 1/2] hwmon: (occ) Prevent power cap command overwriting poll response Eddie James
@ 2022-07-20 20:15 ` Eddie James
2022-08-04 2:55 ` [PATCH linux dev-5.15 1/2] hwmon: (occ) Prevent power cap command overwriting poll response Joel Stanley
1 sibling, 0 replies; 3+ messages in thread
From: Eddie James @ 2022-07-20 20:15 UTC (permalink / raw)
To: openbmc; +Cc: joel
Retrying for checksum failure doesn't work since the response length
gets zeroed out in the submit function. Fix this by resetting the
response length to its original value before the retry.
Fixes: fe6200e9c8be ("hwmon (occ): Retry for checksum failure")
Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
drivers/hwmon/occ/p9_sbe.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/hwmon/occ/p9_sbe.c b/drivers/hwmon/occ/p9_sbe.c
index ad2fcc31db78..037046c3921d 100644
--- a/drivers/hwmon/occ/p9_sbe.c
+++ b/drivers/hwmon/occ/p9_sbe.c
@@ -82,6 +82,7 @@ static bool p9_sbe_occ_save_ffdc(struct p9_sbe_occ *ctx, const void *resp,
static int p9_sbe_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len,
void *resp, size_t resp_len)
{
+ size_t original_resp_len = resp_len;
struct p9_sbe_occ *ctx = to_p9_sbe_occ(occ);
int rc, i;
@@ -98,6 +99,7 @@ static int p9_sbe_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len,
}
if (rc != -EBADE)
return rc;
+ resp_len = original_resp_len;
}
switch (((struct occ_response *)resp)->return_status) {
--
2.31.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH linux dev-5.15 1/2] hwmon: (occ) Prevent power cap command overwriting poll response
2022-07-20 20:15 [PATCH linux dev-5.15 1/2] hwmon: (occ) Prevent power cap command overwriting poll response Eddie James
2022-07-20 20:15 ` [PATCH linux dev-5.15 2/2] hwmon (occ): Fix response length in checksum retry Eddie James
@ 2022-08-04 2:55 ` Joel Stanley
1 sibling, 0 replies; 3+ messages in thread
From: Joel Stanley @ 2022-08-04 2:55 UTC (permalink / raw)
To: Eddie James; +Cc: openbmc
On Wed, 20 Jul 2022 at 20:16, Eddie James <eajames@linux.ibm.com> wrote:
>
> Currently, the response to the power cap command overwrites the
> first eight bytes of the poll response, since the commands use
> the same buffer. This means that user's get the wrong data between
> the time of sending the power cap and the next poll response update.
> Fix this by specifying a different buffer for the power cap command
> response.
>
> Fixes: 5b5513b88002 ("hwmon: Add On-Chip Controller (OCC) hwmon driver")
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> Link: https://lore.kernel.org/r/20220628203029.51747-1-eajames@linux.ibm.com
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
I've applied this and the second patch.
> ---
> drivers/hwmon/occ/common.c | 5 +++--
> drivers/hwmon/occ/common.h | 3 ++-
> drivers/hwmon/occ/p8_i2c.c | 13 +++++++------
> drivers/hwmon/occ/p9_sbe.c | 10 ++++------
> 4 files changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/hwmon/occ/common.c b/drivers/hwmon/occ/common.c
> index f00cd59f1d19..1757f3ab842e 100644
> --- a/drivers/hwmon/occ/common.c
> +++ b/drivers/hwmon/occ/common.c
> @@ -145,7 +145,7 @@ static int occ_poll(struct occ *occ)
> cmd[6] = 0; /* checksum lsb */
>
> /* mutex should already be locked if necessary */
> - rc = occ->send_cmd(occ, cmd, sizeof(cmd));
> + rc = occ->send_cmd(occ, cmd, sizeof(cmd), &occ->resp, sizeof(occ->resp));
> if (rc) {
> occ->last_error = rc;
> if (occ->error_count++ > OCC_ERROR_COUNT_THRESHOLD)
> @@ -182,6 +182,7 @@ static int occ_set_user_power_cap(struct occ *occ, u16 user_power_cap)
> {
> int rc;
> u8 cmd[8];
> + u8 resp[8];
> __be16 user_power_cap_be = cpu_to_be16(user_power_cap);
>
> cmd[0] = 0; /* sequence number */
> @@ -198,7 +199,7 @@ static int occ_set_user_power_cap(struct occ *occ, u16 user_power_cap)
> if (rc)
> return rc;
>
> - rc = occ->send_cmd(occ, cmd, sizeof(cmd));
> + rc = occ->send_cmd(occ, cmd, sizeof(cmd), resp, sizeof(resp));
>
> mutex_unlock(&occ->lock);
>
> diff --git a/drivers/hwmon/occ/common.h b/drivers/hwmon/occ/common.h
> index 2dd4a4d240c0..726943af9a07 100644
> --- a/drivers/hwmon/occ/common.h
> +++ b/drivers/hwmon/occ/common.h
> @@ -96,7 +96,8 @@ struct occ {
>
> int powr_sample_time_us; /* average power sample time */
> u8 poll_cmd_data; /* to perform OCC poll command */
> - int (*send_cmd)(struct occ *occ, u8 *cmd, size_t len);
> + int (*send_cmd)(struct occ *occ, u8 *cmd, size_t len, void *resp,
> + size_t resp_len);
>
> unsigned long next_update;
> struct mutex lock; /* lock OCC access */
> diff --git a/drivers/hwmon/occ/p8_i2c.c b/drivers/hwmon/occ/p8_i2c.c
> index 9e61e1fb5142..c35c07964d85 100644
> --- a/drivers/hwmon/occ/p8_i2c.c
> +++ b/drivers/hwmon/occ/p8_i2c.c
> @@ -111,7 +111,8 @@ static int p8_i2c_occ_putscom_be(struct i2c_client *client, u32 address,
> be32_to_cpu(data1));
> }
>
> -static int p8_i2c_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len)
> +static int p8_i2c_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len,
> + void *resp, size_t resp_len)
> {
> int i, rc;
> unsigned long start;
> @@ -120,7 +121,7 @@ static int p8_i2c_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len)
> const long wait_time = msecs_to_jiffies(OCC_CMD_IN_PRG_WAIT_MS);
> struct p8_i2c_occ *ctx = to_p8_i2c_occ(occ);
> struct i2c_client *client = ctx->client;
> - struct occ_response *resp = &occ->resp;
> + struct occ_response *or = (struct occ_response *)resp;
>
> start = jiffies;
>
> @@ -151,7 +152,7 @@ static int p8_i2c_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len)
> return rc;
>
> /* wait for OCC */
> - if (resp->return_status == OCC_RESP_CMD_IN_PRG) {
> + if (or->return_status == OCC_RESP_CMD_IN_PRG) {
> rc = -EALREADY;
>
> if (time_after(jiffies, start + timeout))
> @@ -163,7 +164,7 @@ static int p8_i2c_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len)
> } while (rc);
>
> /* check the OCC response */
> - switch (resp->return_status) {
> + switch (or->return_status) {
> case OCC_RESP_CMD_IN_PRG:
> rc = -ETIMEDOUT;
> break;
> @@ -192,8 +193,8 @@ static int p8_i2c_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len)
> if (rc < 0)
> return rc;
>
> - data_length = get_unaligned_be16(&resp->data_length);
> - if (data_length > OCC_RESP_DATA_BYTES)
> + data_length = get_unaligned_be16(&or->data_length);
> + if ((data_length + 7) > resp_len)
> return -EMSGSIZE;
>
> /* fetch the rest of the response data */
> diff --git a/drivers/hwmon/occ/p9_sbe.c b/drivers/hwmon/occ/p9_sbe.c
> index d34d6a007e8d..ad2fcc31db78 100644
> --- a/drivers/hwmon/occ/p9_sbe.c
> +++ b/drivers/hwmon/occ/p9_sbe.c
> @@ -79,13 +79,11 @@ static bool p9_sbe_occ_save_ffdc(struct p9_sbe_occ *ctx, const void *resp,
> return notify;
> }
>
> -static int p9_sbe_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len)
> +static int p9_sbe_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len,
> + void *resp, size_t resp_len)
> {
> - struct occ_response *resp = &occ->resp;
> struct p9_sbe_occ *ctx = to_p9_sbe_occ(occ);
> - size_t resp_len = sizeof(*resp);
> - int i;
> - int rc;
> + int rc, i;
>
> for (i = 0; i < OCC_CHECKSUM_RETRIES; ++i) {
> rc = fsi_occ_submit(ctx->sbe, cmd, len, resp, &resp_len);
> @@ -102,7 +100,7 @@ static int p9_sbe_occ_send_cmd(struct occ *occ, u8 *cmd, size_t len)
> return rc;
> }
>
> - switch (resp->return_status) {
> + switch (((struct occ_response *)resp)->return_status) {
> case OCC_RESP_CMD_IN_PRG:
> rc = -ETIMEDOUT;
> break;
> --
> 2.31.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-08-04 2:56 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-20 20:15 [PATCH linux dev-5.15 1/2] hwmon: (occ) Prevent power cap command overwriting poll response Eddie James
2022-07-20 20:15 ` [PATCH linux dev-5.15 2/2] hwmon (occ): Fix response length in checksum retry Eddie James
2022-08-04 2:55 ` [PATCH linux dev-5.15 1/2] hwmon: (occ) Prevent power cap command overwriting poll response Joel Stanley
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).