From: Rob Barnes <robbarnes@google.com>
To: bleung@chromium.org, tzungbi@kernel.org
Cc: tomhughes@chromium.org, chrome-platform@lists.linux.dev,
linux-kernel@vger.kernel.org, Rob Barnes <robbarnes@google.com>
Subject: [PATCH v4] platform/chrome: cros_ec_proto: Fix deferred response
Date: Thu, 27 Aug 2026 12:05:11 -0600 [thread overview]
Message-ID: <20260827180511.2825660-1-robbarnes@google.com> (raw)
When cros_ec_wait_until_complete() succeeds after an
EC_RES_IN_PROGRESS status, it previously returned the 4-byte
transfer size of EC_CMD_GET_COMMS_STATUS. For 0-byte payload
commands (such as EC_CMD_FLASH_ERASE), userspace received 4
bytes instead of 0, causing response size validation errors.
For commands expecting non-zero response payloads, the kernel
exited without issuing EC_CMD_RESEND_RESPONSE, leaving response
buffers uninitialized.
Refactor cros_ec_wait_until_complete() to pass orig_msg, issue
EC_CMD_RESEND_RESPONSE into orig_msg->data when insize > 0, and
return 0 for 0-byte response commands.
Signed-off-by: Rob Barnes <robbarnes@google.com>
---
v4:
- Rebase against upstream chrome-platform/for-next branch (Tzung-Bi).
- Send patch using git-send-email via Google Mail Relay to prevent MTA line-wrapping (Tzung-Bi).
v3:
- Rebase against upstream chrome-platform tree (Tzung-Bi).
v2:
- Drop redundant commit description sentences (Tzung-Bi).
- Align function signature parameters to open parenthesis (Tzung-Bi).
- Check orig_msg->insize == 0 and add comments explaining the logic (Tzung-Bi / Rob).
- Use u32 instead of uint32_t (Tzung-Bi).
- Rename 0bytes test to 0byte (Tzung-Bi).
- Check header immutability for insize in test cases (Tzung-Bi).
---
drivers/platform/chrome/cros_ec_proto.c | 32 +++-
drivers/platform/chrome/cros_ec_proto_test.c | 158 ++++++++++++++++++-
2 files changed, 183 insertions(+), 7 deletions(-)
diff --git a/drivers/platform/chrome/cros_ec_proto.c b/drivers/platform/chrome/cros_ec_proto.c
index 1d8d9168ec1a..6d7c57e381e5 100644
--- a/drivers/platform/chrome/cros_ec_proto.c
+++ b/drivers/platform/chrome/cros_ec_proto.c
@@ -138,7 +138,8 @@ static int cros_ec_xfer_command(struct cros_ec_device *ec_dev, struct cros_ec_co
return ret;
}
-static int cros_ec_wait_until_complete(struct cros_ec_device *ec_dev, uint32_t *result)
+static int cros_ec_wait_until_complete(struct cros_ec_device *ec_dev,
+ struct cros_ec_command *orig_msg)
{
DEFINE_RAW_FLEX(struct cros_ec_command, msg, data,
sizeof(struct ec_response_get_comms_status));
@@ -161,7 +162,7 @@ static int cros_ec_wait_until_complete(struct cros_ec_device *ec_dev, uint32_t *
if (ret < 0)
return ret;
- *result = msg->result;
+ orig_msg->result = msg->result;
if (msg->result != EC_RES_SUCCESS)
return ret;
@@ -170,8 +171,31 @@ static int cros_ec_wait_until_complete(struct cros_ec_device *ec_dev, uint32_t *
break;
}
- if (!(status->flags & EC_COMMS_STATUS_PROCESSING))
+ if (!(status->flags & EC_COMMS_STATUS_PROCESSING)) {
+ u32 orig_cmd, orig_outsize, orig_version;
+
+ /* If no response payload is expected, return 0. */
+ if (orig_msg->insize == 0)
+ return 0;
+
+ /*
+ * Request the response using EC_CMD_RESEND_RESPONSE.
+ * Restore the original message fields so it appears
+ * to be a direct response to the original command.
+ */
+ orig_cmd = orig_msg->command;
+ orig_outsize = orig_msg->outsize;
+ orig_version = orig_msg->version;
+
+ orig_msg->command = EC_CMD_RESEND_RESPONSE;
+ orig_msg->outsize = 0;
+ orig_msg->version = 0;
+ ret = cros_ec_xfer_command(ec_dev, orig_msg);
+ orig_msg->command = orig_cmd;
+ orig_msg->outsize = orig_outsize;
+ orig_msg->version = orig_version;
return ret;
+ }
}
if (i >= EC_COMMAND_RETRIES)
@@ -185,7 +209,7 @@ static int cros_ec_send_command(struct cros_ec_device *ec_dev, struct cros_ec_co
int ret = cros_ec_xfer_command(ec_dev, msg);
if (msg->result == EC_RES_IN_PROGRESS)
- ret = cros_ec_wait_until_complete(ec_dev, &msg->result);
+ ret = cros_ec_wait_until_complete(ec_dev, msg);
return ret;
}
diff --git a/drivers/platform/chrome/cros_ec_proto_test.c b/drivers/platform/chrome/cros_ec_proto_test.c
index 3f281996a686..a7b1d64f78ce 100644
--- a/drivers/platform/chrome/cros_ec_proto_test.c
+++ b/drivers/platform/chrome/cros_ec_proto_test.c
@@ -1744,7 +1744,7 @@ static void cros_ec_proto_test_cmd_xfer_protocol_v2_no_op(struct kunit *test)
KUNIT_EXPECT_EQ(test, ret, -EIO);
}
-static void cros_ec_proto_test_cmd_xfer_in_progress_normal(struct kunit *test)
+static void cros_ec_proto_test_cmd_xfer_in_progress_payload_0byte(struct kunit *test)
{
struct cros_ec_proto_test_priv *priv = test->priv;
struct cros_ec_device *ec_dev = &priv->ec_dev;
@@ -1774,7 +1774,7 @@ static void cros_ec_proto_test_cmd_xfer_in_progress_normal(struct kunit *test)
}
ret = cros_ec_cmd_xfer(ec_dev, &msg);
- KUNIT_EXPECT_EQ(test, ret, sizeof(struct ec_response_get_comms_status));
+ KUNIT_EXPECT_EQ(test, ret, 0);
KUNIT_EXPECT_EQ(test, msg.result, EC_RES_SUCCESS);
@@ -1799,6 +1799,156 @@ static void cros_ec_proto_test_cmd_xfer_in_progress_normal(struct kunit *test)
KUNIT_EXPECT_EQ(test, cros_kunit_ec_pkt_xfer_mock_called, 2);
}
+static void cros_ec_proto_test_cmd_xfer_in_progress_payload_4bytes(struct kunit *test)
+{
+ struct cros_ec_proto_test_priv *priv = test->priv;
+ struct cros_ec_device *ec_dev = &priv->ec_dev;
+ struct ec_xfer_mock *mock;
+ u8 resp_data[4] = {0x11, 0x22, 0x33, 0x44};
+ struct {
+ struct cros_ec_command msg;
+ u8 data[4];
+ } buf;
+ struct cros_ec_command *msg = &buf.msg;
+ int ret;
+
+ memset(&buf, 0, sizeof(buf));
+ msg->version = 1;
+ msg->command = 0x1234;
+ msg->outsize = 2;
+ msg->insize = sizeof(buf.data);
+
+ ec_dev->pkt_xfer = cros_kunit_ec_pkt_xfer_mock;
+
+ /* For initial command returning EC_RES_IN_PROGRESS. */
+ {
+ mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_IN_PROGRESS, 0);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ }
+
+ /* For EC_CMD_GET_COMMS_STATUS status polling. */
+ {
+ struct ec_response_get_comms_status *data;
+
+ mock = cros_kunit_ec_xfer_mock_add(test, sizeof(*data));
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+
+ data = (struct ec_response_get_comms_status *)mock->o_data;
+ data->flags = 0;
+ }
+
+ /* For EC_CMD_RESEND_RESPONSE returning 4 bytes payload. */
+ {
+ mock = cros_kunit_ec_xfer_mock_add(test, sizeof(resp_data));
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ memcpy(mock->o_data, resp_data, sizeof(resp_data));
+ }
+
+ ret = cros_ec_cmd_xfer(ec_dev, msg);
+ KUNIT_EXPECT_EQ(test, ret, 4);
+ KUNIT_EXPECT_EQ(test, msg->result, EC_RES_SUCCESS);
+ KUNIT_EXPECT_EQ(test, msg->command, 0x1234);
+ KUNIT_EXPECT_EQ(test, msg->outsize, 2);
+ KUNIT_EXPECT_EQ(test, msg->insize, sizeof(buf.data));
+ KUNIT_EXPECT_EQ(test, msg->version, 1);
+ KUNIT_EXPECT_EQ(test, memcmp(msg->data, resp_data, sizeof(resp_data)), 0);
+
+ /* Verify mock sequence */
+ {
+ mock = cros_kunit_ec_xfer_mock_next();
+ KUNIT_EXPECT_PTR_NE(test, mock, NULL);
+
+ mock = cros_kunit_ec_xfer_mock_next();
+ KUNIT_EXPECT_PTR_NE(test, mock, NULL);
+ KUNIT_EXPECT_EQ(test, mock->msg.command, EC_CMD_GET_COMMS_STATUS);
+
+ mock = cros_kunit_ec_xfer_mock_next();
+ KUNIT_EXPECT_PTR_NE(test, mock, NULL);
+ KUNIT_EXPECT_EQ(test, mock->msg.command, EC_CMD_RESEND_RESPONSE);
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
+ KUNIT_EXPECT_EQ(test, mock->msg.version, 0);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize, 4);
+ }
+
+ KUNIT_EXPECT_EQ(test, cros_kunit_ec_pkt_xfer_mock_called, 3);
+}
+
+static void cros_ec_proto_test_cmd_xfer_in_progress_payload_gt4bytes(struct kunit *test)
+{
+ struct cros_ec_proto_test_priv *priv = test->priv;
+ struct cros_ec_device *ec_dev = &priv->ec_dev;
+ struct ec_xfer_mock *mock;
+ u8 resp_data[16];
+ struct {
+ struct cros_ec_command msg;
+ u8 data[16];
+ } buf;
+ struct cros_ec_command *msg = &buf.msg;
+ int ret, i;
+
+ for (i = 0; i < sizeof(resp_data); ++i)
+ resp_data[i] = (u8)(i + 1);
+
+ memset(&buf, 0, sizeof(buf));
+ msg->version = 0;
+ msg->command = 0x5678;
+ msg->insize = sizeof(buf.data);
+
+ ec_dev->pkt_xfer = cros_kunit_ec_pkt_xfer_mock;
+
+ /* For initial command returning EC_RES_IN_PROGRESS. */
+ {
+ mock = cros_kunit_ec_xfer_mock_addx(test, 0, EC_RES_IN_PROGRESS, 0);
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ }
+
+ /* For EC_CMD_GET_COMMS_STATUS status polling. */
+ {
+ struct ec_response_get_comms_status *data;
+
+ mock = cros_kunit_ec_xfer_mock_add(test, sizeof(*data));
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+
+ data = (struct ec_response_get_comms_status *)mock->o_data;
+ data->flags = 0;
+ }
+
+ /* For EC_CMD_RESEND_RESPONSE returning 16 bytes payload. */
+ {
+ mock = cros_kunit_ec_xfer_mock_add(test, sizeof(resp_data));
+ KUNIT_ASSERT_PTR_NE(test, mock, NULL);
+ memcpy(mock->o_data, resp_data, sizeof(resp_data));
+ }
+
+ ret = cros_ec_cmd_xfer(ec_dev, msg);
+ KUNIT_EXPECT_EQ(test, ret, 16);
+ KUNIT_EXPECT_EQ(test, msg->result, EC_RES_SUCCESS);
+ KUNIT_EXPECT_EQ(test, msg->command, 0x5678);
+ KUNIT_EXPECT_EQ(test, msg->outsize, 0);
+ KUNIT_EXPECT_EQ(test, msg->insize, sizeof(buf.data));
+ KUNIT_EXPECT_EQ(test, msg->version, 0);
+ KUNIT_EXPECT_EQ(test, memcmp(msg->data, resp_data, sizeof(resp_data)), 0);
+
+ /* Verify mock sequence */
+ {
+ mock = cros_kunit_ec_xfer_mock_next();
+ KUNIT_EXPECT_PTR_NE(test, mock, NULL);
+
+ mock = cros_kunit_ec_xfer_mock_next();
+ KUNIT_EXPECT_PTR_NE(test, mock, NULL);
+ KUNIT_EXPECT_EQ(test, mock->msg.command, EC_CMD_GET_COMMS_STATUS);
+
+ mock = cros_kunit_ec_xfer_mock_next();
+ KUNIT_EXPECT_PTR_NE(test, mock, NULL);
+ KUNIT_EXPECT_EQ(test, mock->msg.command, EC_CMD_RESEND_RESPONSE);
+ KUNIT_EXPECT_EQ(test, mock->msg.outsize, 0);
+ KUNIT_EXPECT_EQ(test, mock->msg.version, 0);
+ KUNIT_EXPECT_EQ(test, mock->msg.insize, 16);
+ }
+
+ KUNIT_EXPECT_EQ(test, cros_kunit_ec_pkt_xfer_mock_called, 3);
+}
+
static void cros_ec_proto_test_cmd_xfer_in_progress_retries_eagain(struct kunit *test)
{
struct cros_ec_proto_test_priv *priv = test->priv;
@@ -2703,7 +2853,9 @@ static struct kunit_case cros_ec_proto_test_cases[] = {
KUNIT_CASE(cros_ec_proto_test_cmd_xfer_protocol_v3_no_op),
KUNIT_CASE(cros_ec_proto_test_cmd_xfer_protocol_v2_normal),
KUNIT_CASE(cros_ec_proto_test_cmd_xfer_protocol_v2_no_op),
- KUNIT_CASE(cros_ec_proto_test_cmd_xfer_in_progress_normal),
+ KUNIT_CASE(cros_ec_proto_test_cmd_xfer_in_progress_payload_0byte),
+ KUNIT_CASE(cros_ec_proto_test_cmd_xfer_in_progress_payload_4bytes),
+ KUNIT_CASE(cros_ec_proto_test_cmd_xfer_in_progress_payload_gt4bytes),
KUNIT_CASE(cros_ec_proto_test_cmd_xfer_in_progress_retries_eagain),
KUNIT_CASE(cros_ec_proto_test_cmd_xfer_in_progress_retries_status_processing),
KUNIT_CASE(cros_ec_proto_test_cmd_xfer_in_progress_xfer_error),
--
2.55.0.897.gb25b4bd76c-goog
next reply other threads:[~2026-08-27 18:05 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-27 18:05 Rob Barnes [this message]
2026-08-28 5:31 ` [PATCH v4] platform/chrome: cros_ec_proto: Fix deferred response Tzung-Bi Shih
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=20260827180511.2825660-1-robbarnes@google.com \
--to=robbarnes@google.com \
--cc=bleung@chromium.org \
--cc=chrome-platform@lists.linux.dev \
--cc=linux-kernel@vger.kernel.org \
--cc=tomhughes@chromium.org \
--cc=tzungbi@kernel.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.