[PATCH u-boot v2019.04-aspeed-openbmc v2 0/6] FIT verification

[PATCH u-boot v2019.04-aspeed-openbmc v2 0/6] FIT verification

[Joel Stanley]

As part of our effort to enable secure boot in openbmc, this turns on
FIT verification in the SPL and u-boot.

It adjusts the SRAM layout to accommodate the extra code size, moving
the heap to the non-parity checked 24KB of SRAM.

It also modifies the way the SPL is built, including disabling features.
This reduces the SPL size to 59716, meaning we could choose to leave
ymodem support in the build for now. Please voice your thoughts when
reviewing.

Joel Stanley (6):
  ast2600: Modify SPL SRAM layout
  config: ast2600: Enable FIT signature verification
  ast2600: Allow selection of SPL boot devices
  config: ast2600: Disable unused features
  config: ast2600: Disable SPL ymodem support
  config: ast2600: Reduce SPL image size

 arch/arm/mach-aspeed/ast2600/Kconfig       | 12 ++++++++++++
 arch/arm/mach-aspeed/ast2600/spl_boot.c    |  9 +++++++++
 configs/ast2600_openbmc_spl_emmc_defconfig | 13 +++++++++++--
 include/configs/evb_ast2600a1_spl.h        |  4 ++--
 4 files changed, 34 insertions(+), 4 deletions(-)

-- 
2.29.2

[PATCH u-boot v2019.04-aspeed-openbmc v2 1/6] ast2600: Modify SPL SRAM layout

[Joel Stanley]

The SRAM is 89KB on the A1 and beyond:

 0x1000_0000 to 0x1000_ffff: 64KB, with parity check
 0x1001_0000 to 0x1001_5fff: 24KB, w/o parity check
 0x1001_6000 to 0x1001_63ff: 1KB, w/o parity check, each byte write once

Allow the image to fill the full 64KB payload size (max that secure boot
supports) and place the stack at the top of the 24KB of SRAM.

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

diff --git a/include/configs/evb_ast2600a1_spl.h b/include/configs/evb_ast2600a1_spl.h
index 69f3c32ce1d5..a39988820add 100644
--- a/include/configs/evb_ast2600a1_spl.h
+++ b/include/configs/evb_ast2600a1_spl.h
@@ -25,8 +25,8 @@
 
 /* SPL */
 #define CONFIG_SPL_TEXT_BASE		0x00000000
-#define CONFIG_SPL_MAX_SIZE		0x0000E800
-#define CONFIG_SPL_STACK		0x10010000
+#define CONFIG_SPL_MAX_SIZE		0x00010000
+#define CONFIG_SPL_STACK		0x10016000
 #define CONFIG_SPL_BSS_START_ADDR	0x90000000
 #define CONFIG_SPL_BSS_MAX_SIZE		0x00100000
 
-- 
2.29.2

[PATCH u-boot v2019.04-aspeed-openbmc v2 2/6] config: ast2600: Enable FIT signature verification

[Joel Stanley]

This turns on FIT signature verification for the OpenBMC SPL
configuration, for both the SPL and u-boot.

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

diff --git a/configs/ast2600_openbmc_spl_emmc_defconfig b/configs/ast2600_openbmc_spl_emmc_defconfig
index 68d18652c980..20f2e7019cb3 100644
--- a/configs/ast2600_openbmc_spl_emmc_defconfig
+++ b/configs/ast2600_openbmc_spl_emmc_defconfig
@@ -35,6 +35,9 @@ CONFIG_ARMV7_BOOT_SEC_DEFAULT=y
 CONFIG_ARMV7_PSCI_NR_CPUS=2
 CONFIG_NR_DRAM_BANKS=1
 CONFIG_FIT=y
+CONFIG_FIT_SIGNATURE=y
+CONFIG_SPL_FIT_SIGNATURE=y
+CONFIG_SPL_LOAD_FIT=y
 CONFIG_USE_BOOTARGS=y
 CONFIG_BOOTARGS="console=ttyS4,115200n8 root=/dev/ram rw"
 CONFIG_USE_BOOTCOMMAND=y
-- 
2.29.2

