[U-Boot,04/15] spl: fit: simplify logic for FDT loading for non-OS boots

Message ID 1505330989-25602-5-git-send-email-philipp.tomsich@theobroma-systems.com
State New
Delegated to: Philipp Tomsich
Headers show
Series
  • [U-Boot,01/15] image: add IH_OS_ARM_TRUSTED_FIRMWARE for ARM Trusted Firmware
Related show

Commit Message

Philipp Tomsich Sept. 13, 2017, 7:29 p.m.
To better support bootin through an ATF or OPTEE, we need to
streamline some of the logic for when the FDT is appended to an image:
depending on the image type, we'd like to append the FDT not at all
(the case for the OS boot), to the 'firmware' image (if it is a
U-Boot) or to one of the loadables (if the 'firmware' is an ATF, an
OPTEE, or some other image-type and U-Boot is listed in the
loadabled).

To achieve this goal, we drop the os_boot flag and track the type of
image loaded.  If it is of type IH_OS_U_BOOT, we append the FDT.

Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
---

 common/spl/spl_fit.c | 86 ++++++++++++++++++++++++++++++++++------------------
 1 file changed, 56 insertions(+), 30 deletions(-)

Comments

York Sun Sept. 13, 2017, 9:16 p.m. | #1
On 09/13/2017 12:30 PM, Philipp Tomsich wrote:
> To better support bootin through an ATF or OPTEE, we need to
> streamline some of the logic for when the FDT is appended to an image:
> depending on the image type, we'd like to append the FDT not at all
> (the case for the OS boot), to the 'firmware' image (if it is a
> U-Boot) or to one of the loadables (if the 'firmware' is an ATF, an
> OPTEE, or some other image-type and U-Boot is listed in the
> loadabled).
> 
> To achieve this goal, we drop the os_boot flag and track the type of
> image loaded.  If it is of type IH_OS_U_BOOT, we append the FDT.
> 
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
> 
>   common/spl/spl_fit.c | 86 ++++++++++++++++++++++++++++++++++------------------
>   1 file changed, 56 insertions(+), 30 deletions(-)

This change seems OK. Make sure you test booting OS. Let me know if you 
need help.

York
Philipp Tomsich Sept. 13, 2017, 9:21 p.m. | #2
York,

> On 13 Sep 2017, at 23:16, York Sun <york.sun@nxp.com> wrote:
> 
> On 09/13/2017 12:30 PM, Philipp Tomsich wrote:
>> To better support bootin through an ATF or OPTEE, we need to
>> streamline some of the logic for when the FDT is appended to an image:
>> depending on the image type, we'd like to append the FDT not at all
>> (the case for the OS boot), to the 'firmware' image (if it is a
>> U-Boot) or to one of the loadables (if the 'firmware' is an ATF, an
>> OPTEE, or some other image-type and U-Boot is listed in the
>> loadabled).
>> 
>> To achieve this goal, we drop the os_boot flag and track the type of
>> image loaded.  If it is of type IH_OS_U_BOOT, we append the FDT.
>> 
>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>> ---
>> 
>>  common/spl/spl_fit.c | 86 ++++++++++++++++++++++++++++++++++------------------
>>  1 file changed, 56 insertions(+), 30 deletions(-)
> 
> This change seems OK. Make sure you test booting OS. Let me know if you 
> need help.

I’d appreciate if you could test this on your end as well. I currently only
have test setups that need to go through ATF before entering an OS (as
the PSCI implementation lives in ATF).

Regards,
Philipp.
York Sun Sept. 13, 2017, 9:24 p.m. | #3
On 09/13/2017 02:21 PM, Dr. Philipp Tomsich wrote:
> York,
> 
>> On 13 Sep 2017, at 23:16, York Sun <york.sun@nxp.com> wrote:
>>
>> On 09/13/2017 12:30 PM, Philipp Tomsich wrote:
>>> To better support bootin through an ATF or OPTEE, we need to
>>> streamline some of the logic for when the FDT is appended to an image:
>>> depending on the image type, we'd like to append the FDT not at all
>>> (the case for the OS boot), to the 'firmware' image (if it is a
>>> U-Boot) or to one of the loadables (if the 'firmware' is an ATF, an
>>> OPTEE, or some other image-type and U-Boot is listed in the
>>> loadabled).
>>>
>>> To achieve this goal, we drop the os_boot flag and track the type of
>>> image loaded.  If it is of type IH_OS_U_BOOT, we append the FDT.
>>>
>>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>>> ---
>>>
>>>   common/spl/spl_fit.c | 86 ++++++++++++++++++++++++++++++++++------------------
>>>   1 file changed, 56 insertions(+), 30 deletions(-)
>>
>> This change seems OK. Make sure you test booting OS. Let me know if you
>> need help.
> 
> I’d appreciate if you could test this on your end as well. I currently only
> have test setups that need to go through ATF before entering an OS (as
> the PSCI implementation lives in ATF).

Philipp,

I can test _this_ patch on my board. Looks like it doesn't depend on 
other patches.

York
Philipp Tomsich Sept. 13, 2017, 10:11 p.m. | #4
York,

