[PATCH 0/5] aspeed: Add to CI

[PATCH 0/5] aspeed: Add to CI

[Joel Stanley]

The Aspeed AST2600 is modelled in Qemu. This makes some configuration
changes so it can be added to CI.

It has a depednency on the u-boot-test-hooks patches I sent here:

 https://lore.kernel.org/u-boot/20220624023420.3925916-1-joel@jms.id.au

I've given it a run on Azure and the tests passed.

Joel Stanley (5):
  config/ast2600: Enable CRC32
  config/ast2600: Make position independent
  config/ast2600: Disable hash hardware accel
  ast2600: Configure u-boot-with-spl.bin target
  CI: Add Aspeed AST2600

 include/configs/evb_ast2600.h | 3 +++
 .azure-pipelines.yml          | 3 +++
 .gitlab-ci.yml                | 6 ++++++
 configs/evb-ast2600_defconfig | 7 ++++---
 4 files changed, 16 insertions(+), 3 deletions(-)

-- 
2.35.1

[PATCH 1/5] config/ast2600: Enable CRC32

[Joel Stanley]

Useful for testing images with the default hash type.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 configs/evb-ast2600_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/evb-ast2600_defconfig b/configs/evb-ast2600_defconfig
index f84b723bbba3..53ba36a28374 100644
--- a/configs/evb-ast2600_defconfig
+++ b/configs/evb-ast2600_defconfig
@@ -35,6 +35,7 @@ CONFIG_SPL_SIZE_LIMIT_SUBTRACT_MALLOC=y
 CONFIG_SPL_SYS_MALLOC_SIMPLE=y
 CONFIG_SPL_STACK_R=y
 CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x2000000
+CONFIG_SPL_CRC32=y
 CONFIG_SPL_FIT_IMAGE_TINY=y
 CONFIG_SPL_DM_RESET=y
 CONFIG_SPL_RAM_SUPPORT=y
-- 
2.35.1

[PATCH 2/5] config/ast2600: Make position independent

[Joel Stanley]

Allows loading one u-boot from another. Useful for testing on hardware.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 configs/evb-ast2600_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/evb-ast2600_defconfig b/configs/evb-ast2600_defconfig
index 53ba36a28374..f3a6cb222020 100644
--- a/configs/evb-ast2600_defconfig
+++ b/configs/evb-ast2600_defconfig
@@ -1,5 +1,6 @@
 CONFIG_ARM=y
 CONFIG_SYS_DCACHE_OFF=y
+CONFIG_POSITION_INDEPENDENT=y
 CONFIG_SPL_SYS_THUMB_BUILD=y
 CONFIG_ARCH_ASPEED=y
 CONFIG_SYS_TEXT_BASE=0x80000000
-- 
2.35.1

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

[Joel Stanley]

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@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
 CONFIG_ASPEED_GPIO=y
 CONFIG_DM_I2C=y
 CONFIG_MISC=y
-- 
2.35.1

[PATCH 4/5] ast2600: Configure u-boot-with-spl.bin target

[Joel Stanley]

For the u-boot-with-spl.bin target to be useful for the AST2600, set the
maximum SPL size which also sets the padding length.

The normal way of loading u-boot is as a FIT, so configure u-boot.img as
the SPL playload.

With this the following simple steps can be used to build and boot a
system:

  make u-boot-with-spl.bin
  truncate -s 64M u-boot-with-spl.bin
  qemu-system-arm -nographic -M ast2600-evb \
    -drive file=u-boot-with-spl.bin,if=mtd,format=raw

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 include/configs/evb_ast2600.h | 3 +++
 configs/evb-ast2600_defconfig | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/include/configs/evb_ast2600.h b/include/configs/evb_ast2600.h
index 3c2155da46df..f5ac88447b52 100644
--- a/include/configs/evb_ast2600.h
+++ b/include/configs/evb_ast2600.h
@@ -10,6 +10,9 @@
 
 #define CONFIG_SYS_UBOOT_BASE		CONFIG_SYS_TEXT_BASE
 
+/* The maximum size the AST2600 bootrom can load is 64KB */
+#define CONFIG_SPL_MAX_SIZE		65536
+
 /* Misc */
 #define STR_HELPER(s)	#s
 #define STR(s)		STR_HELPER(s)
diff --git a/configs/evb-ast2600_defconfig b/configs/evb-ast2600_defconfig
index 160bccff48e2..5230515f7ab6 100644
--- a/configs/evb-ast2600_defconfig
+++ b/configs/evb-ast2600_defconfig
@@ -20,6 +20,8 @@ CONFIG_SPL_SIZE_LIMIT=0x10000
 CONFIG_SPL=y
 # CONFIG_ARMV7_NONSEC is not set
 CONFIG_SYS_LOAD_ADDR=0x83000000
