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

[Joel Stanley]

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
> >
>