[Buildroot] [PATCH] package/wolfssl: Disable broken asm implementations on 32-bit Arm

[Buildroot] [PATCH] package/wolfssl: Disable broken asm implementations on 32-bit Arm

[Ben Hutchings]

wolfSSL has ARMv8-A assembly implementations of some functions for
both A64 and A32 ISAs.  However, some of the A32 versions use r11,
which is usually not allowed:

wolfcrypt/src/port/arm/armv8-aes.c: In function 'wc_AesCbcEncrypt':
wolfcrypt/src/port/arm/armv8-aes.c:3303:5: error: fp cannot be used in 'asm' here
 3303 |     }
      |     ^

That can be fixed by adding the compiler flag -fomit-frame-pointer,
but then there is another failure:

/tmp/ccV19DQV.s: Assembler messages:
/tmp/ccV19DQV.s:248: Error: first transfer register must be even -- `ldrd r11,r10,[r14,#4*14]'
make[3]: *** [Makefile:5858: wolfcrypt/src/port/arm/src_libwolfssl_la-armv8-chacha.lo] Error 1

This is definitely not a valid instruction in A32, which suggests that
this code isn't being tested at all upstream.  So disable it here.

Signed-off-by: Ben Hutchings <ben.hutchings@mind.be>
---
 package/wolfssl/wolfssl.mk | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/package/wolfssl/wolfssl.mk b/package/wolfssl/wolfssl.mk
index 58b6f296ab..ff6a65c397 100644
--- a/package/wolfssl/wolfssl.mk
+++ b/package/wolfssl/wolfssl.mk
@@ -33,14 +33,12 @@ WOLFSSL_CONF_OPTS += --disable-sslv3
 endif
 
 # enable ARMv8 hardware acceleration
-ifeq ($(BR2_ARM_CPU_ARMV8A),y)
+ifeq ($(BR2_aarch64),y)
 WOLFSSL_CONF_OPTS += --enable-armasm
 # the flag -mstrict-align is needed to prevent build errors caused by
 # some inline assembly in parts of the AES structure using the "m"
 # constraint
-ifeq ($(BR2_aarch64),y)
 WOLFSSL_CONF_ENV += CPPFLAGS="$(TARGET_CPPFLAGS) -mstrict-align"
-endif
 else
 WOLFSSL_CONF_OPTS += --disable-armasm
 endif
-- 
2.30.2

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] package/wolfssl: Disable broken asm implementations on 32-bit Arm

[Thomas Petazzoni via buildroot]

Hello Dimi,

Could you give some feedback on the below patch that touches wolfssl?
Do me it looks good, but perhaps you want to improve something in
upstream wolfssl in relation to this issue, such as detecting the CPU
architecture capabilities automatically?

Best regards,

Thomas Petazzoni

On Tue, 14 Jun 2022 21:24:45 +0200
Ben Hutchings <ben.hutchings@mind.be> wrote:

> wolfSSL has ARMv8-A assembly implementations of some functions for
> both A64 and A32 ISAs.  However, some of the A32 versions use r11,
> which is usually not allowed:
> 
> wolfcrypt/src/port/arm/armv8-aes.c: In function 'wc_AesCbcEncrypt':
> wolfcrypt/src/port/arm/armv8-aes.c:3303:5: error: fp cannot be used in 'asm' here
>  3303 |     }
>       |     ^
> 
> That can be fixed by adding the compiler flag -fomit-frame-pointer,
> but then there is another failure:
> 
> /tmp/ccV19DQV.s: Assembler messages:
> /tmp/ccV19DQV.s:248: Error: first transfer register must be even -- `ldrd r11,r10,[r14,#4*14]'
> make[3]: *** [Makefile:5858: wolfcrypt/src/port/arm/src_libwolfssl_la-armv8-chacha.lo] Error 1
> 
> This is definitely not a valid instruction in A32, which suggests that
> this code isn't being tested at all upstream.  So disable it here.
> 
> Signed-off-by: Ben Hutchings <ben.hutchings@mind.be>
> ---
>  package/wolfssl/wolfssl.mk | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/package/wolfssl/wolfssl.mk b/package/wolfssl/wolfssl.mk
> index 58b6f296ab..ff6a65c397 100644
> --- a/package/wolfssl/wolfssl.mk
> +++ b/package/wolfssl/wolfssl.mk
> @@ -33,14 +33,12 @@ WOLFSSL_CONF_OPTS += --disable-sslv3
>  endif
>  
>  # enable ARMv8 hardware acceleration
> -ifeq ($(BR2_ARM_CPU_ARMV8A),y)
> +ifeq ($(BR2_aarch64),y)
>  WOLFSSL_CONF_OPTS += --enable-armasm
>  # the flag -mstrict-align is needed to prevent build errors caused by
>  # some inline assembly in parts of the AES structure using the "m"
>  # constraint
> -ifeq ($(BR2_aarch64),y)
>  WOLFSSL_CONF_ENV += CPPFLAGS="$(TARGET_CPPFLAGS) -mstrict-align"
> -endif
>  else
>  WOLFSSL_CONF_OPTS += --disable-armasm
>  endif



-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] package/wolfssl: Disable broken asm implementations on 32-bit Arm

