Change in coreboot[master]: soc/intel/common: Implement error codes for for heci_send_receive()

[Felix Held (Code Review)]

Felix Held has submitted this change. ( https://review.coreboot.org/c/coreboot/+/62299 )

Change subject: soc/intel/common: Implement error codes for for heci_send_receive()
......................................................................

soc/intel/common: Implement error codes for for heci_send_receive()

The patch implements below changes:
1. Implements different error codes and use them in appropriate
 failure scenarios of below functions:
  a. heci_send()
  b. recv_one_message()
  c. heci_receive()

2. As heci_send_receive() is updated to return appropriate error codes
 in different error scenarios of sending and receiving the HECI
 commands. As the function is updated to return 0 when success, and
 non-zero values in the failure scenarios, so all caller function have
 been updated.

BUG=b:220652101
TEST=Verified CSE RX and TX APIs return error codes appropriately in
the simulated error scenarios.

Signed-off-by: Sridhar Siricilla 
Change-Id: Ibedee748ed6d81436c6b125f2eb2722be3f5f8f0
Reviewed-on: https://review.coreboot.org/c/coreboot/+/62299
Tested-by: build bot (Jenkins) 
Reviewed-by: Tim Wawrzynczak 
---
M src/soc/intel/apollolake/cse.c
M src/soc/intel/common/block/cse/cse.c
M src/soc/intel/common/block/cse/cse_eop.c
M src/soc/intel/common/block/cse/cse_lite.c
M src/soc/intel/common/block/include/intelblocks/cse.h
5 files changed, 86 insertions(+), 47 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Tim Wawrzynczak: Looks good to me, approved

diff --git a/src/soc/intel/apollolake/cse.c b/src/soc/intel/apollolake/cse.c
index 7d1e6e7..0f11809 100644
--- a/src/soc/intel/apollolake/cse.c
+++ b/src/soc/intel/apollolake/cse.c
@@ -78,7 +78,7 @@

reply_size = sizeof(rmsg);

-	if (!heci_send_receive(&msg, sizeof(msg), &rmsg, &reply_size, HECI_MKHI_ADDR)) {
+	if (heci_send_receive(&msg, sizeof(msg), &rmsg, &reply_size, HECI_MKHI_ADDR)) {
    	printk(BIOS_ERR, "HECI: Failed to read file\n");
    	return 0;
    }
diff --git a/src/soc/intel/common/block/cse/cse.c b/src/soc/intel/common/block/cse/cse.c
index 18f9ba8..810973b 100644
--- a/src/soc/intel/common/block/cse/cse.c
+++ b/src/soc/intel/common/block/cse/cse.c
@@ -407,7 +407,7 @@
  * Returns 1 on success and 0 otherwise.
  * In case of error heci_reset() may be required.
  */
-static int
+static enum cse_tx_rx_status
 heci_send(const void *msg, size_t len, uint8_t host_addr, uint8_t client_addr)
 {
    uint8_t retry;
@@ -416,7 +416,7 @@
    const uint8_t *p;

if (!msg || !len)
-		return 0;
+		return CSE_TX_ERR_INPUT;

clear_int();

@@ -455,40 +455,41 @@
    	} while (remaining > 0 && sent != 0);

if (!remaining)
-			return 1;
+			return CSE_TX_RX_SUCCESS;
    }
-	return 0;
+
+	return CSE_TX_ERR_CSE_NOT_READY;
 }

