diff options
| author | Rob Barnes <robbarnes@google.com> | 2026-08-31 18:51:26 -0600 |
|---|---|---|
| committer | Tzung-Bi Shih <tzungbi@kernel.org> | 2026-09-01 02:16:14 +0000 |
| commit | c55749415623a58eba3eb527da023d29b43fbf5d (patch) | |
| tree | fad76f35f00557c9fcbc4bebdb8ef994a78c79ce | |
| parent | b57af7f8d194617e965d9713a767cc012774a2b9 (diff) | |
platform/chrome: cros_ec_proto: Fix deferred response
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.
Fixes: 2c7589af3c4d ("mfd: cros_ec: add proto v3 skeleton")
Signed-off-by: Rob Barnes <robbarnes@google.com>
Link: https://lore.kernel.org/r/20260901005126.3344929-1-robbarnes@google.com
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
| -rw-r--r-- | drivers/platform/chrome/cros_ec_proto.c | 32 | ||||
| -rw-r--r-- | 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 724d1313f6b2..b13312c1b093 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..cbaaf98d6877 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->insize = sizeof(buf.data); + + ec_dev->max_response = 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->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->max_response = 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), |
