From 0aed42f2eeb8628f10eb8a8f7491bc3873ede5f3 Mon Sep 17 00:00:00 2001 From: Dawid Niedzwiecki Date: Wed, 25 Oct 2023 12:22:04 +0200 Subject: [PATCH] mgmt: ec_host_cmd: improve handling buffer sizes Add the len_max rx structure member to indicate maximum number of bytes possible to receive. It is needed to send information about our protocol parameters to host. Also, limit the maximum size of request/responses for backends that uses buffers provided by the handler. Signed-off-by: Dawid Niedzwiecki --- include/zephyr/mgmt/ec_host_cmd/backend.h | 7 +++++-- .../backends/ec_host_cmd_backend_espi.c | 4 ++++ .../backends/ec_host_cmd_backend_shi_ite.c | 3 ++- .../backends/ec_host_cmd_backend_shi_npcx.c | 1 + .../backends/ec_host_cmd_backend_spi_stm32.c | 16 ++++++++++++++++ .../backends/ec_host_cmd_backend_uart.c | 18 +++++++++++++++++- subsys/mgmt/ec_host_cmd/ec_host_cmd_handler.c | 2 ++ 7 files changed, 47 insertions(+), 4 deletions(-) diff --git a/include/zephyr/mgmt/ec_host_cmd/backend.h b/include/zephyr/mgmt/ec_host_cmd/backend.h index fe0581b092e..ed61e0e33c2 100644 --- a/include/zephyr/mgmt/ec_host_cmd/backend.h +++ b/include/zephyr/mgmt/ec_host_cmd/backend.h @@ -43,11 +43,14 @@ struct ec_host_cmd_rx_ctx { /** * Buffer to hold received data. The buffer is provided by the handler if * CONFIG_EC_HOST_CMD_HANDLER_RX_BUFFER_SIZE > 0. Otherwise, the backend should provide - * the buffer on its own and overwrites @a buf pointer in the init function. + * the buffer on its own and overwrites @a buf pointer and @a len_max + * in the init function. */ uint8_t *buf; /** Number of bytes written to @a buf by backend. */ size_t len; + /** Maximum number of bytes to receive with one request packet. */ + size_t len_max; }; /** @@ -63,7 +66,7 @@ struct ec_host_cmd_tx_buf { void *buf; /** Number of bytes to write from @a buf. */ size_t len; - /** Size of @a buf. */ + /** Maximum number of bytes to send with one response packet. */ size_t len_max; }; diff --git a/subsys/mgmt/ec_host_cmd/backends/ec_host_cmd_backend_espi.c b/subsys/mgmt/ec_host_cmd/backends/ec_host_cmd_backend_espi.c index dd7dfa9099c..09224ffdac2 100644 --- a/subsys/mgmt/ec_host_cmd/backends/ec_host_cmd_backend_espi.c +++ b/subsys/mgmt/ec_host_cmd/backends/ec_host_cmd_backend_espi.c @@ -114,6 +114,10 @@ static int ec_host_cmd_espi_init(const struct ec_host_cmd_backend *backend, espi_read_lpc_request(hc_espi->espi_dev, ECUSTOM_HOST_CMD_GET_PARAM_MEMORY_SIZE, &tx->len_max); + /* Set the max len for RX as the min of buffer to store data and shared memory. */ + hc_espi->rx_ctx->len_max = + MIN(CONFIG_EC_HOST_CMD_HANDLER_RX_BUFFER_SIZE, hc_espi->tx->len_max); + hc_espi->state = ESPI_STATE_READY_TO_RECV; return 0; diff --git a/subsys/mgmt/ec_host_cmd/backends/ec_host_cmd_backend_shi_ite.c b/subsys/mgmt/ec_host_cmd/backends/ec_host_cmd_backend_shi_ite.c index e84b7d351e3..8c11765ee82 100644 --- a/subsys/mgmt/ec_host_cmd/backends/ec_host_cmd_backend_shi_ite.c +++ b/subsys/mgmt/ec_host_cmd/backends/ec_host_cmd_backend_shi_ite.c @@ -455,8 +455,9 @@ static int shi_ite_backend_init(const struct ec_host_cmd_backend *backend, data->tx = tx; rx_ctx->buf = data->in_msg; + rx_ctx->len_max = CONFIG_EC_HOST_CMD_BACKEND_SHI_MAX_REQUEST; tx->buf = data->out_msg + sizeof(out_preamble); - data->tx->len_max = sizeof(data->out_msg) - EC_SHI_PREAMBLE_LENGTH - EC_SHI_PAST_END_LENGTH; + data->tx->len_max = CONFIG_EC_HOST_CMD_BACKEND_SHI_MAX_RESPONSE; return 0; } diff --git a/subsys/mgmt/ec_host_cmd/backends/ec_host_cmd_backend_shi_npcx.c b/subsys/mgmt/ec_host_cmd/backends/ec_host_cmd_backend_shi_npcx.c index 9e4c6b08dd2..f98f3bf7584 100644 --- a/subsys/mgmt/ec_host_cmd/backends/ec_host_cmd_backend_shi_npcx.c +++ b/subsys/mgmt/ec_host_cmd/backends/ec_host_cmd_backend_shi_npcx.c @@ -882,6 +882,7 @@ static int shi_npcx_backend_init(const struct ec_host_cmd_backend *backend, data->tx = tx; rx_ctx->buf = data->in_msg; + rx_ctx->len_max = CONFIG_EC_HOST_CMD_BACKEND_SHI_MAX_REQUEST; tx->buf = data->out_msg_padded + SHI_OUT_START_PAD; tx->len_max = CONFIG_EC_HOST_CMD_BACKEND_SHI_MAX_RESPONSE; diff --git a/subsys/mgmt/ec_host_cmd/backends/ec_host_cmd_backend_spi_stm32.c b/subsys/mgmt/ec_host_cmd/backends/ec_host_cmd_backend_spi_stm32.c index c4e045ac800..52ee12ea0fd 100644 --- a/subsys/mgmt/ec_host_cmd/backends/ec_host_cmd_backend_spi_stm32.c +++ b/subsys/mgmt/ec_host_cmd/backends/ec_host_cmd_backend_spi_stm32.c @@ -96,6 +96,14 @@ BUILD_ASSERT(DT_NODE_HAS_COMPAT_STATUS(DT_CHOSEN(zephyr_host_cmd_spi_backend), #define EC_HOST_CMD_ST_STM32_FIFO #endif /* st_stm32_spi_fifo */ +/* + * Max data size for a version 3 request/response packet. This is big enough + * to handle a request/response header, flash write offset/size, and 512 bytes + * of flash data. + */ +#define SPI_MAX_REQ_SIZE 0x220 +#define SPI_MAX_RESP_SIZE 0x220 + /* Enumeration to maintain different states of incoming request from * host */ @@ -667,6 +675,14 @@ static int ec_host_cmd_spi_init(const struct ec_host_cmd_backend *backend, hc_spi->tx->buf = (uint8_t *)hc_spi->tx->buf + sizeof(out_preamble); hc_spi->tx->len_max = hc_spi->tx->len_max - sizeof(out_preamble) - EC_SPI_PAST_END_LENGTH; + /* Limit the requset/response max sizes */ + if (hc_spi->rx_ctx->len_max > SPI_MAX_REQ_SIZE) { + hc_spi->rx_ctx->len_max = SPI_MAX_REQ_SIZE; + } + if (hc_spi->tx->len_max > SPI_MAX_RESP_SIZE) { + hc_spi->tx->len_max = SPI_MAX_RESP_SIZE; + } + ret = spi_init(hc_spi); if (ret) { return ret; diff --git a/subsys/mgmt/ec_host_cmd/backends/ec_host_cmd_backend_uart.c b/subsys/mgmt/ec_host_cmd/backends/ec_host_cmd_backend_uart.c index 0d43c9ccd79..2e77d3f0ccd 100644 --- a/subsys/mgmt/ec_host_cmd/backends/ec_host_cmd_backend_uart.c +++ b/subsys/mgmt/ec_host_cmd/backends/ec_host_cmd_backend_uart.c @@ -101,7 +101,15 @@ static int request_expected_size(const struct ec_host_cmd_request_header *r) } /* Timeout after receiving first byte */ -#define UART_REQ_RX_TIMEOUT K_MSEC(150) +#define UART_REQ_RX_TIMEOUT K_MSEC(150) + +/* + * Max data size for a version 3 request/response packet. This is big enough + * to handle a request/response header, flash write offset/size and 512 bytes + * of request payload or 224 bytes of response payload. + */ +#define UART_MAX_REQ_SIZE 0x220 +#define UART_MAX_RESP_SIZE 0x100 static void rx_timeout(struct k_work *work) { @@ -242,6 +250,14 @@ static int ec_host_cmd_uart_init(const struct ec_host_cmd_backend *backend, hc_uart->rx_ctx = rx_ctx; hc_uart->tx_buf = tx; + /* Limit the requset/response max sizes */ + if (hc_uart->rx_ctx->len_max > UART_MAX_REQ_SIZE) { + hc_uart->rx_ctx->len_max = UART_MAX_REQ_SIZE; + } + if (hc_uart->tx_buf->len_max > UART_MAX_RESP_SIZE) { + hc_uart->tx_buf->len_max = UART_MAX_RESP_SIZE; + } + k_work_init_delayable(&hc_uart->timeout_work, rx_timeout); uart_callback_set(hc_uart->uart_dev, uart_callback, hc_uart); ret = uart_rx_enable(hc_uart->uart_dev, hc_uart->rx_ctx->buf, hc_uart->rx_buf_size, 0); 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 c8f471411e7..b8bf8cfbe6a 100644 --- a/subsys/mgmt/ec_host_cmd/ec_host_cmd_handler.c +++ b/subsys/mgmt/ec_host_cmd/ec_host_cmd_handler.c @@ -47,6 +47,8 @@ static struct ec_host_cmd ec_host_cmd = { .rx_ctx = { .buf = COND_CODE_1(CONFIG_EC_HOST_CMD_HANDLER_RX_BUFFER_DEF, (hc_rx_buffer), (NULL)), + .len_max = COND_CODE_1(CONFIG_EC_HOST_CMD_HANDLER_RX_BUFFER_DEF, + (CONFIG_EC_HOST_CMD_HANDLER_RX_BUFFER_SIZE), (0)), }, .tx = { .buf = COND_CODE_1(CONFIG_EC_HOST_CMD_HANDLER_TX_BUFFER_DEF, (hc_tx_buffer),