[PATCH u-boot v2019.04-aspeed-openbmc 00/11] Use HACE to

[PATCH u-boot v2019.04-aspeed-openbmc 00/11] Use HACE to

[Joel Stanley]

This series adds support to u-boot to using the HACE hardware in the
AST2600 to perform SHA hashing during boot.

The first five patches rearrange the way the ast2600 SPL boots, using
the common loading code so we can perform FIT verification on the u-boot
proper.

image-fit has been submitted upstream but not merged yet as it broke
boards that were performing hashing but not checking the result(!).

The final four patches add support for HACE and enable it in the emmc
configuration.

I intend to merge these patches into the openbmc u-boot tree while we
wait for upstream to accept the FIT hashing changes.

A large gotcha of enabling the HACE: it will break checking FIT images
that are in SPI NOR. u-boot will try to load the FIT directly from
memory-mapped flash, and the HACE engine can only read from SDRAM
regions. A fallback mechanism would need to be implemented if someone
was interested in supporting FIT + HACE + SPI NOR.

Joel Stanley (11):
  aspeed: Build secboot only when enabled
  ast2600: Specify boot order
  ast2600: Configure emmc boot options
  ast2600: spl: Support common boot devices
  config: ast2600: Enable common eMMC SPL loader
  image-fit: use hashing infra
  hash: Allow for SHA512 hardware implementations
  ast2600: Add HACE to device tree
  clk: aspeed: Add HACE yclk to ast2600
  crypto: Add driver for Aspeed HACE
  configs/openbmc: Enable hw accelerated sha

 arch/arm/dts/ast2600-rainier.dts           |   5 +
 arch/arm/dts/ast2600-tacoma.dts            |   5 +
 arch/arm/dts/ast2600.dtsi                  |   9 +
 arch/arm/dts/ast2600a1-evb.dts             |   4 +
 arch/arm/mach-aspeed/ast2600/Makefile      |   3 +-
 arch/arm/mach-aspeed/ast2600/spl.c         |  16 ++
 common/hash.c                              |  24 +-
 common/image-fit.c                         |  16 +-
 configs/ast2600_openbmc_spl_emmc_defconfig |  12 +-
 drivers/clk/aspeed/clk_ast2600.c           |  22 ++
 drivers/crypto/Kconfig                     |  16 ++
 drivers/crypto/Makefile                    |   1 +
 drivers/crypto/aspeed_hace.c               | 250 +++++++++++++++++++++
 include/configs/aspeed-common.h            |   2 +-
 include/configs/evb_ast2600a1_spl.h        |   4 +
 include/hw_sha.h                           |  26 +++
 lib/Kconfig                                |  15 +-
 17 files changed, 415 insertions(+), 15 deletions(-)
 create mode 100644 drivers/crypto/aspeed_hace.c

-- 
2.30.2

[PATCH u-boot v2019.04-aspeed-openbmc 01/11] aspeed: Build secboot only when enabled

[Joel Stanley]

The configuration option controls if the secboot code is used. When
there are no callers it is removed from the final u-boot binary. Instead
of relying on the linker to do this, only add it to the build system if
enabled.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 arch/arm/mach-aspeed/ast2600/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-aspeed/ast2600/Makefile b/arch/arm/mach-aspeed/ast2600/Makefile
index 0abac4c233e4..aafc4b2fe37f 100644
--- a/arch/arm/mach-aspeed/ast2600/Makefile
+++ b/arch/arm/mach-aspeed/ast2600/Makefile
@@ -1,2 +1,3 @@
-obj-y   += platform.o board_common.o scu_info.o utils.o cache.o crypto.o aspeed_verify.o
+obj-y   += platform.o board_common.o scu_info.o utils.o cache.o
+obj-$(CONFIG_ASPEED_SECURE_BOOT) += crypto.o aspeed_verify.o
 obj-$(CONFIG_SPL_BUILD) += spl.o spl_boot.o
-- 
2.30.2

Re: [PATCH u-boot v2019.04-aspeed-openbmc 01/11] aspeed: Build secboot only when enabled

[Klaus Heinrich Kiwi]

On 4/13/2021 5:07 AM, Joel Stanley wrote:
> The configuration option controls if the secboot code is used. When
> there are no callers it is removed from the final u-boot binary. Instead
> of relying on the linker to do this, only add it to the build system if
> enabled.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>   arch/arm/mach-aspeed/ast2600/Makefile | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-aspeed/ast2600/Makefile b/arch/arm/mach-aspeed/ast2600/Makefile
> index 0abac4c233e4..aafc4b2fe37f 100644
> --- a/arch/arm/mach-aspeed/ast2600/Makefile
> +++ b/arch/arm/mach-aspeed/ast2600/Makefile
> @@ -1,2 +1,3 @@
> -obj-y   += platform.o board_common.o scu_info.o utils.o cache.o crypto.o aspeed_verify.o
> +obj-y   += platform.o board_common.o scu_info.o utils.o cache.o
> +obj-$(CONFIG_ASPEED_SECURE_BOOT) += crypto.o aspeed_verify.o
>   obj-$(CONFIG_SPL_BUILD) += spl.o spl_boot.o
> 

Reviewed-by: Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>

-- 
Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>

[PATCH u-boot v2019.04-aspeed-openbmc 02/11] ast2600: Specify boot order

[Joel Stanley]

Try to boot from the strapped device, but fall back to the UART.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 arch/arm/mach-aspeed/ast2600/spl.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/mach-aspeed/ast2600/spl.c b/arch/arm/mach-aspeed/ast2600/spl.c
index 54f89b0e8431..d794421b4070 100644
--- a/arch/arm/mach-aspeed/ast2600/spl.c
+++ b/arch/arm/mach-aspeed/ast2600/spl.c
@@ -51,6 +51,12 @@ u32 spl_boot_device(void)
 	return BOOT_DEVICE_NONE;
 }
 
