Re: [PATCH] arm: crypto: add a check for crypto extensions support.

2025-08-28 Thread Ahmad Fatoum
Hi,

On 27.08.25 17:53, [email protected] wrote:
> From: Chali Anis 
> 
> In some configuration the CPU may raise an exception bacause of an
> unknown instruction if it does not support Crypto Extensions for
> example in some BCM281X (RPi3B in my case) when running barebox
> as an EFI Payload, where the EFI stops with a synchronous execption
> See bellow:
> Synchronous Exception at 0x37BFF548
> SP 0x37F798C0 ELR 0x37BFF548
> SPSR 0x2209 FPSR 0x
> ESR 0x0200 FAR 0x14F64325185430BF
> ESR : EC 0x00 IL 0x1 ISS 0x

Thanks for your patch. It's good to have the check, but are you sure
you don't have the ARMv8.0 Crypto Extensions?

arm_cpu_lowlevel_init() enables SIMD, so maybe not calling that
is the reason you got the crash?

For EFI payloads in particular, I don't think we should call
the whole arm_cpu_lowlevel_init(), but maybe there is an argument to be made
that FP/SIMD at least should be enabled. I am not sure if that alone
could lead to issues affecting the EFI loader though.

Cheers,
Ahmad

> 
> Signed-off-by: Chali Anis 
> ---
>  arch/arm/crypto/sha1-ce-glue.c | 7 +++
>  arch/arm/crypto/sha2-ce-glue.c | 9 +++--
>  arch/arm/include/asm/sysreg.h  | 6 ++
>  3 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/crypto/sha1-ce-glue.c b/arch/arm/crypto/sha1-ce-glue.c
> index 5b49237573fa..fea15f295a00 100644
> --- a/arch/arm/crypto/sha1-ce-glue.c
> +++ b/arch/arm/crypto/sha1-ce-glue.c
> @@ -14,6 +14,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  MODULE_DESCRIPTION("SHA1 secure hash using ARMv8 Crypto Extensions");
>  MODULE_AUTHOR("Ard Biesheuvel ");
> @@ -88,6 +89,12 @@ static struct digest_algo m = {
>  
>  static int sha1_ce_mod_init(void)
>  {
> + uint64_t isar0;
> +
> + isar0 = read_sysreg(ID_AA64ISAR0_EL1);
> + if (!(isar0 & ID_AA64ISAR0_EL1_SHA1_MASK))
> + return -EOPNOTSUPP;
> +
>   return digest_algo_register(&m);
>  }
>  coredevice_initcall(sha1_ce_mod_init);
> diff --git a/arch/arm/crypto/sha2-ce-glue.c b/arch/arm/crypto/sha2-ce-glue.c
> index 88cbc7993dac..8479b3c60cb3 100644
> --- a/arch/arm/crypto/sha2-ce-glue.c
> +++ b/arch/arm/crypto/sha2-ce-glue.c
> @@ -14,8 +14,7 @@
>  #include 
>  #include 
>  #include 
> -
> -#include 
> +#include 
>  
>  MODULE_DESCRIPTION("SHA-224/SHA-256 secure hash using ARMv8 Crypto 
> Extensions");
>  MODULE_AUTHOR("Ard Biesheuvel ");
> @@ -116,6 +115,12 @@ static struct digest_algo sha256 = {
>  
>  static int sha256_ce_digest_register(void)
>  {
> + uint64_t isar0;
> +
> + isar0 = read_sysreg(ID_AA64ISAR0_EL1);
> + if (!(isar0 & ID_AA64ISAR0_EL1_SHA2_MASK))
> + return -EOPNOTSUPP;
> +
>   return digest_algo_register(&sha256);
>  }
>  coredevice_initcall(sha256_ce_digest_register);
> diff --git a/arch/arm/include/asm/sysreg.h b/arch/arm/include/asm/sysreg.h
> index 7d567e08d8b7..6f47016e54e8 100644
> --- a/arch/arm/include/asm/sysreg.h
> +++ b/arch/arm/include/asm/sysreg.h
> @@ -12,6 +12,12 @@
>  #include 
>  #include 
>  
> +/*
> + *   ARM64 registers
> + */
> +#define ID_AA64ISAR0_EL1_SHA1_MASK  0xF00UL
> +#define ID_AA64ISAR0_EL1_SHA2_MASK  0xF000UL
> +
>  /*
>   * Unlike read_cpuid, calls to read_sysreg are never expected to be
>   * optimized away or replaced with synthetic values.


-- 
Pengutronix e.K.   | |
Steuerwalder Str. 21   | http://www.pengutronix.de/  |
31137 Hildesheim, Germany  | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |



Re: [PATCH] arm: crypto: add a check for crypto extensions support.

2025-08-28 Thread Sascha Hauer


On Wed, 27 Aug 2025 11:53:07 -0400, [email protected] wrote:
> In some configuration the CPU may raise an exception bacause of an
> unknown instruction if it does not support Crypto Extensions for
> example in some BCM281X (RPi3B in my case) when running barebox
> as an EFI Payload, where the EFI stops with a synchronous execption
> See bellow:
> Synchronous Exception at 0x37BFF548
> SP 0x37F798C0 ELR 0x37BFF548
> SPSR 0x2209 FPSR 0x
> ESR 0x0200 FAR 0x14F64325185430BF
> ESR : EC 0x00 IL 0x1 ISS 0x
> 
> [...]

Applied, thanks!

[1/1] arm: crypto: add a check for crypto extensions support.
  https://git.pengutronix.de/cgit/barebox/commit/?id=20d75a6f932f (link may 
not be stable)

Best regards,
-- 
Sascha Hauer 




Re: [PATCH] arm: crypto: add a check for crypto extensions support.

2025-08-27 Thread Sascha Hauer
Hi,

On Wed, Aug 27, 2025 at 02:06:06AM -0400, [email protected] wrote:
> From: Chali Anis 
> 
> In some configuration the CPU may raise an exception bacause of an
> unknown instruction if it does not support Crypto Extensions for
> example in some BCM281X (RPi3B in my case) when running barebox
> as an EFI Payload, where the EFI stops with a synchronous execption
> See bellow:
> Synchronous Exception at 0x37BFF548
> SP 0x37F798C0 ELR 0x37BFF548
> SPSR 0x2209 FPSR 0x
> ESR 0x0200 FAR 0x14F64325185430BF
> ESR : EC 0x00 IL 0x1 ISS 0x
> 
> Signed-off-by: Chali Anis 
> ---
>  arch/arm/crypto/sha1-ce-glue.c | 6 ++
>  arch/arm/crypto/sha2-ce-glue.c | 6 ++
>  2 files changed, 12 insertions(+)
> 
> diff --git a/arch/arm/crypto/sha1-ce-glue.c b/arch/arm/crypto/sha1-ce-glue.c
> index 5b49237573fa..3c5213774429 100644
> --- a/arch/arm/crypto/sha1-ce-glue.c
> +++ b/arch/arm/crypto/sha1-ce-glue.c
> @@ -88,6 +88,12 @@ static struct digest_algo m = {
>  
>  static int sha1_ce_mod_init(void)
>  {
> + uint64_t isar0;
> +
> + asm volatile("mrs %0, ID_AA64ISAR0_EL1" : "=r"(isar0));

You could use read_sysreg():

#include 

isar0 = read_sysreg(ID_AA64ISAR0_EL1);

> + if (!(isar0 & 0xF00))
> + return -EOPNOTSUPP;

defines for this bitmask would be cool. I just looked at the kernel and
it seems a bit overkill for barebox, but maybe something like

#define ID_AA64ISAR0_EL1_SHA1_MASK  GENMASK(11, 8)
#define ID_AA64ISAR0_EL1_SHA2_MASK  GENMASK(15, 12)

in asm/sysreg.h would do it as a start.

Sascha

-- 
Pengutronix e.K.   | |
Steuerwalder Str. 21   | http://www.pengutronix.de/  |
31137 Hildesheim, Germany  | Phone: +49-5121-206917-0|
Amtsgericht Hildesheim, HRA 2686   | Fax:   +49-5121-206917- |