[PATCH v3 0/3] Random Number Generator fixes

[PATCH v3 0/3] Random Number Generator fixes

[Weizhao Ouyang]

This series aim to fix smccc bind issue and add a list command for RNG
devices.

Changelog:

v2 --> v3
- remove fallback smc call
- add Fixes tag

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   |  5 ++++-
 drivers/rng/smccc_trng.c  |  2 +-
 include/linux/arm-smccc.h |  6 ++++++
 4 files changed, 29 insertions(+), 7 deletions(-)

-- 
2.39.2

[PATCH v3 1/3] firmware: psci: Fix bind_smccc_features psci check

[Weizhao Ouyang]

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>
---
v3: remove fallback smc call
v2: check SMCCC_ARCH_FEATURES
---
 drivers/firmware/psci.c   | 5 ++++-
 include/linux/arm-smccc.h | 6 ++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
index c6b9efab41..03544d76ed 100644
--- a/drivers/firmware/psci.c
+++ b/drivers/firmware/psci.c
@@ -135,10 +135,13 @@ 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;
 
+	if (invoke_psci_fn(ARM_SMCCC_VERSION, 0, 0, 0) < ARM_SMCCC_VERSION_1_1)
+		return 0;
+
 	if (psci_method == PSCI_METHOD_HVC)
 		pdata->invoke_fn = smccc_invoke_hvc;
 	else
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__
-- 
2.39.2

[PATCH v3 2/3] driver: rng: Fix SMCCC TRNG crash

[Weizhao Ouyang]

Fix a SMCCC TRNG null pointer crash due to a failed smccc feature
binding.

Fixes: 53355bb86c25 ("drivers: rng: add smccc trng driver")
Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Signed-off-by: Weizhao Ouyang <o451686892@gmail.com>
---
v3: add Fixes tag
---
 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 */
-- 
2.39.2

[PATCH v3 3/3] cmd: rng: Add rng list command

[Weizhao Ouyang]

The 'rng list' command probes all RNG devices and list those devices
that are successfully probed. Also update the help info.

Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Signed-off-by: Weizhao Ouyang <o451686892@gmail.com>
---
 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"
 );
-- 
2.39.2

Re: [PATCH v3 2/3] driver: rng: Fix SMCCC TRNG crash

[Matthias Brugger]

On Wed, Jan 31, 2024 at 02:14:25PM +0000, Weizhao Ouyang wrote:
> Fix a SMCCC TRNG null pointer crash due to a failed smccc feature
> binding.
> 
> Fixes: 53355bb86c25 ("drivers: rng: add smccc trng driver")
> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Signed-off-by: Weizhao Ouyang <o451686892@gmail.com>

Reviewed-by: Matthias Brugger <mbrugger@suse.com>

> ---
> v3: add Fixes tag
> ---
>  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 */
> -- 
> 2.39.2

Re: [PATCH v3 3/3] cmd: rng: Add rng list command

[Matthias Brugger]

On Wed, Jan 31, 2024 at 02:14:26PM +0000, Weizhao Ouyang wrote:
> The 'rng list' command probes all RNG devices and list those devices
> that are successfully probed. Also update the help info.
> 
> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Signed-off-by: Weizhao Ouyang <o451686892@gmail.com>

Reviewed-by: Matthias Brugger <mbrugger@suse.com>

> ---
>  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"
>  );
> -- 
> 2.39.2

Re: [PATCH v3 1/3] firmware: psci: Fix bind_smccc_features psci check

[Abdellatif El Khlifi]

Hi Weizhao,

