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

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(-)

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

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>