diff mbox series

[1/3] spl: fit: Minimally parse OS properties with FIT_IMAGE_TINY

Message ID 20200507232035.31892-1-samuel@sholland.org
State Changes Requested
Delegated to: Jagannadha Sutradharudu Teki
Headers show
Series [1/3] spl: fit: Minimally parse OS properties with FIT_IMAGE_TINY | expand

Commit Message

Samuel Holland May 7, 2020, 11:20 p.m. UTC
Some boards, specifically 64-bit Allwinner boards (sun50i), are
extremely limited on SPL size. One strategy that was used to make space
was to remove the FIT "os" property parsing code, because it uses a
rather large lookup table.

However, this forces the legacy FIT parsing code path, which requires
the "firmware" entry in the FIT to reference the U-Boot binary, even if
U-Boot is not the next binary in the boot sequence (for example, on
sun50i boards, ATF is run first).

This prevents the same FIT image from being used with a SPL with
CONFIG_SPL_FIT_IMAGE_TINY=n and CONFIG_SPL_ATF=y, because the boot
method selection code looks at `spl_image.os`, which is only set from
the "firmware" entry's "os" property.

To be able to use CONFIG_SPL_ATF=y, the "firmware" entry in the FIT
must be ATF, and U-Boot must be a loadable. For this to work, we need to
parse the "os" property just enough to tell U-Boot from other images, so
we can find it in the loadables list to append the FDT, and so we don't
try to append the FDT to ATF (which could clobber adjacent firmware).

So add the minimal code necessary to distinguish U-Boot/non-U-Boot
loadables with CONFIG_SPL_FIT_IMAGE_TINY=y. This adds about 300 bytes,
much less than the 7400 bytes added by CONFIG_SPL_FIT_IMAGE_TINY=n.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---
 common/spl/Kconfig   |  4 +---
 common/spl/spl_fit.c | 17 ++++++++++++++++-
 2 files changed, 17 insertions(+), 4 deletions(-)

Comments

Patrick Wildt May 8, 2020, 9:47 a.m. UTC | #1
On Thu, May 07, 2020 at 06:20:33PM -0500, Samuel Holland wrote:
> Some boards, specifically 64-bit Allwinner boards (sun50i), are
> extremely limited on SPL size. One strategy that was used to make space
> was to remove the FIT "os" property parsing code, because it uses a
> rather large lookup table.
> 
> However, this forces the legacy FIT parsing code path, which requires
> the "firmware" entry in the FIT to reference the U-Boot binary, even if
> U-Boot is not the next binary in the boot sequence (for example, on
> sun50i boards, ATF is run first).
> 
> This prevents the same FIT image from being used with a SPL with
> CONFIG_SPL_FIT_IMAGE_TINY=n and CONFIG_SPL_ATF=y, because the boot
> method selection code looks at `spl_image.os`, which is only set from
> the "firmware" entry's "os" property.
> 
> To be able to use CONFIG_SPL_ATF=y, the "firmware" entry in the FIT
> must be ATF, and U-Boot must be a loadable. For this to work, we need to
> parse the "os" property just enough to tell U-Boot from other images, so
> we can find it in the loadables list to append the FDT, and so we don't
> try to append the FDT to ATF (which could clobber adjacent firmware).
> 
> So add the minimal code necessary to distinguish U-Boot/non-U-Boot
> loadables with CONFIG_SPL_FIT_IMAGE_TINY=y. This adds about 300 bytes,
> much less than the 7400 bytes added by CONFIG_SPL_FIT_IMAGE_TINY=n.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>

Acked-by: Patrick Wildt <patrick@blueri.se>