> -	if (request_psci_features(ARM_SMCCC_ARCH_FEATURES) ==
> +	if (request_psci_features(ARM_SMCCC_VERSION) ==
>  	    PSCI_RET_NOT_SUPPORTED)
>  		return 0;
>  
> +	if (invoke_psci_fn(ARM_SMCCC_VERSION, 0, 0, 0) < ARM_SMCCC_VERSION_1_1)
> +		return 0;

It makes sense to me, thanks.

> 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

Apart from ARM_SMCCC_VERSION_1_1, are the other ARM_SMCCC_VERSION_1_x defines needed ?

Cheers,
Abdellatif

Re: [PATCH v3 3/3] cmd: rng: Add rng list command

[Igor Opaniuk]

Reviewed-by: Igor Opaniuk <igor.opaniuk@foundries.io>

On Wed, Jan 31, 2024 at 3:15 PM Weizhao Ouyang <o451686892@gmail.com> wrote:
>
> The 'rng list' command probes all RNG devices and list those devices
> that are successfully probed. Also update the help info.
>
> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Signed-off-by: Weizhao Ouyang <o451686892@gmail.com>
> ---
>  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"
>  );
> --
> 2.39.2
>


-- 
Best regards - Freundliche Grüsse - Meilleures salutations

Igor Opaniuk
Senior Software Engineer, Embedded & Security
E: igor.opaniuk@foundries.io
W: www.foundries.io

Re: [PATCH v3 1/3] firmware: psci: Fix bind_smccc_features psci check

[Igor Opaniuk]

Hello Weizhao,

On Wed, Jan 31, 2024 at 3:15 PM Weizhao Ouyang <o451686892@gmail.com> wrote:
>
> According to PSCI specification DEN0022F, PSCI_FEATURES is used to check
> whether the SMCCC is implemented by discovering SMCCC_VERSION.
>
Could you please add more details to your commit message or as a comment
explaining what exact steps should be done for a full discovery sequence of Arm
Architecture Service functions, so people don't need to search for
that information explicitly?

For instance:
Step 1: Determine if SMCCC_VERSION is implemented
- Use firmware data, device tree PSCI node, or ACPI FADT PSCI flag, to
determine whether a
PSCI implementation is present.
- Use PSCI_VERSION to learn whether PSCI_FEATURES is provided. PSCI_FEATURES is
mandatory from version 1.0 of PSCI
Step 2. etc.

I would just pull that info from the latest SMC Calling Convention version 1.5,
from 9 Appendix B: Discovery of Arm Architecture Service functions.

Thank you!
> Signed-off-by: Weizhao Ouyang <o451686892@gmail.com>
> ---
> v3: remove fallback smc call
> v2: check SMCCC_ARCH_FEATURES
> ---
>  drivers/firmware/psci.c   | 5 ++++-
>  include/linux/arm-smccc.h | 6 ++++++
>  2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
> index c6b9efab41..03544d76ed 100644
> --- a/drivers/firmware/psci.c
> +++ b/drivers/firmware/psci.c
> @@ -135,10 +135,13 @@ 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;
>
> +       if (invoke_psci_fn(ARM_SMCCC_VERSION, 0, 0, 0) < ARM_SMCCC_VERSION_1_1)
> +               return 0;
> +
IMO, to fully comply with SMC Calling Convention version 1.5
we should also check for SMCCC_ARCH_WORKAROUND_1:

From 9 Appendix B: Discovery of Arm Architecture Service functions,
Step 2: Determine if Arm Architecture Service function is implemented
- Use SMCCC_VERSION to learn whether the calling convention complies
to version 1.1 or above.
- Use SMCCC_ARCH_FEATURES to query whether the Arm Architecture
Service function is implemented
on this system <--- we lack of this step

>         if (psci_method == PSCI_METHOD_HVC)
>                 pdata->invoke_fn = smccc_invoke_hvc;
>         else
> 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__
> --
> 2.39.2
>


-- 
Best regards - Freundliche Grüsse - Meilleures salutations

Igor Opaniuk
Senior Software Engineer, Embedded & Security
E: igor.opaniuk@foundries.io
W: www.foundries.io

Re: [PATCH v3 1/3] firmware: psci: Fix bind_smccc_features psci check

[Weizhao Ouyang]

Hi Abdellatif,