[PATCH u-boot v2019.04-aspeed-openbmc v2 3/6] ast2600: Allow selection of SPL boot devices

[Joel Stanley]

The AST2600 SPL can boot from a number of sources, with or without the
AST2600 secure boot feature. It may be desirable to disable some of
these, so put them behind the defines for the drivers that are used.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 arch/arm/mach-aspeed/ast2600/Kconfig    | 12 ++++++++++++
 arch/arm/mach-aspeed/ast2600/spl_boot.c |  9 +++++++++
 2 files changed, 21 insertions(+)

diff --git a/arch/arm/mach-aspeed/ast2600/Kconfig b/arch/arm/mach-aspeed/ast2600/Kconfig
index dd991e87c795..518f41b558d3 100644
--- a/arch/arm/mach-aspeed/ast2600/Kconfig
+++ b/arch/arm/mach-aspeed/ast2600/Kconfig
@@ -53,6 +53,18 @@ config TARGET_SLT_AST2600
 
 endchoice
 
+config ASPEED_SECBOOT_BL2
+	bool "ASPEED secure boot BL2 support"
+	depends on ASPEED_AST2600
+	help
+	  Enable ASPEED's "secboot" secure boot support for verifying
+	  the SPL's playload ("BL2").
+
+	  Enable this is if you're using secure boot support in the AST2600 (or similar)
+	  to verify your u-boot proper.
+
+	  Disable this is if you are using u-boot's vboot to verify u-boot.
+
 source "board/aspeed/evb_ast2600a0/Kconfig"
 source "board/aspeed/evb_ast2600a1/Kconfig"
 source "board/aspeed/ncsi_ast2600a0/Kconfig"
diff --git a/arch/arm/mach-aspeed/ast2600/spl_boot.c b/arch/arm/mach-aspeed/ast2600/spl_boot.c
index 58a22f646e08..98cf72bf440d 100644
--- a/arch/arm/mach-aspeed/ast2600/spl_boot.c
+++ b/arch/arm/mach-aspeed/ast2600/spl_boot.c
@@ -42,6 +42,7 @@ static int aspeed_secboot_spl_ram_load_image(struct spl_image_info *spl_image,
 }
 SPL_LOAD_IMAGE_METHOD("RAM with Aspeed Secure Boot", 0, ASPEED_SECBOOT_DEVICE_RAM, aspeed_secboot_spl_ram_load_image);
 