> On 13 Sep 2017, at 23:24, York Sun <york.sun@nxp.com> wrote:
> 
> On 09/13/2017 02:21 PM, Dr. Philipp Tomsich wrote:
>> York,
>> 
>>> On 13 Sep 2017, at 23:16, York Sun <york.sun@nxp.com> wrote:
>>> 
>>> On 09/13/2017 12:30 PM, Philipp Tomsich wrote:
>>>> To better support bootin through an ATF or OPTEE, we need to
>>>> streamline some of the logic for when the FDT is appended to an image:
>>>> depending on the image type, we'd like to append the FDT not at all
>>>> (the case for the OS boot), to the 'firmware' image (if it is a
>>>> U-Boot) or to one of the loadables (if the 'firmware' is an ATF, an
>>>> OPTEE, or some other image-type and U-Boot is listed in the
>>>> loadabled).
>>>> 
>>>> To achieve this goal, we drop the os_boot flag and track the type of
>>>> image loaded.  If it is of type IH_OS_U_BOOT, we append the FDT.
>>>> 
>>>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>>>> ---
>>>> 
>>>>  common/spl/spl_fit.c | 86 ++++++++++++++++++++++++++++++++++------------------
>>>>  1 file changed, 56 insertions(+), 30 deletions(-)
>>> 
>>> This change seems OK. Make sure you test booting OS. Let me know if you
>>> need help.
>> 
>> I’d appreciate if you could test this on your end as well. I currently only
>> have test setups that need to go through ATF before entering an OS (as
>> the PSCI implementation lives in ATF).
> 
> Philipp,
> 
> I can test _this_ patch on my board. Looks like it doesn't depend on 
> other patches.

The other patches shouldn’t interfere (and have been exercised quite
a bit on our end): I am only worried about possibly breaking some of
your changes for your scenario.

Philipp.
York Sun Sept. 14, 2017, 5:51 p.m. | #5
On 09/13/2017 03:11 PM, Dr. Philipp Tomsich wrote:
> York,
> 
>> On 13 Sep 2017, at 23:24, York Sun <york.sun@nxp.com> wrote:
>>
>> On 09/13/2017 02:21 PM, Dr. Philipp Tomsich wrote:
>>> York,
>>>
>>>> On 13 Sep 2017, at 23:16, York Sun <york.sun@nxp.com> wrote:
>>>>
>>>> On 09/13/2017 12:30 PM, Philipp Tomsich wrote:
>>>>> To better support bootin through an ATF or OPTEE, we need to
>>>>> streamline some of the logic for when the FDT is appended to an image:
>>>>> depending on the image type, we'd like to append the FDT not at all
>>>>> (the case for the OS boot), to the 'firmware' image (if it is a
>>>>> U-Boot) or to one of the loadables (if the 'firmware' is an ATF, an
>>>>> OPTEE, or some other image-type and U-Boot is listed in the
>>>>> loadabled).
>>>>>
>>>>> To achieve this goal, we drop the os_boot flag and track the type of
>>>>> image loaded.  If it is of type IH_OS_U_BOOT, we append the FDT.
>>>>>
>>>>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>>>>> ---
>>>>>
>>>>>   common/spl/spl_fit.c | 86 ++++++++++++++++++++++++++++++++++------------------
>>>>>   1 file changed, 56 insertions(+), 30 deletions(-)
>>>>
>>>> This change seems OK. Make sure you test booting OS. Let me know if you
>>>> need help.
>>>
>>> I’d appreciate if you could test this on your end as well. I currently only
>>> have test setups that need to go through ATF before entering an OS (as
>>> the PSCI implementation lives in ATF).
>>
>> Philipp,
>>
>> I can test _this_ patch on my board. Looks like it doesn't depend on
>> other patches.
> 
> The other patches shouldn’t interfere (and have been exercised quite
> a bit on our end): I am only worried about possibly breaking some of
> your changes for your scenario.
> 

Acked-by: York Sun <york.sun@nxp.com>
Simon Glass Sept. 17, 2017, 5:54 p.m. | #6
On 13 September 2017 at 13:29, Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> To better support bootin through an ATF or OPTEE, we need to
> streamline some of the logic for when the FDT is appended to an image:
> depending on the image type, we'd like to append the FDT not at all
> (the case for the OS boot), to the 'firmware' image (if it is a
> U-Boot) or to one of the loadables (if the 'firmware' is an ATF, an
> OPTEE, or some other image-type and U-Boot is listed in the
> loadabled).
>
> To achieve this goal, we drop the os_boot flag and track the type of
> image loaded.  If it is of type IH_OS_U_BOOT, we append the FDT.
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>
>  common/spl/spl_fit.c | 86 ++++++++++++++++++++++++++++++++++------------------
>  1 file changed, 56 insertions(+), 30 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Patch

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 49ccf1c..9f05e1e 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -215,6 +215,30 @@  static int spl_load_fit_image(struct spl_load_info *info, ulong sector,
 	return 0;
 }
 