On Thu, Feb 1, 2024 at 7:40 PM Abdellatif El Khlifi
<abdellatif.elkhlifi@arm.com> wrote:
>
> Hi Weizhao,
>
> > -     if (request_psci_features(ARM_SMCCC_ARCH_FEATURES) ==
> > +     if (request_psci_features(ARM_SMCCC_VERSION) ==
> >           PSCI_RET_NOT_SUPPORTED)
> >               return 0;
> >
> > +     if (invoke_psci_fn(ARM_SMCCC_VERSION, 0, 0, 0) < ARM_SMCCC_VERSION_1_1)
> > +             return 0;
>
> It makes sense to me, thanks.
>
> > 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
>
> Apart from ARM_SMCCC_VERSION_1_1, are the other ARM_SMCCC_VERSION_1_x defines needed ?

I'm trying to synchronize with linux kernel, it might be a bit odd to
add only one version.

BR,
Weizhao

>
> Cheers,
> Abdellatif

Re: [PATCH v3 1/3] firmware: psci: Fix bind_smccc_features psci check

[Weizhao Ouyang]

Hi Igor,

On Thu, Feb 1, 2024 at 10:36 PM Igor Opaniuk <igor.opaniuk@foundries.io> wrote:
>
> Hello Weizhao,
>
> On Wed, Jan 31, 2024 at 3:15 PM Weizhao Ouyang <o451686892@gmail.com> wrote:
> >
> > According to PSCI specification DEN0022F, PSCI_FEATURES is used to check
> > whether the SMCCC is implemented by discovering SMCCC_VERSION.
> >
> Could you please add more details to your commit message or as a comment
> explaining what exact steps should be done for a full discovery sequence of Arm
> Architecture Service functions, so people don't need to search for
> that information explicitly?
>
> For instance:
> Step 1: Determine if SMCCC_VERSION is implemented
> - Use firmware data, device tree PSCI node, or ACPI FADT PSCI flag, to
> determine whether a
> PSCI implementation is present.
> - Use PSCI_VERSION to learn whether PSCI_FEATURES is provided. PSCI_FEATURES is
> mandatory from version 1.0 of PSCI
> Step 2. etc.
>
> I would just pull that info from the latest SMC Calling Convention version 1.5,
> from 9 Appendix B: Discovery of Arm Architecture Service functions.
>
> Thank you!
> > Signed-off-by: Weizhao Ouyang <o451686892@gmail.com>
> > ---
> > v3: remove fallback smc call
> > v2: check SMCCC_ARCH_FEATURES
> > ---
> >  drivers/firmware/psci.c   | 5 ++++-
> >  include/linux/arm-smccc.h | 6 ++++++
> >  2 files changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
> > index c6b9efab41..03544d76ed 100644
> > --- a/drivers/firmware/psci.c
> > +++ b/drivers/firmware/psci.c
> > @@ -135,10 +135,13 @@ 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;
> >
> > +       if (invoke_psci_fn(ARM_SMCCC_VERSION, 0, 0, 0) < ARM_SMCCC_VERSION_1_1)
> > +               return 0;
> > +
> IMO, to fully comply with SMC Calling Convention version 1.5
> we should also check for SMCCC_ARCH_WORKAROUND_1:
>
> From 9 Appendix B: Discovery of Arm Architecture Service functions,
> Step 2: Determine if Arm Architecture Service function is implemented
> - Use SMCCC_VERSION to learn whether the calling convention complies
> to version 1.1 or above.
> - Use SMCCC_ARCH_FEATURES to query whether the Arm Architecture
> Service function is implemented
> on this system <--- we lack of this step

Thanks for your review. The 9 Appendix B describes an approach to
discovery the maximize ability without causing unsafe behavior on
existing platforms. Regarding the second step, it is just using
SMCCC_ARCH_WORKAROUND_1 as an example to test SMCCC_ARCH_FEATURES.

For the U-Boot case, we can revisit this from two perspectives:
1. SMCCC_ARCH_FEATURES is MANDATORY from SMCCC v1.1.
2. SMCCC_VERSION is OPTIONAL for SMCCC v1.0, so we should consider a
safe behavior.
3. U-Boot is using CONFIG_ARM_SMCCC_FEATURES to probe and bind SMCCC
service driver if this driver is reported as supported.
4. U-Boot SMCCC service driver can embed its discovery process in
is_supported() callback.

So now we can choose the following approach in U-Boot:
- Use firmware data (check "arm,psci-1.0") to determine whether a PSCI
implementation is present.
- Use PSCI_VERSION (psci_0_2_get_version() major version >= 0) to learn
whether PSCI_FEATURES is provided.
- Use PSCI_FEATURES with the SMCCC_VERSION Function Identifier as a
parameter to determine that SMCCC_VERSION is implemented.
- Use SMCCC_VERSION to learn whether the calling convention complies to
version 1.1 or above.
- Trying to probe and bind SMCCC service driver.

BR,
Weizhao

>
> >         if (psci_method == PSCI_METHOD_HVC)
> >                 pdata->invoke_fn = smccc_invoke_hvc;
> >         else
> > 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__
> > --
> > 2.39.2
> >
>
>
> --
> Best regards - Freundliche Grüsse - Meilleures salutations
>
> Igor Opaniuk
> Senior Software Engineer, Embedded & Security
> E: igor.opaniuk@foundries.io
> W: www.foundries.io

Re: [PATCH v3 1/3] firmware: psci: Fix bind_smccc_features psci check

[Igor Opaniuk]

Hello Weizhao,

On Fri, Feb 2, 2024 at 4:43 AM Weizhao Ouyang <o451686892@gmail.com> wrote:
>
> Hi Igor,
>
> On Thu, Feb 1, 2024 at 10:36 PM Igor Opaniuk <igor.opaniuk@foundries.io> wrote:
> >
> > Hello Weizhao,
> >
> > On Wed, Jan 31, 2024 at 3:15 PM Weizhao Ouyang <o451686892@gmail.com> wrote:
> > >
> > > According to PSCI specification DEN0022F, PSCI_FEATURES is used to check
> > > whether the SMCCC is implemented by discovering SMCCC_VERSION.
> > >
> > Could you please add more details to your commit message or as a comment
> > explaining what exact steps should be done for a full discovery sequence of Arm
> > Architecture Service functions, so people don't need to search for
> > that information explicitly?
> >
> > For instance:
> > Step 1: Determine if SMCCC_VERSION is implemented
> > - Use firmware data, device tree PSCI node, or ACPI FADT PSCI flag, to
> > determine whether a
> > PSCI implementation is present.
> > - Use PSCI_VERSION to learn whether PSCI_FEATURES is provided. PSCI_FEATURES is
> > mandatory from version 1.0 of PSCI
> > Step 2. etc.
> >
> > I would just pull that info from the latest SMC Calling Convention version 1.5,
> > from 9 Appendix B: Discovery of Arm Architecture Service functions.
> >
> > Thank you!
> > > Signed-off-by: Weizhao Ouyang <o451686892@gmail.com>
> > > ---
> > > v3: remove fallback smc call
> > > v2: check SMCCC_ARCH_FEATURES
> > > ---
> > >  drivers/firmware/psci.c   | 5 ++++-
> > >  include/linux/arm-smccc.h | 6 ++++++
> > >  2 files changed, 10 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/firmware/psci.c b/drivers/firmware/psci.c
> > > index c6b9efab41..03544d76ed 100644
> > > --- a/drivers/firmware/psci.c
> > > +++ b/drivers/firmware/psci.c
> > > @@ -135,10 +135,13 @@ 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;
> > >
> > > +       if (invoke_psci_fn(ARM_SMCCC_VERSION, 0, 0, 0) < ARM_SMCCC_VERSION_1_1)
> > > +               return 0;
> > > +
> > IMO, to fully comply with SMC Calling Convention version 1.5
> > we should also check for SMCCC_ARCH_WORKAROUND_1:
> >
> > From 9 Appendix B: Discovery of Arm Architecture Service functions,
> > Step 2: Determine if Arm Architecture Service function is implemented
> > - Use SMCCC_VERSION to learn whether the calling convention complies
> > to version 1.1 or above.
> > - Use SMCCC_ARCH_FEATURES to query whether the Arm Architecture
> > Service function is implemented
> > on this system <--- we lack of this step
>
> Thanks for your review. The 9 Appendix B describes an approach to
> discovery the maximize ability without causing unsafe behavior on
> existing platforms. Regarding the second step, it is just using
> SMCCC_ARCH_WORKAROUND_1 as an example to test SMCCC_ARCH_FEATURES.
>
> For the U-Boot case, we can revisit this from two perspectives:
> 1. SMCCC_ARCH_FEATURES is MANDATORY from SMCCC v1.1.
> 2. SMCCC_VERSION is OPTIONAL for SMCCC v1.0, so we should consider a
> safe behavior.
> 3. U-Boot is using CONFIG_ARM_SMCCC_FEATURES to probe and bind SMCCC
> service driver if this driver is reported as supported.
> 4. U-Boot SMCCC service driver can embed its discovery process in
> is_supported() callback.
>
> So now we can choose the following approach in U-Boot:
> - Use firmware data (check "arm,psci-1.0") to determine whether a PSCI
> implementation is present.
> - Use PSCI_VERSION (psci_0_2_get_version() major version >= 0) to learn
> whether PSCI_FEATURES is provided.
> - Use PSCI_FEATURES with the SMCCC_VERSION Function Identifier as a
> parameter to determine that SMCCC_VERSION is implemented.
> - Use SMCCC_VERSION to learn whether the calling convention complies to
> version 1.1 or above.
> - Trying to probe and bind SMCCC service driver.

Thanks for the detailed explanation!
Reviewed-by: Igor Opaniuk <igor.opaniuk@foundries.io>

>
> BR,
> Weizhao
>
> >
> > >         if (psci_method == PSCI_METHOD_HVC)
> > >                 pdata->invoke_fn = smccc_invoke_hvc;
> > >         else
> > > 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__
> > > --
> > > 2.39.2
> > >
> >
> >
> > --
> > Best regards - Freundliche Grüsse - Meilleures salutations
> >
> > Igor Opaniuk
> > Senior Software Engineer, Embedded & Security
> > E: igor.opaniuk@foundries.io
> > W: www.foundries.io



-- 
Best regards - Freundliche Grüsse - Meilleures salutations

Igor Opaniuk
Senior Software Engineer, Embedded & Security
E: igor.opaniuk@foundries.io
W: www.foundries.io

Re: [PATCH v3 3/3] cmd: rng: Add rng list command

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 597 bytes --]

