ec_host_cmd: increase stack size, change thread priority and alignment

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ś <mb@semihalf.com>

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

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);

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)