+static int spl_fit_append_fdt(struct spl_image_info *spl_image,
+			      struct spl_load_info *info, ulong sector,
+			      void *fit, int images, ulong base_offset)
+{
+	struct spl_image_info image_info;
+	int node, ret;
+
+	/* Figure out which device tree the board wants to use */
+	node = spl_fit_get_image_node(fit, images, FIT_FDT_PROP, 0);
+	if (node < 0) {
+		debug("%s: cannot find FDT node\n", __func__);
+		return node;
+	}
+
+	/*
+	 * Read the device tree and place it after the image.
+	 * Align the destination address to ARCH_DMA_MINALIGN.
+	 */
+	image_info.load_addr = spl_image->load_addr + spl_image->size;
+	ret = spl_load_fit_image(info, sector, fit, base_offset, node,
+				 &image_info);
+	return ret;
+}
+
 int spl_load_simple_fit(struct spl_image_info *spl_image,
 			struct spl_load_info *info, ulong sector, void *fit)
 {
@@ -222,7 +246,6 @@  int spl_load_simple_fit(struct spl_image_info *spl_image,
 	ulong size;
 	unsigned long count;
 	struct spl_image_info image_info;
-	bool boot_os = false;
 	int node = -1;
 	int images, ret;
 	int base_offset, align_len = ARCH_DMA_MINALIGN - 1;
@@ -270,17 +293,18 @@  int spl_load_simple_fit(struct spl_image_info *spl_image,
 		return -1;
 	}
 
+	/*
+	 * Find the U-Boot image using the following search order:
+	 *   - start at 'firmware' (e.g. an ARM Trusted Firmware)
+	 *   - fall back 'kernel' (e.g. a Falcon-mode OS boot
+	 *   - fall back to using the first 'loadables' entry
+	 */
+	if (node < 0)
+		node = spl_fit_get_image_node(fit, images, "firmware", 0);
 #ifdef CONFIG_SPL_OS_BOOT
-	/* Find OS image first */
-	node = spl_fit_get_image_node(fit, images, FIT_KERNEL_PROP, 0);
 	if (node < 0)
-		debug("No kernel image.\n");
-	else
-		boot_os = true;
+		node = spl_fit_get_image_node(fit, images, FIT_KERNEL_PROP, 0);
 #endif
-	/* find the U-Boot image */
-	if (node < 0)
-		node = spl_fit_get_image_node(fit, images, "firmware", 0);
 	if (node < 0) {
 		debug("could not find firmware image, trying loadables...\n");
 		node = spl_fit_get_image_node(fit, images, "loadables", 0);
@@ -302,34 +326,29 @@  int spl_load_simple_fit(struct spl_image_info *spl_image,
 	if (ret)
 		return ret;
 
-#ifdef CONFIG_SPL_OS_BOOT
+	/*
+	 * For backward compatibility, we treat the first node that is
+	 * as a U-Boot image, if no OS-type has been declared.
+	 */
 	if (!fit_image_get_os(fit, node, &spl_image->os))
 		debug("Image OS is %s\n", genimg_get_os_name(spl_image->os));
-#else
-	spl_image->os = IH_OS_U_BOOT;
+#if !defined(CONFIG_SPL_OS_BOOT)
+	else
+		spl_image->os = IH_OS_U_BOOT;
 #endif
 
-	if (!boot_os) {
-		/* Figure out which device tree the board wants to use */
-		node = spl_fit_get_image_node(fit, images, FIT_FDT_PROP, 0);
-		if (node < 0) {
-			debug("%s: cannot find FDT node\n", __func__);
-			return node;
-		}
-
-		/*
-		 * Read the device tree and place it after the image.
-		 * Align the destination address to ARCH_DMA_MINALIGN.
-		 */
-		image_info.load_addr = spl_image->load_addr + spl_image->size;
-		ret = spl_load_fit_image(info, sector, fit, base_offset, node,
-					 &image_info);
-		if (ret < 0)
-			return ret;
-	}
+	/*
+	 * Booting a next-stage U-Boot may require us to append the FDT.
+	 * We allow this to fail, as the U-Boot image might embed its FDT.
+	 */
+	if (spl_image->os == IH_OS_U_BOOT)
+		spl_fit_append_fdt(spl_image, info, sector, fit,
+				   images, base_offset);
 
 	/* Now check if there are more images for us to load */
 	for (; ; index++) {
+		uint8_t os_type = IH_OS_INVALID;
+
 		node = spl_fit_get_image_node(fit, images, "loadables", index);
 		if (node < 0)
 			break;
@@ -339,6 +358,13 @@  int spl_load_simple_fit(struct spl_image_info *spl_image,
 		if (ret < 0)
 			continue;
 
+		if (!fit_image_get_os(fit, node, &os_type))
+			debug("Loadable is %s\n", genimg_get_os_name(os_type));
+
+		if (spl_image->os == IH_OS_U_BOOT)
+			spl_fit_append_fdt(spl_image, info, sector,
+					   fit, images, base_offset);
+
 		/*
 		 * If the "firmware" image did not provide an entry point,
 		 * use the first valid entry point from the loadables.