Re: [PATCH v6] hw/misc/aspeed_hace: Fix SG Accumulative hashing

[Joel Stanley]

On Wed, 23 Oct 2024 at 02:35, Cédric Le Goater <clg@redhat.com> wrote:
>
> On 10/22/24 13:54, Joel Stanley wrote:
> > On Wed, 16 Oct 2024 at 01:23, Jamin Lin <jamin_lin@aspeedtech.com> wrote:
> >
> >> 3. Test HACE model with u-boot hash command
> >> a. load test file to address 83000000 via tftp
> >> ast# tftp 83000000 jamin_lin/32MB
> >> b. get sha256
> >> ast# hash sha256 83000000 2000000
> >> sha256 for 83000000 ... 84ffffff ==> 
> >> 1ddcccdba742d762e2b8da0bceaf4778727c5eba54a24d7ae0c573c65414f736
> >> c. get sha384
> >> ast# hash sha384 83000000 2000000
> >> sha384 for 83000000 ... 84ffffff ==> 
> >> 825d9b24bb797695545b3cbd2f373b9738627c7a1878e620415570a57c7faed77916d47084c954254f101fc0f10c0591
> >> d. get sha512
> >> ast# hash sha512 83000000 2000000
> >> sha512 for 83000000 ... 84ffffff ==> 
> >> b5ae725b2dc1e521f48eae37dd82c3d5fc94f7acb5fff3dabf1caa4bb4b5bcfb498e7cc1fbaa97dda2664bff99f9f8e778f823e95afaf76fbf0985181522e478
> >
> > I attempted this same test and noticed that the 'hash' command was not
> > using the hardware. You can see this by putting some printf or
> > breakpoint in eg hw/misc/aspeed_hace.c do_hash_operation. There's some
> > missing work on the u-boot side to move the "hash" command over to the
> > hash uclass, so it can be used to test this code path (or add support
> > for the old API to the hace driver).
> >
> > Separately, I attempted to test with u-boot by enabling hash
> > verification of the FIT image, and it fails to calculate the correct
> > SHA.
> >
> > I think to have any confidence that this model works, we need to add
> > some testing to qemu. I did this for the initial version of the model
> > in tests/qtest/aspeed_hace-test.c.
>
> There are "accumulative mode" tests in QEMU, which were added by commit
> e0c371a0d23b ("tests/qtest: Add test for Aspeed HACE accumulative mode")
> They pass today with this patch. Are you suggesting we should add more?

I was trying to find a test case that showed the behaviour was broken
before (segfault?) and fixed after, but haven't had any luck so far.

The tests pass before this patch, and they pass after it. By that
logic there's no problem merging this one:

Reviewed-by: Joel Stanley <joel@jms.id.au>

Someone (aspeed?) should take a todo to resolve the HACE situation in u-boot.

Cheers,

Joel