* [PATCH v4] platform/chrome: cros_ec_proto: Fix deferred response
@ 2026-08-27 18:05 Rob Barnes
2026-08-28 5:31 ` Tzung-Bi Shih
0 siblings, 1 reply; 2+ messages in thread
From: Rob Barnes @ 2026-08-27 18:05 UTC (permalink / raw)
To: bleung, tzungbi; +Cc: tomhughes, chrome-platform, linux-kernel, Rob Barnes
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
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH v4] platform/chrome: cros_ec_proto: Fix deferred response
2026-08-27 18:05 [PATCH v4] platform/chrome: cros_ec_proto: Fix deferred response Rob Barnes
@ 2026-08-28 5:31 ` Tzung-Bi Shih
0 siblings, 0 replies; 2+ messages in thread
From: Tzung-Bi Shih @ 2026-08-28 5:31 UTC (permalink / raw)
To: Rob Barnes; +Cc: bleung, tomhughes, chrome-platform, linux-kernel
On Thu, Aug 27, 2026 at 12:05:11PM -0600, Rob Barnes wrote:
> - Send patch using git-send-email via Google Mail Relay to prevent MTA line-wrapping (Tzung-Bi).
The patch can be applied via `git am` after the fixing. However, KUnit test
emits errors after applying the patch:
$ ./tools/testing/kunit/kunit.py run \
--arch=x86_64 \
--kconfig_add CONFIG_CHROME_PLATFORMS=y \
--kconfig_add CONFIG_CROS_EC=y \
cros_ec*
> diff --git a/drivers/platform/chrome/cros_ec_proto_test.c b/drivers/platform/chrome/cros_ec_proto_test.c
...
> +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;
It needs to set `ec_dev->max_request` before it can send the payload. Given
the test doesn't verify the output payload, just removing the assignment of
`msg->outsize` and using default value 0 could be the simplest fix.
Otherwise:
cros_ec_proto_test: request of size 2 is too big (max: 0)
# cros_ec_proto_test_cmd_xfer_in_progress_payload_4bytes:
Expected ret == 4, but
ret == -90 (0xffffffffffffffa6)
> + msg->insize = sizeof(buf.data);
Similar here, it needs to set `ec_dev->max_response` before it can receive
payload (cros_ec_cmd_xfer() clamps it). Otherwise:
# cros_ec_proto_test_cmd_xfer_in_progress_payload_4bytes:
Expected msg->insize == sizeof(buf.data), but
msg->insize == 0 (0x0)
sizeof(buf.data) == 4 (0x4)
> + 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);
Following the above suggestion, this needs to be dropped.
> +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;
Same here:
ec_dev->max_response = sizeof(buf.data);
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-08-28 5:31 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-27 18:05 [PATCH v4] platform/chrome: cros_ec_proto: Fix deferred response Rob Barnes
2026-08-28 5:31 ` Tzung-Bi Shih
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®