[PATCH v2 0/3] Random Number Generator fixes
This series aim to fix smccc bind issue and add a list command for RNG devices.
Changelog:
v1 --> v2 - check SMCCC_ARCH_FEATURES - update commit message and rng help info
Weizhao Ouyang (3): firmware: psci: Fix bind_smccc_features psci check driver: rng: Fix SMCCC TRNG crash cmd: rng: Add rng list command
cmd/rng.c | 23 ++++++++++++++++++----- drivers/firmware/psci.c | 9 ++++++++- drivers/rng/smccc_trng.c | 2 +- include/linux/arm-smccc.h | 6 ++++++ 4 files changed, 33 insertions(+), 7 deletions(-)
According to PSCI specification DEN0022F, PSCI_FEATURES is used to check whether the SMCCC is implemented by discovering SMCCC_VERSION.
Signed-off-by: Weizhao Ouyang o451686892@gmail.com --- v2: check SMCCC_ARCH_FEATURES --- drivers/firmware/psci.c | 9 ++++++++- include/linux/arm-smccc.h | 6 ++++++ 2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c index c6b9efab41..ed701cd1e4 100644 --- a/drivers/firmware/psci.c +++ b/drivers/firmware/psci.c @@ -135,7 +135,7 @@ static int bind_smccc_features(struct udevice *dev, int psci_method) PSCI_VERSION_MAJOR(psci_0_2_get_version()) == 0) return 0;
- if (request_psci_features(ARM_SMCCC_ARCH_FEATURES) == + if (request_psci_features(ARM_SMCCC_VERSION) == PSCI_RET_NOT_SUPPORTED) return 0;
@@ -144,6 +144,13 @@ static int bind_smccc_features(struct udevice *dev, int psci_method) else pdata->invoke_fn = smccc_invoke_smc;
+ /* + * SMCCC_ARCH_FEATURES is MANDATORY from SMCCC v1.1, but we still remain + * the invoke_fn() even the SMCCC version is v1.0. + */ + if (invoke_psci_fn(ARM_SMCCC_VERSION, 0, 0, 0) < ARM_SMCCC_VERSION_1_1) + return 0; + feature_cnt = ll_entry_count(struct arm_smccc_feature, arm_smccc_feature); feature = ll_entry_start(struct arm_smccc_feature, arm_smccc_feature);
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h index f44e9e8f93..da3d29aabe 100644 --- a/include/linux/arm-smccc.h +++ b/include/linux/arm-smccc.h @@ -55,8 +55,14 @@ #define ARM_SMCCC_QUIRK_NONE 0 #define ARM_SMCCC_QUIRK_QCOM_A6 1 /* Save/restore register a6 */
+#define ARM_SMCCC_VERSION 0x80000000 #define ARM_SMCCC_ARCH_FEATURES 0x80000001
+#define ARM_SMCCC_VERSION_1_0 0x10000 +#define ARM_SMCCC_VERSION_1_1 0x10001 +#define ARM_SMCCC_VERSION_1_2 0x10002 +#define ARM_SMCCC_VERSION_1_3 0x10003 + #define ARM_SMCCC_RET_NOT_SUPPORTED ((unsigned long)-1)
#ifndef __ASSEMBLY__
On 31.01.24 12:12, Weizhao Ouyang wrote:
According to PSCI specification DEN0022F, PSCI_FEATURES is used to check whether the SMCCC is implemented by discovering SMCCC_VERSION.
Signed-off-by: Weizhao Ouyang o451686892@gmail.com
v2: check SMCCC_ARCH_FEATURES
drivers/firmware/psci.c | 9 ++++++++- include/linux/arm-smccc.h | 6 ++++++ 2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c index c6b9efab41..ed701cd1e4 100644 --- a/drivers/firmware/psci.c +++ b/drivers/firmware/psci.c @@ -135,7 +135,7 @@ static int bind_smccc_features(struct udevice *dev, int psci_method) PSCI_VERSION_MAJOR(psci_0_2_get_version()) == 0) return 0;
- if (request_psci_features(ARM_SMCCC_ARCH_FEATURES) ==
- if (request_psci_features(ARM_SMCCC_VERSION) == PSCI_RET_NOT_SUPPORTED) return 0;
@@ -144,6 +144,13 @@ static int bind_smccc_features(struct udevice *dev, int psci_method) else pdata->invoke_fn = smccc_invoke_smc;
- /*
* SMCCC_ARCH_FEATURES is MANDATORY from SMCCC v1.1, but we still remain* the invoke_fn() even the SMCCC version is v1.0.
This sentence is a bit hard to understand. Did you mean,
"but we keep calling invoke_fn() even if the SMCC version is v1.0" ?
*/- if (invoke_psci_fn(ARM_SMCCC_VERSION, 0, 0, 0) < ARM_SMCCC_VERSION_1_1)
return 0;
Why do we leave the function for invoke_psci_fn(ARM_SMCCC_VERSION, 0, 0, 0) == ARM_SMCCC_VERSION_1_0 despite the comment above?
Best regards
Heinrich
- feature_cnt = ll_entry_count(struct arm_smccc_feature, arm_smccc_feature); feature = ll_entry_start(struct arm_smccc_feature, arm_smccc_feature);
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h index f44e9e8f93..da3d29aabe 100644 --- a/include/linux/arm-smccc.h +++ b/include/linux/arm-smccc.h @@ -55,8 +55,14 @@ #define ARM_SMCCC_QUIRK_NONE 0 #define ARM_SMCCC_QUIRK_QCOM_A6 1 /* Save/restore register a6 */
+#define ARM_SMCCC_VERSION 0x80000000 #define ARM_SMCCC_ARCH_FEATURES 0x80000001
+#define ARM_SMCCC_VERSION_1_0 0x10000 +#define ARM_SMCCC_VERSION_1_1 0x10001 +#define ARM_SMCCC_VERSION_1_2 0x10002 +#define ARM_SMCCC_VERSION_1_3 0x10003
#define ARM_SMCCC_RET_NOT_SUPPORTED ((unsigned long)-1)
#ifndef __ASSEMBLY__
On Wed, Jan 31, 2024 at 8:27 PM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 31.01.24 12:12, Weizhao Ouyang wrote:
According to PSCI specification DEN0022F, PSCI_FEATURES is used to check whether the SMCCC is implemented by discovering SMCCC_VERSION.
Signed-off-by: Weizhao Ouyang o451686892@gmail.com
v2: check SMCCC_ARCH_FEATURES
drivers/firmware/psci.c | 9 ++++++++- include/linux/arm-smccc.h | 6 ++++++ 2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c index c6b9efab41..ed701cd1e4 100644 --- a/drivers/firmware/psci.c +++ b/drivers/firmware/psci.c @@ -135,7 +135,7 @@ static int bind_smccc_features(struct udevice *dev, int psci_method) PSCI_VERSION_MAJOR(psci_0_2_get_version()) == 0) return 0;
if (request_psci_features(ARM_SMCCC_ARCH_FEATURES) ==
if (request_psci_features(ARM_SMCCC_VERSION) == PSCI_RET_NOT_SUPPORTED) return 0;@@ -144,6 +144,13 @@ static int bind_smccc_features(struct udevice *dev, int psci_method) else pdata->invoke_fn = smccc_invoke_smc;
/** SMCCC_ARCH_FEATURES is MANDATORY from SMCCC v1.1, but we still remain* the invoke_fn() even the SMCCC version is v1.0.This sentence is a bit hard to understand. Did you mean,
"but we keep calling invoke_fn() even if the SMCC version is v1.0" ?
*/if (invoke_psci_fn(ARM_SMCCC_VERSION, 0, 0, 0) < ARM_SMCCC_VERSION_1_1)return 0;Why do we leave the function for invoke_psci_fn(ARM_SMCCC_VERSION, 0, 0, 0) == ARM_SMCCC_VERSION_1_0 despite the comment above?
Previously I was trying to leave the driver a fallback SMC call for SMCCC v1.0, but since the arm-ffa driver not use the legacy SMC invoke function and also we can directly call invoke_psci_fn(), so maybe we can drop this fallback function. Will update in v3.
BR, Weizhao
Best regards
Heinrich
feature_cnt = ll_entry_count(struct arm_smccc_feature, arm_smccc_feature); feature = ll_entry_start(struct arm_smccc_feature, arm_smccc_feature);diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h index f44e9e8f93..da3d29aabe 100644 --- a/include/linux/arm-smccc.h +++ b/include/linux/arm-smccc.h @@ -55,8 +55,14 @@ #define ARM_SMCCC_QUIRK_NONE 0 #define ARM_SMCCC_QUIRK_QCOM_A6 1 /* Save/restore register a6 */
+#define ARM_SMCCC_VERSION 0x80000000 #define ARM_SMCCC_ARCH_FEATURES 0x80000001
+#define ARM_SMCCC_VERSION_1_0 0x10000 +#define ARM_SMCCC_VERSION_1_1 0x10001 +#define ARM_SMCCC_VERSION_1_2 0x10002 +#define ARM_SMCCC_VERSION_1_3 0x10003
#define ARM_SMCCC_RET_NOT_SUPPORTED ((unsigned long)-1)
#ifndef __ASSEMBLY__
Fix a SMCCC TRNG null pointer crash due to a failed smccc feature binding.
Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de Signed-off-by: Weizhao Ouyang o451686892@gmail.com --- drivers/rng/smccc_trng.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/rng/smccc_trng.c b/drivers/rng/smccc_trng.c index 3a4bb33941..3087cb991a 100644 --- a/drivers/rng/smccc_trng.c +++ b/drivers/rng/smccc_trng.c @@ -166,7 +166,7 @@ static int smccc_trng_probe(struct udevice *dev) struct smccc_trng_priv *priv = dev_get_priv(dev); struct arm_smccc_res res;
- if (!(smccc_trng_is_supported(smccc->invoke_fn))) + if (!smccc || !(smccc_trng_is_supported(smccc->invoke_fn))) return -ENODEV;
/* At least one of 64bit and 32bit interfaces is available */
Etienne should have been on the CC list. He's in ./scripts/get_maintainer.pl so I'm not sure what went wrong there... I've added him.
On Wed, Jan 31, 2024 at 11:12:16AM +0000, Weizhao Ouyang wrote:
Fix a SMCCC TRNG null pointer crash due to a failed smccc feature binding.
Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de Signed-off-by: Weizhao Ouyang o451686892@gmail.com
drivers/rng/smccc_trng.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/rng/smccc_trng.c b/drivers/rng/smccc_trng.c index 3a4bb33941..3087cb991a 100644 --- a/drivers/rng/smccc_trng.c +++ b/drivers/rng/smccc_trng.c @@ -166,7 +166,7 @@ static int smccc_trng_probe(struct udevice *dev) struct smccc_trng_priv *priv = dev_get_priv(dev); struct arm_smccc_res res;
- if (!(smccc_trng_is_supported(smccc->invoke_fn)))
- if (!smccc || !(smccc_trng_is_supported(smccc->invoke_fn)))
To me it seems a bit strange that dm_priv_to_rw() can return NULL... Anyway, probably the Fixes tag should point to when the driver was added.
Fixes: 53355bb86c25 ("drivers: rng: add smccc trng driver")
regards, dan carpenter
On Wed, Jan 31, 2024 at 02:48:58PM +0300, Dan Carpenter wrote:
Etienne should have been on the CC list. He's in ./scripts/get_maintainer.pl so I'm not sure what went wrong there... I've added him.
Hm... Etienne's @linaro.org address bounces. Apparently MAINTAINERS needs to be updated.
regards, dan carpenter
On Wed, Jan 31, 2024 at 11:12:16AM +0000, Weizhao Ouyang wrote:
Fix a SMCCC TRNG null pointer crash due to a failed smccc feature binding.
Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de Signed-off-by: Weizhao Ouyang o451686892@gmail.com
drivers/rng/smccc_trng.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/rng/smccc_trng.c b/drivers/rng/smccc_trng.c index 3a4bb33941..3087cb991a 100644 --- a/drivers/rng/smccc_trng.c +++ b/drivers/rng/smccc_trng.c @@ -166,7 +166,7 @@ static int smccc_trng_probe(struct udevice *dev) struct smccc_trng_priv *priv = dev_get_priv(dev); struct arm_smccc_res res;
- if (!(smccc_trng_is_supported(smccc->invoke_fn)))
- if (!smccc || !(smccc_trng_is_supported(smccc->invoke_fn)))
To me it seems a bit strange that dm_priv_to_rw() can return NULL... Anyway, probably the Fixes tag should point to when the driver was added.
Fixes: 53355bb86c25 ("drivers: rng: add smccc trng driver")
regards, dan carpenter
+cc Etienne
On Wed, 31 Jan 2024 at 13:49, Dan Carpenter dan.carpenter@linaro.org wrote:
Etienne should have been on the CC list. He's in ./scripts/get_maintainer.pl so I'm not sure what went wrong there... I've added him.
On Wed, Jan 31, 2024 at 11:12:16AM +0000, Weizhao Ouyang wrote:
Fix a SMCCC TRNG null pointer crash due to a failed smccc feature binding.
Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de Signed-off-by: Weizhao Ouyang o451686892@gmail.com
drivers/rng/smccc_trng.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/rng/smccc_trng.c b/drivers/rng/smccc_trng.c index 3a4bb33941..3087cb991a 100644 --- a/drivers/rng/smccc_trng.c +++ b/drivers/rng/smccc_trng.c @@ -166,7 +166,7 @@ static int smccc_trng_probe(struct udevice *dev) struct smccc_trng_priv *priv = dev_get_priv(dev); struct arm_smccc_res res;
if (!(smccc_trng_is_supported(smccc->invoke_fn)))
if (!smccc || !(smccc_trng_is_supported(smccc->invoke_fn)))To me it seems a bit strange that dm_priv_to_rw() can return NULL... Anyway, probably the Fixes tag should point to when the driver was added.
Fixes: 53355bb86c25 ("drivers: rng: add smccc trng driver")
regards, dan carpenter
The 'rng list' command probes all RNG devices and list those devices that are successfully probed. Also update the help info.
Signed-off-by: Weizhao Ouyang o451686892@gmail.com --- v2: update commit message and rng help info --- cmd/rng.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/cmd/rng.c b/cmd/rng.c index 52f722c7af..b073a6c849 100644 --- a/cmd/rng.c +++ b/cmd/rng.c @@ -19,6 +19,22 @@ static int do_rng(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) struct udevice *dev; int ret = CMD_RET_SUCCESS;
+ if (argc == 2 && !strcmp(argv[1], "list")) { + int idx = 0; + + uclass_foreach_dev_probe(UCLASS_RNG, dev) { + idx++; + printf("RNG #%d - %s\n", dev->seq_, dev->name); + } + + if (!idx) { + log_err("No RNG device\n"); + return CMD_RET_FAILURE; + } + + return CMD_RET_SUCCESS; + } + switch (argc) { case 1: devnum = 0; @@ -56,12 +72,9 @@ static int do_rng(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) return ret; }
-U_BOOT_LONGHELP(rng, - "[dev [n]]\n" - " - print n random bytes(max 64) read from dev\n"); - U_BOOT_CMD( rng, 3, 0, do_rng, "print bytes from the hardware random number generator", - rng_help_text + "list - list all the probed rng devices\n" + "rng [dev] [n] - print n random bytes(max 64) read from dev\n" );
On 31.01.24 12:12, Weizhao Ouyang wrote:
The 'rng list' command probes all RNG devices and list those devices that are successfully probed. Also update the help info.
Signed-off-by: Weizhao Ouyang o451686892@gmail.com
v2: update commit message and rng help info
Thanks for updating.
Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de
cmd/rng.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-)
diff --git a/cmd/rng.c b/cmd/rng.c index 52f722c7af..b073a6c849 100644 --- a/cmd/rng.c +++ b/cmd/rng.c @@ -19,6 +19,22 @@ static int do_rng(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) struct udevice *dev; int ret = CMD_RET_SUCCESS;
- if (argc == 2 && !strcmp(argv[1], "list")) {
int idx = 0;uclass_foreach_dev_probe(UCLASS_RNG, dev) {idx++;printf("RNG #%d - %s\n", dev->seq_, dev->name);}if (!idx) {log_err("No RNG device\n");return CMD_RET_FAILURE;}return CMD_RET_SUCCESS;- }
- switch (argc) { case 1: devnum = 0;
@@ -56,12 +72,9 @@ static int do_rng(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) return ret; }
-U_BOOT_LONGHELP(rng,
- "[dev [n]]\n"
- " - print n random bytes(max 64) read from dev\n");
- U_BOOT_CMD( rng, 3, 0, do_rng, "print bytes from the hardware random number generator",
- rng_help_text
- "list - list all the probed rng devices\n"
- "rng [dev] [n] - print n random bytes(max 64) read from dev\n" );
participants (4)
-
Dan Carpenter -
Heinrich Schuchardt -
Ilias Apalodimas -
Weizhao Ouyang