On Wed, Jan 31, 2024 at 02:14:26PM +0000, Weizhao Ouyang wrote:

> The 'rng list' command probes all RNG devices and list those devices
> that are successfully probed. Also update the help info.
> 
> Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Signed-off-by: Weizhao Ouyang <o451686892@gmail.com>
> Reviewed-by: Matthias Brugger <mbrugger@suse.com>
> Reviewed-by: Igor Opaniuk <igor.opaniuk@foundries.io>
> ---
>  cmd/rng.c | 23 ++++++++++++++++++-----
>  1 file changed, 18 insertions(+), 5 deletions(-)

Please update doc/usage/cmd/rng.rst as well, thanks.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v3 3/3] cmd: rng: Add rng list command

[Weizhao Ouyang]

Hi Tom,

On Tue, Feb 6, 2024 at 2:14 AM Tom Rini <trini@konsulko.com> wrote:
>
> On Wed, Jan 31, 2024 at 02:14:26PM +0000, Weizhao Ouyang wrote:
>
> > The 'rng list' command probes all RNG devices and list those devices
> > that are successfully probed. Also update the help info.
> >
> > Reviewed-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> > Signed-off-by: Weizhao Ouyang <o451686892@gmail.com>
> > Reviewed-by: Matthias Brugger <mbrugger@suse.com>
> > Reviewed-by: Igor Opaniuk <igor.opaniuk@foundries.io>
> > ---
> >  cmd/rng.c | 23 ++++++++++++++++++-----
> >  1 file changed, 18 insertions(+), 5 deletions(-)
>
> Please update doc/usage/cmd/rng.rst as well, thanks.

Ok, I will update it.

BR,
Weizhao

>
> --
> Tom