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

From: Dimi Tomov <dimi@tpm.dev>
To: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Ben Hutchings <ben.hutchings@mind.be>, buildroot@buildroot.org
Subject: Re: [Buildroot] [PATCH] package/wolfssl: Disable broken asm implementations on 32-bit Arm
Date: Mon, 01 Aug 2022 21:53:18 +0300	[thread overview]
Message-ID: <b4e1a3f6777f709531b2619e0136a971@tpm.dev> (raw)
In-Reply-To: <20220722225120.2472bbab@windsurf>

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