[PATCH 3/5] config/ast2600: Disable hash hardware accel

[Steven Lee]

Hi Joel,

I was wondering if you could share the commit hash of u-boot you tested.
I would like to test it on qemu.

Thanks,
Steven

************* Email Confidentiality Notice ********************
DISCLAIMER:
This message (and any attachments) may contain legally privileged and/or other confidential information. If you have received it in error, please notify the sender by reply e-mail and immediately delete the e-mail and any attachments without copying or disclosing the contents. Thank you.

-----Original Message-----
From: Joel Stanley <joel at jms.id.au> 
Sent: Monday, June 27, 2022 3:40 PM
To: Cédric Le Goater <clg at kaod.org>
Cc: ChiaWei Wang <chiawei_wang at aspeedtech.com>; u-boot at lists.denx.de; Ryan Chen <ryan_chen at aspeedtech.com>; BMC-SW <BMC-SW at aspeedtech.com>
Subject: Re: [PATCH 3/5] config/ast2600: Disable hash hardware accel

On Mon, 27 Jun 2022 at 07:12, Cédric Le Goater <clg at kaod.org> wrote:
>
> Hello Chiawei,
>
> On 6/27/22 02:39, ChiaWei Wang wrote:
> > Reply again to leave record on mailing list.

Sorry, I re-sent it to get it on the list and managed to miss that for the second time.

> >
> >> From: joel.stan at gmail.com <joel.stan at gmail.com> On Behalf Of Joel 
> >> Stanley
> >> Sent: Friday, June 24, 2022 10:50 AM
> >>
> >> The Qemu model or the u-boot driver is unable to correctly compute 
> >> the
> >> SHA256 hash used in a FIT. Disable it by default while that issue 
> >> is worked out to enable boot testing in Qemu.
> >>
> >> Signed-off-by: Joel Stanley <joel at jms.id.au>
> >> ---
> >>   configs/evb-ast2600_defconfig | 3 ---
> >>   1 file changed, 3 deletions(-)
> >>
> >> diff --git a/configs/evb-ast2600_defconfig 
> >> b/configs/evb-ast2600_defconfig index f3a6cb222020..160bccff48e2 
> >> 100644
> >> --- a/configs/evb-ast2600_defconfig
> >> +++ b/configs/evb-ast2600_defconfig
> >> @@ -59,9 +59,6 @@ CONFIG_REGMAP=y
> >>   CONFIG_SPL_OF_TRANSLATE=y
> >>   CONFIG_CLK=y
> >>   CONFIG_SPL_CLK=y
> >> -CONFIG_DM_HASH=y
> >> -CONFIG_HASH_ASPEED=y
> >> -CONFIG_ASPEED_ACRY=y
> >
> > Per our previous discussion, SPL code size still exists if all of AST2600 features are upstream-ed.
> > Therefore, HW-assisted crypto drivers are needed.
> >
> > In addition, the current drivers works fine on real EVB to verify Hash + RSA signature (including the SHA256 in question).
> > This issue described in commit message should be attributed to incomplete QEMU emulation.
>
> When activating some debug in the hace driver :
>
>    U-Boot SPL 2022.07-rc5-dirty (Jun 27 2022 - 09:01:28 +0200)
>    already initialized, aspeed_2600_sdmc_write: SDMC is locked! (write to MCR04 blocked)
>    Trying to boot from RAM
>    ## Checking hash(es) for config conf-1 ... OK
>    ## Checking hash(es) for Image firmware-1 ... crc32Unsupported hash algorithm 'crc32'
>     error!
>    Unsupported hash algorithm for 'hash-1' hash node in 'firmware-1' 
> image node
>
> It seems the problem comes from the unsupported 'crc32' algo.
> See aspeed_hace_init().

Well spotted. It needs a fallback implementation of the algorithms the hash API supports but the hardware driver does not implement.

So we have three downsides of using the HACE driver:

 - Cannot DMA from SPI NOR, requiring a copy to RAM
 - Missing MD5 and CRC32 implementations, breaking the hash API
 - Broken Qemu emulation, meaning we cannot use it in OpenBMC as all commits will fail CI

Obviously we can fix or find workarounds for these issues. However I suggest while they are worked on we leave the HACE disabled in the defconfig, so we can have build coverage in u-boot CI.

Once Aspeed completes the upstreaming of its u-boot port and therefore hits the 64KB space limit, then we can look at re-enabling HACE in the defconfig. Hopefully by then you will have resolved the issues with the Qemu model.

Cheers,

Joel



>
> Thanks,
>
> C.
>
>
> >
> > Therefore, fixing QEMU should be the right way to go instead of disabling these options for real HW.
> >
> > Chiawei
> >
> >>   CONFIG_ASPEED_GPIO=y
> >>   CONFIG_DM_I2C=y
> >>   CONFIG_MISC=y
> >> --
> >> 2.35.1
> >
>