[PATCH v5 1/2] firmware: zynqmp: fix write to an uninitialised pointer in xilinx_pm_request

When a caller is not interested in the returned message, the ret_payload pointer is set to NULL in the u-boot-sources. In this case, under EL3, the memory from address 0x0 would be overwritten by xilinx_pm_request with the returned IPI message, damaging the original data under this address. The patch, in case ret_payload is NULL, assigns the pointer to the array holding the IPI message being sent.
Signed-off-by: Adrian Fiergolski adrian.fiergolski@fastree3d.com --- Fix casting of ret (ingore v4). drivers/firmware/firmware-zynqmp.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c index d4dc856baf..8273437dd9 100644 --- a/drivers/firmware/firmware-zynqmp.c +++ b/drivers/firmware/firmware-zynqmp.c @@ -154,6 +154,8 @@ U_BOOT_DRIVER(zynqmp_power) = { int __maybe_unused xilinx_pm_request(u32 api_id, u32 arg0, u32 arg1, u32 arg2, u32 arg3, u32 *ret_payload) { + int ret; + debug("%s at EL%d, API ID: 0x%0x\n", __func__, current_el(), api_id);
if (IS_ENABLED(CONFIG_SPL_BUILD) || current_el() == 3) { @@ -165,6 +167,12 @@ int __maybe_unused xilinx_pm_request(u32 api_id, u32 arg0, u32 arg1, u32 arg2, */ u32 regs[] = {api_id, arg0, arg1, arg2, arg3};
+ /* + * Use regs array in case ret_payload is NULL + */ + if (ret_payload == NULL) + ret_payload = regs; + if (api_id == PM_FPGA_LOAD) { /* Swap addr_hi/low because of incompatibility */ u32 temp = regs[1]; @@ -174,6 +182,8 @@ int __maybe_unused xilinx_pm_request(u32 api_id, u32 arg0, u32 arg1, u32 arg2, }
ipi_req(regs, PAYLOAD_ARG_CNT, ret_payload, PAYLOAD_ARG_CNT); + + ret = (int)ret_payload[0]; #else return -EPERM; #endif @@ -198,8 +208,9 @@ int __maybe_unused xilinx_pm_request(u32 api_id, u32 arg0, u32 arg1, u32 arg2, ret_payload[4] = (u32)regs.regs[2]; }
+ ret = (ret_payload) ? ret_payload[0] : 0; } - return (ret_payload) ? ret_payload[0] : 0; + return ret; }
static const struct udevice_id zynqmp_firmware_ids[] = {

This patch fixes xilinx_pm_request to return a valid status from xilinx_pm_request in EL2.
Signed-off-by: Adrian Fiergolski adrian.fiergolski@fastree3d.com --- drivers/firmware/firmware-zynqmp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c index 8273437dd9..34ad7fb985 100644 --- a/drivers/firmware/firmware-zynqmp.c +++ b/drivers/firmware/firmware-zynqmp.c @@ -208,7 +208,7 @@ int __maybe_unused xilinx_pm_request(u32 api_id, u32 arg0, u32 arg1, u32 arg2, ret_payload[4] = (u32)regs.regs[2]; }
- ret = (ret_payload) ? ret_payload[0] : 0; + ret = (int)regs.regs[0]; } return ret; }

On 10/14/21 14:43, Adrian Fiergolski wrote:
This patch fixes xilinx_pm_request to return a valid status from xilinx_pm_request in EL2.
Signed-off-by: Adrian Fiergolski adrian.fiergolski@fastree3d.com
drivers/firmware/firmware-zynqmp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/firmware/firmware-zynqmp.c b/drivers/firmware/firmware-zynqmp.c index 8273437dd9..34ad7fb985 100644 --- a/drivers/firmware/firmware-zynqmp.c +++ b/drivers/firmware/firmware-zynqmp.c @@ -208,7 +208,7 @@ int __maybe_unused xilinx_pm_request(u32 api_id, u32 arg0, u32 arg1, u32 arg2, ret_payload[4] = (u32)regs.regs[2]; }
ret = (ret_payload) ? ret_payload[0] : 0;
} return ret; }ret = (int)regs.regs[0];
I have tested this series and our expectation that regs.regs[0] is return value is not unfortunately correct. mmio_read/write are not aligned to it. And when you try to boot you will see hang without any message.
I sent 2 patches as suggestion how we could do it via ipi_req(). Please take a look at it and let me know your opinion.
Thanks, Michal
participants (2)
-
Adrian Fiergolski
-
Michal Simek