[PATCH 0/5] aspeed: Add to CI
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(-)
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
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
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
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
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
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
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
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
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/aspe...
AspeedTech-BMC: https://github.com/AspeedTech-BMC/u-boot/blob/aspeed-master-v2019.04/drivers...
Regards, Steven
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
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
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"
participants (4)
-
ChiaWei Wang -
Cédric Le Goater -
Joel Stanley -
Steven Lee