+CONFIG_SPL_PAYLOAD="u-boot.img"
+CONFIG_BUILD_TARGET="u-boot-with-spl.bin"
 # CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set
 CONFIG_FIT=y
 CONFIG_SPL_FIT_SIGNATURE=y
-- 
2.35.1

[PATCH 5/5] CI: Add Aspeed AST2600

[Joel Stanley]

The AST2600 has a Qemu model that allows testing. Create a SPI NOR image
containing the combined SPL and u-boot FIT image.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 .azure-pipelines.yml | 3 +++
 .gitlab-ci.yml       | 6 ++++++
 2 files changed, 9 insertions(+)

diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml
index ad540ea63536..bdc515ebcdc1 100644
--- a/.azure-pipelines.yml
+++ b/.azure-pipelines.yml
@@ -261,6 +261,9 @@ stages:
         evb_ast2500:
           TEST_PY_BD: "evb-ast2500"
           TEST_PY_ID: "--id qemu"
+        evb_ast2600:
+          TEST_PY_BD: "evb-ast2600"
+          TEST_PY_ID: "--id qemu"
         vexpress_ca9x4:
           TEST_PY_BD: "vexpress_ca9x4"
           TEST_PY_ID: "--id qemu"
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index c6a608f7e2a7..f9cd41750791 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -272,6 +272,12 @@ evb-ast2500 test.py:
     TEST_PY_ID: "--id qemu"
   <<: *buildman_and_testpy_dfn
 
+evb-ast2600 test.py:
+  variables:
+    TEST_PY_BD: "evb-ast2600"
+    TEST_PY_ID: "--id qemu"
+  <<: *buildman_and_testpy_dfn
+
 sandbox_flattree test.py:
   variables:
     TEST_PY_BD: "sandbox_flattree"
-- 
2.35.1

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

[ChiaWei Wang]

Reply again to leave record on mailing list.

> From: joel.stan@gmail.com <joel.stan@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@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.

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

RE: [PATCH 4/5] ast2600: Configure u-boot-with-spl.bin target

[ChiaWei Wang]

Reply again to leave record on mailing list.

> From: joel.stan@gmail.com <joel.stan@gmail.com> On Behalf Of Joel Stanley
> Sent: Friday, June 24, 2022 10:50 AM
> 
> For the u-boot-with-spl.bin target to be useful for the AST2600, set the
> maximum SPL size which also sets the padding length.
> 
> The normal way of loading u-boot is as a FIT, so configure u-boot.img as the
> SPL playload.
> 
> With this the following simple steps can be used to build and boot a
> system:
> 
>   make u-boot-with-spl.bin
>   truncate -s 64M u-boot-with-spl.bin
>   qemu-system-arm -nographic -M ast2600-evb \
>     -drive file=u-boot-with-spl.bin,if=mtd,format=raw
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  include/configs/evb_ast2600.h | 3 +++
>  configs/evb-ast2600_defconfig | 2 ++
>  2 files changed, 5 insertions(+)
> 
> diff --git a/include/configs/evb_ast2600.h b/include/configs/evb_ast2600.h
> index 3c2155da46df..f5ac88447b52 100644
> --- a/include/configs/evb_ast2600.h
> +++ b/include/configs/evb_ast2600.h
> @@ -10,6 +10,9 @@
> 
>  #define CONFIG_SYS_UBOOT_BASE		CONFIG_SYS_TEXT_BASE
> 
> +/* The maximum size the AST2600 bootrom can load is 64KB */
> +#define CONFIG_SPL_MAX_SIZE		65536

Please define this to the Kconfig option CONFIG_SPL_SIZE_LIMIT to avoid inconsistent definitions on SPL image size limitation.

Chiawei

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

[Cédric Le Goater]

Hello Chiawei,

On 6/27/22 02:39, ChiaWei Wang wrote:
> Reply again to leave record on mailing list.
> 
>> From: joel.stan@gmail.com <joel.stan@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@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().

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
> 

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

[Joel Stanley]

On Mon, 27 Jun 2022 at 07:12, Cédric Le Goater <clg@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@gmail.com <joel.stan@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@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
> >
>