+void board_boot_order(u32 *spl_boot_list)
+{
+	spl_boot_list[0] = spl_boot_device();
+	spl_boot_list[1] = ASPEED_BOOT_DEVICE_UART;
+}
+
 #ifdef CONFIG_SPL_OS_BOOT
 int spl_start_uboot(void)
 {
-- 
2.30.2

Re: [PATCH u-boot v2019.04-aspeed-openbmc 02/11] ast2600: Specify boot order

[Klaus Heinrich Kiwi]

On 4/13/2021 5:07 AM, Joel Stanley wrote:
> Try to boot from the strapped device, but fall back to the UART.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
Reviewed-by: Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>
> ---
>   arch/arm/mach-aspeed/ast2600/spl.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/arch/arm/mach-aspeed/ast2600/spl.c b/arch/arm/mach-aspeed/ast2600/spl.c
> index 54f89b0e8431..d794421b4070 100644
> --- a/arch/arm/mach-aspeed/ast2600/spl.c
> +++ b/arch/arm/mach-aspeed/ast2600/spl.c
> @@ -51,6 +51,12 @@ u32 spl_boot_device(void)
>   	return BOOT_DEVICE_NONE;
>   }
> 
> +void board_boot_order(u32 *spl_boot_list)
> +{
> +	spl_boot_list[0] = spl_boot_device();
> +	spl_boot_list[1] = ASPEED_BOOT_DEVICE_UART;
> +}
> +
>   #ifdef CONFIG_SPL_OS_BOOT
>   int spl_start_uboot(void)
>   {
> 

-- 
Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>

[PATCH u-boot v2019.04-aspeed-openbmc 03/11] ast2600: Configure emmc boot options

[Joel Stanley]

Assume a u-boot that is 1MB, minus 64KB for the SPL, that is linked and
loaded to an address in DRAM.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 include/configs/aspeed-common.h     | 2 +-
 include/configs/evb_ast2600a1_spl.h | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/configs/aspeed-common.h b/include/configs/aspeed-common.h
index 876958735b75..093c59c0eadc 100755
--- a/include/configs/aspeed-common.h
+++ b/include/configs/aspeed-common.h
@@ -82,6 +82,6 @@
 #define PHY_ANEG_TIMEOUT		800
 
 /* Uboot size */
-#define CONFIG_SYS_MONITOR_LEN (1024 * 1024)
+#define CONFIG_SYS_MONITOR_LEN ((1024 - 64) * 1024)
 
 #endif	/* __ASPEED_COMMON_CONFIG_H */
diff --git a/include/configs/evb_ast2600a1_spl.h b/include/configs/evb_ast2600a1_spl.h
index a39988820add..4d8405e9dbc0 100644
--- a/include/configs/evb_ast2600a1_spl.h
+++ b/include/configs/evb_ast2600a1_spl.h
@@ -13,6 +13,7 @@
 #define CONFIG_SYS_MEMTEST_END		(CONFIG_SYS_MEMTEST_START + 0x5000000)
 
 #define CONFIG_SYS_UBOOT_BASE		CONFIG_SYS_TEXT_BASE
+#define CONFIG_SYS_UBOOT_START 		0x81000000
 
 /* Memory Info */
 #define CONFIG_SYS_LOAD_ADDR		0x83000000
@@ -32,4 +33,7 @@
 
 #define CONFIG_SUPPORT_EMMC_BOOT
 
+#define CONFIG_SYS_MMCSD_RAW_MODE_ARGS_SECTOR	0x80	/* address 0x10000 */
+#define CONFIG_SYS_MMCSD_RAW_MODE_ARGS_SECTORS	0xf0000	/* 960KiB (1M - 64K) */
+
 #endif	/* __CONFIG_H */
-- 
2.30.2

[PATCH u-boot v2019.04-aspeed-openbmc 04/11] ast2600: spl: Support common boot devices

[Joel Stanley]

Aspeed's SDK has some custom boot devices. There's also the common SPL
code for loading boot images from various devices that the system can be
configured to use.

This will use the Aspeed device types first and then fall back to the
generic ones.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 arch/arm/mach-aspeed/ast2600/spl.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/mach-aspeed/ast2600/spl.c b/arch/arm/mach-aspeed/ast2600/spl.c
index d794421b4070..527e14f8e3b6 100644
--- a/arch/arm/mach-aspeed/ast2600/spl.c
+++ b/arch/arm/mach-aspeed/ast2600/spl.c
@@ -34,6 +34,7 @@ void board_init_f(ulong dummy)
 
 u32 spl_boot_device(void)
 {
+#if IS_ENABLED(CONFIG_ASPEED_SECURE_BOOT)
 	switch (aspeed_bootmode()) {
 	case AST_BOOTMODE_EMMC:
 		return (IS_ENABLED(CONFIG_ASPEED_SECURE_BOOT))?
@@ -47,6 +48,15 @@ u32 spl_boot_device(void)
 	default:
 		break;
 	}
+#endif
+	switch (aspeed_bootmode()) {
+	case AST_BOOTMODE_EMMC:
+		return BOOT_DEVICE_MMC1;
+	case AST_BOOTMODE_SPI:
+		return BOOT_DEVICE_SPI;
+	case AST_BOOTMODE_UART:
+		return BOOT_DEVICE_UART;
+	}
 
 	return BOOT_DEVICE_NONE;
 }
-- 
2.30.2

Re: [PATCH u-boot v2019.04-aspeed-openbmc 04/11] ast2600: spl: Support common boot devices

[Klaus Heinrich Kiwi]

On 4/13/2021 5:07 AM, Joel Stanley wrote:
> Aspeed's SDK has some custom boot devices. There's also the common SPL
> code for loading boot images from various devices that the system can be
> configured to use.
> 
> This will use the Aspeed device types first and then fall back to the
> generic ones.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>   arch/arm/mach-aspeed/ast2600/spl.c | 10 ++++++++++
>   1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm/mach-aspeed/ast2600/spl.c b/arch/arm/mach-aspeed/ast2600/spl.c
> index d794421b4070..527e14f8e3b6 100644
> --- a/arch/arm/mach-aspeed/ast2600/spl.c
> +++ b/arch/arm/mach-aspeed/ast2600/spl.c
> @@ -34,6 +34,7 @@ void board_init_f(ulong dummy)
> 
>   u32 spl_boot_device(void)
>   {
> +#if IS_ENABLED(CONFIG_ASPEED_SECURE_BOOT)
>   	switch (aspeed_bootmode()) {
>   	case AST_BOOTMODE_EMMC:
>   		return (IS_ENABLED(CONFIG_ASPEED_SECURE_BOOT))?
> @@ -47,6 +48,15 @@ u32 spl_boot_device(void)
>   	default:
>   		break;
>   	}
> +#endif
> +	switch (aspeed_bootmode()) {
> +	case AST_BOOTMODE_EMMC:
> +		return BOOT_DEVICE_MMC1;
> +	case AST_BOOTMODE_SPI:
> +		return BOOT_DEVICE_SPI;
> +	case AST_BOOTMODE_UART:
> +		return BOOT_DEVICE_UART;
> +	}
> 
>   	return BOOT_DEVICE_NONE;
>   }
> 

Reviewed-by: Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>

-- 
Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>

[PATCH u-boot v2019.04-aspeed-openbmc 05/11] config: ast2600: Enable common eMMC SPL loader

[Joel Stanley]

Notabily the link address changes, as this is used as the load address
by the loader.

Given the Aspeed loaders are linking u-boot at 0x10000 but running it
from RAM, the u-boot relocation code must be fine with this setup.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 configs/ast2600_openbmc_spl_emmc_defconfig | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/configs/ast2600_openbmc_spl_emmc_defconfig b/configs/ast2600_openbmc_spl_emmc_defconfig
index 6daf6343478b..e59d3595ebf0 100644
--- a/configs/ast2600_openbmc_spl_emmc_defconfig
+++ b/configs/ast2600_openbmc_spl_emmc_defconfig
@@ -7,7 +7,7 @@ CONFIG_SYS_THUMB_BUILD=y
 # CONFIG_SPL_USE_ARCH_MEMSET is not set
 CONFIG_SPL_LDSCRIPT="arch/$(ARCH)/mach-aspeed/ast2600/u-boot-spl.lds"
 CONFIG_ARCH_ASPEED=y
-CONFIG_SYS_TEXT_BASE=0x10000
+CONFIG_SYS_TEXT_BASE=0x81000000
 CONFIG_ASPEED_AST2600=y
 CONFIG_ASPEED_UBOOT_SPI_BASE=0x10000
 CONFIG_ASPEED_UBOOT_SPI_SIZE=0xd0000
@@ -47,11 +47,12 @@ CONFIG_SYS_CONSOLE_ENV_OVERWRITE=y
 CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_ARCH_EARLY_INIT_R=y
 CONFIG_BOARD_EARLY_INIT_F=y
-# CONFIG_SPL_RAW_IMAGE_SUPPORT is not set
 # CONFIG_SPL_LEGACY_IMAGE_SUPPORT is not set
 CONFIG_SPL_SYS_MALLOC_SIMPLE=y
 CONFIG_SPL_STACK_R=y
 CONFIG_SPL_SEPARATE_BSS=y
+CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y
+CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x80
 CONFIG_SPL_FIT_IMAGE_TINY=y
 CONFIG_SPL_DM_RESET=y
 CONFIG_SPL_RAM_SUPPORT=y
-- 
2.30.2

Re: [PATCH u-boot v2019.04-aspeed-openbmc 05/11] config: ast2600: Enable common eMMC SPL loader

[Klaus Heinrich Kiwi]

On 4/13/2021 5:07 AM, Joel Stanley wrote:
> Notabily the link address changes, as this is used as the load address
> by the loader.
> 
> Given the Aspeed loaders are linking u-boot at 0x10000 but running it
> from RAM, the u-boot relocation code must be fine with this setup.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>   configs/ast2600_openbmc_spl_emmc_defconfig | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/configs/ast2600_openbmc_spl_emmc_defconfig b/configs/ast2600_openbmc_spl_emmc_defconfig
> index 6daf6343478b..e59d3595ebf0 100644
> --- a/configs/ast2600_openbmc_spl_emmc_defconfig
> +++ b/configs/ast2600_openbmc_spl_emmc_defconfig
> @@ -7,7 +7,7 @@ CONFIG_SYS_THUMB_BUILD=y
>   # CONFIG_SPL_USE_ARCH_MEMSET is not set
>   CONFIG_SPL_LDSCRIPT="arch/$(ARCH)/mach-aspeed/ast2600/u-boot-spl.lds"
>   CONFIG_ARCH_ASPEED=y
> -CONFIG_SYS_TEXT_BASE=0x10000
> +CONFIG_SYS_TEXT_BASE=0x81000000
>   CONFIG_ASPEED_AST2600=y
>   CONFIG_ASPEED_UBOOT_SPI_BASE=0x10000
>   CONFIG_ASPEED_UBOOT_SPI_SIZE=0xd0000
> @@ -47,11 +47,12 @@ CONFIG_SYS_CONSOLE_ENV_OVERWRITE=y
>   CONFIG_DISPLAY_BOARDINFO_LATE=y
>   CONFIG_ARCH_EARLY_INIT_R=y
>   CONFIG_BOARD_EARLY_INIT_F=y
> -# CONFIG_SPL_RAW_IMAGE_SUPPORT is not set
Guess this is unrelated?
But still, isn't this required for booting from UART? I had
the impression that the SPL ymodem boot would not work with
fitImages.


>   # CONFIG_SPL_LEGACY_IMAGE_SUPPORT is not set
>   CONFIG_SPL_SYS_MALLOC_SIMPLE=y
>   CONFIG_SPL_STACK_R=y
>   CONFIG_SPL_SEPARATE_BSS=y
> +CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y
> +CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x80
>   CONFIG_SPL_FIT_IMAGE_TINY=y
>   CONFIG_SPL_DM_RESET=y
>   CONFIG_SPL_RAM_SUPPORT=y
> 

-- 
Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>

Re: [PATCH u-boot v2019.04-aspeed-openbmc 05/11] config: ast2600: Enable common eMMC SPL loader

[Joel Stanley]

On Tue, 13 Apr 2021 at 20:47, Klaus Heinrich Kiwi
<klaus@linux.vnet.ibm.com> wrote:
>
>
>
> On 4/13/2021 5:07 AM, Joel Stanley wrote:
> > Notabily the link address changes, as this is used as the load address
> > by the loader.
> >
> > Given the Aspeed loaders are linking u-boot at 0x10000 but running it
> > from RAM, the u-boot relocation code must be fine with this setup.
> >
> > Signed-off-by: Joel Stanley <joel@jms.id.au>
> > ---
> >   configs/ast2600_openbmc_spl_emmc_defconfig | 5 +++--
> >   1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/configs/ast2600_openbmc_spl_emmc_defconfig b/configs/ast2600_openbmc_spl_emmc_defconfig
> > index 6daf6343478b..e59d3595ebf0 100644
> > --- a/configs/ast2600_openbmc_spl_emmc_defconfig
> > +++ b/configs/ast2600_openbmc_spl_emmc_defconfig
> > @@ -7,7 +7,7 @@ CONFIG_SYS_THUMB_BUILD=y
> >   # CONFIG_SPL_USE_ARCH_MEMSET is not set
> >   CONFIG_SPL_LDSCRIPT="arch/$(ARCH)/mach-aspeed/ast2600/u-boot-spl.lds"
> >   CONFIG_ARCH_ASPEED=y
> > -CONFIG_SYS_TEXT_BASE=0x10000
> > +CONFIG_SYS_TEXT_BASE=0x81000000
> >   CONFIG_ASPEED_AST2600=y
> >   CONFIG_ASPEED_UBOOT_SPI_BASE=0x10000
> >   CONFIG_ASPEED_UBOOT_SPI_SIZE=0xd0000
> > @@ -47,11 +47,12 @@ CONFIG_SYS_CONSOLE_ENV_OVERWRITE=y
> >   CONFIG_DISPLAY_BOARDINFO_LATE=y
> >   CONFIG_ARCH_EARLY_INIT_R=y
> >   CONFIG_BOARD_EARLY_INIT_F=y
> > -# CONFIG_SPL_RAW_IMAGE_SUPPORT is not set
> Guess this is unrelated?
> But still, isn't this required for booting from UART? I had
> the impression that the SPL ymodem boot would not work with
> fitImages.

The common spl code that we're now using the boot the system has
support for parsing the fit. See ymodem_read_fit in
common/spl/spl_ymodem.c.

You were correct that the aspeed spl code did not support fit.

Cheers,

Joel

[PATCH u-boot v2019.04-aspeed-openbmc 06/11] image-fit: use hashing infra

[Joel Stanley]

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 common/image-fit.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/common/image-fit.c b/common/image-fit.c
index e64949dfa73d..b9c3d79b83e1 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -1135,9 +1135,22 @@ int fit_set_timestamp(void *fit, int noffset, time_t timestamp)
  *     0, on success
  *    -1, when algo is unsupported
  */
-int calculate_hash(const void *data, int data_len, const char *algo,
+int calculate_hash(const void *data, int data_len, const char *algo_name,
 			uint8_t *value, int *value_len)
 {
+	struct hash_algo *algo;
+
+	if (hash_lookup_algo(algo_name, &algo)) {
+		debug("Unsupported hash alogrithm\n");
+		return -1;
+	}
+
+	algo->hash_func_ws(data, data_len, value, algo->chunk_size);
+	*value_len = algo->digest_size;
+
+	return 0;
+
+#if 0
 	if (IMAGE_ENABLE_CRC32 && strcmp(algo, "crc32") == 0) {
 		*((uint32_t *)value) = crc32_wd(0, data, data_len,
 							CHUNKSZ_CRC32);
@@ -1167,6 +1180,7 @@ int calculate_hash(const void *data, int data_len, const char *algo,
 		return -1;
 	}
 	return 0;
+#endif
 }
 
 static int fit_image_check_hash(const void *fit, int noffset, const void *data,
-- 
2.30.2

Re: [PATCH u-boot v2019.04-aspeed-openbmc 06/11] image-fit: use hashing infra

[Klaus Heinrich Kiwi]

On 4/13/2021 5:07 AM, Joel Stanley wrote:
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>   common/image-fit.c | 16 +++++++++++++++-
>   1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/common/image-fit.c b/common/image-fit.c
> index e64949dfa73d..b9c3d79b83e1 100644
> --- a/common/image-fit.c
> +++ b/common/image-fit.c
> @@ -1135,9 +1135,22 @@ int fit_set_timestamp(void *fit, int noffset, time_t timestamp)
>    *     0, on success
>    *    -1, when algo is unsupported
>    */
> -int calculate_hash(const void *data, int data_len, const char *algo,
> +int calculate_hash(const void *data, int data_len, const char *algo_name,
>   			uint8_t *value, int *value_len)
Is the API changing here, or you just needed the 'algo' variable name for something else?
Are all callers OK with that?

>   {
> +	struct hash_algo *algo;
> +
> +	if (hash_lookup_algo(algo_name, &algo)) {
> +		debug("Unsupported hash alogrithm\n");
> +		return -1;
> +	}
> +
> +	algo->hash_func_ws(data, data_len, value, algo->chunk_size);
> +	*value_len = algo->digest_size;
> +
> +	return 0;
> +
> +#if 0

Can you expand the rationale behind keeping this dead code around?

>   	if (IMAGE_ENABLE_CRC32 && strcmp(algo, "crc32") == 0) {
>   		*((uint32_t *)value) = crc32_wd(0, data, data_len,
>   							CHUNKSZ_CRC32);
> @@ -1167,6 +1180,7 @@ int calculate_hash(const void *data, int data_len, const char *algo,
>   		return -1;
>   	}
>   	return 0;
> +#endif
>   }
> 
>   static int fit_image_check_hash(const void *fit, int noffset, const void *data,
> 

-- 
Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>

Re: [PATCH u-boot v2019.04-aspeed-openbmc 06/11] image-fit: use hashing infra

[Joel Stanley]

On Tue, 13 Apr 2021 at 12:38, Klaus Heinrich Kiwi
<klaus@linux.vnet.ibm.com> wrote:
>
>
>
> On 4/13/2021 5:07 AM, Joel Stanley wrote:
> > Signed-off-by: Joel Stanley <joel@jms.id.au>
> > ---
> >   common/image-fit.c | 16 +++++++++++++++-
> >   1 file changed, 15 insertions(+), 1 deletion(-)
> >
> > diff --git a/common/image-fit.c b/common/image-fit.c
> > index e64949dfa73d..b9c3d79b83e1 100644
> > --- a/common/image-fit.c
> > +++ b/common/image-fit.c
> > @@ -1135,9 +1135,22 @@ int fit_set_timestamp(void *fit, int noffset, time_t timestamp)
> >    *     0, on success
> >    *    -1, when algo is unsupported
> >    */
> > -int calculate_hash(const void *data, int data_len, const char *algo,
> > +int calculate_hash(const void *data, int data_len, const char *algo_name,
> >                       uint8_t *value, int *value_len)
> Is the API changing here, or you just needed the 'algo' variable name for something else?
> Are all callers OK with that?
>
> >   {
> > +     struct hash_algo *algo;
> > +
> > +     if (hash_lookup_algo(algo_name, &algo)) {
> > +             debug("Unsupported hash alogrithm\n");
> > +             return -1;
> > +     }
> > +
> > +     algo->hash_func_ws(data, data_len, value, algo->chunk_size);
> > +     *value_len = algo->digest_size;
> > +
> > +     return 0;
> > +
> > +#if 0
>
> Can you expand the rationale behind keeping this dead code around?

Good catch, I sent the wrong version of the patch. I'll send v2 with
the version that went upstream.

[PATCH u-boot v2019.04-aspeed-openbmc 07/11] hash: Allow for SHA512 hardware implementations

[Joel Stanley]

Similar to support for SHA1 and SHA256, allow the use of hardware hashing
engine by enabling the algorithm and setting  CONFIG_SHA_HW_ACCEL /
CONFIG_SHA_PROG_HW_ACCEL.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 common/hash.c    | 24 ++++++++++++++++++++++--
 include/hw_sha.h | 26 ++++++++++++++++++++++++++
 lib/Kconfig      | 15 +++++++--------
 3 files changed, 55 insertions(+), 10 deletions(-)

diff --git a/common/hash.c b/common/hash.c
index c00ec4d36c41..a19cba07d779 100644
--- a/common/hash.c
+++ b/common/hash.c
@@ -86,7 +86,7 @@ static int hash_finish_sha256(struct hash_algo *algo, void *ctx, void
 }
 #endif
 
-#if defined(CONFIG_SHA384)
+#if defined(CONFIG_SHA384) && !defined(CONFIG_SHA_PROG_HW_ACCEL)
 static int hash_init_sha384(struct hash_algo *algo, void **ctxp)
 {
 	sha512_context *ctx = malloc(sizeof(sha512_context));
@@ -114,7 +114,7 @@ static int hash_finish_sha384(struct hash_algo *algo, void *ctx, void
 }
 #endif
 
-#if defined(CONFIG_SHA512)
+#if defined(CONFIG_SHA512) && !defined(CONFIG_SHA_PROG_HW_ACCEL)
 static int hash_init_sha512(struct hash_algo *algo, void **ctxp)
 {
 	sha512_context *ctx = malloc(sizeof(sha512_context));
@@ -249,10 +249,20 @@ static struct hash_algo hash_algo[] = {
 		.name		= "sha384",
 		.digest_size	= SHA384_SUM_LEN,
 		.chunk_size	= CHUNKSZ_SHA384,
+#ifdef CONFIG_SHA_HW_ACCEL
+		.hash_func_ws	= hw_sha384,
+#else
 		.hash_func_ws	= sha384_csum_wd,
+#endif
+#ifdef CONFIG_SHA_PROG_HW_ACCEL
+		.hash_init	= hw_sha_init,
+		.hash_update	= hw_sha_update,
+		.hash_finish	= hw_sha_finish,
+#else
 		.hash_init	= hash_init_sha384,
 		.hash_update	= hash_update_sha384,
 		.hash_finish	= hash_finish_sha384,
+#endif
 	},
 #endif
 #ifdef CONFIG_SHA512
@@ -260,10 +270,20 @@ static struct hash_algo hash_algo[] = {
 		.name		= "sha512",
 		.digest_size	= SHA512_SUM_LEN,
 		.chunk_size	= CHUNKSZ_SHA512,
+#ifdef CONFIG_SHA_HW_ACCEL
+		.hash_func_ws	= hw_sha512,
+#else
 		.hash_func_ws	= sha512_csum_wd,
+#endif
+#ifdef CONFIG_SHA_PROG_HW_ACCEL
+		.hash_init	= hw_sha_init,
+		.hash_update	= hw_sha_update,
+		.hash_finish	= hw_sha_finish,
+#else
 		.hash_init	= hash_init_sha512,
 		.hash_update	= hash_update_sha512,
 		.hash_finish	= hash_finish_sha512,
+#endif
 	},
 #endif
 	{
diff --git a/include/hw_sha.h b/include/hw_sha.h
index 991e496a3cb2..8cdf821218a0 100644
--- a/include/hw_sha.h
+++ b/include/hw_sha.h
@@ -8,6 +8,32 @@
 #define __HW_SHA_H
 #include <hash.h>
 
+/**
+ * Computes hash value of input pbuf using h/w acceleration
+ *
+ * @param in_addr	A pointer to the input buffer
+ * @param bufleni	Byte length of input buffer
+ * @param out_addr	A pointer to the output buffer. When complete
+ *			64 bytes are copied to pout[0]...pout[63]. Thus, a user
+ *			should allocate at least 64 bytes at pOut in advance.
+ * @param chunk_size	chunk size for sha512
+ */
+void hw_sha512(const uchar * in_addr, uint buflen,
+			uchar * out_addr, uint chunk_size);
+
+/**
+ * Computes hash value of input pbuf using h/w acceleration
+ *
+ * @param in_addr	A pointer to the input buffer
+ * @param bufleni	Byte length of input buffer
+ * @param out_addr	A pointer to the output buffer. When complete
+ *			48 bytes are copied to pout[0]...pout[47]. Thus, a user
+ *			should allocate at least 48 bytes at pOut in advance.
+ * @param chunk_size	chunk size for sha384
+ */
+void hw_sha384(const uchar * in_addr, uint buflen,
+			uchar * out_addr, uint chunk_size);
+
 /**
  * Computes hash value of input pbuf using h/w acceleration
  *
diff --git a/lib/Kconfig b/lib/Kconfig
index 984a783fd16f..f77272d0a94a 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -273,19 +273,18 @@ config SHA384
 config SHA_HW_ACCEL
 	bool "Enable hashing using hardware"
 	help
-	  This option enables hardware acceleration
-	  for SHA1/SHA256 hashing.
-	  This affects the 'hash' command and also the
-	  hash_lookup_algo() function.
+	  This option enables hardware acceleration for SHA hashing.
+	  This affects the 'hash' command and also the hash_lookup_algo()
+	  function.
 
 config SHA_PROG_HW_ACCEL
 	bool "Enable Progressive hashing support using hardware"
 	depends on SHA_HW_ACCEL
 	help
-	  This option enables hardware-acceleration for
-	  SHA1/SHA256 progressive hashing.
-	  Data can be streamed in a block at a time and the hashing
-	  is performed in hardware.
+	  This option enables hardware-acceleration for SHA progressive
+	  hashing.
+	  Data can be streamed in a block at a time and the hashing is
+	  performed in hardware.
 
 config MD5
 	bool
-- 
2.30.2

Re: [PATCH u-boot v2019.04-aspeed-openbmc 07/11] hash: Allow for SHA512 hardware implementations

[Klaus Heinrich Kiwi]

On 4/13/2021 5:07 AM, Joel Stanley wrote:
> Similar to support for SHA1 and SHA256, allow the use of hardware hashing
> engine by enabling the algorithm and setting  CONFIG_SHA_HW_ACCEL /
> CONFIG_SHA_PROG_HW_ACCEL.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>   common/hash.c    | 24 ++++++++++++++++++++++--
>   include/hw_sha.h | 26 ++++++++++++++++++++++++++
>   lib/Kconfig      | 15 +++++++--------
>   3 files changed, 55 insertions(+), 10 deletions(-)
> 
> diff --git a/common/hash.c b/common/hash.c
> index c00ec4d36c41..a19cba07d779 100644
> --- a/common/hash.c
> +++ b/common/hash.c
> @@ -86,7 +86,7 @@ static int hash_finish_sha256(struct hash_algo *algo, void *ctx, void
>   }
>   #endif
> 
> -#if defined(CONFIG_SHA384)
> +#if defined(CONFIG_SHA384) && !defined(CONFIG_SHA_PROG_HW_ACCEL)
>   static int hash_init_sha384(struct hash_algo *algo, void **ctxp)
>   {
>   	sha512_context *ctx = malloc(sizeof(sha512_context));
> @@ -114,7 +114,7 @@ static int hash_finish_sha384(struct hash_algo *algo, void *ctx, void
>   }
>   #endif
> 
> -#if defined(CONFIG_SHA512)
> +#if defined(CONFIG_SHA512) && !defined(CONFIG_SHA_PROG_HW_ACCEL)
>   static int hash_init_sha512(struct hash_algo *algo, void **ctxp)
>   {
>   	sha512_context *ctx = malloc(sizeof(sha512_context));
> @@ -249,10 +249,20 @@ static struct hash_algo hash_algo[] = {
>   		.name		= "sha384",
>   		.digest_size	= SHA384_SUM_LEN,
>   		.chunk_size	= CHUNKSZ_SHA384,
> +#ifdef CONFIG_SHA_HW_ACCEL
> +		.hash_func_ws	= hw_sha384,
> +#else
>   		.hash_func_ws	= sha384_csum_wd,
> +#endif
> +#ifdef CONFIG_SHA_PROG_HW_ACCEL
> +		.hash_init	= hw_sha_init,
> +		.hash_update	= hw_sha_update,
> +		.hash_finish	= hw_sha_finish,
> +#else
>   		.hash_init	= hash_init_sha384,
>   		.hash_update	= hash_update_sha384,
>   		.hash_finish	= hash_finish_sha384,
> +#endif
>   	},
>   #endif
>   #ifdef CONFIG_SHA512
> @@ -260,10 +270,20 @@ static struct hash_algo hash_algo[] = {
>   		.name		= "sha512",
>   		.digest_size	= SHA512_SUM_LEN,
>   		.chunk_size	= CHUNKSZ_SHA512,
> +#ifdef CONFIG_SHA_HW_ACCEL
> +		.hash_func_ws	= hw_sha512,
> +#else
>   		.hash_func_ws	= sha512_csum_wd,
> +#endif
> +#ifdef CONFIG_SHA_PROG_HW_ACCEL
> +		.hash_init	= hw_sha_init,
> +		.hash_update	= hw_sha_update,
> +		.hash_finish	= hw_sha_finish,
> +#else
>   		.hash_init	= hash_init_sha512,
>   		.hash_update	= hash_update_sha512,
>   		.hash_finish	= hash_finish_sha512,
> +#endif
>   	},
>   #endif
>   	{
> diff --git a/include/hw_sha.h b/include/hw_sha.h
> index 991e496a3cb2..8cdf821218a0 100644
> --- a/include/hw_sha.h
> +++ b/include/hw_sha.h
> @@ -8,6 +8,32 @@
>   #define __HW_SHA_H
>   #include <hash.h>
> 
> +/**
> + * Computes hash value of input pbuf using h/w acceleration
> + *
> + * @param in_addr	A pointer to the input buffer
> + * @param bufleni	Byte length of input buffer
> + * @param out_addr	A pointer to the output buffer. When complete
> + *			64 bytes are copied to pout[0]...pout[63]. Thus, a user
> + *			should allocate at least 64 bytes at pOut in advance.
> + * @param chunk_size	chunk size for sha512
> + */
> +void hw_sha512(const uchar * in_addr, uint buflen,
> +			uchar * out_addr, uint chunk_size);
> +
> +/**
> + * Computes hash value of input pbuf using h/w acceleration
> + *
> + * @param in_addr	A pointer to the input buffer
> + * @param bufleni	Byte length of input buffer
> + * @param out_addr	A pointer to the output buffer. When complete
> + *			48 bytes are copied to pout[0]...pout[47]. Thus, a user
> + *			should allocate at least 48 bytes at pOut in advance.
> + * @param chunk_size	chunk size for sha384
> + */
> +void hw_sha384(const uchar * in_addr, uint buflen,
> +			uchar * out_addr, uint chunk_size);
> +
>   /**
>    * Computes hash value of input pbuf using h/w acceleration
>    *
> diff --git a/lib/Kconfig b/lib/Kconfig
> index 984a783fd16f..f77272d0a94a 100644
> --- a/lib/Kconfig
> +++ b/lib/Kconfig
> @@ -273,19 +273,18 @@ config SHA384
>   config SHA_HW_ACCEL
>   	bool "Enable hashing using hardware"
>   	help
> -	  This option enables hardware acceleration
> -	  for SHA1/SHA256 hashing.
> -	  This affects the 'hash' command and also the
> -	  hash_lookup_algo() function.
> +	  This option enables hardware acceleration for SHA hashing.
> +	  This affects the 'hash' command and also the hash_lookup_algo()
> +	  function.
> 
>   config SHA_PROG_HW_ACCEL
>   	bool "Enable Progressive hashing support using hardware"
>   	depends on SHA_HW_ACCEL
>   	help
> -	  This option enables hardware-acceleration for
> -	  SHA1/SHA256 progressive hashing.
> -	  Data can be streamed in a block at a time and the hashing
> -	  is performed in hardware.
> +	  This option enables hardware-acceleration for SHA progressive
> +	  hashing.
> +	  Data can be streamed in a block at a time and the hashing is
> +	  performed in hardware.
> 
>   config MD5
>   	bool
> 

Reviewed-by: Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>

-- 
Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>

[PATCH u-boot v2019.04-aspeed-openbmc 08/11] ast2600: Add HACE to device tree

[Joel Stanley]

HACE is the Hash and Crypto Egine in the AST2600.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 arch/arm/dts/ast2600-rainier.dts | 5 +++++
 arch/arm/dts/ast2600-tacoma.dts  | 5 +++++
 arch/arm/dts/ast2600.dtsi        | 9 +++++++++
 arch/arm/dts/ast2600a1-evb.dts   | 4 ++++
 4 files changed, 23 insertions(+)

diff --git a/arch/arm/dts/ast2600-rainier.dts b/arch/arm/dts/ast2600-rainier.dts
index 67e177baf1bd..aae507b4c23d 100755
--- a/arch/arm/dts/ast2600-rainier.dts
+++ b/arch/arm/dts/ast2600-rainier.dts
@@ -103,3 +103,8 @@
 	pinctrl-0 = <&pinctrl_emmc_default>;
 	sdhci-drive-type = <1>;
 };
+
+&hace {
+	u-boot,dm-pre-reloc;
+	status = "okay";
+};
diff --git a/arch/arm/dts/ast2600-tacoma.dts b/arch/arm/dts/ast2600-tacoma.dts
index 85d1e3902b11..c8ed5e35a74c 100755
--- a/arch/arm/dts/ast2600-tacoma.dts
+++ b/arch/arm/dts/ast2600-tacoma.dts
@@ -94,3 +94,8 @@
 	pinctrl-0 = <&pinctrl_emmc_default>;
 	sdhci-drive-type = <1>;
 };
+
+&hace {
+	u-boot,dm-pre-reloc;
+	status = "okay";
+};
diff --git a/arch/arm/dts/ast2600.dtsi b/arch/arm/dts/ast2600.dtsi
index e619f7118886..57ea98a47b67 100644
--- a/arch/arm/dts/ast2600.dtsi
+++ b/arch/arm/dts/ast2600.dtsi
@@ -304,6 +304,15 @@
 
 			};
 
+			hace: hace@1e6d0000 {
+				compatible = "aspeed,ast2600-hace";
+				reg = <0x1e6d0000 0x200>;
+				interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>;
+				clocks = <&scu ASPEED_CLK_GATE_YCLK>;
+				clock-names = "yclk";
+				status = "disabled";
+			};
+
 			smp-memram@0 {
 				compatible = "aspeed,ast2600-smpmem", "syscon";
 				reg = <0x1e6e2180 0x40>;
diff --git a/arch/arm/dts/ast2600a1-evb.dts b/arch/arm/dts/ast2600a1-evb.dts
index 2827e00c0eb4..2ae6e3bdf192 100644
--- a/arch/arm/dts/ast2600a1-evb.dts
+++ b/arch/arm/dts/ast2600a1-evb.dts
@@ -301,3 +301,7 @@
 &display_port {
 	status = "okay";
 };
+
+&hace {
+	status = "okay";
+};
-- 
2.30.2

Re: [PATCH u-boot v2019.04-aspeed-openbmc 08/11] ast2600: Add HACE to device tree

[Klaus Heinrich Kiwi]

On 4/13/2021 5:07 AM, Joel Stanley wrote:
> HACE is the Hash and Crypto Egine in the AST2600.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
Reviewed-by: Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>
> ---
>   arch/arm/dts/ast2600-rainier.dts | 5 +++++
>   arch/arm/dts/ast2600-tacoma.dts  | 5 +++++
>   arch/arm/dts/ast2600.dtsi        | 9 +++++++++
>   arch/arm/dts/ast2600a1-evb.dts   | 4 ++++
>   4 files changed, 23 insertions(+)
> 
> diff --git a/arch/arm/dts/ast2600-rainier.dts b/arch/arm/dts/ast2600-rainier.dts
> index 67e177baf1bd..aae507b4c23d 100755
> --- a/arch/arm/dts/ast2600-rainier.dts
> +++ b/arch/arm/dts/ast2600-rainier.dts
> @@ -103,3 +103,8 @@
>   	pinctrl-0 = <&pinctrl_emmc_default>;
>   	sdhci-drive-type = <1>;
>   };
> +
> +&hace {
> +	u-boot,dm-pre-reloc;
> +	status = "okay";
> +};
> diff --git a/arch/arm/dts/ast2600-tacoma.dts b/arch/arm/dts/ast2600-tacoma.dts
> index 85d1e3902b11..c8ed5e35a74c 100755
> --- a/arch/arm/dts/ast2600-tacoma.dts
> +++ b/arch/arm/dts/ast2600-tacoma.dts
> @@ -94,3 +94,8 @@
>   	pinctrl-0 = <&pinctrl_emmc_default>;
>   	sdhci-drive-type = <1>;
>   };
> +
> +&hace {
> +	u-boot,dm-pre-reloc;
> +	status = "okay";
> +};
> diff --git a/arch/arm/dts/ast2600.dtsi b/arch/arm/dts/ast2600.dtsi
> index e619f7118886..57ea98a47b67 100644
> --- a/arch/arm/dts/ast2600.dtsi
> +++ b/arch/arm/dts/ast2600.dtsi
> @@ -304,6 +304,15 @@
> 
>   			};
> 
> +			hace: hace@1e6d0000 {
> +				compatible = "aspeed,ast2600-hace";
> +				reg = <0x1e6d0000 0x200>;
> +				interrupts = <GIC_SPI 4 IRQ_TYPE_LEVEL_HIGH>;
> +				clocks = <&scu ASPEED_CLK_GATE_YCLK>;
> +				clock-names = "yclk";
> +				status = "disabled";
> +			};
> +
>   			smp-memram@0 {
>   				compatible = "aspeed,ast2600-smpmem", "syscon";
>   				reg = <0x1e6e2180 0x40>;
> diff --git a/arch/arm/dts/ast2600a1-evb.dts b/arch/arm/dts/ast2600a1-evb.dts
> index 2827e00c0eb4..2ae6e3bdf192 100644
> --- a/arch/arm/dts/ast2600a1-evb.dts
> +++ b/arch/arm/dts/ast2600a1-evb.dts
> @@ -301,3 +301,7 @@
>   &display_port {
>   	status = "okay";
>   };
> +
> +&hace {
> +	status = "okay";
> +};
> 

-- 
Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>

[PATCH u-boot v2019.04-aspeed-openbmc 09/11] clk: aspeed: Add HACE yclk to ast2600

[Joel Stanley]

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/clk/aspeed/clk_ast2600.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/clk/aspeed/clk_ast2600.c b/drivers/clk/aspeed/clk_ast2600.c
index 4c00008a5dfd..ba9a0a0a9a5a 100644
--- a/drivers/clk/aspeed/clk_ast2600.c
+++ b/drivers/clk/aspeed/clk_ast2600.c
@@ -1066,6 +1066,25 @@ static ulong ast2600_enable_usbbhclk(struct ast2600_scu *scu)
 	return 0;
 }
 
+/* also known as yclk */
+static ulong ast2600_enable_haceclk(struct ast2600_scu *scu)
+{
+	u32 reset_bit;
+	u32 clkstop_bit;
+
+	reset_bit = BIT(ASPEED_RESET_HACE);
+	clkstop_bit = BIT(13);
+
+	writel(reset_bit, &scu->sysreset_ctrl1);
+	udelay(100);
+	writel(clkstop_bit, &scu->clk_stop_clr_ctrl1);
+	mdelay(20);
+
+	writel(reset_bit, &scu->sysreset_clr_ctrl1);
+
+	return 0;
+}
+
 static int ast2600_clk_enable(struct clk *clk)
 {
 	struct ast2600_clk_priv *priv = dev_get_priv(clk->dev);
@@ -1104,6 +1123,9 @@ static int ast2600_clk_enable(struct clk *clk)
 		case ASPEED_CLK_GATE_USBPORT2CLK:
 			ast2600_enable_usbbhclk(priv->scu);
 			break;
+		case ASPEED_CLK_GATE_YCLK:
+			ast2600_enable_haceclk(priv->scu);
+			break;
 		default:
 			pr_debug("can't enable clk \n");
 			return -ENOENT;
-- 
2.30.2

Re: [PATCH u-boot v2019.04-aspeed-openbmc 09/11] clk: aspeed: Add HACE yclk to ast2600

[Klaus Heinrich Kiwi]

On 4/13/2021 5:07 AM, Joel Stanley wrote:
> Signed-off-by: Joel Stanley <joel@jms.id.au>
Reviewed-by: Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>
> ---
>   drivers/clk/aspeed/clk_ast2600.c | 22 ++++++++++++++++++++++
>   1 file changed, 22 insertions(+)
> 
> diff --git a/drivers/clk/aspeed/clk_ast2600.c b/drivers/clk/aspeed/clk_ast2600.c
> index 4c00008a5dfd..ba9a0a0a9a5a 100644
> --- a/drivers/clk/aspeed/clk_ast2600.c
> +++ b/drivers/clk/aspeed/clk_ast2600.c
> @@ -1066,6 +1066,25 @@ static ulong ast2600_enable_usbbhclk(struct ast2600_scu *scu)
>   	return 0;
>   }
> 
> +/* also known as yclk */
> +static ulong ast2600_enable_haceclk(struct ast2600_scu *scu)
> +{
> +	u32 reset_bit;
> +	u32 clkstop_bit;
> +
> +	reset_bit = BIT(ASPEED_RESET_HACE);
> +	clkstop_bit = BIT(13);
> +
> +	writel(reset_bit, &scu->sysreset_ctrl1);
> +	udelay(100);
> +	writel(clkstop_bit, &scu->clk_stop_clr_ctrl1);
> +	mdelay(20);
> +
> +	writel(reset_bit, &scu->sysreset_clr_ctrl1);
> +
> +	return 0;
> +}
> +
>   static int ast2600_clk_enable(struct clk *clk)
>   {
>   	struct ast2600_clk_priv *priv = dev_get_priv(clk->dev);
> @@ -1104,6 +1123,9 @@ static int ast2600_clk_enable(struct clk *clk)
>   		case ASPEED_CLK_GATE_USBPORT2CLK:
>   			ast2600_enable_usbbhclk(priv->scu);
>   			break;
> +		case ASPEED_CLK_GATE_YCLK:
> +			ast2600_enable_haceclk(priv->scu);
> +			break;
>   		default:
>   			pr_debug("can't enable clk \n");
>   			return -ENOENT;
> 

-- 
Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>

[PATCH u-boot v2019.04-aspeed-openbmc 10/11] crypto: Add driver for Aspeed HACE

[Joel Stanley]

The HACE supports MD5, SHA1 and SHA2 family hash functions. This driver
uses it in a polling mode to perform hash calculations over buffers
placed in DRAM.

Co-developed-by: Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>
Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 drivers/crypto/Kconfig       |  16 +++
 drivers/crypto/Makefile      |   1 +
 drivers/crypto/aspeed_hace.c | 250 +++++++++++++++++++++++++++++++++++
 3 files changed, 267 insertions(+)
 create mode 100644 drivers/crypto/aspeed_hace.c

diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index 1ea116be7503..f78e41e0e9e7 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -2,4 +2,20 @@ menu "Hardware crypto devices"
 
 source drivers/crypto/fsl/Kconfig
 
+config ASPEED_HACE
+	bool "ASPEED Hash and Crypto Engine"
+	select SHA_HW_ACCEL
+	select SHA_PROG_HW_ACCEL
+	depends on ASPEED_AST2600
+	help
+	 Select this option to enable a driver for using the SHA engine in
+	 the ASPEED BMC SoCs.
+
+	 Enabling this allows the use of SHA operations in hardware without requiring the
+	 SHA software implementations, saving code size.
+
+	 Due to hardware limitations it cannot be used with a FIT placed in SPI
+	 FLASH. Data can only be hashed if it is in SDRAM, making this relevant
+	 for MMC and network boot only.
+
 endmenu
diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile
index efbd1d3fca05..ac93b1295954 100644
--- a/drivers/crypto/Makefile
+++ b/drivers/crypto/Makefile
@@ -4,5 +4,6 @@
 # 	http://www.samsung.com
 
 obj-$(CONFIG_EXYNOS_ACE_SHA)	+= ace_sha.o
+obj-$(CONFIG_ASPEED_HACE)	+= aspeed_hace.o
 obj-y += rsa_mod_exp/
 obj-y += fsl/
diff --git a/drivers/crypto/aspeed_hace.c b/drivers/crypto/aspeed_hace.c
new file mode 100644
index 000000000000..473d4d7391b7
--- /dev/null
+++ b/drivers/crypto/aspeed_hace.c
@@ -0,0 +1,250 @@
+/*
+ * (C) Copyright ASPEED Technology Inc.
+ * Copyright 2021 IBM Corp.
+ *
+ * SPDX-License-Identifier:	GPL-2.0-or-later
+ */
+
+#include <common.h>
+#include <clk.h>
+
+#include <log.h>
+#include <asm/io.h>
+#include <malloc.h>
+#include <hash.h>
+
+#include <dm/device.h>
+#include <dm/fdtaddr.h>
+
+#include <linux/bitops.h>
+#include <linux/delay.h>
+#include <linux/kernel.h>
+#include <linux/iopoll.h>
+
+#define ASPEED_HACE_STS			0x1C
+#define  HACE_RSA_ISR			BIT(13)
+#define  HACE_CRYPTO_ISR		BIT(12)
+#define  HACE_HASH_ISR			BIT(9)
+#define  HACE_RSA_BUSY			BIT(2)
+#define  HACE_CRYPTO_BUSY		BIT(1)
+#define  HACE_HASH_BUSY			BIT(0)
+#define ASPEED_HACE_HASH_SRC		0x20
+#define ASPEED_HACE_HASH_DIGEST_BUFF	0x24
+#define ASPEED_HACE_HASH_KEY_BUFF	0x28
+#define ASPEED_HACE_HASH_DATA_LEN	0x2C
+#define  HACE_SG_LAST			BIT(31)
+#define ASPEED_HACE_HASH_CMD		0x30
+#define  HACE_SHA_BE_EN			BIT(3)
+#define  HACE_MD5_LE_EN			BIT(2)
+#define  HACE_ALGO_MD5			0
+#define  HACE_ALGO_SHA1			BIT(5)
+#define  HACE_ALGO_SHA224		BIT(6)
+#define  HACE_ALGO_SHA256		(BIT(4) | BIT(6))
+#define  HACE_ALGO_SHA512		(BIT(5) | BIT(6))
+#define  HACE_ALGO_SHA384		(BIT(5) | BIT(6) | BIT(10))
+#define  HACE_SG_EN			BIT(18)
+
+#define ASPEED_MAX_SG			32
+
+struct aspeed_sg {
+	u32 len;
+	u32 addr;
+};
+
+struct aspeed_hash_ctx {
+	u32 method;
+	u32 digest_size;
+	u32 len;
+	u32 count;
+	struct aspeed_sg list[ASPEED_MAX_SG] __attribute__((aligned(8)));
+};
+
+struct aspeed_hace {
+	struct clk clk;
+};
+
+static phys_addr_t base;
+
+static int aspeed_hace_wait_completion(u32 reg, u32 flag, int timeout_us)
+{
+	u32 val;
+
+	return readl_poll_timeout(reg, val, (val & flag) == flag, timeout_us);
+}
+
+static int digest_object(const void *src, unsigned int length, void *digest,
+		  u32 method)
+{
+	if (!((u32)src & BIT(31))) {
+		debug("HACE src out of bounds: can only copy from SDRAM\n");
+		return -EINVAL;
+	}
+
+	if ((u32)digest & 0x7) {
+		debug("HACE dest alignment incorrect: %p\n", digest);
+		return -EINVAL;
+	}
+
+	writel((u32)src, base + ASPEED_HACE_HASH_SRC);
+	writel((u32)digest, base + ASPEED_HACE_HASH_DIGEST_BUFF);
+	writel(length, base + ASPEED_HACE_HASH_DATA_LEN);
+	writel(HACE_SHA_BE_EN | method, base + ASPEED_HACE_HASH_CMD);
+
+	/* SHA512 hashing appears to have a througput of about 12MB/s */
+	return aspeed_hace_wait_completion(base + ASPEED_HACE_STS,
+			HACE_HASH_ISR,
+			1000 + (length >> 3));
+}
+
+void hw_sha1(const unsigned char *pbuf, unsigned int buf_len,
+	       unsigned char *pout, unsigned int chunk_size)
+{
+	int rc;
+
+	rc = digest_object(pbuf, buf_len, pout, HACE_ALGO_SHA1);
+	if (rc)
+		debug("HACE failure: %d\n", rc);
+}
+
+void hw_sha256(const unsigned char *pbuf, unsigned int buf_len,
+	       unsigned char *pout, unsigned int chunk_size)
+{
+	int rc;
+
+	rc = digest_object(pbuf, buf_len, pout, HACE_ALGO_SHA256);
+	if (rc)
+		debug("HACE failure: %d\n", rc);
+}
+
+void hw_sha512(const unsigned char *pbuf, unsigned int buf_len,
+	       unsigned char *pout, unsigned int chunk_size)
+{
+	int rc;
+
+	rc = digest_object(pbuf, buf_len, pout, HACE_ALGO_SHA512);
+	if (rc)
+		debug("HACE failure: %d\n", rc);
+}
+
+#if IS_ENABLED(CONFIG_SHA_PROG_HW_ACCEL)
+int hw_sha_init(struct hash_algo *algo, void **ctxp)
+{
+	struct aspeed_hash_ctx *ctx;
+	u32 method;
+
+	if (!strcmp(algo->name, "sha1")) {
+		method = HACE_ALGO_SHA1;
+	}
+	else if (!strcmp(algo->name, "sha256")) {
+		method = HACE_ALGO_SHA256;
+	}
+	else if (!strcmp(algo->name, "sha512")) {
+		method = HACE_ALGO_SHA512;
+	}
+	else  {
+		return -ENOTSUPP;
+	}
+
+	ctx = calloc(1, sizeof(*ctx));
+
+	if (ctx == NULL) {
+		debug("Cannot allocate memory for context\n");
+		return -ENOMEM;
+	}
+	ctx->method = method | HACE_SG_EN;
+	ctx->digest_size = algo->digest_size;
+	*ctxp = ctx;
+
+	return 0;
+}
+
+int hw_sha_update(struct hash_algo *algo, void *hash_ctx, const void *buf,
+		  unsigned int size, int is_last)
+{
+	struct aspeed_hash_ctx *ctx = hash_ctx;
+	struct aspeed_sg *sg = &ctx->list[ctx->count];
+
+	if (ctx->count >= ARRAY_SIZE(ctx->list)) {
+		debug("HACE error: Reached maximum number of hash segments\n");
+		free(ctx);
+		return -EINVAL;
+	}
+
+	sg->addr = (u32)buf;
+	sg->len = size;
+	if (is_last)
+		sg->len |= HACE_SG_LAST;
+
+	ctx->count++;
+	ctx->len += size;
+
+	return 0;
+}
+
+int hw_sha_finish(struct hash_algo *algo, void *hash_ctx, void *dest_buf, int size)
+{
+	struct aspeed_hash_ctx *ctx = hash_ctx;
+	int rc;
+
+	if (size < ctx->digest_size) {
+		debug("HACE error: insufficient size on destination buffer\n");
+		free(ctx);
+		return -EINVAL;
+	}
+
+	rc = digest_object(ctx->list, ctx->len, dest_buf, ctx->method);
+	if (rc)
+		debug("HACE Scatter-Gather failure\n");
+
+	free(ctx);
+
+	return rc;
+}
+#endif
+
+static int aspeed_hace_probe(struct udevice *dev)
+{
+	struct aspeed_hace *hace = dev_get_priv(dev);
+	int ret;
+
+	ret = clk_get_by_index(dev, 0, &hace->clk);
+	if (ret < 0) {
+		debug("Can't get clock for %s: %d\n", dev->name, ret);
+		return ret;
+	}
+
+	ret = clk_enable(&hace->clk);
+	if (ret) {
+		debug("Failed to enable fsi clock (%d)\n", ret);
+		return ret;
+	}
+
+	/* As the crypto code does not pass us any driver state */
+	base = devfdt_get_addr(dev);
+
+	return ret;
+}
+
+static int aspeed_hace_remove(struct udevice *dev)
+{
+	struct aspeed_hace *hace = dev_get_priv(dev);
+
+	clk_disable(&hace->clk);
+
+	return 0;
+}
+
+static const struct udevice_id aspeed_hace_ids[] = {
+	{ .compatible = "aspeed,ast2600-hace" },
+	{ }
+};
+
+U_BOOT_DRIVER(aspeed_hace) = {
+	.name		= "aspeed_hace",
+	.id		= UCLASS_MISC,
+	.of_match	= aspeed_hace_ids,
+	.probe		= aspeed_hace_probe,
+	.remove 	= aspeed_hace_remove,
+	.priv_auto_alloc_size = sizeof(struct aspeed_hace),
+	.flags  = DM_FLAG_PRE_RELOC,
+};
-- 
2.30.2

Re: [PATCH u-boot v2019.04-aspeed-openbmc 10/11] crypto: Add driver for Aspeed HACE

[Klaus Heinrich Kiwi]

Hi Joel,

On 4/13/2021 5:07 AM, Joel Stanley wrote:
> The HACE supports MD5, SHA1 and SHA2 family hash functions. This driver
> uses it in a polling mode to perform hash calculations over buffers
> placed in DRAM.
> 
> Co-developed-by: Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>   drivers/crypto/Kconfig       |  16 +++
>   drivers/crypto/Makefile      |   1 +
>   drivers/crypto/aspeed_hace.c | 250 +++++++++++++++++++++++++++++++++++
>   3 files changed, 267 insertions(+)
>   create mode 100644 drivers/crypto/aspeed_hace.c
> 
> diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
> index 1ea116be7503..f78e41e0e9e7 100644
> --- a/drivers/crypto/Kconfig
> +++ b/drivers/crypto/Kconfig
> @@ -2,4 +2,20 @@ menu "Hardware crypto devices"
> 
>   source drivers/crypto/fsl/Kconfig
> 
> +config ASPEED_HACE
> +	bool "ASPEED Hash and Crypto Engine"
> +	select SHA_HW_ACCEL
> +	select SHA_PROG_HW_ACCEL
> +	depends on ASPEED_AST2600
> +	help
> +	 Select this option to enable a driver for using the SHA engine in
> +	 the ASPEED BMC SoCs.
> +
> +	 Enabling this allows the use of SHA operations in hardware without requiring the
> +	 SHA software implementations, saving code size.
> +
> +	 Due to hardware limitations it cannot be used with a FIT placed in SPI
> +	 FLASH. Data can only be hashed if it is in SDRAM, making this relevant
> +	 for MMC and network boot only.
> +
>   endmenu
> diff --git a/drivers/crypto/Makefile b/drivers/crypto/Makefile
> index efbd1d3fca05..ac93b1295954 100644
> --- a/drivers/crypto/Makefile
> +++ b/drivers/crypto/Makefile
> @@ -4,5 +4,6 @@
>   # 	http://www.samsung.com
> 
>   obj-$(CONFIG_EXYNOS_ACE_SHA)	+= ace_sha.o
> +obj-$(CONFIG_ASPEED_HACE)	+= aspeed_hace.o
>   obj-y += rsa_mod_exp/
>   obj-y += fsl/
> diff --git a/drivers/crypto/aspeed_hace.c b/drivers/crypto/aspeed_hace.c
> new file mode 100644
> index 000000000000..473d4d7391b7
> --- /dev/null
> +++ b/drivers/crypto/aspeed_hace.c
> @@ -0,0 +1,250 @@
> +/*
> + * (C) Copyright ASPEED Technology Inc.
> + * Copyright 2021 IBM Corp.
> + *
> + * SPDX-License-Identifier:	GPL-2.0-or-later
> + */
> +
> +#include <common.h>
> +#include <clk.h>
> +
> +#include <log.h>
> +#include <asm/io.h>
> +#include <malloc.h>
> +#include <hash.h>
> +
> +#include <dm/device.h>
> +#include <dm/fdtaddr.h>
> +
> +#include <linux/bitops.h>
> +#include <linux/delay.h>
> +#include <linux/kernel.h>
> +#include <linux/iopoll.h>
> +
> +#define ASPEED_HACE_STS			0x1C
> +#define  HACE_RSA_ISR			BIT(13)
> +#define  HACE_CRYPTO_ISR		BIT(12)
> +#define  HACE_HASH_ISR			BIT(9)
> +#define  HACE_RSA_BUSY			BIT(2)
> +#define  HACE_CRYPTO_BUSY		BIT(1)
> +#define  HACE_HASH_BUSY			BIT(0)
> +#define ASPEED_HACE_HASH_SRC		0x20
> +#define ASPEED_HACE_HASH_DIGEST_BUFF	0x24
> +#define ASPEED_HACE_HASH_KEY_BUFF	0x28
> +#define ASPEED_HACE_HASH_DATA_LEN	0x2C
> +#define  HACE_SG_LAST			BIT(31)
> +#define ASPEED_HACE_HASH_CMD		0x30
> +#define  HACE_SHA_BE_EN			BIT(3)
> +#define  HACE_MD5_LE_EN			BIT(2)
> +#define  HACE_ALGO_MD5			0
> +#define  HACE_ALGO_SHA1			BIT(5)
> +#define  HACE_ALGO_SHA224		BIT(6)
> +#define  HACE_ALGO_SHA256		(BIT(4) | BIT(6))
> +#define  HACE_ALGO_SHA512		(BIT(5) | BIT(6))
> +#define  HACE_ALGO_SHA384		(BIT(5) | BIT(6) | BIT(10))
> +#define  HACE_SG_EN			BIT(18)
> +
> +#define ASPEED_MAX_SG			32
> +
> +struct aspeed_sg {
> +	u32 len;
> +	u32 addr;
> +};
> +
> +struct aspeed_hash_ctx {
> +	u32 method;
> +	u32 digest_size;
> +	u32 len;
> +	u32 count;
> +	struct aspeed_sg list[ASPEED_MAX_SG] __attribute__((aligned(8)));
> +};
> +
> +struct aspeed_hace {
> +	struct clk clk;
> +};
> +
> +static phys_addr_t base;
> +
> +static int aspeed_hace_wait_completion(u32 reg, u32 flag, int timeout_us)
> +{
> +	u32 val;
> +
> +	return readl_poll_timeout(reg, val, (val & flag) == flag, timeout_us);
> +}
> +
> +static int digest_object(const void *src, unsigned int length, void *digest,
> +		  u32 method)
> +{
> +	if (!((u32)src & BIT(31))) {
> +		debug("HACE src out of bounds: can only copy from SDRAM\n");
> +		return -EINVAL;
> +	}
> +
> +	if ((u32)digest & 0x7) {
> +		debug("HACE dest alignment incorrect: %p\n", digest);
> +		return -EINVAL;
> +	}
> +
> +	writel((u32)src, base + ASPEED_HACE_HASH_SRC);
> +	writel((u32)digest, base + ASPEED_HACE_HASH_DIGEST_BUFF);
> +	writel(length, base + ASPEED_HACE_HASH_DATA_LEN);
> +	writel(HACE_SHA_BE_EN | method, base + ASPEED_HACE_HASH_CMD);
> +
> +	/* SHA512 hashing appears to have a througput of about 12MB/s */
> +	return aspeed_hace_wait_completion(base + ASPEED_HACE_STS,
> +			HACE_HASH_ISR,
> +			1000 + (length >> 3));

In some of my previous testing with the un-cleaned patchset
(https://github.com/klauskiwi/u-boot/tree/hace_sg_work), the
Qemu implementation (Cedric's Aspeed-6.0 branch) worked fine,
but on hardware I was getting errors until I explicitly
cleared the HACE_HASH_ISR before attemptinga new command..
It makes sense since the readl_poll_timeout() would return
immediately, without completing the command, if the HASH_ISR bit
is set.

> +}
> +
> +void hw_sha1(const unsigned char *pbuf, unsigned int buf_len,
> +	       unsigned char *pout, unsigned int chunk_size)
> +{
> +	int rc;
> +
> +	rc = digest_object(pbuf, buf_len, pout, HACE_ALGO_SHA1);
> +	if (rc)
> +		debug("HACE failure: %d\n", rc);
> +}
> +
> +void hw_sha256(const unsigned char *pbuf, unsigned int buf_len,
> +	       unsigned char *pout, unsigned int chunk_size)
> +{
> +	int rc;
> +
> +	rc = digest_object(pbuf, buf_len, pout, HACE_ALGO_SHA256);
> +	if (rc)
> +		debug("HACE failure: %d\n", rc);
> +}
> +
> +void hw_sha512(const unsigned char *pbuf, unsigned int buf_len,
> +	       unsigned char *pout, unsigned int chunk_size)
> +{
> +	int rc;
> +
> +	rc = digest_object(pbuf, buf_len, pout, HACE_ALGO_SHA512);
> +	if (rc)
> +		debug("HACE failure: %d\n", rc);
> +}
> +
> +#if IS_ENABLED(CONFIG_SHA_PROG_HW_ACCEL)
> +int hw_sha_init(struct hash_algo *algo, void **ctxp)
> +{
> +	struct aspeed_hash_ctx *ctx;
> +	u32 method;
> +
> +	if (!strcmp(algo->name, "sha1")) {
> +		method = HACE_ALGO_SHA1;
> +	}
> +	else if (!strcmp(algo->name, "sha256")) {
> +		method = HACE_ALGO_SHA256;
> +	}
> +	else if (!strcmp(algo->name, "sha512")) {
> +		method = HACE_ALGO_SHA512;
> +	}
> +	else  {
> +		return -ENOTSUPP;
> +	}
> +
> +	ctx = calloc(1, sizeof(*ctx));
> +
> +	if (ctx == NULL) {
> +		debug("Cannot allocate memory for context\n");
> +		return -ENOMEM;
> +	}
> +	ctx->method = method | HACE_SG_EN;
> +	ctx->digest_size = algo->digest_size;
> +	*ctxp = ctx;
> +
> +	return 0;
> +}
> +
> +int hw_sha_update(struct hash_algo *algo, void *hash_ctx, const void *buf,
> +		  unsigned int size, int is_last)
> +{
> +	struct aspeed_hash_ctx *ctx = hash_ctx;
> +	struct aspeed_sg *sg = &ctx->list[ctx->count];
> +
> +	if (ctx->count >= ARRAY_SIZE(ctx->list)) {
> +		debug("HACE error: Reached maximum number of hash segments\n");
> +		free(ctx);
> +		return -EINVAL;
> +	}
> +
> +	sg->addr = (u32)buf;
> +	sg->len = size;
> +	if (is_last)
> +		sg->len |= HACE_SG_LAST;
> +
> +	ctx->count++;
> +	ctx->len += size;
> +
> +	return 0;
> +}
> +
> +int hw_sha_finish(struct hash_algo *algo, void *hash_ctx, void *dest_buf, int size)
> +{
> +	struct aspeed_hash_ctx *ctx = hash_ctx;
> +	int rc;
> +
> +	if (size < ctx->digest_size) {
> +		debug("HACE error: insufficient size on destination buffer\n");
> +		free(ctx);
> +		return -EINVAL;
> +	}
> +
> +	rc = digest_object(ctx->list, ctx->len, dest_buf, ctx->method);
> +	if (rc)
> +		debug("HACE Scatter-Gather failure\n");
> +
> +	free(ctx);
> +
> +	return rc;
> +}
> +#endif
> +
> +static int aspeed_hace_probe(struct udevice *dev)
> +{
> +	struct aspeed_hace *hace = dev_get_priv(dev);
> +	int ret;
> +
> +	ret = clk_get_by_index(dev, 0, &hace->clk);
> +	if (ret < 0) {
> +		debug("Can't get clock for %s: %d\n", dev->name, ret);
> +		return ret;
> +	}
> +
> +	ret = clk_enable(&hace->clk);
> +	if (ret) {
> +		debug("Failed to enable fsi clock (%d)\n", ret);
> +		return ret;
> +	}
> +
> +	/* As the crypto code does not pass us any driver state */
> +	base = devfdt_get_addr(dev);
> +
> +	return ret;
> +}
> +
> +static int aspeed_hace_remove(struct udevice *dev)
> +{
> +	struct aspeed_hace *hace = dev_get_priv(dev);
> +
> +	clk_disable(&hace->clk);
> +
> +	return 0;
> +}
> +
> +static const struct udevice_id aspeed_hace_ids[] = {
> +	{ .compatible = "aspeed,ast2600-hace" },
> +	{ }
> +};
> +
> +U_BOOT_DRIVER(aspeed_hace) = {
> +	.name		= "aspeed_hace",
> +	.id		= UCLASS_MISC,
> +	.of_match	= aspeed_hace_ids,
> +	.probe		= aspeed_hace_probe,
> +	.remove 	= aspeed_hace_remove,
> +	.priv_auto_alloc_size = sizeof(struct aspeed_hace),
> +	.flags  = DM_FLAG_PRE_RELOC,
> +};
> 

I've tested your patchset with Cedric's Aspeed-6.0 but looks
like the probe function is never called. Reading through the code
a bit more, looks like you need to explicitly probe this device
somewhere in board_init_r (that is, after sdram was initialized),
since functions like dm_scan_fdt() and dm_extended_scan_fdt() will
only scan subnodes of the top level, and the clocks node.

This is what I get (with some added printfs of mine):

qemu-system-arm: warning: Aspeed iBT has no chardev backend
qemu-system-arm: warning: nic ftgmac100.0 has no peer
qemu-system-arm: warning: nic ftgmac100.1 has no peer
qemu-system-arm: warning: nic ftgmac100.2 has no peer
qemu-system-arm: warning: nic ftgmac100.3 has no peer
aspeed_timer_ctrl_pulse_enable: Timer does not support pulse mode
aspeed_timer_ctrl_pulse_enable: Timer does not support pulse mode
aspeed_timer_ctrl_pulse_enable: Timer does not support pulse mode
aspeed_timer_ctrl_pulse_enable: Timer does not support pulse mode
aspeed_ast2600_scu_write: SCU is locked!
aspeed_ast2600_scu_write: SCU is locked!
aspeed_smc_write: not implemented: 0x18

U-Boot SPL 2019.04 (Apr 13 2021 - 17:57:20 +0000)
aspeed_soc.io: unimplemented device read  (size 4, offset 0x0f500c)
aspeed_soc.io: unimplemented device write (size 4, offset 0x0f500c, value 0x00000000)
Trying to boot from MMC1
SD: CMD8 in a wrong state
## Checking hash(es) for Image uboot ... sha512,rsa4096:autogenerated-uboot-4096-key
digest_object: ASPEED_HACE_STS='0xe59ff03c'

digest_object: SCU080h='0xf7ff7f8a'

digest_object: writing '0x90200104' to ASPEED_HACE_HASH_SRC

digest_object: writing '0x902ffa50' to ASPEED_HACE_HASH_DIGEST_BUFF

digest_object: writing '0x0005ccd4' to ASPEED_HACE_HASH_DATA_LEN

digest_object: writing '0x00040068' to ASPEED_HACE_HASH_CMD
HACE Scatter-Gather failure
rsa_verify: Error in checksum calculation
- Failed to verify required signature 'key-autogenerated-uboot-4096-key'
  error!
Unable to verify required signature for '' hash node in 'uboot' image node
mmc_load_image_raw_sector: mmc block read error
Trying to boot from UART
CCQEMU: Terminated

-- 
Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>

Re: [PATCH u-boot v2019.04-aspeed-openbmc 10/11] crypto: Add driver for Aspeed HACE

[Klaus Heinrich Kiwi]

On 4/13/2021 5:41 PM, Klaus Heinrich Kiwi wrote:
> I've tested your patchset with Cedric's Aspeed-6.0 but looks
> like the probe function is never called. Reading through the code
> a bit more, looks like you need to explicitly probe this device
> somewhere in board_init_r (that is, after sdram was initialized),
> since functions like dm_scan_fdt() and dm_extended_scan_fdt() will
> only scan subnodes of the top level, and the clocks node.
> 
> This is what I get (with some added printfs of mine):

I've played around a bit more, and got it to work on Qemu with the following changes:

  * Added a board-specific spl_board_init() initializing the HACE driver at
    the SPL's board_init_r() timeframe. Enabled that on the defconfig file.
  * Because the driver model is using some pre-sdram malloc pool space,
    the changes above were causing the probing of the sdram itself to fail.
    Corrected by increasing the pre-sdram malloc pool to 0x1000.

 From 4ddc9fe2727a54f2d8c088d1c7046f4a3ab56314 Mon Sep 17 00:00:00 2001
From: Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>
Date: Wed, 14 Apr 2021 16:29:15 -0300
Subject: [PATCH] fixup: Enable HACE probing on SPL, expand malloc pool

Signed-off-by: Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>
---
  arch/arm/mach-aspeed/ast2600/spl.c         | 15 +++++++++++++++
  configs/ast2600_openbmc_spl_emmc_defconfig |  3 ++-
  2 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-aspeed/ast2600/spl.c b/arch/arm/mach-aspeed/ast2600/spl.c
index 527e14f8e3..2b1e8477eb 100644
--- a/arch/arm/mach-aspeed/ast2600/spl.c
+++ b/arch/arm/mach-aspeed/ast2600/spl.c
@@ -86,3 +86,18 @@ struct image_header *spl_get_load_buffer(ssize_t offset, size_t size)
  {
  	return (struct image_header *)(CONFIG_SYS_LOAD_ADDR);
  }
+
+#if CONFIG_IS_ENABLED(BOARD_INIT)
+void spl_board_init(void)
+{
+#ifdef CONFIG_ASPEED_HACE
+	struct udevice *dev;
+
+	if (uclass_get_device_by_driver(UCLASS_MISC, DM_GET_DRIVER(aspeed_hace),
+									&dev))
+		debug("Warning: HACE initialization failure\n");
+#endif
+
+	return;
+}
+#endif /* CONFIG_IS_ENABLED(BOARD_INIT) */
\ No newline at end of file
diff --git a/configs/ast2600_openbmc_spl_emmc_defconfig b/configs/ast2600_openbmc_spl_emmc_defconfig
index 05069084cb..e856df2398 100644
--- a/configs/ast2600_openbmc_spl_emmc_defconfig
+++ b/configs/ast2600_openbmc_spl_emmc_defconfig
@@ -24,7 +24,7 @@ CONFIG_ASPEED_KERNEL_FIT_DRAM_BASE=0x83000000
  CONFIG_TARGET_EVB_AST2600A1=y
  CONFIG_SPL_LIBCOMMON_SUPPORT=y
  CONFIG_SPL_LIBGENERIC_SUPPORT=y
-CONFIG_SYS_MALLOC_F_LEN=0x800
+CONFIG_SYS_MALLOC_F_LEN=0x1000
  CONFIG_SPL_MMC_SUPPORT=y
  CONFIG_SPL_SERIAL_SUPPORT=y
  CONFIG_SPL_DRIVERS_MISC_SUPPORT=y
@@ -48,6 +48,7 @@ CONFIG_SYS_CONSOLE_ENV_OVERWRITE=y
  CONFIG_DISPLAY_BOARDINFO_LATE=y
  CONFIG_ARCH_EARLY_INIT_R=y
  CONFIG_BOARD_EARLY_INIT_F=y
+CONFIG_SPL_BOARD_INIT=y
  # CONFIG_SPL_LEGACY_IMAGE_SUPPORT is not set
  CONFIG_SPL_SYS_MALLOC_SIMPLE=y
  CONFIG_SPL_STACK_R=y
-- 
2.25.1


However, when I tried to test it on a Rainier, it failed:
U-Boot SPL 2019.04 (Apr 14 2021 - 19:31:59 +0000)
already initialized, Trying to boot from MMC1
## Checking hash(es) for Image uboot ... sha512,rsa4096:autogenerated-uboot-4096-key- Failed to verify required signature 'key-autogenera'
  error!
Unable to verify required signature for '' hash node in 'uboot' image node
mmc_load_image_raw_sector: mmc block read error
Trying to boot from UART
CCCCC\x06�P

(and yes, I had since disabled my debugging printf's). I wonder if the HASH_ISR
may need to be explicitly cleared, although I'd expect it to work for the first
command at least.

Another interesting thing is that the SPL tries to boot from UART, but neither
my fitImages, Legacy images or even RAW images are working.. Not sure if we need
some special handling of those images before feeding them to the spl ymodem loader?

  -Klaus

-- 
Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>

Re: [PATCH u-boot v2019.04-aspeed-openbmc 10/11] crypto: Add driver for Aspeed HACE

[Joel Stanley]

On Wed, 14 Apr 2021 at 20:28, Klaus Heinrich Kiwi
<klaus@linux.vnet.ibm.com> wrote:
>
>
>
> On 4/13/2021 5:41 PM, Klaus Heinrich Kiwi wrote:
> > I've tested your patchset with Cedric's Aspeed-6.0 but looks
> > like the probe function is never called. Reading through the code
> > a bit more, looks like you need to explicitly probe this device
> > somewhere in board_init_r (that is, after sdram was initialized),
> > since functions like dm_scan_fdt() and dm_extended_scan_fdt() will
> > only scan subnodes of the top level, and the clocks node.
> >
> > This is what I get (with some added printfs of mine):
>
> I've played around a bit more, and got it to work on Qemu with the following changes:
>
>   * Added a board-specific spl_board_init() initializing the HACE driver at
>     the SPL's board_init_r() timeframe. Enabled that on the defconfig file.
>   * Because the driver model is using some pre-sdram malloc pool space,
>     the changes above were causing the probing of the sdram itself to fail.
>     Corrected by increasing the pre-sdram malloc pool to 0x1000.

Thanks. I had added something similar when debugging this yesterday.

I had tested the changes, but must have mixed up which images were
being loaded into qemu.

> However, when I tried to test it on a Rainier, it failed:
> U-Boot SPL 2019.04 (Apr 14 2021 - 19:31:59 +0000)
> already initialized, Trying to boot from MMC1
> ## Checking hash(es) for Image uboot ... sha512,rsa4096:autogenerated-uboot-4096-key- Failed to verify required signature 'key-autogenera'
>   error!
> Unable to verify required signature for '' hash node in 'uboot' image node
> mmc_load_image_raw_sector: mmc block read error
> Trying to boot from UART
> CCCCC �P
>
> (and yes, I had since disabled my debugging printf's). I wonder if the HASH_ISR
> may need to be explicitly cleared, although I'd expect it to work for the first
> command at least.
>
> Another interesting thing is that the SPL tries to boot from UART, but neither
> my fitImages, Legacy images or even RAW images are working.. Not sure if we need
> some special handling of those images before feeding them to the spl ymodem loader?

I wasn't able to get the SPL to load any images - raw binaries or FIT
- from eMMC either. Something is going wrong, but I am unsure what it
is. I will continue to debug.

Cheers,

Joel

Re: [PATCH u-boot v2019.04-aspeed-openbmc 10/11] crypto: Add driver for Aspeed HACE

[Klaus Heinrich Kiwi]

On 4/14/2021 11:32 PM, Joel Stanley wrote:
>> Another interesting thing is that the SPL tries to boot from UART, but neither
>> my fitImages, Legacy images or even RAW images are working.. Not sure if we need
>> some special handling of those images before feeding them to the spl ymodem loader?
> I wasn't able to get the SPL to load any images - raw binaries or FIT
> - from eMMC either. Something is going wrong, but I am unsure what it
> is. I will continue to debug.

I was able to make it work on real hardware (rainier100) with the following changes
(in addition to the other fixups already mentioned in this thread):

 From a2a2819455ec5de689fd0702cce20bfb18ec11b1 Mon Sep 17 00:00:00 2001
From: Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>
Date: Thu, 15 Apr 2021 15:16:37 -0300
Subject: [PATCH] HACE fixups:

  * The AST2600 workbook mentions that the list structure passed to
    ASPEED_HACE_HASH_SRC needs to be 8-byte aligned. Normally, glibc's
    malloc() would always align memory to (at least) 8-bytes, but that's
    the case with u-boot's pre-sdram malloc() implementation. So we need
    to explicitly align the context to 8-bytes with malign().

  * The __atribute__ ((align 8)) doesn't have an effect in struct
    elements. Remove it.

  * Since the struct aspeed_hash_ctx->list element is what we need to
    make sure is aligned to 9-bytes, move that to the first element of
    the array, and call-out the fact that this needs to be aligned in
    the declaration.

   * Clear HACE_HASH_ISR before issuing new command

Signed-off-by: Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>
---
  drivers/crypto/aspeed_hace.c | 15 +++++++++++++--
  1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/crypto/aspeed_hace.c b/drivers/crypto/aspeed_hace.c
index 473d4d7391..0551fe6c83 100644
--- a/drivers/crypto/aspeed_hace.c
+++ b/drivers/crypto/aspeed_hace.c
@@ -51,12 +51,19 @@ struct aspeed_sg {
  	u32 addr;
  };
  
+
+/*
+ * Note: element 'list' below needs to be 8-byte aligned,
+ *       keep it as the first element so that we can always
+ *       guarantee that when allocating the struct (that should
+ *       also be 8-byte aligned)
+ */
  struct aspeed_hash_ctx {
+	struct aspeed_sg list[ASPEED_MAX_SG];
  	u32 method;
  	u32 digest_size;
  	u32 len;
  	u32 count;
-	struct aspeed_sg list[ASPEED_MAX_SG] __attribute__((aligned(8)));
  };
  
  struct aspeed_hace {
@@ -85,6 +92,9 @@ static int digest_object(const void *src, unsigned int length, void *digest,
  		return -EINVAL;
  	}
  
+	/* clear any pending interrupt */
+	writel(HACE_HASH_ISR, base + ASPEED_HACE_STS);
+
  	writel((u32)src, base + ASPEED_HACE_HASH_SRC);
  	writel((u32)digest, base + ASPEED_HACE_HASH_DIGEST_BUFF);
  	writel(length, base + ASPEED_HACE_HASH_DATA_LEN);
@@ -145,12 +155,13 @@ int hw_sha_init(struct hash_algo *algo, void **ctxp)
  		return -ENOTSUPP;
  	}
  
-	ctx = calloc(1, sizeof(*ctx));
+	ctx = memalign(8, sizeof(struct aspeed_hash_ctx));
  
  	if (ctx == NULL) {
  		debug("Cannot allocate memory for context\n");
  		return -ENOMEM;
  	}
+	ctx->len = ctx->count = 0;
  	ctx->method = method | HACE_SG_EN;
  	ctx->digest_size = algo->digest_size;
  	*ctxp = ctx;
-- 
2.25.1





-- 
Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>

Re: [PATCH u-boot v2019.04-aspeed-openbmc 10/11] crypto: Add driver for Aspeed HACE

[Joel Stanley]

On Thu, 15 Apr 2021 at 21:38, Klaus Heinrich Kiwi
<klaus@linux.vnet.ibm.com> wrote:
>
>
>
> On 4/14/2021 11:32 PM, Joel Stanley wrote:
> >> Another interesting thing is that the SPL tries to boot from UART, but neither
> >> my fitImages, Legacy images or even RAW images are working.. Not sure if we need
> >> some special handling of those images before feeding them to the spl ymodem loader?
> > I wasn't able to get the SPL to load any images - raw binaries or FIT
> > - from eMMC either. Something is going wrong, but I am unsure what it
> > is. I will continue to debug.
>
> I was able to make it work on real hardware (rainier100) with the following changes
> (in addition to the other fixups already mentioned in this thread):
>
>  From a2a2819455ec5de689fd0702cce20bfb18ec11b1 Mon Sep 17 00:00:00 2001
> From: Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>
> Date: Thu, 15 Apr 2021 15:16:37 -0300
> Subject: [PATCH] HACE fixups:
>
>   * The AST2600 workbook mentions that the list structure passed to
>     ASPEED_HACE_HASH_SRC needs to be 8-byte aligned. Normally, glibc's
>     malloc() would always align memory to (at least) 8-bytes, but that's
>     the case with u-boot's pre-sdram malloc() implementation. So we need
>     to explicitly align the context to 8-bytes with malign().

We're not using the HACE engine pre-SDRAM, so we should be ok.

common/dlmalloc.c:

    8-byte alignment is guaranteed by normal malloc calls, so don't
    bother calling memalign with an argument of 8 or less.

Were you able to observe any allocations that were not aligned?

>
>   * The __atribute__ ((align 8)) doesn't have an effect in struct
>     elements. Remove it.

Agreed.

>
>   * Since the struct aspeed_hash_ctx->list element is what we need to
>     make sure is aligned to 9-bytes, move that to the first element of
>     the array, and call-out the fact that this needs to be aligned in
>     the declaration.

9 bytes? Did you mean 8?

Regardless, the array in the structure should be aligned as there will
be no padding earlier in the struct. I have added a runtime check for
this and have not hit that in my testing so far.

>
>    * Clear HACE_HASH_ISR before issuing new command

This makes sense.

I will send out a new series for review. I've tested it thoroughly in
qemu, and have tested the u-boot -> Linux path on hardware.

Cheers,

Joel

>
> Signed-off-by: Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>
> ---
>   drivers/crypto/aspeed_hace.c | 15 +++++++++++++--
>   1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/crypto/aspeed_hace.c b/drivers/crypto/aspeed_hace.c
> index 473d4d7391..0551fe6c83 100644
> --- a/drivers/crypto/aspeed_hace.c
> +++ b/drivers/crypto/aspeed_hace.c
> @@ -51,12 +51,19 @@ struct aspeed_sg {
>         u32 addr;
>   };
>
> +
> +/*
> + * Note: element 'list' below needs to be 8-byte aligned,
> + *       keep it as the first element so that we can always
> + *       guarantee that when allocating the struct (that should
> + *       also be 8-byte aligned)
> + */
>   struct aspeed_hash_ctx {
> +       struct aspeed_sg list[ASPEED_MAX_SG];
>         u32 method;
>         u32 digest_size;
>         u32 len;
>         u32 count;
> -       struct aspeed_sg list[ASPEED_MAX_SG] __attribute__((aligned(8)));
>   };
>
>   struct aspeed_hace {
> @@ -85,6 +92,9 @@ static int digest_object(const void *src, unsigned int length, void *digest,
>                 return -EINVAL;
>         }
>
> +       /* clear any pending interrupt */
> +       writel(HACE_HASH_ISR, base + ASPEED_HACE_STS);
> +
>         writel((u32)src, base + ASPEED_HACE_HASH_SRC);
>         writel((u32)digest, base + ASPEED_HACE_HASH_DIGEST_BUFF);
>         writel(length, base + ASPEED_HACE_HASH_DATA_LEN);
> @@ -145,12 +155,13 @@ int hw_sha_init(struct hash_algo *algo, void **ctxp)
>                 return -ENOTSUPP;
>         }
>
> -       ctx = calloc(1, sizeof(*ctx));
> +       ctx = memalign(8, sizeof(struct aspeed_hash_ctx));
>
>         if (ctx == NULL) {
>                 debug("Cannot allocate memory for context\n");
>                 return -ENOMEM;
>         }
> +       ctx->len = ctx->count = 0;
>         ctx->method = method | HACE_SG_EN;
>         ctx->digest_size = algo->digest_size;
>         *ctxp = ctx;
> --
> 2.25.1
>
>
>
>
>
> --
> Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>

Re: [PATCH u-boot v2019.04-aspeed-openbmc 10/11] crypto: Add driver for Aspeed HACE

[Klaus Heinrich Kiwi]

Hi Joel,

On 4/19/2021 9:49 AM, Joel Stanley wrote:
> On Thu, 15 Apr 2021 at 21:38, Klaus Heinrich Kiwi

>>
>>    * The AST2600 workbook mentions that the list structure passed to
>>      ASPEED_HACE_HASH_SRC needs to be 8-byte aligned. Normally, glibc's
>>      malloc() would always align memory to (at least) 8-bytes, but that's
>>      the case with u-boot's pre-sdram malloc() implementation. So we need
>>      to explicitly align the context to 8-bytes with malign().
> 
> We're not using the HACE engine pre-SDRAM, so we should be ok.
> 
> common/dlmalloc.c:
> 
>      8-byte alignment is guaranteed by normal malloc calls, so don't
>      bother calling memalign with an argument of 8 or less.
> 
> Were you able to observe any allocations that were not aligned?

yes, I thought I had. And by pre-sdram I guess I was trying to say pre-relocation.
But that might have been an artifact of trying to probe/initialize hace in the
SPL, so I might have mixed things.

I was sure that Qemu and Hardware were behaving differently, since the same
image would work on qemu but fail on hardware. Perhaps we should put a warning
on Qemu for unaligned access instead of just masking.

  
>>
>>    * The __atribute__ ((align 8)) doesn't have an effect in struct
>>      elements. Remove it.
> 
> Agreed.
> 
>>
>>    * Since the struct aspeed_hash_ctx->list element is what we need to
>>      make sure is aligned to 9-bytes, move that to the first element of
>>      the array, and call-out the fact that this needs to be aligned in
>>      the declaration.
> 
> 9 bytes? Did you mean 8?
> I did. A typo.

> Regardless, the array in the structure should be aligned as there will
> be no padding earlier in the struct. I have added a runtime check for
> this and have not hit that in my testing so far.
> 

My change involved moving the array to the first element exactly to not
risk changing the u32 elements before it to something else and breaking
alignment.

>>
>>     * Clear HACE_HASH_ISR before issuing new command
> 
> This makes sense.
> 
> I will send out a new series for review. I've tested it thoroughly in
> qemu, and have tested the u-boot -> Linux path on hardware.
> 
> Cheers,
> 
> Joel

Sounds good. I can give it a spin on rain100 and check if it's all good.

-- 
Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>

[PATCH u-boot v2019.04-aspeed-openbmc 11/11] configs/openbmc: Enable hw accelerated sha

[Joel Stanley]

SHA512 will be used by the openbmc secure boot implementation when
verifying FIT images of both u-boot proper and the kernel.

The hash command is useful, and adds only a small amount of binary size
given the algorithms are already included in the image.

Using hardware acceleration instead of a software implementation saves a
significant amount of binary size (approx. 12KB for the SPL).

Note that the hardware implementation is only useful when booting from a
payload in memory, as is the case with MMC or network boot. It cannot be
used when booting from SPI NOR.

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

diff --git a/configs/ast2600_openbmc_spl_emmc_defconfig b/configs/ast2600_openbmc_spl_emmc_defconfig
index e59d3595ebf0..05069084cbc5 100644
--- a/configs/ast2600_openbmc_spl_emmc_defconfig
+++ b/configs/ast2600_openbmc_spl_emmc_defconfig
@@ -36,6 +36,7 @@ CONFIG_ARMV7_BOOT_SEC_DEFAULT=y
 CONFIG_ARMV7_PSCI_NR_CPUS=2
 CONFIG_NR_DRAM_BANKS=1
 CONFIG_FIT=y
+CONFIG_FIT_ENABLE_SHA512_SUPPORT=y
 CONFIG_FIT_SIGNATURE=y
 CONFIG_SPL_FIT_SIGNATURE=y
 CONFIG_SPL_LOAD_FIT=y
@@ -53,6 +54,8 @@ CONFIG_SPL_STACK_R=y
 CONFIG_SPL_SEPARATE_BSS=y
 CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y
 CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x80
+CONFIG_SPL_SHA256_SUPPORT=y
+CONFIG_SPL_SHA512_SUPPORT=y
 CONFIG_SPL_FIT_IMAGE_TINY=y
 CONFIG_SPL_DM_RESET=y
 CONFIG_SPL_RAM_SUPPORT=y
@@ -80,6 +83,7 @@ CONFIG_CMD_DHCP=y
 CONFIG_CMD_MII=y
 CONFIG_CMD_PING=y
 CONFIG_CMD_NCSI=y
+CONFIG_CMD_HASH=y
 CONFIG_CMD_EXT2=y
 CONFIG_CMD_EXT4=y
 CONFIG_CMD_EXT4_WRITE=y
@@ -97,6 +101,7 @@ CONFIG_SYSCON=y
 CONFIG_SPL_OF_TRANSLATE=y
 CONFIG_CLK=y
 CONFIG_SPL_CLK=y
+CONFIG_ASPEED_HACE=y
 CONFIG_DM_GPIO=y
 CONFIG_ASPEED_GPIO=y
 CONFIG_DM_I2C=y
@@ -135,4 +140,6 @@ CONFIG_USE_TINY_PRINTF=y
 CONFIG_SPL_TINY_MEMSET=y
 CONFIG_TPM=y
 CONFIG_SPL_TPM=y
+CONFIG_SHA_HW_ACCEL=y
+CONFIG_SHA_PROG_HW_ACCEL=y
 # CONFIG_EFI_LOADER is not set
-- 
2.30.2

Re: [PATCH u-boot v2019.04-aspeed-openbmc 11/11] configs/openbmc: Enable hw accelerated sha

[Klaus Heinrich Kiwi]

On 4/13/2021 5:07 AM, Joel Stanley wrote:
> SHA512 will be used by the openbmc secure boot implementation when
> verifying FIT images of both u-boot proper and the kernel.
> 
> The hash command is useful, and adds only a small amount of binary size
> given the algorithms are already included in the image.
> 
> Using hardware acceleration instead of a software implementation saves a
> significant amount of binary size (approx. 12KB for the SPL).
> 
> Note that the hardware implementation is only useful when booting from a
> payload in memory, as is the case with MMC or network boot. It cannot be
> used when booting from SPI NOR.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
Reviewed-by: Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>
> ---
>   configs/ast2600_openbmc_spl_emmc_defconfig | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/configs/ast2600_openbmc_spl_emmc_defconfig b/configs/ast2600_openbmc_spl_emmc_defconfig
> index e59d3595ebf0..05069084cbc5 100644
> --- a/configs/ast2600_openbmc_spl_emmc_defconfig
> +++ b/configs/ast2600_openbmc_spl_emmc_defconfig
> @@ -36,6 +36,7 @@ CONFIG_ARMV7_BOOT_SEC_DEFAULT=y
>   CONFIG_ARMV7_PSCI_NR_CPUS=2
>   CONFIG_NR_DRAM_BANKS=1
>   CONFIG_FIT=y
> +CONFIG_FIT_ENABLE_SHA512_SUPPORT=y
>   CONFIG_FIT_SIGNATURE=y
>   CONFIG_SPL_FIT_SIGNATURE=y
>   CONFIG_SPL_LOAD_FIT=y
> @@ -53,6 +54,8 @@ CONFIG_SPL_STACK_R=y
>   CONFIG_SPL_SEPARATE_BSS=y
>   CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y
>   CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x80
> +CONFIG_SPL_SHA256_SUPPORT=y
> +CONFIG_SPL_SHA512_SUPPORT=y
>   CONFIG_SPL_FIT_IMAGE_TINY=y
>   CONFIG_SPL_DM_RESET=y
>   CONFIG_SPL_RAM_SUPPORT=y
> @@ -80,6 +83,7 @@ CONFIG_CMD_DHCP=y
>   CONFIG_CMD_MII=y
>   CONFIG_CMD_PING=y
>   CONFIG_CMD_NCSI=y
> +CONFIG_CMD_HASH=y
>   CONFIG_CMD_EXT2=y
>   CONFIG_CMD_EXT4=y
>   CONFIG_CMD_EXT4_WRITE=y
> @@ -97,6 +101,7 @@ CONFIG_SYSCON=y
>   CONFIG_SPL_OF_TRANSLATE=y
>   CONFIG_CLK=y
>   CONFIG_SPL_CLK=y
> +CONFIG_ASPEED_HACE=y
>   CONFIG_DM_GPIO=y
>   CONFIG_ASPEED_GPIO=y
>   CONFIG_DM_I2C=y
> @@ -135,4 +140,6 @@ CONFIG_USE_TINY_PRINTF=y
>   CONFIG_SPL_TINY_MEMSET=y
>   CONFIG_TPM=y
>   CONFIG_SPL_TPM=y
> +CONFIG_SHA_HW_ACCEL=y
> +CONFIG_SHA_PROG_HW_ACCEL=y
>   # CONFIG_EFI_LOADER is not set
> 

-- 
Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>

Re: [PATCH u-boot v2019.04-aspeed-openbmc 11/11] configs/openbmc: Enable hw accelerated sha

[Klaus Heinrich Kiwi]

On 4/13/2021 5:42 PM, Klaus Heinrich Kiwi wrote:
> 
> 
> On 4/13/2021 5:07 AM, Joel Stanley wrote:
>> SHA512 will be used by the openbmc secure boot implementation when
>> verifying FIT images of both u-boot proper and the kernel.
>>
>> The hash command is useful, and adds only a small amount of binary size
>> given the algorithms are already included in the image.
>>
>> Using hardware acceleration instead of a software implementation saves a
>> significant amount of binary size (approx. 12KB for the SPL).
>>
>> Note that the hardware implementation is only useful when booting from a
>> payload in memory, as is the case with MMC or network boot. It cannot be
>> used when booting from SPI NOR.
>>
>> Signed-off-by: Joel Stanley <joel@jms.id.au>
> Reviewed-by: Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>
>> ---
>>   configs/ast2600_openbmc_spl_emmc_defconfig | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/configs/ast2600_openbmc_spl_emmc_defconfig b/configs/ast2600_openbmc_spl_emmc_defconfig
>> index e59d3595ebf0..05069084cbc5 100644
>> --- a/configs/ast2600_openbmc_spl_emmc_defconfig
>> +++ b/configs/ast2600_openbmc_spl_emmc_defconfig
>> @@ -36,6 +36,7 @@ CONFIG_ARMV7_BOOT_SEC_DEFAULT=y
>>   CONFIG_ARMV7_PSCI_NR_CPUS=2
>>   CONFIG_NR_DRAM_BANKS=1
>>   CONFIG_FIT=y
>> +CONFIG_FIT_ENABLE_SHA512_SUPPORT=y
>>   CONFIG_FIT_SIGNATURE=y
>>   CONFIG_SPL_FIT_SIGNATURE=y
>>   CONFIG_SPL_LOAD_FIT=y
>> @@ -53,6 +54,8 @@ CONFIG_SPL_STACK_R=y
>>   CONFIG_SPL_SEPARATE_BSS=y
>>   CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y
>>   CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x80
>> +CONFIG_SPL_SHA256_SUPPORT=y
>> +CONFIG_SPL_SHA512_SUPPORT=y
>>   CONFIG_SPL_FIT_IMAGE_TINY=y
>>   CONFIG_SPL_DM_RESET=y
>>   CONFIG_SPL_RAM_SUPPORT=y
>> @@ -80,6 +83,7 @@ CONFIG_CMD_DHCP=y
>>   CONFIG_CMD_MII=y
>>   CONFIG_CMD_PING=y
>>   CONFIG_CMD_NCSI=y
>> +CONFIG_CMD_HASH=y
>>   CONFIG_CMD_EXT2=y
>>   CONFIG_CMD_EXT4=y
>>   CONFIG_CMD_EXT4_WRITE=y
>> @@ -97,6 +101,7 @@ CONFIG_SYSCON=y
>>   CONFIG_SPL_OF_TRANSLATE=y
>>   CONFIG_CLK=y
>>   CONFIG_SPL_CLK=y
>> +CONFIG_ASPEED_HACE=y
>>   CONFIG_DM_GPIO=y
>>   CONFIG_ASPEED_GPIO=y
>>   CONFIG_DM_I2C=y
>> @@ -135,4 +140,6 @@ CONFIG_USE_TINY_PRINTF=y
>>   CONFIG_SPL_TINY_MEMSET=y
>>   CONFIG_TPM=y
>>   CONFIG_SPL_TPM=y
>> +CONFIG_SHA_HW_ACCEL=y
>> +CONFIG_SHA_PROG_HW_ACCEL=y

Doesn't invalidate my reviewed-by-tag, but I just noted that these are redundant
due to CONFIG_ASPEED_HACE=y.

But I'm fine if you want yo keep them explicit as well.

  -Klaus

>>   # CONFIG_EFI_LOADER is not set
>>
> 

-- 
Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>