+#if IS_ENABLED(CONFIG_SPL_MMC_SUPPORT)
 static int aspeed_spl_mmc_load_image(struct spl_image_info *spl_image,
 				      struct spl_boot_device *bootdev)
 {
@@ -101,6 +102,7 @@ static int aspeed_spl_mmc_load_image(struct spl_image_info *spl_image,
 }
 SPL_LOAD_IMAGE_METHOD("MMC", 0, ASPEED_BOOT_DEVICE_MMC, aspeed_spl_mmc_load_image);
 
+#if IS_ENABLED(ASPEED_SECBOOT_BL2)
 static int aspeed_secboot_spl_mmc_load_image(struct spl_image_info *spl_image,
 				      struct spl_boot_device *bootdev)
 {
@@ -161,7 +163,10 @@ static int aspeed_secboot_spl_mmc_load_image(struct spl_image_info *spl_image,
 	return 0;
 }
 SPL_LOAD_IMAGE_METHOD("MMC with Aspeed Secure Boot", 0, ASPEED_SECBOOT_DEVICE_MMC, aspeed_secboot_spl_mmc_load_image);
+#endif /* ASPEED_SECBOOT_BL2 */
+#endif
 
+#if IS_ENABLED(CONFIG_SPL_YMODEM_SUPPORT)
 static int getcymodem(void)
 {
 	if (tstc())
@@ -204,6 +209,8 @@ end_stream:
 }
 SPL_LOAD_IMAGE_METHOD("UART", 0, ASPEED_BOOT_DEVICE_UART, aspeed_spl_ymodem_load_image);
 
+
+#if IS_ENABLED(ASPEED_SECBOOT_BL2)
 static int aspeed_secboot_spl_ymodem_load_image(struct spl_image_info *spl_image,
 		struct spl_boot_device *bootdev)
 {
@@ -245,3 +252,5 @@ end_stream:
 	return ret;
 }
 SPL_LOAD_IMAGE_METHOD("UART with Aspeed Secure Boot", 0, ASPEED_SECBOOT_DEVICE_UART, aspeed_secboot_spl_ymodem_load_image);
+#endif /* ASPEED_SECBOOT_BL2 */
+#endif
-- 
2.29.2

[PATCH u-boot v2019.04-aspeed-openbmc v2 4/6] config: ast2600: Disable unused features

[Joel Stanley]

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

diff --git a/configs/ast2600_openbmc_spl_emmc_defconfig b/configs/ast2600_openbmc_spl_emmc_defconfig
index 20f2e7019cb3..3f2de64e077d 100644
--- a/configs/ast2600_openbmc_spl_emmc_defconfig
+++ b/configs/ast2600_openbmc_spl_emmc_defconfig
@@ -83,6 +83,7 @@ CONFIG_CMD_EXT4_WRITE=y
 CONFIG_CMD_FAT=y
 CONFIG_CMD_FS_GENERIC=y
 CONFIG_CMD_MTDPARTS=y
+# CONFIG_SPL_DOS_PARTITION is not set
 # CONFIG_SPL_EFI_PARTITION is not set
 CONFIG_SPL_OF_CONTROL=y
 CONFIG_ENV_IS_IN_MMC=y
@@ -100,6 +101,7 @@ CONFIG_SYS_I2C_ASPEED=y
 CONFIG_MISC=y
 CONFIG_ASPEED_AHBC=y
 CONFIG_DM_MMC=y
+# CONFIG_MMC_VERBOSE is not set
 CONFIG_SPL_MMC_TINY=y
 CONFIG_MMC_SDHCI=y
 CONFIG_MMC_SDHCI_ASPEED=y
@@ -128,6 +130,7 @@ CONFIG_DM_SPI=y
 CONFIG_SYSRESET=y
 CONFIG_WDT=y
 CONFIG_USE_TINY_PRINTF=y
+# CONFIG_REGEX is not set
 CONFIG_TPM=y
 CONFIG_SPL_TPM=y
 # CONFIG_EFI_LOADER is not set
-- 
2.29.2

[PATCH u-boot v2019.04-aspeed-openbmc v2 5/6] config: ast2600: Disable SPL ymodem support

[Joel Stanley]

This feature consumes 3656 bytes. Without it systems will need to load a
new SPL over the UART with the AST2600's recovery feature if they flash
a bad u-boot proper.

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

diff --git a/configs/ast2600_openbmc_spl_emmc_defconfig b/configs/ast2600_openbmc_spl_emmc_defconfig
index 3f2de64e077d..c55a70c5c1c9 100644
--- a/configs/ast2600_openbmc_spl_emmc_defconfig
+++ b/configs/ast2600_openbmc_spl_emmc_defconfig
@@ -55,7 +55,6 @@ CONFIG_SPL_DM_RESET=y
 CONFIG_SPL_RAM_SUPPORT=y
 CONFIG_SPL_RAM_DEVICE=y
 CONFIG_SPL_WATCHDOG_SUPPORT=y
-CONFIG_SPL_YMODEM_SUPPORT=y
 CONFIG_HUSH_PARSER=y
 # CONFIG_AUTO_COMPLETE is not set
 CONFIG_SYS_PROMPT="ast# "
-- 
2.29.2

[PATCH u-boot v2019.04-aspeed-openbmc v2 6/6] config: ast2600: Reduce SPL image size

[Joel Stanley]

This modifies some features of the SPL to ensure it fits in the 64KB
payload size.

This set of options reduceds the binary size by 4760 bytes with GCC 10.

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

diff --git a/configs/ast2600_openbmc_spl_emmc_defconfig b/configs/ast2600_openbmc_spl_emmc_defconfig
index c55a70c5c1c9..345225131075 100644
--- a/configs/ast2600_openbmc_spl_emmc_defconfig
+++ b/configs/ast2600_openbmc_spl_emmc_defconfig
@@ -2,8 +2,9 @@ CONFIG_ARM=y
 CONFIG_SYS_CONFIG_NAME="evb_ast2600a1_spl"
 CONFIG_SYS_DCACHE_OFF=y
 CONFIG_POSITION_INDEPENDENT=y
-CONFIG_SPL_SYS_THUMB_BUILD=y
 CONFIG_SYS_THUMB_BUILD=y
+# CONFIG_SPL_USE_ARCH_MEMCPY is not set
+# 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
@@ -51,6 +52,8 @@ CONFIG_BOARD_EARLY_INIT_F=y
 CONFIG_SPL_SYS_MALLOC_SIMPLE=y
 CONFIG_SPL_STACK_R=y
 CONFIG_SPL_SEPARATE_BSS=y
+# CONFIG_TPL_BANNER_PRINT is not set
+CONFIG_SPL_FIT_IMAGE_TINY=y
 CONFIG_SPL_DM_RESET=y
 CONFIG_SPL_RAM_SUPPORT=y
 CONFIG_SPL_RAM_DEVICE=y
@@ -130,6 +133,7 @@ CONFIG_SYSRESET=y
 CONFIG_WDT=y
 CONFIG_USE_TINY_PRINTF=y
 # CONFIG_REGEX is not set
+CONFIG_SPL_TINY_MEMSET=y
 CONFIG_TPM=y
 CONFIG_SPL_TPM=y
 # CONFIG_EFI_LOADER is not set
-- 
2.29.2

Re: [PATCH u-boot v2019.04-aspeed-openbmc v2 3/6] ast2600: Allow selection of SPL boot devices

[Klaus Heinrich Kiwi]

On 1/27/2021 4:00 AM, Joel Stanley wrote:
> The AST2600 SPL can boot from a number of sources, with or without the
> AST2600 secure boot feature. It may be desirable to disable some of
> these, so put them behind the defines for the drivers that are used.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>
Reviewed-by: Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>


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

Re: [PATCH u-boot v2019.04-aspeed-openbmc v2 4/6] config: ast2600: Disable unused features

[Klaus Heinrich Kiwi]

Hi Joel,

> +# CONFIG_SPL_DOS_PARTITION is not set
Sounds good.


> +# CONFIG_MMC_VERBOSE is not set
>   CONFIG_SPL_MMC_TINY=y
>   CONFIG_MMC_SDHCI=y
>   CONFIG_MMC_SDHCI_ASPEED=y
> @@ -128,6 +130,7 @@ CONFIG_DM_SPI=y
>   CONFIG_SYSRESET=y
>   CONFIG_WDT=y
>   CONFIG_USE_TINY_PRINTF=y
> +# CONFIG_REGEX is not set
Are those influencing the SPL size or just U-boot proper?

At any rate, I don't have objections.

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

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

Re: [PATCH u-boot v2019.04-aspeed-openbmc v2 5/6] config: ast2600: Disable SPL ymodem support

[Klaus Heinrich Kiwi]

On 1/27/2021 4:00 AM, Joel Stanley wrote:
> This feature consumes 3656 bytes. Without it systems will need to load a
> new SPL over the UART with the AST2600's recovery feature if they flash
> a bad u-boot proper.
No objections here. Just a bit confusing since that in the introductory letter
you mention we *could* leave that enabled. So if that's not necessary, why
bundle that with this patchset (and not separate)?

> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>   configs/ast2600_openbmc_spl_emmc_defconfig | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/configs/ast2600_openbmc_spl_emmc_defconfig b/configs/ast2600_openbmc_spl_emmc_defconfig
> index 3f2de64e077d..c55a70c5c1c9 100644
> --- a/configs/ast2600_openbmc_spl_emmc_defconfig
> +++ b/configs/ast2600_openbmc_spl_emmc_defconfig
> @@ -55,7 +55,6 @@ CONFIG_SPL_DM_RESET=y
>   CONFIG_SPL_RAM_SUPPORT=y
>   CONFIG_SPL_RAM_DEVICE=y
>   CONFIG_SPL_WATCHDOG_SUPPORT=y
> -CONFIG_SPL_YMODEM_SUPPORT=y
>   CONFIG_HUSH_PARSER=y
>   # CONFIG_AUTO_COMPLETE is not set
>   CONFIG_SYS_PROMPT="ast# "

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

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

Re: [PATCH u-boot v2019.04-aspeed-openbmc v2 6/6] config: ast2600: Reduce SPL image size

[Klaus Heinrich Kiwi]

Hi Joel,

On 1/27/2021 4:00 AM, Joel Stanley wrote:
> This modifies some features of the SPL to ensure it fits in the 64KB
> payload size.
> 
> This set of options reduceds the binary size by 4760 bytes with GCC 10.
typo here..


> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>   configs/ast2600_openbmc_spl_emmc_defconfig | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/configs/ast2600_openbmc_spl_emmc_defconfig b/configs/ast2600_openbmc_spl_emmc_defconfig
> index c55a70c5c1c9..345225131075 100644
> --- a/configs/ast2600_openbmc_spl_emmc_defconfig
> +++ b/configs/ast2600_openbmc_spl_emmc_defconfig
> @@ -2,8 +2,9 @@ CONFIG_ARM=y
>   CONFIG_SYS_CONFIG_NAME="evb_ast2600a1_spl"
>   CONFIG_SYS_DCACHE_OFF=y
>   CONFIG_POSITION_INDEPENDENT=y
> -CONFIG_SPL_SYS_THUMB_BUILD=y
Are we sure this is reducing the size? From the Kconfig file..
"Thumb instruction set provides better code density"

>   CONFIG_SYS_THUMB_BUILD=y
> +# CONFIG_SPL_USE_ARCH_MEMCPY is not set
> +# CONFIG_SPL_USE_ARCH_MEMSET is not set
Ack, sounds good.

>   CONFIG_SPL_LDSCRIPT="arch/$(ARCH)/mach-aspeed/ast2600/u-boot-spl.lds"
>   CONFIG_ARCH_ASPEED=y
>   CONFIG_SYS_TEXT_BASE=0x10000
> @@ -51,6 +52,8 @@ CONFIG_BOARD_EARLY_INIT_F=y
>   CONFIG_SPL_SYS_MALLOC_SIMPLE=y
>   CONFIG_SPL_STACK_R=y
>   CONFIG_SPL_SEPARATE_BSS=y
> +# CONFIG_TPL_BANNER_PRINT is not set
Is this unrelated?

> +CONFIG_SPL_FIT_IMAGE_TINY=y
I am unsure about this one. I know that we *may* need that to
secureboot, but we may loose good tracking of the image that
was actually loaded in the fdt, which sounds like a desirable
feature in secureboot scenarios, specially where we don't have
a TPM for measurements.

I'd put that low on the priority list (i.e., below the ymodem support)?

>   CONFIG_SPL_DM_RESET=y
>   CONFIG_SPL_RAM_SUPPORT=y
>   CONFIG_SPL_RAM_DEVICE=y
> @@ -130,6 +133,7 @@ CONFIG_SYSRESET=y
>   CONFIG_WDT=y
>   CONFIG_USE_TINY_PRINTF=y
>   # CONFIG_REGEX is not set
bikeshedding, but I'd recommend combining the necessary changes to make
SPL fit the 64KB size in one patch, and enable the SPL signing in another
patch in the same set, while leaving out unrelated / optional changes
to another set.

> +CONFIG_SPL_TINY_MEMSET=y
ack

>   CONFIG_TPM=y
>   CONFIG_SPL_TPM=y
>   # CONFIG_EFI_LOADER is not set

Thanks,

  -Klaus

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

Re: [PATCH u-boot v2019.04-aspeed-openbmc v2 2/6] config: ast2600: Enable FIT signature verification

[Klaus Heinrich Kiwi]

On 1/27/2021 4:00 AM, Joel Stanley wrote:
> This turns on FIT signature verification for the OpenBMC SPL
> configuration, for both the SPL and u-boot.
> 
>   CONFIG_NR_DRAM_BANKS=1
>   CONFIG_FIT=y
> +CONFIG_FIT_SIGNATURE=y
> +CONFIG_SPL_FIT_SIGNATURE=y
> +CONFIG_SPL_LOAD_FIT=y

See my comments on patch 6, but feels like this should logically go after the size-reduction patches.
>   CONFIG_USE_BOOTARGS=y
>   CONFIG_BOOTARGS="console=ttyS4,115200n8 root=/dev/ram rw"
>   CONFIG_USE_BOOTCOMMAND=y
> 

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

Re: [PATCH u-boot v2019.04-aspeed-openbmc v2 6/6] config: ast2600: Reduce SPL image size

[Joel Stanley]

On Wed, 27 Jan 2021 at 19:46, Klaus Heinrich Kiwi
<klaus@linux.vnet.ibm.com> wrote:
>
> Hi Joel,
>
> On 1/27/2021 4:00 AM, Joel Stanley wrote:
> > This modifies some features of the SPL to ensure it fits in the 64KB
> > payload size.
> >
> > This set of options reduceds the binary size by 4760 bytes with GCC 10.
> typo here..
>
>
> > Signed-off-by: Joel Stanley <joel@jms.id.au>
> > ---
> >   configs/ast2600_openbmc_spl_emmc_defconfig | 6 +++++-
> >   1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/configs/ast2600_openbmc_spl_emmc_defconfig b/configs/ast2600_openbmc_spl_emmc_defconfig
> > index c55a70c5c1c9..345225131075 100644
> > --- a/configs/ast2600_openbmc_spl_emmc_defconfig
> > +++ b/configs/ast2600_openbmc_spl_emmc_defconfig
> > @@ -2,8 +2,9 @@ CONFIG_ARM=y
> >   CONFIG_SYS_CONFIG_NAME="evb_ast2600a1_spl"
> >   CONFIG_SYS_DCACHE_OFF=y
> >   CONFIG_POSITION_INDEPENDENT=y
> > -CONFIG_SPL_SYS_THUMB_BUILD=y
> Are we sure this is reducing the size? From the Kconfig file..
> "Thumb instruction set provides better code density"

This is a defconfing change only. We are still building with thumb;
check the output .config.

>
> >   CONFIG_SYS_THUMB_BUILD=y
> > +# CONFIG_SPL_USE_ARCH_MEMCPY is not set
> > +# CONFIG_SPL_USE_ARCH_MEMSET is not set
> Ack, sounds good.
>
> >   CONFIG_SPL_LDSCRIPT="arch/$(ARCH)/mach-aspeed/ast2600/u-boot-spl.lds"
> >   CONFIG_ARCH_ASPEED=y
> >   CONFIG_SYS_TEXT_BASE=0x10000
> > @@ -51,6 +52,8 @@ CONFIG_BOARD_EARLY_INIT_F=y
> >   CONFIG_SPL_SYS_MALLOC_SIMPLE=y
> >   CONFIG_SPL_STACK_R=y
> >   CONFIG_SPL_SEPARATE_BSS=y
> > +# CONFIG_TPL_BANNER_PRINT is not set
> Is this unrelated?

Yes, this can be dropped.

>
> > +CONFIG_SPL_FIT_IMAGE_TINY=y
> I am unsure about this one. I know that we *may* need that to
> secureboot, but we may loose good tracking of the image that
> was actually loaded in the fdt, which sounds like a desirable
> feature in secureboot scenarios, specially where we don't have
> a TPM for measurements.

I don't see any need in our design for updating the u-boot device tree
with any information from the SPL. If we have that requirement in the
future we can consider turning this code on.

>
> I'd put that low on the priority list (i.e., below the ymodem support)?
>
> >   CONFIG_SPL_DM_RESET=y
> >   CONFIG_SPL_RAM_SUPPORT=y
> >   CONFIG_SPL_RAM_DEVICE=y
> > @@ -130,6 +133,7 @@ CONFIG_SYSRESET=y
> >   CONFIG_WDT=y
> >   CONFIG_USE_TINY_PRINTF=y
> >   # CONFIG_REGEX is not set
> bikeshedding, but I'd recommend combining the necessary changes to make
> SPL fit the 64KB size in one patch, and enable the SPL signing in another
> patch in the same set, while leaving out unrelated / optional changes
> to another set.

ok.

>
> > +CONFIG_SPL_TINY_MEMSET=y
> ack
>
> >   CONFIG_TPM=y
> >   CONFIG_SPL_TPM=y
> >   # CONFIG_EFI_LOADER is not set
>
> Thanks,
>
>   -Klaus
>
> --
> Klaus Heinrich Kiwi <klaus@linux.vnet.ibm.com>

Re: [PATCH u-boot v2019.04-aspeed-openbmc v2 0/6] FIT verification

[Andrew Jeffery]

On Wed, 27 Jan 2021, at 17:30, Joel Stanley wrote:
> As part of our effort to enable secure boot in openbmc, this turns on
> FIT verification in the SPL and u-boot.
> 
> It adjusts the SRAM layout to accommodate the extra code size, moving
> the heap to the non-parity checked 24KB of SRAM.
> 
> It also modifies the way the SPL is built, including disabling features.
> This reduces the SPL size to 59716, meaning we could choose to leave
> ymodem support in the build for now. Please voice your thoughts when
> reviewing.

If we leave ymodem in, after signing we have 1536 bytes spare*.

I suggest we drop the patch disabling ymodem support so we don't have to build 
a separate SPL config if we want to recover.

Andrew

* Signing requires 512-byte alignment, and aligns the signed image size to a 
512-byte boundary. So: (64×1024)−((59716+3656+(512−1))&(~(512−1))+512)

Re: [PATCH u-boot v2019.04-aspeed-openbmc v2 1/6] ast2600: Modify SPL SRAM layout

[Andrew Jeffery]

On Wed, 27 Jan 2021, at 17:30, Joel Stanley wrote:
> The SRAM is 89KB on the A1 and beyond:
> 
>  0x1000_0000 to 0x1000_ffff: 64KB, with parity check
>  0x1001_0000 to 0x1001_5fff: 24KB, w/o parity check
>  0x1001_6000 to 0x1001_63ff: 1KB, w/o parity check, each byte write once
> 
> Allow the image to fill the full 64KB payload size (max that secure boot
> supports) and place the stack at the top of the 24KB of SRAM.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>

I've pushed a change to github that enables socsec to sign larger SPLs:

https://github.com/amboar/socsec/commit/e28d00cb8278d61b02cb65c320ab4bfa70c79ae1

Acked-by: Andrew Jeffery <andrew@aj.id.au>

Re: [PATCH u-boot v2019.04-aspeed-openbmc v2 5/6] config: ast2600: Disable SPL ymodem support

[Andrew Jeffery]

On Wed, 27 Jan 2021, at 17:30, Joel Stanley wrote:
> This feature consumes 3656 bytes. Without it systems will need to load a
> new SPL over the UART with the AST2600's recovery feature if they flash
> a bad u-boot proper.
> 
> Signed-off-by: Joel Stanley <joel@jms.id.au>

As mentioned in my reply to the cover letter, I think we should drop this one.

Andrew

Re: [PATCH u-boot v2019.04-aspeed-openbmc v2 4/6] config: ast2600: Disable unused features

[Andrew Jeffery]

On Thu, 28 Jan 2021, at 05:59, Klaus Heinrich Kiwi wrote:
> Hi Joel,
> 
> > +# CONFIG_SPL_DOS_PARTITION is not set
> Sounds good.
> 
> 
> > +# CONFIG_MMC_VERBOSE is not set
> >   CONFIG_SPL_MMC_TINY=y
> >   CONFIG_MMC_SDHCI=y
> >   CONFIG_MMC_SDHCI_ASPEED=y
> > @@ -128,6 +130,7 @@ CONFIG_DM_SPI=y
> >   CONFIG_SYSRESET=y
> >   CONFIG_WDT=y
> >   CONFIG_USE_TINY_PRINTF=y
> > +# CONFIG_REGEX is not set
> Are those influencing the SPL size or just U-boot proper?

The SPL needs MMC support to load u-boot, so I assume CONFIG_MMC_VERBOSE=n is 
worthwhile.

Acked-by: Andrew Jeffery <andrew@aj.id.au>