RE: [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@jms.id.au> 
Sent: Monday, June 27, 2022 3:40 PM
To: Cédric Le Goater <clg@kaod.org>
Cc: ChiaWei Wang <chiawei_wang@aspeedtech.com>; u-boot@lists.denx.de; Ryan Chen <ryan_chen@aspeedtech.com>; BMC-SW <BMC-SW@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@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@gmail.com <joel.stan@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@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
> >
>

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

[Joel Stanley]

[-- Attachment #1: Type: text/plain, Size: 2957 bytes --]

On Mon, 27 Jun 2022 at 08:48, Steven Lee <steven_lee@aspeedtech.com> wrote:
>
> 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.

I recommend using master with the patch that fixes FIT hash checking:

https://lore.kernel.org/r/20220620070117.3443066-1-joel@jms.id.au

I use a script to build the image (also attached to this email):

https://ozlabs.org/~joel/build-ast2600-spl.sh

Run that script from the u-boot source tree. It provides an example
qemu commandline when it finishes:

/usr/bin/qemu-system-arm -M ast2600-evb -nographic -drive
file=ast2600-obj/test.img,if=mtd,format=raw -nic user,tftp=/srv/tftp/
U-Boot SPL 2022.07-rc5-00010-g75967970850a (Jun 27 2022 - 19:00:15 +0930)
Trying to boot from RAM
## Checking hash(es) for config conf-1 ... OK
## Checking hash(es) for Image firmware-1 ... sha256 error!
Bad hash value for 'hash-1' hash node in 'firmware-1' image node

One thing to note is the conf-1 check succeeds, but the firmware-1
check fails. I suspect this is because the conf-1 check is less than
64 bytes, so it only requires one pass of the HACE. That's also why
the qemu unit test you wrote works; it only tests one pass, so doesn't
trigger the accumulation part of the model.

I was running with this patch to see the output of the hash operation:

Author: Joel Stanley <joel@jms.id.au>
Date:   Sat Jun 18 18:20:08 2022 +0930

    fit: Print hash results on failure

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

diff --git a/boot/image-fit.c b/boot/image-fit.c
index df3e5df8836a..63aa46e51270 100644
--- a/boot/image-fit.c
+++ b/boot/image-fit.c
@@ -1302,7 +1302,18 @@ static int fit_image_check_hash(const void
*fit, int noffset, const void *data,
                *err_msgp = "Bad hash value len";
                return -1;
        } else if (memcmp(value, fit_value, value_len) != 0) {
+               int i;
                *err_msgp = "Bad hash value";
+               printf("\ncalculated: ");
+               for (i=0; i<value_len; i++)
+                       printf("%02x ", value[i]);
+               printf("\n");
+
+               printf("  expected: ");
+               for (i=0; i<value_len; i++)
+                       printf("%02x ", fit_value[i]);
+               printf("\n");
+
                return -1;
        }


If you forget the FIT hash checking patch I linked above, the
calculated value is zero. The value with that fix applied is non-zero,
but incorrect:

## Checking hash(es) for config conf-1 ... OK
## Checking hash(es) for Image firmware-1 ... sha256
calculated: ae e0 4c 59 7c ec 06 72 68 6c 97 86 ea 6c da d0 6d 66 69
18 0d a2 29 05 15 60 ed 38 b0 31 9b 7b
  expected: 21 b0 f9 d8 2c 54 51 58 b7 22 bd 79 26 4a 99 c9 42 45 fd
5d f3 3f 4e 66 d2 67 cb bf 5d fa eb ab

If it helps, here's a tree with u-master plus the two patches I mentioned:

 git clone -b aspeed-test https://github.com/shenki/u-boot/

Cheers,

Joel

[-- Attachment #2: build-ast2600-spl.sh --]
[-- Type: application/x-sh, Size: 2224 bytes --]

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

[Steven Lee]

The 06/27/2022 18:06, Joel Stanley wrote:
> On Mon, 27 Jun 2022 at 08:48, Steven Lee <steven_lee@aspeedtech.com> wrote:
> >
> > 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.
> 
> I recommend using master with the patch that fixes FIT hash checking:
> 
> https://lore.kernel.org/r/20220620070117.3443066-1-joel@jms.id.au
> 
> I use a script to build the image (also attached to this email):
> 
> https://ozlabs.org/~joel/build-ast2600-spl.sh
> 
> Run that script from the u-boot source tree. It provides an example
> qemu commandline when it finishes:

Hi Joel,

Thanks for providing the information.
It seems that 2022.07 hace driver calculating hash by direct access +
accumulative mode, and aspeedtech-bmc uboot calculating hash by
scatter-gather + accumulative mode.

Currently, qemu only supports scatter-gather + accumulative mode.
https://github.com/qemu/qemu/blob/master/hw/misc/aspeed_hace.c#L197-L224

Reference:
U-Boot SPL 2022.07-rc5-08402-gad4f0cd35a:
https://github.com/shenki/u-boot/blob/aspeed-test/drivers/crypto/aspeed/aspeed_hace.c#L125

AspeedTech-BMC:
https://github.com/AspeedTech-BMC/u-boot/blob/aspeed-master-v2019.04/drivers/crypto/aspeed_hace.c#L165

Regards,
Steven