[Dimi Tomov]

Hello Thomas,,

Sorry for my late response, it seems my email filters are not properly 
set for buildroot.

My domain expertise is with wolfTPM and the related wolfSSL 
capabilities, such as crypto callbacks, AES CFB, etc.

I can only assume that the optimization was designed for a specific SoC 
and later generalized.

Thanks,

Dimi

On 2022-07-22 11:51 PM, Thomas Petazzoni wrote:
> Hello Dimi,
> 
> Could you give some feedback on the below patch that touches wolfssl?
> Do me it looks good, but perhaps you want to improve something in
> upstream wolfssl in relation to this issue, such as detecting the CPU
> architecture capabilities automatically?
> 
> Best regards,
> 
> Thomas Petazzoni
> 
> On Tue, 14 Jun 2022 21:24:45 +0200
> Ben Hutchings <ben.hutchings@mind.be> wrote:
> 
>> wolfSSL has ARMv8-A assembly implementations of some functions for
>> both A64 and A32 ISAs.  However, some of the A32 versions use r11,
>> which is usually not allowed:
>> 
>> wolfcrypt/src/port/arm/armv8-aes.c: In function 'wc_AesCbcEncrypt':
>> wolfcrypt/src/port/arm/armv8-aes.c:3303:5: error: fp cannot be used in 
>> 'asm' here
>>  3303 |     }
>>       |     ^
>> 
>> That can be fixed by adding the compiler flag -fomit-frame-pointer,
>> but then there is another failure:
>> 
>> /tmp/ccV19DQV.s: Assembler messages:
>> /tmp/ccV19DQV.s:248: Error: first transfer register must be even -- 
>> `ldrd r11,r10,[r14,#4*14]'
>> make[3]: *** [Makefile:5858: 
>> wolfcrypt/src/port/arm/src_libwolfssl_la-armv8-chacha.lo] Error 1
>> 
>> This is definitely not a valid instruction in A32, which suggests that
>> this code isn't being tested at all upstream.  So disable it here.
>> 
>> Signed-off-by: Ben Hutchings <ben.hutchings@mind.be>
>> ---
>>  package/wolfssl/wolfssl.mk | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>> 
>> diff --git a/package/wolfssl/wolfssl.mk b/package/wolfssl/wolfssl.mk
>> index 58b6f296ab..ff6a65c397 100644
>> --- a/package/wolfssl/wolfssl.mk
>> +++ b/package/wolfssl/wolfssl.mk
>> @@ -33,14 +33,12 @@ WOLFSSL_CONF_OPTS += --disable-sslv3
>>  endif
>> 
>>  # enable ARMv8 hardware acceleration
>> -ifeq ($(BR2_ARM_CPU_ARMV8A),y)
>> +ifeq ($(BR2_aarch64),y)
>>  WOLFSSL_CONF_OPTS += --enable-armasm
>>  # the flag -mstrict-align is needed to prevent build errors caused by
>>  # some inline assembly in parts of the AES structure using the "m"
>>  # constraint
>> -ifeq ($(BR2_aarch64),y)
>>  WOLFSSL_CONF_ENV += CPPFLAGS="$(TARGET_CPPFLAGS) -mstrict-align"
>> -endif
>>  else
>>  WOLFSSL_CONF_OPTS += --disable-armasm
>>  endif

-- 
Founder of TPM.dev
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] package/wolfssl: Disable broken asm implementations on 32-bit Arm

[Thomas Petazzoni via buildroot]

Hello Ben,

On Tue, 14 Jun 2022 21:24:45 +0200
Ben Hutchings <ben.hutchings@mind.be> wrote:

> wolfSSL has ARMv8-A assembly implementations of some functions for
> both A64 and A32 ISAs.  However, some of the A32 versions use r11,
> which is usually not allowed:
> 
> wolfcrypt/src/port/arm/armv8-aes.c: In function 'wc_AesCbcEncrypt':
> wolfcrypt/src/port/arm/armv8-aes.c:3303:5: error: fp cannot be used in 'asm' here
>  3303 |     }
>       |     ^
> 
> That can be fixed by adding the compiler flag -fomit-frame-pointer,
> but then there is another failure:
> 
> /tmp/ccV19DQV.s: Assembler messages:
> /tmp/ccV19DQV.s:248: Error: first transfer register must be even -- `ldrd r11,r10,[r14,#4*14]'
> make[3]: *** [Makefile:5858: wolfcrypt/src/port/arm/src_libwolfssl_la-armv8-chacha.lo] Error 1
> 
> This is definitely not a valid instruction in A32, which suggests that
> this code isn't being tested at all upstream.  So disable it here.
> 
> Signed-off-by: Ben Hutchings <ben.hutchings@mind.be>
> ---
>  package/wolfssl/wolfssl.mk | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)

