From 6173d914d673249ec47c080909c31a1654545913 Mon Sep 17 00:00:00 2001 From: Naman Trivedi Manojbhai Date: Tue, 7 Mar 2023 12:41:12 +0530 Subject: [PATCH] fix(xilinx): handle CRC failure in IPI callback Currently, if CRC validation fails during IPI communication, pm_ipi_buff_read_callb() logs error message but don't return error code to upper layers. Added CRC failure specific error code which will be returned by pm_ipi_buff_read_callb() if CRC validation fails. Signed-off-by: Naman Trivedi Manojbhai Change-Id: I2eaca073e2bf325a8c86b1820bdd7cca487b783e --- plat/xilinx/common/include/pm_ipi.h | 2 +- plat/xilinx/common/pm_service/pm_ipi.c | 11 ++++++++++- plat/xilinx/versal/pm_service/pm_api_sys.c | 10 +++++++--- plat/xilinx/versal/pm_service/pm_api_sys.h | 2 +- plat/xilinx/versal/pm_service/pm_svc_main.c | 19 +++++++++++++++++-- plat/xilinx/zynqmp/pm_service/pm_api_sys.c | 9 ++++++--- plat/xilinx/zynqmp/pm_service/pm_api_sys.h | 2 +- plat/xilinx/zynqmp/pm_service/pm_svc_main.c | 6 +++++- 8 files changed, 48 insertions(+), 13 deletions(-) diff --git a/plat/xilinx/common/include/pm_ipi.h b/plat/xilinx/common/include/pm_ipi.h index 52dfc4766..ede71dfe1 100644 --- a/plat/xilinx/common/include/pm_ipi.h +++ b/plat/xilinx/common/include/pm_ipi.h @@ -25,7 +25,7 @@ enum pm_ret_status pm_ipi_send_non_blocking(const struct pm_proc *proc, enum pm_ret_status pm_ipi_send_sync(const struct pm_proc *proc, uint32_t payload[PAYLOAD_ARG_CNT], uint32_t *value, size_t count); -void pm_ipi_buff_read_callb(uint32_t *value, size_t count); +enum pm_ret_status pm_ipi_buff_read_callb(uint32_t *value, size_t count); void pm_ipi_irq_enable(const struct pm_proc *proc); void pm_ipi_irq_clear(const struct pm_proc *proc); uint32_t pm_ipi_irq_status(const struct pm_proc *proc); diff --git a/plat/xilinx/common/pm_service/pm_ipi.c b/plat/xilinx/common/pm_service/pm_ipi.c index 81f1939f4..37d038423 100644 --- a/plat/xilinx/common/pm_service/pm_ipi.c +++ b/plat/xilinx/common/pm_service/pm_ipi.c @@ -188,17 +188,20 @@ static enum pm_ret_status pm_ipi_buff_read(const struct pm_proc *proc, * * This callback function fills requested data in @value from ipi response * buffer. + * @return Returns status, either success or error */ -void pm_ipi_buff_read_callb(uint32_t *value, size_t count) +enum pm_ret_status pm_ipi_buff_read_callb(uint32_t *value, size_t count) { size_t i; #if IPI_CRC_CHECK + uint32_t *payload_ptr = value; size_t j; unsigned int response_payload[PAYLOAD_ARG_CNT] = {0}; #endif uintptr_t buffer_base = IPI_BUFFER_REMOTE_BASE + IPI_BUFFER_TARGET_LOCAL_OFFSET + IPI_BUFFER_REQ_OFFSET; + enum pm_ret_status ret = PM_RET_SUCCESS; if (count > IPI_BUFFER_MAX_WORDS) { count = IPI_BUFFER_MAX_WORDS; @@ -218,8 +221,14 @@ void pm_ipi_buff_read_callb(uint32_t *value, size_t count) calculate_crc(response_payload, IPI_W0_TO_W6_SIZE)) { NOTICE("ERROR in CRC response payload value:0x%x\n", response_payload[PAYLOAD_CRC_POS]); + ret = PM_RET_ERROR_INVALID_CRC; + /* Payload data is invalid as CRC validation failed + * Clear the payload to avoid leakage of data to upper layers + */ + memset(payload_ptr, 0, count); } #endif + return ret; } /** diff --git a/plat/xilinx/versal/pm_service/pm_api_sys.c b/plat/xilinx/versal/pm_service/pm_api_sys.c index db9fae433..cc99f1197 100644 --- a/plat/xilinx/versal/pm_service/pm_api_sys.c +++ b/plat/xilinx/versal/pm_service/pm_api_sys.c @@ -196,19 +196,23 @@ enum pm_ret_status pm_req_wakeup(uint32_t target, uint32_t set_address, * 1 - Ack IPI after reading payload * * Read value from ipi buffer response buffer. + * @return Returns status, either success or error */ -void pm_get_callbackdata(uint32_t *data, size_t count, uint32_t flag, uint32_t ack) +enum pm_ret_status pm_get_callbackdata(uint32_t *data, size_t count, uint32_t flag, uint32_t ack) { + enum pm_ret_status ret = PM_RET_SUCCESS; /* Return if interrupt is not from PMU */ if (pm_ipi_irq_status(primary_proc) == 0) { - return; + return ret; } - pm_ipi_buff_read_callb(data, count); + ret = pm_ipi_buff_read_callb(data, count); if (ack != 0U) { pm_ipi_irq_clear(primary_proc); } + + return ret; } /** diff --git a/plat/xilinx/versal/pm_service/pm_api_sys.h b/plat/xilinx/versal/pm_service/pm_api_sys.h index c539aa743..8625e9568 100644 --- a/plat/xilinx/versal/pm_service/pm_api_sys.h +++ b/plat/xilinx/versal/pm_service/pm_api_sys.h @@ -38,7 +38,7 @@ enum pm_ret_status pm_req_wakeup(uint32_t target, uint32_t set_address, uintptr_t address, uint8_t ack, uint32_t flag); enum pm_ret_status pm_set_wakeup_source(uint32_t target, uint32_t device_id, uint8_t enable, uint32_t flag); -void pm_get_callbackdata(uint32_t *data, size_t count, uint32_t flag, +enum pm_ret_status pm_get_callbackdata(uint32_t *data, size_t count, uint32_t flag, uint32_t ack); enum pm_ret_status pm_pll_set_param(uint32_t clk_id, uint32_t param, uint32_t value, uint32_t flag); diff --git a/plat/xilinx/versal/pm_service/pm_svc_main.c b/plat/xilinx/versal/pm_service/pm_svc_main.c index c90f9e1c3..185bfdb65 100644 --- a/plat/xilinx/versal/pm_service/pm_svc_main.c +++ b/plat/xilinx/versal/pm_service/pm_svc_main.c @@ -48,12 +48,17 @@ static uint64_t ipi_fiq_handler(uint32_t id, uint32_t flags, void *handle, void *cookie) { uint32_t payload[4] = {0}; + enum pm_ret_status ret; VERBOSE("Received IPI FIQ from firmware\n"); (void)plat_ic_acknowledge_interrupt(); - pm_get_callbackdata(payload, ARRAY_SIZE(payload), 0, 0); + ret = pm_get_callbackdata(payload, ARRAY_SIZE(payload), 0, 0); + if (ret != PM_RET_SUCCESS) { + payload[0] = ret; + } + switch (payload[0]) { case PM_INIT_SUSPEND_CB: case PM_NOTIFY_CB: @@ -61,6 +66,11 @@ static uint64_t ipi_fiq_handler(uint32_t id, uint32_t flags, void *handle, notify_os(); } break; + case PM_RET_ERROR_INVALID_CRC: + pm_ipi_irq_clear(primary_proc); + WARN("Invalid CRC in the payload\n"); + break; + default: pm_ipi_irq_clear(primary_proc); WARN("Invalid IPI payload\n"); @@ -274,8 +284,13 @@ static uintptr_t TF_A_specific_handler(uint32_t api_id, uint32_t *pm_arg, case PM_GET_CALLBACK_DATA: { uint32_t result[4] = {0}; + enum pm_ret_status ret; + + ret = pm_get_callbackdata(result, ARRAY_SIZE(result), security_flag, 1U); + if (ret != 0) { + result[0] = ret; + } - pm_get_callbackdata(result, ARRAY_SIZE(result), security_flag, 1U); SMC_RET2(handle, (uint64_t)result[0] | ((uint64_t)result[1] << 32U), (uint64_t)result[2] | ((uint64_t)result[3] << 32U)); diff --git a/plat/xilinx/zynqmp/pm_service/pm_api_sys.c b/plat/xilinx/zynqmp/pm_service/pm_api_sys.c index 99594f756..58491a07a 100644 --- a/plat/xilinx/zynqmp/pm_service/pm_api_sys.c +++ b/plat/xilinx/zynqmp/pm_service/pm_api_sys.c @@ -687,16 +687,19 @@ enum pm_ret_status pm_aes_engine(uint32_t address_high, * @data - array of PAYLOAD_ARG_CNT elements * * Read value from ipi buffer response buffer. + * @return Returns status, either success or error */ -void pm_get_callbackdata(uint32_t *data, size_t count) +enum pm_ret_status pm_get_callbackdata(uint32_t *data, size_t count) { + enum pm_ret_status ret = PM_RET_SUCCESS; /* Return if interrupt is not from PMU */ if (!pm_ipi_irq_status(primary_proc)) { - return; + return ret; } - pm_ipi_buff_read_callb(data, count); + ret = pm_ipi_buff_read_callb(data, count); pm_ipi_irq_clear(primary_proc); + return ret; } /** diff --git a/plat/xilinx/zynqmp/pm_service/pm_api_sys.h b/plat/xilinx/zynqmp/pm_service/pm_api_sys.h index c4fe038df..1341e7b37 100644 --- a/plat/xilinx/zynqmp/pm_service/pm_api_sys.h +++ b/plat/xilinx/zynqmp/pm_service/pm_api_sys.h @@ -128,7 +128,7 @@ enum pm_ret_status pm_secure_rsaaes(uint32_t address_low, uint32_t size, uint32_t flags); uint32_t pm_get_shutdown_scope(void); -void pm_get_callbackdata(uint32_t *data, size_t count); +enum pm_ret_status pm_get_callbackdata(uint32_t *data, size_t count); enum pm_ret_status pm_ioctl(enum pm_node_id nid, uint32_t ioctl_id, uint32_t arg1, diff --git a/plat/xilinx/zynqmp/pm_service/pm_svc_main.c b/plat/xilinx/zynqmp/pm_service/pm_svc_main.c index c907773b4..b35859d5d 100644 --- a/plat/xilinx/zynqmp/pm_service/pm_svc_main.c +++ b/plat/xilinx/zynqmp/pm_service/pm_svc_main.c @@ -365,7 +365,11 @@ uint64_t pm_smc_handler(uint32_t smc_fid, uint64_t x1, uint64_t x2, uint64_t x3, SMC_RET1(handle, (uint64_t)ret); case PM_GET_CALLBACK_DATA: - pm_get_callbackdata(result, ARRAY_SIZE(result)); + ret = pm_get_callbackdata(result, ARRAY_SIZE(result)); + if (ret != PM_RET_SUCCESS) { + result[0] = ret; + } + SMC_RET2(handle, (uint64_t)result[0] | ((uint64_t)result[1] << 32), (uint64_t)result[2] | ((uint64_t)result[3] << 32)); -- 2.39.5