> ---
>  common/spl/Kconfig   |  4 +---
>  common/spl/spl_fit.c | 17 ++++++++++++++++-
>  2 files changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/common/spl/Kconfig b/common/spl/Kconfig
> index 9feadb5e43..f2fa12354d 100644
> --- a/common/spl/Kconfig
> +++ b/common/spl/Kconfig
> @@ -448,9 +448,7 @@ config SPL_FIT_IMAGE_TINY
>  	  Enable this to reduce the size of the FIT image loading code
>  	  in SPL, if space for the SPL binary is very tight.
>  
> -	  This removes the detection of image types (which forces the
> -	  first image to be treated as having a U-Boot style calling
> -	  convention) and skips the recording of each loaded payload
> +	  This skips the recording of each loaded payload
>  	  (i.e. loadable) into the FDT (modifying the loaded FDT to
>  	  ensure this information is available to the next image
>  	  invoked).
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index c51e4beb1c..b9dd4211aa 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -464,7 +464,22 @@ static int spl_fit_record_loadable(const void *fit, int images, int index,
>  static int spl_fit_image_get_os(const void *fit, int noffset, uint8_t *os)
>  {
>  #if CONFIG_IS_ENABLED(FIT_IMAGE_TINY) && !defined(CONFIG_SPL_OS_BOOT)
> -	return -ENOTSUPP;
> +	const char *name = fdt_getprop(fit, noffset, FIT_OS_PROP, NULL);
> +
> +	if (!name)
> +		return -ENOENT;
> +
> +	/*
> +	 * We don't care what the type of the image actually is,
> +	 * only whether or not it is U-Boot. This saves some
> +	 * space by omitting the large table of OS types.
> +	 */
> +	if (!strcmp(name, "u-boot"))
> +		*os = IH_OS_U_BOOT;
> +	else
> +		*os = IH_OS_INVALID;
> +
> +	return 0;
>  #else
>  	return fit_image_get_os(fit, noffset, os);
>  #endif
> -- 
> 2.24.1
>
Jagan Teki June 1, 2020, 5:04 p.m. UTC | #2
On Fri, May 8, 2020 at 4:49 AM Samuel Holland <samuel@sholland.org> wrote:
>
> Some boards, specifically 64-bit Allwinner boards (sun50i), are
> extremely limited on SPL size. One strategy that was used to make space
> was to remove the FIT "os" property parsing code, because it uses a
> rather large lookup table.
>
> However, this forces the legacy FIT parsing code path, which requires
> the "firmware" entry in the FIT to reference the U-Boot binary, even if
> U-Boot is not the next binary in the boot sequence (for example, on
> sun50i boards, ATF is run first).
>
> This prevents the same FIT image from being used with a SPL with
> CONFIG_SPL_FIT_IMAGE_TINY=n and CONFIG_SPL_ATF=y, because the boot
> method selection code looks at `spl_image.os`, which is only set from
> the "firmware" entry's "os" property.
>
> To be able to use CONFIG_SPL_ATF=y, the "firmware" entry in the FIT
> must be ATF, and U-Boot must be a loadable. For this to work, we need to
> parse the "os" property just enough to tell U-Boot from other images, so
> we can find it in the loadables list to append the FDT, and so we don't
> try to append the FDT to ATF (which could clobber adjacent firmware).
>
> So add the minimal code necessary to distinguish U-Boot/non-U-Boot
> loadables with CONFIG_SPL_FIT_IMAGE_TINY=y. This adds about 300 bytes,
> much less than the 7400 bytes added by CONFIG_SPL_FIT_IMAGE_TINY=n.
>
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---

+ Andre
diff mbox series

Patch

diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index 9feadb5e43..f2fa12354d 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -448,9 +448,7 @@  config SPL_FIT_IMAGE_TINY
 	  Enable this to reduce the size of the FIT image loading code
 	  in SPL, if space for the SPL binary is very tight.
 
-	  This removes the detection of image types (which forces the
-	  first image to be treated as having a U-Boot style calling
-	  convention) and skips the recording of each loaded payload
+	  This skips the recording of each loaded payload
 	  (i.e. loadable) into the FDT (modifying the loaded FDT to
 	  ensure this information is available to the next image
 	  invoked).
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index c51e4beb1c..b9dd4211aa 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -464,7 +464,22 @@  static int spl_fit_record_loadable(const void *fit, int images, int index,
 static int spl_fit_image_get_os(const void *fit, int noffset, uint8_t *os)
 {
 #if CONFIG_IS_ENABLED(FIT_IMAGE_TINY) && !defined(CONFIG_SPL_OS_BOOT)
-	return -ENOTSUPP;
+	const char *name = fdt_getprop(fit, noffset, FIT_OS_PROP, NULL);
+
+	if (!name)
+		return -ENOENT;
+
+	/*
+	 * We don't care what the type of the image actually is,
+	 * only whether or not it is U-Boot. This saves some
+	 * space by omitting the large table of OS types.
+	 */
+	if (!strcmp(name, "u-boot"))
+		*os = IH_OS_U_BOOT;
+	else
+		*os = IH_OS_INVALID;
+
+	return 0;
 #else
 	return fit_image_get_os(fit, noffset, os);
 #endif