From 9bdde3e76691d3978784e35340d8cd8bb8ff6a73 Mon Sep 17 00:00:00 2001 From: Gwendal Grignou Date: Wed, 31 May 2017 17:05:30 -0700 Subject: [PATCH] npcx: Fix response size max_response_packet_size was incorrectly set to response_size + SPI header/footer, leading to the command handler to return EC_RES_OVERFLOW when the response buffer size was set by the host to a larger size then response_size. It is happening since flashrom does not limit itself to 128 bytes command since cl/264034. The SHI repsone buffer is laid out as follow: ,.... out_msg_padded / | |< SHI_OUT_START_PAD >|< SHI_MAX_RESPONSE_SIZE >|< SHI_OUT_END_PAD >| +---+-----------------+-------------------------+-------------------+ | | | | | +---+-----------------+-------------------------+-------------------+ | \ | ------- | \ | EC_SPI_FRAME_START_LENGTH | \..... out_msg BUG=b:35571522,chromium:725580 BRANCH=gru TEST=Before flashrom would fail: cros_ec_set_max_size: sending protoinfo command cros_ec_set_max_size: rc:12 cros_ec_set_max_size: max_write:536 max_read:163 ... Reading flash... __cros_ec_command_dev_v2(): Command 0x11 returned result: 11 Ater, flashrom works: cros_ec_set_max_size: sending protoinfo command cros_ec_set_max_size: rc:12 cros_ec_set_max_size: max_write:536 max_read:160 ... Reading flash... done.SUCCESS Verified that cros_ec.c/cros_ec_spi.c set some space for header and footer in addition to max_response_packet_size. Change-Id: I0de7ee5e8109e9277692113f2bb1d4a4758be9f6 Signed-off-by: Gwendal Grignou Reviewed-on: https://chromium-review.googlesource.com/520585 Reviewed-by: Aseda Aboagye --- chip/npcx/shi.c | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/chip/npcx/shi.c b/chip/npcx/shi.c index ed4b108311..ec584e82ee 100644 --- a/chip/npcx/shi.c +++ b/chip/npcx/shi.c @@ -111,14 +111,17 @@ /* * 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. + * of flash data: + * sizeof(ec_host_request): 8 + * sizoef(ec_params_flash_write): 8 + * payload 512 + * */ #define SHI_MAX_REQUEST_SIZE 0x220 #ifdef NPCX_SHI_BYPASS_OVER_256B /* Make sure SHI_MAX_RESPONSE_SIZE won't exceed 256 bytes */ -#define SHI_MAX_RESPONSE_SIZE (160 + EC_SPI_PAST_END_LENGTH + \ - EC_SPI_FRAME_START_LENGTH + sizeof(struct ec_host_response)) +#define SHI_MAX_RESPONSE_SIZE (160 + sizeof(struct ec_host_response)) BUILD_ASSERT(SHI_MAX_RESPONSE_SIZE <= SHI_BYPASS_BOUNDARY); #else #define SHI_MAX_RESPONSE_SIZE 0x220 @@ -129,9 +132,13 @@ BUILD_ASSERT(SHI_MAX_RESPONSE_SIZE <= SHI_BYPASS_BOUNDARY); * message, including protocol overhead. The pointers after the protocol * overhead, as passed to the host command handler, must be 32-bit aligned. */ -#define SHI_OUT_PAD ((4 - EC_SPI_FRAME_START_LENGTH) % 4) -static uint8_t out_msg_padded[SHI_OUT_PAD + SHI_MAX_RESPONSE_SIZE] __aligned(4); -static uint8_t * const out_msg = out_msg_padded + SHI_OUT_PAD; +#define SHI_OUT_START_PAD (4 * (EC_SPI_FRAME_START_LENGTH / 4 + 1)) +#define SHI_OUT_END_PAD (4 * (EC_SPI_PAST_END_LENGTH / 4 + 1)) +static uint8_t out_msg_padded[SHI_OUT_START_PAD + + SHI_MAX_RESPONSE_SIZE + + SHI_OUT_END_PAD] __aligned(4); +static uint8_t * const out_msg = + out_msg_padded + SHI_OUT_START_PAD - EC_SPI_FRAME_START_LENGTH; static uint8_t in_msg[SHI_MAX_REQUEST_SIZE] __aligned(4); /* Parameters used by host protocols */ @@ -285,7 +292,7 @@ void shi_handle_host_package(void) shi_packet.response = out_msg + EC_SPI_FRAME_START_LENGTH; /* Reserve space for frame start and trailing past-end byte */ - shi_packet.response_max = SHI_MAX_RESPONSE_SIZE - SHI_PROTO3_OVERHEAD; + shi_packet.response_max = SHI_MAX_RESPONSE_SIZE; shi_packet.response_size = 0; shi_packet.driver_result = EC_RES_SUCCESS;