From 0d423c4a78984dd02f6596d6fd9dd40446eec517 Mon Sep 17 00:00:00 2001 From: Alexey Romanov Date: Wed, 30 Aug 2023 17:08:50 +0300 Subject: [PATCH 1/2] drivers: meson: sm: correct meson_sm_* API retval handling 1. Following the ARM SMC32 calling convention, the return value from secure monitor is a 32-bit signed integer. This patch changes the type of the return value of the function meson_sm_call(). 2. Now, when meson_sm_call() returns a 32-bit signed integer, we need to ensure that this value is not negative. It is important to check that the return value is not negative in both the meson_sm_call_read() and meson_sm_call_write() functions. 3. Add a comment explaining why it is necessary to check if the SMC return value is equal to 0 in the function meson_sm_call_read(). It is not obvious when reading this code. Signed-off-by: Alexey Romanov Reviewed-by: Neil Armstrong Link: https://lore.kernel.org/r/20230830140850.17130-1-avromanov@salutedevices.com Signed-off-by: Neil Armstrong --- drivers/firmware/meson/meson_sm.c | 20 +++++++++++++------- include/linux/firmware/meson/meson_sm.h | 2 +- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/drivers/firmware/meson/meson_sm.c b/drivers/firmware/meson/meson_sm.c index 9a2656d73600..53bf56e18743 100644 --- a/drivers/firmware/meson/meson_sm.c +++ b/drivers/firmware/meson/meson_sm.c @@ -67,7 +67,7 @@ static u32 meson_sm_get_cmd(const struct meson_sm_chip *chip, return cmd->smc_id; } -static u32 __meson_sm_call(u32 cmd, u32 arg0, u32 arg1, u32 arg2, +static s32 __meson_sm_call(u32 cmd, u32 arg0, u32 arg1, u32 arg2, u32 arg3, u32 arg4) { struct arm_smccc_res res; @@ -102,9 +102,10 @@ static void __iomem *meson_sm_map_shmem(u32 cmd_shmem, unsigned int size) * Return: 0 on success, a negative value on error */ int meson_sm_call(struct meson_sm_firmware *fw, unsigned int cmd_index, - u32 *ret, u32 arg0, u32 arg1, u32 arg2, u32 arg3, u32 arg4) + s32 *ret, u32 arg0, u32 arg1, u32 arg2, u32 arg3, u32 arg4) { - u32 cmd, lret; + u32 cmd; + s32 lret; if (!fw->chip) return -ENOENT; @@ -143,7 +144,7 @@ int meson_sm_call_read(struct meson_sm_firmware *fw, void *buffer, unsigned int bsize, unsigned int cmd_index, u32 arg0, u32 arg1, u32 arg2, u32 arg3, u32 arg4) { - u32 size; + s32 size; int ret; if (!fw->chip) @@ -158,11 +159,16 @@ int meson_sm_call_read(struct meson_sm_firmware *fw, void *buffer, if (meson_sm_call(fw, cmd_index, &size, arg0, arg1, arg2, arg3, arg4) < 0) return -EINVAL; - if (size > bsize) + if (size < 0 || size > bsize) return -EINVAL; ret = size; + /* In some cases (for example GET_CHIP_ID command), + * SMC doesn't return the number of bytes read, even + * though the bytes were actually read into sm_shmem_out. + * So this check is needed. + */ if (!size) size = bsize; @@ -192,7 +198,7 @@ int meson_sm_call_write(struct meson_sm_firmware *fw, void *buffer, unsigned int size, unsigned int cmd_index, u32 arg0, u32 arg1, u32 arg2, u32 arg3, u32 arg4) { - u32 written; + s32 written; if (!fw->chip) return -ENOENT; @@ -208,7 +214,7 @@ int meson_sm_call_write(struct meson_sm_firmware *fw, void *buffer, if (meson_sm_call(fw, cmd_index, &written, arg0, arg1, arg2, arg3, arg4) < 0) return -EINVAL; - if (!written) + if (written <= 0 || written > size) return -EINVAL; return written; diff --git a/include/linux/firmware/meson/meson_sm.h b/include/linux/firmware/meson/meson_sm.h index 95b0da2326a9..8eaf8922ab02 100644 --- a/include/linux/firmware/meson/meson_sm.h +++ b/include/linux/firmware/meson/meson_sm.h @@ -19,7 +19,7 @@ enum { struct meson_sm_firmware; int meson_sm_call(struct meson_sm_firmware *fw, unsigned int cmd_index, - u32 *ret, u32 arg0, u32 arg1, u32 arg2, u32 arg3, u32 arg4); + s32 *ret, u32 arg0, u32 arg1, u32 arg2, u32 arg3, u32 arg4); int meson_sm_call_write(struct meson_sm_firmware *fw, void *buffer, unsigned int b_size, unsigned int cmd_index, u32 arg0, u32 arg1, u32 arg2, u32 arg3, u32 arg4); From 5d59b6a49bb8f6841acf53affc0c4720fe7450e5 Mon Sep 17 00:00:00 2001 From: Rob Herring Date: Fri, 6 Oct 2023 17:46:44 -0500 Subject: [PATCH 2/2] firmware: meson: Use device_get_match_data() Use preferred device_get_match_data() instead of of_match_device() to get the driver match data. With this, adjust the includes to explicitly include the correct headers. Signed-off-by: Rob Herring Reviewed-by: Neil Armstrong Link: https://lore.kernel.org/r/20231006224644.445295-1-robh@kernel.org Signed-off-by: Neil Armstrong --- drivers/firmware/meson/meson_sm.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/firmware/meson/meson_sm.c b/drivers/firmware/meson/meson_sm.c index 53bf56e18743..ed60f1103053 100644 --- a/drivers/firmware/meson/meson_sm.c +++ b/drivers/firmware/meson/meson_sm.c @@ -13,9 +13,10 @@ #include #include #include -#include +#include #include #include +#include #include #include #include @@ -297,7 +298,7 @@ static int __init meson_sm_probe(struct platform_device *pdev) if (!fw) return -ENOMEM; - chip = of_match_device(meson_sm_ids, dev)->data; + chip = device_get_match_data(dev); if (!chip) return -EINVAL;