-static size_t
-recv_one_message(uint32_t *hdr, void *buff, size_t maxlen)
+static enum cse_tx_rx_status
+recv_one_message(uint32_t *hdr, void *buff, size_t maxlen, size_t *recv_len)
 {
    uint32_t reg, *p = buff;
-	size_t recv_slots, recv_len, remainder, i;
+	size_t recv_slots, remainder, i;

/* first get the header */
    if (!wait_read_slots(1))
-		return 0;
+		return CSE_RX_ERR_TIMEOUT;

*hdr = read_slot();
-	recv_len = hdr_get_length(*hdr);
+	*recv_len = hdr_get_length(*hdr);

-	if (!recv_len)
+	if (!*recv_len)
    	printk(BIOS_WARNING, "HECI: message is zero-sized\n");

-	recv_slots = bytes_to_slots(recv_len);
+	recv_slots = bytes_to_slots(*recv_len);

i = 0;
-	if (recv_len > maxlen) {
+	if (*recv_len > maxlen) {
    	printk(BIOS_ERR, "HECI: response is too big\n");
-		return 0;
+		return CSE_RX_ERR_RESP_LEN_MISMATCH;
    }

/* wait for the rest of messages to arrive */
    wait_read_slots(recv_slots);

/* fetch whole slots first */
-	while (i < ALIGN_DOWN(recv_len, SLOT_SIZE)) {
+	while (i < ALIGN_DOWN(*recv_len, SLOT_SIZE)) {
    	*p++ = read_slot();
    	i += SLOT_SIZE;
    }
@@ -498,16 +499,15 @@
     * we received junk
     */
    if (!cse_ready())
-		return 0;
+		return CSE_RX_ERR_CSE_NOT_READY;

-	remainder = recv_len % SLOT_SIZE;
+	remainder = *recv_len % SLOT_SIZE;

if (remainder) {
    	reg = read_slot();
    	memcpy(p, &reg, remainder);
    }
-
-	return recv_len;
+	return CSE_TX_RX_SUCCESS;
 }

/*
@@ -518,15 +518,16 @@
  * and 1 on success.
  * In case of error heci_reset() may be required.
  */
-static int heci_receive(void *buff, size_t *maxlen)
+static enum cse_tx_rx_status heci_receive(void *buff, size_t *maxlen)
 {
    uint8_t retry;
    size_t left, received;
    uint32_t hdr = 0;
    uint8_t *p;
+	enum cse_tx_rx_status ret = CSE_RX_ERR_TIMEOUT;

if (!buff || !maxlen || !*maxlen)
-		return 0;
+		return CSE_RX_ERR_INPUT;

clear_int();

@@ -544,10 +545,10 @@
    	 * complete or we run out of space in caller-provided buffer.
    	 */
    	do {
-			received = recv_one_message(&hdr, p, left);
-			if (!received) {
+			ret = recv_one_message(&hdr, p, left, &received);
+			if (ret) {
    			printk(BIOS_ERR, "HECI: Failed to receive!\n");
-				return 0;
+				return ret;
    		}
    		left -= received;
    		p += received;
@@ -558,27 +559,32 @@

if ((hdr & MEI_HDR_IS_COMPLETE) && received) {
    		*maxlen = p - (uint8_t *) buff;
-			return 1;
+			return CSE_TX_RX_SUCCESS;
    	}
    }
-	return 0;
+
+	return CSE_RX_ERR_CSE_NOT_READY;
 }

-int heci_send_receive(const void *snd_msg, size_t snd_sz, void *rcv_msg, size_t *rcv_sz,
-									uint8_t cse_addr)
+enum cse_tx_rx_status heci_send_receive(const void *snd_msg, size_t snd_sz, void *rcv_msg,
+					size_t *rcv_sz, uint8_t cse_addr)
 {
-	if (!heci_send(snd_msg, snd_sz, BIOS_HOST_ADDR, cse_addr)) {
+	enum cse_tx_rx_status ret;
+
+	ret = heci_send(snd_msg, snd_sz, BIOS_HOST_ADDR, cse_addr);
+	if (ret) {
    	printk(BIOS_ERR, "HECI: send Failed\n");
-		return 0;
+		return ret;
    }

if (rcv_msg != NULL) {
-		if (!heci_receive(rcv_msg, rcv_sz)) {
+		ret = heci_receive(rcv_msg, rcv_sz);
+		if (ret) {
    		printk(BIOS_ERR, "HECI: receive Failed\n");
-			return 0;
+			return ret;
    	}
    }
-	return 1;
+	return ret;
 }

/*
@@ -704,7 +710,7 @@
    	status = heci_send_receive(&msg, sizeof(msg), &reply, &reply_size,
    								HECI_MKHI_ADDR);

-	printk(BIOS_DEBUG, "HECI: Global Reset %s!\n", status ? "success" : "failure");
+	printk(BIOS_DEBUG, "HECI: Global Reset %s!\n", !status ? "success" : "failure");
    return status;
 }

@@ -777,7 +783,7 @@
    	return 0;
    }

-	if (!heci_send_receive(&msg, sizeof(struct hmrfpo_enable_msg),
+	if (heci_send_receive(&msg, sizeof(struct hmrfpo_enable_msg),
    			&resp, &resp_size, HECI_MKHI_ADDR))
    	return 0;

@@ -826,7 +832,7 @@
    	return -1;
    }

-	if (!heci_send_receive(&msg, sizeof(struct hmrfpo_get_status_msg),
+	if (heci_send_receive(&msg, sizeof(struct hmrfpo_get_status_msg),
    			&resp, &resp_size, HECI_MKHI_ADDR)) {
    	printk(BIOS_ERR, "HECI: HMRFPO send/receive fail\n");
    	return -1;
@@ -893,7 +899,7 @@

heci_reset();

-	if (!heci_send_receive(&fw_ver_msg, sizeof(fw_ver_msg), resp, &resp_size,
+	if (heci_send_receive(&fw_ver_msg, sizeof(fw_ver_msg), resp, &resp_size,
    								HECI_MKHI_ADDR))
    	return CB_ERR;

@@ -1171,7 +1177,7 @@
    }

printk(BIOS_DEBUG, "HECI: ME state change send %s!\n",
-							send ? "success" : "failure");
+							!send ? "success" : "failure");
    printk(BIOS_DEBUG, "HECI: ME state change result %s!\n",
    						result ? "success" : "failure");

diff --git a/src/soc/intel/common/block/cse/cse_eop.c b/src/soc/intel/common/block/cse/cse_eop.c
index af6abb8..87da5fc 100644
--- a/src/soc/intel/common/block/cse/cse_eop.c
+++ b/src/soc/intel/common/block/cse/cse_eop.c
@@ -34,7 +34,7 @@

size_t reply_sz = sizeof(reply);

-	if (!heci_send_receive(&msg, sizeof(msg), &reply, &reply_sz, HECI_MEI_ADDR)) {
+	if (heci_send_receive(&msg, sizeof(msg), &reply, &reply_sz, HECI_MEI_ADDR)) {
    	printk(BIOS_ERR, "HECI: Failed to Disable MEI bus\n");
    	return false;
    }
@@ -98,7 +98,7 @@

printk(BIOS_INFO, "HECI: Sending End-of-Post\n");

-	if (!heci_send_receive(&msg, sizeof(msg), &resp, &resp_size, HECI_MKHI_ADDR)) {
+	if (heci_send_receive(&msg, sizeof(msg), &resp, &resp_size, HECI_MKHI_ADDR)) {
    	printk(BIOS_ERR, "HECI: EOP send/receive fail\n");
    	return CSE_EOP_RESULT_ERROR;
    }
diff --git a/src/soc/intel/common/block/cse/cse_lite.c b/src/soc/intel/common/block/cse/cse_lite.c
index 67dae1c..2756814 100644
--- a/src/soc/intel/common/block/cse/cse_lite.c
+++ b/src/soc/intel/common/block/cse/cse_lite.c
@@ -143,7 +143,7 @@

size_t resp_size = sizeof(struct cse_boot_perf_rsp);

-	if (!heci_send_receive(&req, sizeof(req), boot_perf_rsp, &resp_size,
+	if (heci_send_receive(&req, sizeof(req), boot_perf_rsp, &resp_size,
    								HECI_MKHI_ADDR)) {
    	printk(BIOS_ERR, "cse_lite: Could not get boot performance data\n");
    	return false;
@@ -241,7 +241,7 @@

size_t resp_size = sizeof(struct get_bp_info_rsp);

-	if (!heci_send_receive(&info_req, sizeof(info_req), bp_info_rsp, &resp_size,
+	if (heci_send_receive(&info_req, sizeof(info_req), bp_info_rsp, &resp_size,
    								HECI_MKHI_ADDR)) {
    	printk(BIOS_ERR, "cse_lite: Could not get partition info\n");
    	return false;
@@ -294,7 +294,7 @@
    struct mkhi_hdr switch_resp;
    size_t sw_resp_sz = sizeof(struct mkhi_hdr);

-	if (!heci_send_receive(&switch_req, sizeof(switch_req), &switch_resp, &sw_resp_sz,
+	if (heci_send_receive(&switch_req, sizeof(switch_req), &switch_resp, &sw_resp_sz,
    								HECI_MKHI_ADDR))
    	return false;

@@ -331,7 +331,7 @@
    struct mkhi_hdr data_clr_rsp;
    size_t data_clr_rsp_sz = sizeof(data_clr_rsp);

-	if (!heci_send_receive(&data_clr_rq, sizeof(data_clr_rq), &data_clr_rsp,
+	if (heci_send_receive(&data_clr_rq, sizeof(data_clr_rq), &data_clr_rsp,
    			&data_clr_rsp_sz, HECI_MKHI_ADDR)) {
    	return false;
    }
diff --git a/src/soc/intel/common/block/include/intelblocks/cse.h b/src/soc/intel/common/block/include/intelblocks/cse.h
index f94fc73..8fd8ba0 100644
--- a/src/soc/intel/common/block/include/intelblocks/cse.h
+++ b/src/soc/intel/common/block/include/intelblocks/cse.h
@@ -109,6 +109,39 @@
    struct me_version fitc;
 } __packed;

+/* CSE RX and TX error status */
+enum cse_tx_rx_status {
+	/*
+	 * Transmission of HECI message is success or
+	 * Reception of HECI message is success.
+	 */
+	CSE_TX_RX_SUCCESS = 0,
+
+	 /* Timeout to send a message to CSE */
+	CSE_TX_ERR_TIMEOUT = 1,
+
+	/* Timeout to receive the response message from CSE */
+	CSE_RX_ERR_TIMEOUT = 2,
+
+	/*
+	 * Response length doesn't match with expected
+	 * response message length
+	 */
+	CSE_RX_ERR_RESP_LEN_MISMATCH = 3,
+
+	/* CSE is not ready during TX flow */
+	CSE_TX_ERR_CSE_NOT_READY = 4,
+
+	/* CSE is not ready during RX flow */
+	CSE_RX_ERR_CSE_NOT_READY = 5,
+
+	/* Invalid input arguments provided for TX API */
+	CSE_TX_ERR_INPUT = 6,
+
+	/* Invalid input arguments provided for RX API */
+	CSE_RX_ERR_INPUT = 7,
+};
+
 /* CSE recovery sub-error codes */
 enum csme_failure_reason {
    /* No error */
@@ -300,10 +333,10 @@
  * Send message from BIOS_HOST_ADDR to cse_addr.
  * Sends snd_msg of size snd_sz, and reads message into buffer pointed by
  * rcv_msg of size rcv_sz
- * Returns 0 on failure and 1 on success.
+ * Returns CSE_TX_RX_SUCCESS on success and other enum values on failure scenarios.
  */
-int heci_send_receive(const void *snd_msg, size_t snd_sz, void *rcv_msg, size_t *rcv_sz,
-									uint8_t cse_addr);
+enum cse_tx_rx_status heci_send_receive(const void *snd_msg, size_t snd_sz, void *rcv_msg,
+					size_t *rcv_sz,	uint8_t cse_addr);

/*
  * Attempt device reset. This is useful and perhaps only thing left to do when

8 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.