From 6c30baee93d97c0fd2d41be51fca2e681f8dc9ab Mon Sep 17 00:00:00 2001 From: "Andrew F. Davis" Date: Wed, 10 Apr 2019 11:38:56 -0400 Subject: [PATCH 1/4] ti: k3: drivers: sec_proxy: Fix printf format specifiers The ID of a thread is not used outside for printing it out when something goes wrong. The specifier used is also not consistent. Instead of storing the thread ID, store its name and print that. Signed-off-by: Andrew F. Davis Change-Id: Id137c2f8dfdd5c599e220193344ece903f80af7b --- .../k3/common/drivers/sec_proxy/sec_proxy.c | 44 +++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/plat/ti/k3/common/drivers/sec_proxy/sec_proxy.c b/plat/ti/k3/common/drivers/sec_proxy/sec_proxy.c index 49cecd44c..0cf296b57 100644 --- a/plat/ti/k3/common/drivers/sec_proxy/sec_proxy.c +++ b/plat/ti/k3/common/drivers/sec_proxy/sec_proxy.c @@ -51,14 +51,14 @@ struct k3_sec_proxy_desc { }; /** - * struct k3_sec_proxy_thread - Description of a secure proxy Thread - * @id: Thread ID + * struct k3_sec_proxy_thread - Description of a Secure Proxy Thread + * @name: Thread Name * @data: Thread Data path region for target * @scfg: Secure Config Region for Thread * @rt: RealTime Region for Thread */ struct k3_sec_proxy_thread { - uint32_t id; + const char *name; uintptr_t data; uintptr_t scfg; uintptr_t rt; @@ -83,7 +83,7 @@ struct k3_sec_proxy_mbox { */ #define SP_THREAD(_x) \ [_x] = { \ - .id = _x, \ + .name = #_x, \ .data = SEC_PROXY_THREAD(SEC_PROXY_DATA_BASE, _x), \ .scfg = SEC_PROXY_THREAD(SEC_PROXY_SCFG_BASE, _x), \ .rt = SEC_PROXY_THREAD(SEC_PROXY_RT_BASE, _x), \ @@ -131,7 +131,7 @@ static inline int k3_sec_proxy_verify_thread(struct k3_sec_proxy_thread *spt, /* Check for any errors already available */ if (mmio_read_32(spt->rt + RT_THREAD_STATUS) & RT_THREAD_STATUS_ERROR_MASK) { - ERROR("Thread %d is corrupted, cannot send data\n", spt->id); + ERROR("Thread %s is corrupted, cannot send data\n", spt->name); return -EINVAL; } @@ -139,11 +139,11 @@ static inline int k3_sec_proxy_verify_thread(struct k3_sec_proxy_thread *spt, if ((mmio_read_32(spt->scfg + SCFG_THREAD_CTRL) & SCFG_THREAD_CTRL_DIR_MASK) != (dir << SCFG_THREAD_CTRL_DIR_SHIFT)) { if (dir) - ERROR("Trying to send data on RX Thread %d\n", - spt->id); + ERROR("Trying to send data on RX Thread %s\n", + spt->name); else - ERROR("Trying to receive data on TX Thread %d\n", - spt->id); + ERROR("Trying to receive data on TX Thread %s\n", + spt->name); return -EINVAL; } @@ -151,10 +151,10 @@ static inline int k3_sec_proxy_verify_thread(struct k3_sec_proxy_thread *spt, uint32_t tick_start = (uint32_t)read_cntpct_el0(); uint32_t ticks_per_us = SYS_COUNTER_FREQ_IN_TICKS / 1000000; while (!(mmio_read_32(spt->rt + RT_THREAD_STATUS) & RT_THREAD_STATUS_CUR_CNT_MASK)) { - VERBOSE("Waiting for thread %d to clear\n", spt->id); + VERBOSE("Waiting for thread %s to clear\n", spt->name); if (((uint32_t)read_cntpct_el0() - tick_start) > (spm.desc.timeout_us * ticks_per_us)) { - ERROR("Timeout waiting for thread %d to clear\n", spt->id); + ERROR("Timeout waiting for thread %s to clear\n", spt->name); return -ETIMEDOUT; } } @@ -176,13 +176,13 @@ int k3_sec_proxy_clear_rx_thread(enum k3_sec_proxy_chan_id id) /* Check for any errors already available */ if (mmio_read_32(spt->rt + RT_THREAD_STATUS) & RT_THREAD_STATUS_ERROR_MASK) { - ERROR("Thread %d is corrupted, cannot send data\n", spt->id); + ERROR("Thread %s is corrupted, cannot send data\n", spt->name); return -EINVAL; } /* Make sure thread is configured for right direction */ if (!(mmio_read_32(spt->scfg + SCFG_THREAD_CTRL) & SCFG_THREAD_CTRL_DIR_MASK)) { - ERROR("Cannot clear a transmit thread %d\n", spt->id); + ERROR("Cannot clear a transmit thread %s\n", spt->name); return -EINVAL; } @@ -190,10 +190,10 @@ int k3_sec_proxy_clear_rx_thread(enum k3_sec_proxy_chan_id id) uint32_t try_count = 10; while (mmio_read_32(spt->rt + RT_THREAD_STATUS) & RT_THREAD_STATUS_CUR_CNT_MASK) { if (!(try_count--)) { - ERROR("Could not clear all messages from thread %d\n", spt->id); + ERROR("Could not clear all messages from thread %s\n", spt->name); return -ETIMEDOUT; } - WARN("Clearing message from thread %d\n", spt->id); + WARN("Clearing message from thread %s\n", spt->name); mmio_read_32(spt->data + spm.desc.data_end_offset); } @@ -216,14 +216,14 @@ int k3_sec_proxy_send(enum k3_sec_proxy_chan_id id, const struct k3_sec_proxy_ms ret = k3_sec_proxy_verify_thread(spt, THREAD_IS_TX); if (ret) { - ERROR("Thread %d verification failed (%d)\n", spt->id, ret); + ERROR("Thread %s verification failed (%d)\n", spt->name, ret); return ret; } /* Check the message size */ if (msg->len + sizeof(secure_header) > spm.desc.max_msg_size) { - ERROR("Thread %d message length %lu > max msg size\n", - spt->id, msg->len); + ERROR("Thread %s message length %lu > max msg size\n", + spt->name, msg->len); return -EINVAL; } @@ -263,7 +263,7 @@ int k3_sec_proxy_send(enum k3_sec_proxy_chan_id id, const struct k3_sec_proxy_ms if (data_reg <= spm.desc.data_end_offset) mmio_write_32(spt->data + spm.desc.data_end_offset, 0); - VERBOSE("Message successfully sent on thread %ud\n", id); + VERBOSE("Message successfully sent on thread %s\n", spt->name); return 0; } @@ -275,7 +275,7 @@ int k3_sec_proxy_send(enum k3_sec_proxy_chan_id id, const struct k3_sec_proxy_ms * * Return: 0 if all goes well, else appropriate error message */ -int k3_sec_proxy_recv(uint32_t id, struct k3_sec_proxy_msg *msg) +int k3_sec_proxy_recv(enum k3_sec_proxy_chan_id id, struct k3_sec_proxy_msg *msg) { struct k3_sec_proxy_thread *spt = &spm.threads[id]; union sec_msg_hdr secure_header; @@ -284,7 +284,7 @@ int k3_sec_proxy_recv(uint32_t id, struct k3_sec_proxy_msg *msg) ret = k3_sec_proxy_verify_thread(spt, THREAD_IS_RX); if (ret) { - ERROR("Thread %d verification failed (%d)\n", spt->id, ret); + ERROR("Thread %s verification failed (%d)\n", spt->name, ret); return ret; } @@ -323,7 +323,7 @@ int k3_sec_proxy_recv(uint32_t id, struct k3_sec_proxy_msg *msg) /* TODO: Verify checksum */ (void)secure_header.checksum; - VERBOSE("Message successfully received from thread %ud\n", id); + VERBOSE("Message successfully received from thread %s\n", spt->name); return 0; } From fb98ca5a81aaf010a352e1353ae915c6e4ebfd9b Mon Sep 17 00:00:00 2001 From: "Andrew F. Davis" Date: Wed, 10 Apr 2019 11:45:19 -0400 Subject: [PATCH 2/4] ti: k3: drivers: sec_proxy: Use direction definitions The direction of a thread should be explicitly compared to avoid confusion. Also fixup message wording based on this direction. Signed-off-by: Andrew F. Davis Change-Id: Ia3cf9413cd23af476bb5d2e6d70bee15234cbd11 --- plat/ti/k3/common/drivers/sec_proxy/sec_proxy.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/plat/ti/k3/common/drivers/sec_proxy/sec_proxy.c b/plat/ti/k3/common/drivers/sec_proxy/sec_proxy.c index 0cf296b57..ee1eecf78 100644 --- a/plat/ti/k3/common/drivers/sec_proxy/sec_proxy.c +++ b/plat/ti/k3/common/drivers/sec_proxy/sec_proxy.c @@ -138,7 +138,7 @@ static inline int k3_sec_proxy_verify_thread(struct k3_sec_proxy_thread *spt, /* Make sure thread is configured for right direction */ if ((mmio_read_32(spt->scfg + SCFG_THREAD_CTRL) & SCFG_THREAD_CTRL_DIR_MASK) != (dir << SCFG_THREAD_CTRL_DIR_SHIFT)) { - if (dir) + if (dir == THREAD_IS_TX) ERROR("Trying to send data on RX Thread %s\n", spt->name); else @@ -151,10 +151,12 @@ static inline int k3_sec_proxy_verify_thread(struct k3_sec_proxy_thread *spt, uint32_t tick_start = (uint32_t)read_cntpct_el0(); uint32_t ticks_per_us = SYS_COUNTER_FREQ_IN_TICKS / 1000000; while (!(mmio_read_32(spt->rt + RT_THREAD_STATUS) & RT_THREAD_STATUS_CUR_CNT_MASK)) { - VERBOSE("Waiting for thread %s to clear\n", spt->name); + VERBOSE("Waiting for thread %s to %s\n", + spt->name, (dir == THREAD_IS_TX) ? "empty" : "fill"); if (((uint32_t)read_cntpct_el0() - tick_start) > (spm.desc.timeout_us * ticks_per_us)) { - ERROR("Timeout waiting for thread %s to clear\n", spt->name); + ERROR("Timeout waiting for thread %s to %s\n", + spt->name, (dir == THREAD_IS_TX) ? "empty" : "fill"); return -ETIMEDOUT; } } From 7a469035e96b1e6d9d31d502ed326d53c698a137 Mon Sep 17 00:00:00 2001 From: "Andrew F. Davis" Date: Wed, 10 Apr 2019 11:49:40 -0400 Subject: [PATCH 3/4] ti: k3: drivers: ti_sci: Cleanup sequence ID usage The sequence ID can be set with a message to identify it when it is responded to in the response queue. We assign each message a number and check for this same number to detect response mismatches. Start this at 0 and increase it by one for each message sent, even ones that do not request or wait for a response as one may still be delivered in some cases and we want to detect this. Signed-off-by: Andrew F. Davis Change-Id: I72b4d1ef98bf1c1409d9db9db074af8dfbcd83ea --- plat/ti/k3/common/drivers/ti_sci/ti_sci.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/plat/ti/k3/common/drivers/ti_sci/ti_sci.c b/plat/ti/k3/common/drivers/ti_sci/ti_sci.c index df0b794f8..a376e6ea5 100644 --- a/plat/ti/k3/common/drivers/ti_sci/ti_sci.c +++ b/plat/ti/k3/common/drivers/ti_sci/ti_sci.c @@ -45,7 +45,6 @@ static struct ti_sci_info info = { .host_id = TI_SCI_HOST_ID, .max_msg_size = TI_SCI_MAX_MESSAGE_SIZE, }, - .seq = 0x0a, }; /** @@ -89,10 +88,8 @@ static int ti_sci_setup_one_xfer(uint16_t msg_type, uint32_t msg_flags, tx_message_size < sizeof(*hdr)) return -ERANGE; - info.seq++; - hdr = (struct ti_sci_msg_hdr *)tx_buf; - hdr->seq = info.seq; + hdr->seq = ++info.seq; hdr->type = msg_type; hdr->host = info.desc.host_id; hdr->flags = msg_flags | TI_SCI_FLAG_REQ_ACK_ON_PROCESSED; @@ -425,7 +422,7 @@ int ti_sci_device_put_no_wait(uint32_t id) return -ERANGE; hdr = (struct ti_sci_msg_hdr *)&req; - hdr->seq = info.seq; + hdr->seq = ++info.seq; hdr->type = TI_SCI_MSG_SET_DEVICE_STATE; hdr->host = info.desc.host_id; /* Setup with NORESPONSE flag to keep response queue clean */ @@ -1408,7 +1405,7 @@ int ti_sci_proc_set_boot_ctrl_no_wait(uint8_t proc_id, return -ERANGE; hdr = (struct ti_sci_msg_hdr *)&req; - hdr->seq = info.seq; + hdr->seq = ++info.seq; hdr->type = TISCI_MSG_SET_PROC_BOOT_CTRL; hdr->host = info.desc.host_id; /* Setup with NORESPONSE flag to keep response queue clean */ @@ -1650,7 +1647,7 @@ int ti_sci_proc_wait_boot_status_no_wait(uint8_t proc_id, return -ERANGE; hdr = (struct ti_sci_msg_hdr *)&req; - hdr->seq = info.seq; + hdr->seq = ++info.seq; hdr->type = TISCI_MSG_WAIT_PROC_BOOT_STATUS; hdr->host = info.desc.host_id; /* Setup with NORESPONSE flag to keep response queue clean */ From 71a35273130798aa3f19e0baf793aa598577c323 Mon Sep 17 00:00:00 2001 From: "Andrew F. Davis" Date: Wed, 10 Apr 2019 12:40:12 -0400 Subject: [PATCH 4/4] ti: k3: drivers: ti_sci: Retry message receive on bad sequence ID When we get a sequence ID that does not match what we expect then the we are looking at is not the one we are expecting and so we error out. We can also assume this message is a stale message left in the queue, in this case we can read in the next message and check again for our message. Switch to doing that here. We only retry a set number of times so we don't lock the system if our message is actually lost and will never show up. Signed-off-by: Andrew F. Davis Change-Id: I6c8186ccc45e646d3ba9d431f7d4c451dcd70c5c --- plat/ti/k3/common/drivers/ti_sci/ti_sci.c | 31 ++++++++++++++--------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/plat/ti/k3/common/drivers/ti_sci/ti_sci.c b/plat/ti/k3/common/drivers/ti_sci/ti_sci.c index a376e6ea5..ac33278a9 100644 --- a/plat/ti/k3/common/drivers/ti_sci/ti_sci.c +++ b/plat/ti/k3/common/drivers/ti_sci/ti_sci.c @@ -116,21 +116,28 @@ static inline int ti_sci_get_response(struct ti_sci_xfer *xfer, { struct k3_sec_proxy_msg *msg = &xfer->rx_message; struct ti_sci_msg_hdr *hdr; + unsigned int retry = 5; int ret; - /* Receive the response */ - ret = k3_sec_proxy_recv(chan, msg); - if (ret) { - ERROR("Message receive failed (%d)\n", ret); - return ret; + for (; retry > 0; retry--) { + /* Receive the response */ + ret = k3_sec_proxy_recv(chan, msg); + if (ret) { + ERROR("Message receive failed (%d)\n", ret); + return ret; + } + + /* msg is updated by Secure Proxy driver */ + hdr = (struct ti_sci_msg_hdr *)msg->buf; + + /* Sanity check for message response */ + if (hdr->seq == info.seq) + break; + else + WARN("Message with sequence ID %u is not expected\n", hdr->seq); } - - /* msg is updated by Secure Proxy driver */ - hdr = (struct ti_sci_msg_hdr *)msg->buf; - - /* Sanity check for message response */ - if (hdr->seq != info.seq) { - ERROR("Message for %d is not expected\n", hdr->seq); + if (!retry) { + ERROR("Timed out waiting for message\n"); return -EINVAL; }