[PATCH 0/3] Random Number Generator fixes
This series aim to fix smccc bind issue and add a list command for RNG devices.
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 | 15 +++++++++++++++ drivers/firmware/psci.c | 2 +- drivers/rng/smccc_trng.c | 2 +- include/linux/arm-smccc.h | 1 + 4 files changed, 18 insertions(+), 2 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 --- drivers/firmware/psci.c | 2 +- include/linux/arm-smccc.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c index c6b9efab41..894be8128d 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;
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h index f44e9e8f93..453017eb5f 100644 --- a/include/linux/arm-smccc.h +++ b/include/linux/arm-smccc.h @@ -55,6 +55,7 @@ #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_RET_NOT_SUPPORTED ((unsigned long)-1)
On 25.01.24 15:05, 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
5.15.2 Implementation responsibilities
PSCI_FEATURES should report the presence of SMCCC_VERSION in privileged platform firmware compliant with PSCI v1.0 and later revisions.
Acked-by: Heinrich Schuchardt xypron.glpk@gmx.de
drivers/firmware/psci.c | 2 +- include/linux/arm-smccc.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c index c6b9efab41..894be8128d 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;
diff --git a/include/linux/arm-smccc.h b/include/linux/arm-smccc.h index f44e9e8f93..453017eb5f 100644 --- a/include/linux/arm-smccc.h +++ b/include/linux/arm-smccc.h @@ -55,6 +55,7 @@ #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_RET_NOT_SUPPORTED ((unsigned long)-1)
Hi Weizhao,
On Thu, Jan 25, 2024 at 02:05:00PM +0000, 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
drivers/firmware/psci.c | 2 +- include/linux/arm-smccc.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c index c6b9efab41..894be8128d 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;
I suggest we add checks on the SMCCC version like this:
if SMCCC_VERSION >= 1.1 assume SMCCC_ARCH_FEATURES is supported discover arch features and bind drivers/devices else error
For more details [1][2].
[1]: SMC Calling Convention 1.5 (https://developer.arm.com/documentation/den0028/latest/)
SMCCC_ARCH_FEATURES MANDATORY from SMCCC v1.1
The implementation of this function can be detected by checking the SMCCC version. This function is mandatory if SMCCC_VERSION indicates that version 1.1 or later is implemented.
[2]: Linux checks SMCCC v1.1: https://elixir.bootlin.com/linux/latest/source/drivers/firmware/psci/psci.c#...
Cheers, Abdellatif
Hi Abdellatif,
On Fri, Jan 26, 2024 at 7:20 PM Abdellatif El Khlifi abdellatif.elkhlifi@arm.com wrote:
Hi Weizhao,
On Thu, Jan 25, 2024 at 02:05:00PM +0000, 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
drivers/firmware/psci.c | 2 +- include/linux/arm-smccc.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c index c6b9efab41..894be8128d 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;I suggest we add checks on the SMCCC version like this:
if SMCCC_VERSION >= 1.1 assume SMCCC_ARCH_FEATURES is supported discover arch features and bind drivers/devices else error
Okay, I will add it.
BR, Weizhao
For more details [1][2].
[1]: SMC Calling Convention 1.5 (https://developer.arm.com/documentation/den0028/latest/)
SMCCC_ARCH_FEATURES MANDATORY from SMCCC v1.1
The implementation of this function can be detected by checking the SMCCC version. This function is mandatory if SMCCC_VERSION indicates that version 1.1 or later is implemented.
[2]: Linux checks SMCCC v1.1: https://elixir.bootlin.com/linux/latest/source/drivers/firmware/psci/psci.c#...
Cheers, Abdellatif
Fix a SMCCC TRNG null pointer crash due to a failed smccc feature binding.
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 */
On 25.01.24 15:05, Weizhao Ouyang wrote:
Fix a SMCCC TRNG null pointer crash due to a failed smccc feature binding.
Signed-off-by: Weizhao Ouyang o451686892@gmail.com
Reviewed-by: Heinrich Schuchardt xypron.gpk@gmx.de
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 */
On 25.01.24 15:05, Weizhao Ouyang wrote:
Fix a SMCCC TRNG null pointer crash due to a failed smccc feature binding.
Signed-off-by: Weizhao Ouyang o451686892@gmail.com
Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de
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 */
Add rng list command to list all probed RNG device.
Signed-off-by: Weizhao Ouyang o451686892@gmail.com --- cmd/rng.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/cmd/rng.c b/cmd/rng.c index 52f722c7af..4818133f94 100644 --- a/cmd/rng.c +++ b/cmd/rng.c @@ -18,6 +18,19 @@ static int do_rng(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) int devnum; struct udevice *dev; int ret = CMD_RET_SUCCESS; + int idx; + + if (argc == 2 && !strcmp(argv[1], "list")) { + uclass_foreach_dev_probe(UCLASS_RNG, dev) { + printf("RNG #%d - %s\n", dev->seq_, dev->name); + } + + if (!idx) { + printf("*** no RNG devices available ***\n"); + return CMD_RET_FAILURE; + } + return CMD_RET_SUCCESS; + }
switch (argc) { case 1: @@ -57,6 +70,8 @@ static int do_rng(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) }
U_BOOT_LONGHELP(rng, + "[list]\n" + " - list all the probe rng device\n" "[dev [n]]\n" " - print n random bytes(max 64) read from dev\n");
On 25.01.24 15:05, Weizhao Ouyang wrote:
Add rng list command to list all probed RNG device.
Thank you for your contribution.
Would the following be more accurate?
The 'rng list' command probes all RNG devices and list those devices that are successfully probed.
Signed-off-by: Weizhao Ouyang o451686892@gmail.com
cmd/rng.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/cmd/rng.c b/cmd/rng.c index 52f722c7af..4818133f94 100644 --- a/cmd/rng.c +++ b/cmd/rng.c @@ -18,6 +18,19 @@ static int do_rng(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) int devnum; struct udevice *dev; int ret = CMD_RET_SUCCESS;
- int idx;
Please, put the definition into the code block where it is used.
- if (argc == 2 && !strcmp(argv[1], "list")) {
int idx = 0;
uclass_foreach_dev_probe(UCLASS_RNG, dev) {printf("RNG #%d - %s\n", dev->seq_, dev->name);
++idx;
}if (!idx) {printf("*** no RNG devices available ***\n");
If idx is not initialized, depending on the random value of idx on the stack you get a printout or not, e.g.
=> rng list RNG #0 - riscv_zkr RNG #1 - virtio-rng#8 *** no RNG devices available ***
Do we need the message to be so noisy? How about
log_err("No RNG device\n");
return CMD_RET_FAILURE;}
We prefer having an empty line before return statements.
Best regards
Heinrich
return CMD_RET_SUCCESS;}
switch (argc) { case 1:
@@ -57,6 +70,8 @@ static int do_rng(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) }
U_BOOT_LONGHELP(rng,
- "[list]\n"
- " - list all the probe rng device\n" "[dev [n]]\n" " - print n random bytes(max 64) read from dev\n");
On Thu, Jan 25, 2024 at 10:49 PM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 25.01.24 15:05, Weizhao Ouyang wrote:
Add rng list command to list all probed RNG device.
Thank you for your contribution.
Would the following be more accurate?
The 'rng list' command probes all RNG devices and list those devices that are successfully probed.
Yeah.
Signed-off-by: Weizhao Ouyang o451686892@gmail.com
cmd/rng.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+)
diff --git a/cmd/rng.c b/cmd/rng.c index 52f722c7af..4818133f94 100644 --- a/cmd/rng.c +++ b/cmd/rng.c @@ -18,6 +18,19 @@ static int do_rng(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) int devnum; struct udevice *dev; int ret = CMD_RET_SUCCESS;
int idx;Please, put the definition into the code block where it is used.
Sorry I imported the draft code, will remove it in the next version.
if (argc == 2 && !strcmp(argv[1], "list")) {int idx = 0;
uclass_foreach_dev_probe(UCLASS_RNG, dev) {printf("RNG #%d - %s\n", dev->seq_, dev->name);++idx;
}if (!idx) {printf("*** no RNG devices available ***\n");If idx is not initialized, depending on the random value of idx on the stack you get a printout or not, e.g.
=> rng list RNG #0 - riscv_zkr RNG #1 - virtio-rng#8 *** no RNG devices available ***
Do we need the message to be so noisy? How about
log_err("No RNG device\n");
return CMD_RET_FAILURE;}We prefer having an empty line before return statements.
Best regards
Heinrich
return CMD_RET_SUCCESS;} switch (argc) { case 1:@@ -57,6 +70,8 @@ static int do_rng(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) }
U_BOOT_LONGHELP(rng,
"[list]\n"" - list all the probe rng device\n" "[dev [n]]\n" " - print n random bytes(max 64) read from dev\n");
BR, Weizhao
participants (3)
-
Abdellatif El Khlifi -
Heinrich Schuchardt -
Weizhao Ouyang