From 369596dc95d9f2b71e3a0eca526a02474cd34eac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micha=C5=82=20Barna=C5=9B?= Date: Wed, 8 Jun 2022 16:08:28 +0200 Subject: [PATCH] ec_host_cmd: increase stack size, change thread priority and alignment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit increases the stack size for thread handling the host commands requests. It was required due to the stack being corrupted using earlier default size. The thread priority is now configurable using the Kconfig. It also adds alignment to the tx_buffer since the npcx MCU requires it to work correctly and removes clearing the buffer before use due to the hard time requirements. Tests checking if buffers are cleared are also removed. Signed-off-by: Michał Barnaś --- subsys/mgmt/ec_host_cmd/Kconfig | 11 +++- subsys/mgmt/ec_host_cmd/ec_host_cmd_handler.c | 19 +++--- tests/subsys/mgmt/ec_host_cmd/src/main.c | 63 ------------------- 3 files changed, 17 insertions(+), 76 deletions(-) diff --git a/subsys/mgmt/ec_host_cmd/Kconfig b/subsys/mgmt/ec_host_cmd/Kconfig index 1d503393811..799c3e711e0 100644 --- a/subsys/mgmt/ec_host_cmd/Kconfig +++ b/subsys/mgmt/ec_host_cmd/Kconfig @@ -17,12 +17,21 @@ if EC_HOST_CMD config EC_HOST_CMD_HANDLER_STACK_SIZE int "Stack size for the EC host command handler thread" - default 512 + default 800 config EC_HOST_CMD_HANDLER_TX_BUFFER int "Buffer size in bytes for TX buffer shared by all EC host commands" default 256 +config EC_HOST_CMD_HANDLER_PRIO + int "Priority of host command task" + default 13 + help + Priority of the kernel task that handles the host commands. + If the priority is too low (high in value), the host commands handler may be unable to + process the command on time and the AP will abort the waiting for response and be unable + to boot the system properly. + endif # EC_HOST_CMD endmenu diff --git a/subsys/mgmt/ec_host_cmd/ec_host_cmd_handler.c b/subsys/mgmt/ec_host_cmd/ec_host_cmd_handler.c index c3454e374c4..d4c39809a5e 100644 --- a/subsys/mgmt/ec_host_cmd/ec_host_cmd_handler.c +++ b/subsys/mgmt/ec_host_cmd/ec_host_cmd_handler.c @@ -19,8 +19,12 @@ #define RX_HEADER_SIZE (sizeof(struct ec_host_cmd_request_header)) #define TX_HEADER_SIZE (sizeof(struct ec_host_cmd_response_header)) -/** Used by host command handlers for their response before going over wire */ -uint8_t tx_buffer[CONFIG_EC_HOST_CMD_HANDLER_TX_BUFFER]; +/** + * Used by host command handlers for their response before going over wire. + * Host commands handlers will cast this to respective response structures that may have fields of + * uint32_t or uint64_t, so this buffer must be aligned to protect against the unaligned access. + */ +static uint8_t tx_buffer[CONFIG_EC_HOST_CMD_HANDLER_TX_BUFFER] __aligned(8); static uint8_t cal_checksum(const uint8_t *const buffer, const uint16_t size) { @@ -134,15 +138,6 @@ static void handle_host_cmds_entry(void *arg1, void *arg2, void *arg3) continue; } - /* - * Ensure that RX/TX buffers are cleared between each host - * command to ensure subsequent host command handlers cannot - * read data from previous host command runs. - */ - memset(&rx.buf[rx_valid_data_size], 0, - *rx.len - rx_valid_data_size); - memset(tx_buffer, 0, sizeof(tx_buffer)); - struct ec_host_cmd_handler_args args = { .input_buf = rx.buf + RX_HEADER_SIZE, .input_buf_size = rx_header->data_len, @@ -207,4 +202,4 @@ static void handle_host_cmds_entry(void *arg1, void *arg2, void *arg3) K_THREAD_DEFINE(ec_host_cmd_handler_tid, CONFIG_EC_HOST_CMD_HANDLER_STACK_SIZE, handle_host_cmds_entry, NULL, NULL, NULL, - K_LOWEST_APPLICATION_THREAD_PRIO, 0, 0); + CONFIG_EC_HOST_CMD_HANDLER_PRIO, 0, 0); diff --git a/tests/subsys/mgmt/ec_host_cmd/src/main.c b/tests/subsys/mgmt/ec_host_cmd/src/main.c index 30d12b79f41..2f9142fc84a 100644 --- a/tests/subsys/mgmt/ec_host_cmd/src/main.c +++ b/tests/subsys/mgmt/ec_host_cmd/src/main.c @@ -415,69 +415,6 @@ ZTEST(ec_host_cmd, test_unbounded_handler_response_too_big) verify_tx_error(EC_HOST_CMD_INVALID_RESPONSE); } -ZTEST(ec_host_cmd, test_rx_buffer_cleared_foreach_hostcommand) -{ - host_to_dut->header.prtcl_ver = 3; - host_to_dut->header.cmd_id = EC_CMD_UNBOUNDED; - host_to_dut->header.cmd_ver = 2; - host_to_dut->header.reserved = 0; - host_to_dut->header.data_len = sizeof(host_to_dut->unbounded); - host_to_dut->unbounded.bytes_to_write = 5; - - /* Write data after the entire request message. The host command handler - * always assert that this data is cleared upon receipt. - */ - host_to_dut->raw[4] = 42; - - simulate_rx_data(); - - expected_dut_to_host->header.prtcl_ver = 3; - expected_dut_to_host->header.result = 0; - expected_dut_to_host->header.reserved = 0; - expected_dut_to_host->header.data_len = 5; - expected_dut_to_host->raw[0] = 0; - expected_dut_to_host->raw[1] = 1; - expected_dut_to_host->raw[2] = 2; - expected_dut_to_host->raw[3] = 3; - expected_dut_to_host->raw[4] = 4; - - verify_tx_data(); -} - -ZTEST(ec_host_cmd, test_tx_buffer_cleared_foreach_hostcommand) -{ - host_to_dut->header.prtcl_ver = 3; - host_to_dut->header.cmd_id = EC_CMD_UNBOUNDED; - host_to_dut->header.cmd_ver = 2; - host_to_dut->header.reserved = 0; - host_to_dut->header.data_len = sizeof(host_to_dut->unbounded); - host_to_dut->unbounded.bytes_to_write = 5; - - simulate_rx_data(); - - expected_dut_to_host->header.prtcl_ver = 3; - expected_dut_to_host->header.result = 0; - expected_dut_to_host->header.reserved = 0; - expected_dut_to_host->header.data_len = 5; - expected_dut_to_host->raw[0] = 0; - expected_dut_to_host->raw[1] = 1; - expected_dut_to_host->raw[2] = 2; - expected_dut_to_host->raw[3] = 3; - expected_dut_to_host->raw[4] = 4; - - verify_tx_data(); - - /* Send second command with less bytes to write. Host command handler - * asserts that the previous output data is zero. - */ - - host_to_dut->unbounded.bytes_to_write = 2; - simulate_rx_data(); - - expected_dut_to_host->header.data_len = 2; - verify_tx_data(); -} - #define EC_CMD_TOO_BIG 0x0003 static enum ec_host_cmd_status ec_host_cmd_too_big(struct ec_host_cmd_handler_args *args)