Thanks, I finally applied your patch, after adding a reference to the
autobuilder failure it is fixing. I also pushed two other patches to
fix other wolfssl build failures:

  https://gitlab.com/buildroot.org/buildroot/-/commit/36b8c9494b56b877fea62f17926f747c7c7bfb8d
  https://gitlab.com/buildroot.org/buildroot/-/commit/f79a9c775ff0a59027f274a237d98b5f8d31c022
  https://gitlab.com/buildroot.org/buildroot/-/commit/d8dc5315eb712eca0a5cbf793a6714a47ab6e57e

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] package/wolfssl: Disable broken asm implementations on 32-bit Arm

[Peter Korsgaard]

>>>>> "Thomas" == Thomas Petazzoni via buildroot <buildroot@buildroot.org> writes:

 > Hello Ben,
 > On Tue, 14 Jun 2022 21:24:45 +0200
 > Ben Hutchings <ben.hutchings@mind.be> wrote:

 >> wolfSSL has ARMv8-A assembly implementations of some functions for
 >> both A64 and A32 ISAs.  However, some of the A32 versions use r11,
 >> which is usually not allowed:
 >> 
 >> wolfcrypt/src/port/arm/armv8-aes.c: In function 'wc_AesCbcEncrypt':
 >> wolfcrypt/src/port/arm/armv8-aes.c:3303:5: error: fp cannot be used in 'asm' here
 >> 3303 |     }
 >> |     ^
 >> 
 >> That can be fixed by adding the compiler flag -fomit-frame-pointer,
 >> but then there is another failure:
 >> 
 >> /tmp/ccV19DQV.s: Assembler messages:
 >> /tmp/ccV19DQV.s:248: Error: first transfer register must be even -- `ldrd r11,r10,[r14,#4*14]'
 >> make[3]: *** [Makefile:5858: wolfcrypt/src/port/arm/src_libwolfssl_la-armv8-chacha.lo] Error 1
 >> 
 >> This is definitely not a valid instruction in A32, which suggests that
 >> this code isn't being tested at all upstream.  So disable it here.
 >> 
 >> Signed-off-by: Ben Hutchings <ben.hutchings@mind.be>
 >> ---
 >> package/wolfssl/wolfssl.mk | 4 +---
 >> 1 file changed, 1 insertion(+), 3 deletions(-)

 > Thanks, I finally applied your patch, after adding a reference to the
 > autobuilder failure it is fixing. I also pushed two other patches to
 > fix other wolfssl build failures:

 >   https://gitlab.com/buildroot.org/buildroot/-/commit/36b8c9494b56b877fea62f17926f747c7c7bfb8d
 >   https://gitlab.com/buildroot.org/buildroot/-/commit/f79a9c775ff0a59027f274a237d98b5f8d31c022
 >   https://gitlab.com/buildroot.org/buildroot/-/commit/d8dc5315eb712eca0a5cbf793a6714a47ab6e57e

All 3 committed to 2022.11.x and 2022.02.x, thanks.

-- 
Bye, Peter Korsgaard
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot