diff mbox series

[U-Boot,06/15] spl: fit: implement recording of loadables into /fit-images

Message ID 1505330989-25602-7-git-send-email-philipp.tomsich@theobroma-systems.com
State Accepted
Commit a616c783f22a045e580f101141a9d62775f97365
Delegated to: Philipp Tomsich
Headers show
Series [U-Boot,01/15] image: add IH_OS_ARM_TRUSTED_FIRMWARE for ARM Trusted Firmware | expand

Commit Message

Philipp Tomsich Sept. 13, 2017, 7:29 p.m. UTC
If a FDT was loaded (e.g. to append it to U-Boot image), we store it's
address and record information for all loadables into this FDT.  This
allows us to easily keep track of images for multiple privilege levels
(e.g. with ATF) or of firmware images preloaded into temporary
locations (e.g. PMU firmware that may overlap the SPL stage).

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

 common/spl/spl_fit.c | 95 ++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 81 insertions(+), 14 deletions(-)

Comments

Simon Glass Sept. 17, 2017, 5:53 p.m. UTC | #1
On 13 September 2017 at 13:29, Philipp Tomsich
<philipp.tomsich@theobroma-systems.com> wrote:
> If a FDT was loaded (e.g. to append it to U-Boot image), we store it's
> address and record information for all loadables into this FDT.  This
> allows us to easily keep track of images for multiple privilege levels
> (e.g. with ATF) or of firmware images preloaded into temporary
> locations (e.g. PMU firmware that may overlap the SPL stage).
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>
>  common/spl/spl_fit.c | 95 ++++++++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 81 insertions(+), 14 deletions(-)

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

I wonder if this should be a new CONFIG option to reduce code size for
things that don't need it?
Philipp Tomsich Nov. 23, 2017, 2:51 p.m. UTC | #2
> If a FDT was loaded (e.g. to append it to U-Boot image), we store it's
> address and record information for all loadables into this FDT.  This
> allows us to easily keep track of images for multiple privilege levels
> (e.g. with ATF) or of firmware images preloaded into temporary
> locations (e.g. PMU firmware that may overlap the SPL stage).
> 
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> ---
> 
>  common/spl/spl_fit.c | 95 ++++++++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 81 insertions(+), 14 deletions(-)
> 

Applied to u-boot-rockchip, thanks!
Michal Simek Jan. 18, 2018, 12:56 p.m. UTC | #3
Hi Philipp,


2017-09-13 21:29 GMT+02:00 Philipp Tomsich <
philipp.tomsich@theobroma-systems.com>:

> If a FDT was loaded (e.g. to append it to U-Boot image), we store it's
> address and record information for all loadables into this FDT.  This
> allows us to easily keep track of images for multiple privilege levels
> (e.g. with ATF) or of firmware images preloaded into temporary
> locations (e.g. PMU firmware that may overlap the SPL stage).
>
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
>
>  common/spl/spl_fit.c | 95 ++++++++++++++++++++++++++++++
> ++++++++++++++--------
>  1 file changed, 81 insertions(+), 14 deletions(-)
>
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index 9f05e1e..6dc0969 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -2,7 +2,7 @@
>   * Copyright (C) 2016 Google, Inc
>   * Written by Simon Glass <sjg@chromium.org>
>   *
> - * SPDX-License-Identifier:     GPL-2.0+
> + * SPDX-License-Identifier:    GPL-2.0+
>   */
>
>  #include <common.h>
> @@ -16,22 +16,24 @@
>  #endif
>
>  /**
> - * spl_fit_get_image_node(): By using the matching configuration subnode,
> + * spl_fit_get_image_name(): By using the matching configuration subnode,
>   * retrieve the name of an image, specified by a property name and an
> index
>   * into that.
>   * @fit:       Pointer to the FDT blob.
>   * @images:    Offset of the /images subnode.
>   * @type:      Name of the property within the configuration subnode.
>   * @index:     Index into the list of strings in this property.
> + * @outname:   Name of the image
>   *
> - * Return:     the node offset of the respective image node or a negative
> - *             error number.
> + * Return:     0 on success, or a negative error number
>   */
> -static int spl_fit_get_image_node(const void *fit, int images,
> -                                 const char *type, int index)
> +static int spl_fit_get_image_name(const void *fit, int images,
> +                                 const char *type, int index,
> +                                 char **outname)
>  {
>         const char *name, *str;
> -       int node, conf_node;
> +       __maybe_unused int node;
> +       int conf_node;
>         int len, i;
>
>         conf_node = fit_find_config_node(fit);
> @@ -63,7 +65,35 @@ static int spl_fit_get_image_node(const void *fit, int
> images,
>                 }
>         }
>
> +       *outname = (char *)str;
> +       return 0;
> +}
> +
> +/**
> + * spl_fit_get_image_node(): By using the matching configuration subnode,
> + * retrieve the name of an image, specified by a property name and an
> index
> + * into that.
> + * @fit:       Pointer to the FDT blob.
> + * @images:    Offset of the /images subnode.
> + * @type:      Name of the property within the configuration subnode.
> + * @index:     Index into the list of strings in this property.
> + *
> + * Return:     the node offset of the respective image node or a negative
> + *             error number.
> + */
> +static int spl_fit_get_image_node(const void *fit, int images,
> +                                 const char *type, int index)
> +{
> +       char *str;
> +       int err;
> +       int node;
> +
> +       err = spl_fit_get_image_name(fit, images, type, index, &str);
> +       if (err)
> +               return err;
> +
>         debug("%s: '%s'\n", type, str);
> +
>         node = fdt_subnode_offset(fit, images, str);
>         if (node < 0) {
>                 debug("cannot find image node '%s': %d\n", str, node);
> @@ -116,15 +146,15 @@ static int get_aligned_image_size(struct
> spl_load_info *info, int data_size,
>   * @info:      points to information about the device to load data from
>   * @sector:    the start sector of the FIT image on the device
>   * @fit:       points to the flattened device tree blob describing the FIT
> - *             image
> + *             image
>   * @base_offset: the beginning of the data area containing the actual
>   *             image data, relative to the beginning of the FIT
>   * @node:      offset of the DT node describing the image to load
> (relative
> - *             to @fit)
> + *             to @fit)
>   * @image_info:        will be filled with information about the loaded
> image
> - *             If the FIT node does not contain a "load" (address)
> property,
> - *             the image gets loaded to the address pointed to by the
> - *             load_addr member in this struct.
> + *             If the FIT node does not contain a "load" (address)
> property,
> + *             the image gets loaded to the address pointed to by the
> + *             load_addr member in this struct.
>   *
>   * Return:     0 on success or a negative error number.
>   */
> @@ -236,6 +266,35 @@ static int spl_fit_append_fdt(struct spl_image_info
> *spl_image,
>         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;
> +
> +       /* Make the load-address of the FDT available for the SPL
> framework */
> +       spl_image->fdt_addr = (void *)image_info.load_addr;
> +       /* Try to make space, so we can inject details on the loadables */
> +       ret = fdt_shrink_to_minimum(spl_image->fdt_addr, 8192);
> +
> +       return ret;
> +}
> +
> +static int spl_fit_record_loadable(const void *fit, int images, int index,
> +                                  void *blob, struct spl_image_info
> *image)
> +{
> +       char *name;
> +       int node, ret;
> +
> +       ret = spl_fit_get_image_name(fit, images, "loadables",
> +                                    index, &name);
> +       if (ret < 0)
> +               return ret;
> +
> +       node = spl_fit_get_image_node(fit, images, "loadables", index);
> +
> +       ret = fdt_record_loadable(blob, index, name, image->load_addr,
> +                                 image->size, image->entry_point,
> +                                 fdt_getprop(fit, node, "type", NULL),
> +                                 fdt_getprop(fit, node, "os", NULL));
>


Calling this fdt_record_loadable is causing compilation issue when
CONFIG_ARCH_FIXUP_FDT_MEMORY is disabled.

common/spl/built-in.o: In function `spl_fit_record_loadable':
/mnt/disk/u-boot/common/spl/spl_fit.c:308: undefined reference to
`fdt_record_loadable'
/mnt/disk/u-boot/common/spl/spl_fit.c:308:(.text.spl_load_simple_fit+0x2ec):
relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol
`fdt_record_loadable'
make[1]: *** [spl/u-boot-spl] Error 1

You can see this issue with xilinx_zynqmp_zcu102_rev1_0_defconfig when you
disable that option.
Just a note adding dependency to Kconfig is not a proper solution because
for this combination I need fit image support.

Can you please propose solution?

Thanks,
Michal
Philipp Tomsich Jan. 18, 2018, 1:17 p.m. UTC | #4
Michal,

> On 18 Jan 2018, at 13:56, Michal Simek <monstr@monstr.eu> wrote:
> 
> Hi Philipp,
> 
> 
> 2017-09-13 21:29 GMT+02:00 Philipp Tomsich <philipp.tomsich@theobroma-systems.com>:
> If a FDT was loaded (e.g. to append it to U-Boot image), we store it's
> address and record information for all loadables into this FDT.  This
> allows us to easily keep track of images for multiple privilege levels
> (e.g. with ATF) or of firmware images preloaded into temporary
> locations (e.g. PMU firmware that may overlap the SPL stage).
> 
> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
> ---
> 
>  common/spl/spl_fit.c | 95 ++++++++++++++++++++++++++++++++++++++++++++--------
>  1 file changed, 81 insertions(+), 14 deletions(-)
> 
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index 9f05e1e..6dc0969 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -2,7 +2,7 @@
>   * Copyright (C) 2016 Google, Inc
>   * Written by Simon Glass <sjg@chromium.org>
>   *
> - * SPDX-License-Identifier:     GPL-2.0+
> + * SPDX-License-Identifier:    GPL-2.0+
>   */
> 
>  #include <common.h>
> @@ -16,22 +16,24 @@
>  #endif
> 
>  /**
> - * spl_fit_get_image_node(): By using the matching configuration subnode,
> + * spl_fit_get_image_name(): By using the matching configuration subnode,
>   * retrieve the name of an image, specified by a property name and an index
>   * into that.
>   * @fit:       Pointer to the FDT blob.
>   * @images:    Offset of the /images subnode.
>   * @type:      Name of the property within the configuration subnode.
>   * @index:     Index into the list of strings in this property.
> + * @outname:   Name of the image
>   *
> - * Return:     the node offset of the respective image node or a negative
> - *             error number.
> + * Return:     0 on success, or a negative error number
>   */
> -static int spl_fit_get_image_node(const void *fit, int images,
> -                                 const char *type, int index)
> +static int spl_fit_get_image_name(const void *fit, int images,
> +                                 const char *type, int index,
> +                                 char **outname)
>  {
>         const char *name, *str;
> -       int node, conf_node;
> +       __maybe_unused int node;
> +       int conf_node;
>         int len, i;
> 
>         conf_node = fit_find_config_node(fit);
> @@ -63,7 +65,35 @@ static int spl_fit_get_image_node(const void *fit, int images,
>                 }
>         }
> 
> +       *outname = (char *)str;
> +       return 0;
> +}
> +
> +/**
> + * spl_fit_get_image_node(): By using the matching configuration subnode,
> + * retrieve the name of an image, specified by a property name and an index
> + * into that.
> + * @fit:       Pointer to the FDT blob.
> + * @images:    Offset of the /images subnode.
> + * @type:      Name of the property within the configuration subnode.
> + * @index:     Index into the list of strings in this property.
> + *
> + * Return:     the node offset of the respective image node or a negative
> + *             error number.
> + */
> +static int spl_fit_get_image_node(const void *fit, int images,
> +                                 const char *type, int index)
> +{
> +       char *str;
> +       int err;
> +       int node;
> +
> +       err = spl_fit_get_image_name(fit, images, type, index, &str);
> +       if (err)
> +               return err;
> +
>         debug("%s: '%s'\n", type, str);
> +
>         node = fdt_subnode_offset(fit, images, str);
>         if (node < 0) {
>                 debug("cannot find image node '%s': %d\n", str, node);
> @@ -116,15 +146,15 @@ static int get_aligned_image_size(struct spl_load_info *info, int data_size,
>   * @info:      points to information about the device to load data from
>   * @sector:    the start sector of the FIT image on the device
>   * @fit:       points to the flattened device tree blob describing the FIT
> - *             image
> + *             image
>   * @base_offset: the beginning of the data area containing the actual
>   *             image data, relative to the beginning of the FIT
>   * @node:      offset of the DT node describing the image to load (relative
> - *             to @fit)
> + *             to @fit)
>   * @image_info:        will be filled with information about the loaded image
> - *             If the FIT node does not contain a "load" (address) property,
> - *             the image gets loaded to the address pointed to by the
> - *             load_addr member in this struct.
> + *             If the FIT node does not contain a "load" (address) property,
> + *             the image gets loaded to the address pointed to by the
> + *             load_addr member in this struct.
>   *
>   * Return:     0 on success or a negative error number.
>   */
> @@ -236,6 +266,35 @@ static int spl_fit_append_fdt(struct spl_image_info *spl_image,
>         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;
> +
> +       /* Make the load-address of the FDT available for the SPL framework */
> +       spl_image->fdt_addr = (void *)image_info.load_addr;
> +       /* Try to make space, so we can inject details on the loadables */
> +       ret = fdt_shrink_to_minimum(spl_image->fdt_addr, 8192);
> +
> +       return ret;
> +}
> +
> +static int spl_fit_record_loadable(const void *fit, int images, int index,
> +                                  void *blob, struct spl_image_info *image)
> +{
> +       char *name;
> +       int node, ret;
> +
> +       ret = spl_fit_get_image_name(fit, images, "loadables",
> +                                    index, &name);
> +       if (ret < 0)
> +               return ret;
> +
> +       node = spl_fit_get_image_node(fit, images, "loadables", index);
> +
> +       ret = fdt_record_loadable(blob, index, name, image->load_addr,
> +                                 image->size, image->entry_point,
> +                                 fdt_getprop(fit, node, "type", NULL),
> +                                 fdt_getprop(fit, node, "os", NULL));
> 
> 
> Calling this fdt_record_loadable is causing compilation issue when CONFIG_ARCH_FIXUP_FDT_MEMORY is disabled.
> 
> common/spl/built-in.o: In function `spl_fit_record_loadable':
> /mnt/disk/u-boot/common/spl/spl_fit.c:308: undefined reference to `fdt_record_loadable'
> /mnt/disk/u-boot/common/spl/spl_fit.c:308:(.text.spl_load_simple_fit+0x2ec): relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol `fdt_record_loadable'
> make[1]: *** [spl/u-boot-spl] Error 1
> 
> You can see this issue with xilinx_zynqmp_zcu102_rev1_0_defconfig when you disable that option.
> Just a note adding dependency to Kconfig is not a proper solution because for this combination I need fit image support.
> 
> Can you please propose solution?

Thanks for the heads-up.

Sloppy editing on my part. The fdt_record_loadable function needs to be moved out of
the #ifdef CONFIG_ARCH_FIXUP_FDT_MEMORY guard in common/fdt_support.c.
I’ll send a patch.


> Thanks,
> Michal
>
Michal Simek Jan. 18, 2018, 1:40 p.m. UTC | #5
On 18.1.2018 14:17, Dr. Philipp Tomsich wrote:
> Michal,
> 
>> On 18 Jan 2018, at 13:56, Michal Simek <monstr@monstr.eu> wrote:
>>
>> Hi Philipp,
>>
>>
>> 2017-09-13 21:29 GMT+02:00 Philipp Tomsich <philipp.tomsich@theobroma-systems.com>:
>> If a FDT was loaded (e.g. to append it to U-Boot image), we store it's
>> address and record information for all loadables into this FDT.  This
>> allows us to easily keep track of images for multiple privilege levels
>> (e.g. with ATF) or of firmware images preloaded into temporary
>> locations (e.g. PMU firmware that may overlap the SPL stage).
>>
>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>> ---
>>
>>  common/spl/spl_fit.c | 95 ++++++++++++++++++++++++++++++++++++++++++++--------
>>  1 file changed, 81 insertions(+), 14 deletions(-)
>>
>> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
>> index 9f05e1e..6dc0969 100644
>> --- a/common/spl/spl_fit.c
>> +++ b/common/spl/spl_fit.c
>> @@ -2,7 +2,7 @@
>>   * Copyright (C) 2016 Google, Inc
>>   * Written by Simon Glass <sjg@chromium.org>
>>   *
>> - * SPDX-License-Identifier:     GPL-2.0+
>> + * SPDX-License-Identifier:    GPL-2.0+
>>   */
>>
>>  #include <common.h>
>> @@ -16,22 +16,24 @@
>>  #endif
>>
>>  /**
>> - * spl_fit_get_image_node(): By using the matching configuration subnode,
>> + * spl_fit_get_image_name(): By using the matching configuration subnode,
>>   * retrieve the name of an image, specified by a property name and an index
>>   * into that.
>>   * @fit:       Pointer to the FDT blob.
>>   * @images:    Offset of the /images subnode.
>>   * @type:      Name of the property within the configuration subnode.
>>   * @index:     Index into the list of strings in this property.
>> + * @outname:   Name of the image
>>   *
>> - * Return:     the node offset of the respective image node or a negative
>> - *             error number.
>> + * Return:     0 on success, or a negative error number
>>   */
>> -static int spl_fit_get_image_node(const void *fit, int images,
>> -                                 const char *type, int index)
>> +static int spl_fit_get_image_name(const void *fit, int images,
>> +                                 const char *type, int index,
>> +                                 char **outname)
>>  {
>>         const char *name, *str;
>> -       int node, conf_node;
>> +       __maybe_unused int node;
>> +       int conf_node;
>>         int len, i;
>>
>>         conf_node = fit_find_config_node(fit);
>> @@ -63,7 +65,35 @@ static int spl_fit_get_image_node(const void *fit, int images,
>>                 }
>>         }
>>
>> +       *outname = (char *)str;
>> +       return 0;
>> +}
>> +
>> +/**
>> + * spl_fit_get_image_node(): By using the matching configuration subnode,
>> + * retrieve the name of an image, specified by a property name and an index
>> + * into that.
>> + * @fit:       Pointer to the FDT blob.
>> + * @images:    Offset of the /images subnode.
>> + * @type:      Name of the property within the configuration subnode.
>> + * @index:     Index into the list of strings in this property.
>> + *
>> + * Return:     the node offset of the respective image node or a negative
>> + *             error number.
>> + */
>> +static int spl_fit_get_image_node(const void *fit, int images,
>> +                                 const char *type, int index)
>> +{
>> +       char *str;
>> +       int err;
>> +       int node;
>> +
>> +       err = spl_fit_get_image_name(fit, images, type, index, &str);
>> +       if (err)
>> +               return err;
>> +
>>         debug("%s: '%s'\n", type, str);
>> +
>>         node = fdt_subnode_offset(fit, images, str);
>>         if (node < 0) {
>>                 debug("cannot find image node '%s': %d\n", str, node);
>> @@ -116,15 +146,15 @@ static int get_aligned_image_size(struct spl_load_info *info, int data_size,
>>   * @info:      points to information about the device to load data from
>>   * @sector:    the start sector of the FIT image on the device
>>   * @fit:       points to the flattened device tree blob describing the FIT
>> - *             image
>> + *             image
>>   * @base_offset: the beginning of the data area containing the actual
>>   *             image data, relative to the beginning of the FIT
>>   * @node:      offset of the DT node describing the image to load (relative
>> - *             to @fit)
>> + *             to @fit)
>>   * @image_info:        will be filled with information about the loaded image
>> - *             If the FIT node does not contain a "load" (address) property,
>> - *             the image gets loaded to the address pointed to by the
>> - *             load_addr member in this struct.
>> + *             If the FIT node does not contain a "load" (address) property,
>> + *             the image gets loaded to the address pointed to by the
>> + *             load_addr member in this struct.
>>   *
>>   * Return:     0 on success or a negative error number.
>>   */
>> @@ -236,6 +266,35 @@ static int spl_fit_append_fdt(struct spl_image_info *spl_image,
>>         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;
>> +
>> +       /* Make the load-address of the FDT available for the SPL framework */
>> +       spl_image->fdt_addr = (void *)image_info.load_addr;
>> +       /* Try to make space, so we can inject details on the loadables */
>> +       ret = fdt_shrink_to_minimum(spl_image->fdt_addr, 8192);
>> +
>> +       return ret;
>> +}
>> +
>> +static int spl_fit_record_loadable(const void *fit, int images, int index,
>> +                                  void *blob, struct spl_image_info *image)
>> +{
>> +       char *name;
>> +       int node, ret;
>> +
>> +       ret = spl_fit_get_image_name(fit, images, "loadables",
>> +                                    index, &name);
>> +       if (ret < 0)
>> +               return ret;
>> +
>> +       node = spl_fit_get_image_node(fit, images, "loadables", index);
>> +
>> +       ret = fdt_record_loadable(blob, index, name, image->load_addr,
>> +                                 image->size, image->entry_point,
>> +                                 fdt_getprop(fit, node, "type", NULL),
>> +                                 fdt_getprop(fit, node, "os", NULL));
>>
>>
>> Calling this fdt_record_loadable is causing compilation issue when CONFIG_ARCH_FIXUP_FDT_MEMORY is disabled.
>>
>> common/spl/built-in.o: In function `spl_fit_record_loadable':
>> /mnt/disk/u-boot/common/spl/spl_fit.c:308: undefined reference to `fdt_record_loadable'
>> /mnt/disk/u-boot/common/spl/spl_fit.c:308:(.text.spl_load_simple_fit+0x2ec): relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol `fdt_record_loadable'
>> make[1]: *** [spl/u-boot-spl] Error 1
>>
>> You can see this issue with xilinx_zynqmp_zcu102_rev1_0_defconfig when you disable that option.
>> Just a note adding dependency to Kconfig is not a proper solution because for this combination I need fit image support.
>>
>> Can you please propose solution?
> 
> Thanks for the heads-up.
> 
> Sloppy editing on my part. The fdt_record_loadable function needs to be moved out of
> the #ifdef CONFIG_ARCH_FIXUP_FDT_MEMORY guard in common/fdt_support.c.
> I’ll send a patch.

Will wait for a patch.

Thanks,
Michal
Michal Simek Feb. 1, 2018, 7:51 a.m. UTC | #6
On 18.1.2018 14:17, Dr. Philipp Tomsich wrote:
> Michal,
> 
>> On 18 Jan 2018, at 13:56, Michal Simek <monstr@monstr.eu> wrote:
>>
>> Hi Philipp,
>>
>>
>> 2017-09-13 21:29 GMT+02:00 Philipp Tomsich <philipp.tomsich@theobroma-systems.com>:
>> If a FDT was loaded (e.g. to append it to U-Boot image), we store it's
>> address and record information for all loadables into this FDT.  This
>> allows us to easily keep track of images for multiple privilege levels
>> (e.g. with ATF) or of firmware images preloaded into temporary
>> locations (e.g. PMU firmware that may overlap the SPL stage).
>>
>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>> ---
>>
>>  common/spl/spl_fit.c | 95 ++++++++++++++++++++++++++++++++++++++++++++--------
>>  1 file changed, 81 insertions(+), 14 deletions(-)
>>
>> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
>> index 9f05e1e..6dc0969 100644
>> --- a/common/spl/spl_fit.c
>> +++ b/common/spl/spl_fit.c
>> @@ -2,7 +2,7 @@
>>   * Copyright (C) 2016 Google, Inc
>>   * Written by Simon Glass <sjg@chromium.org>
>>   *
>> - * SPDX-License-Identifier:     GPL-2.0+
>> + * SPDX-License-Identifier:    GPL-2.0+
>>   */
>>
>>  #include <common.h>
>> @@ -16,22 +16,24 @@
>>  #endif
>>
>>  /**
>> - * spl_fit_get_image_node(): By using the matching configuration subnode,
>> + * spl_fit_get_image_name(): By using the matching configuration subnode,
>>   * retrieve the name of an image, specified by a property name and an index
>>   * into that.
>>   * @fit:       Pointer to the FDT blob.
>>   * @images:    Offset of the /images subnode.
>>   * @type:      Name of the property within the configuration subnode.
>>   * @index:     Index into the list of strings in this property.
>> + * @outname:   Name of the image
>>   *
>> - * Return:     the node offset of the respective image node or a negative
>> - *             error number.
>> + * Return:     0 on success, or a negative error number
>>   */
>> -static int spl_fit_get_image_node(const void *fit, int images,
>> -                                 const char *type, int index)
>> +static int spl_fit_get_image_name(const void *fit, int images,
>> +                                 const char *type, int index,
>> +                                 char **outname)
>>  {
>>         const char *name, *str;
>> -       int node, conf_node;
>> +       __maybe_unused int node;
>> +       int conf_node;
>>         int len, i;
>>
>>         conf_node = fit_find_config_node(fit);
>> @@ -63,7 +65,35 @@ static int spl_fit_get_image_node(const void *fit, int images,
>>                 }
>>         }
>>
>> +       *outname = (char *)str;
>> +       return 0;
>> +}
>> +
>> +/**
>> + * spl_fit_get_image_node(): By using the matching configuration subnode,
>> + * retrieve the name of an image, specified by a property name and an index
>> + * into that.
>> + * @fit:       Pointer to the FDT blob.
>> + * @images:    Offset of the /images subnode.
>> + * @type:      Name of the property within the configuration subnode.
>> + * @index:     Index into the list of strings in this property.
>> + *
>> + * Return:     the node offset of the respective image node or a negative
>> + *             error number.
>> + */
>> +static int spl_fit_get_image_node(const void *fit, int images,
>> +                                 const char *type, int index)
>> +{
>> +       char *str;
>> +       int err;
>> +       int node;
>> +
>> +       err = spl_fit_get_image_name(fit, images, type, index, &str);
>> +       if (err)
>> +               return err;
>> +
>>         debug("%s: '%s'\n", type, str);
>> +
>>         node = fdt_subnode_offset(fit, images, str);
>>         if (node < 0) {
>>                 debug("cannot find image node '%s': %d\n", str, node);
>> @@ -116,15 +146,15 @@ static int get_aligned_image_size(struct spl_load_info *info, int data_size,
>>   * @info:      points to information about the device to load data from
>>   * @sector:    the start sector of the FIT image on the device
>>   * @fit:       points to the flattened device tree blob describing the FIT
>> - *             image
>> + *             image
>>   * @base_offset: the beginning of the data area containing the actual
>>   *             image data, relative to the beginning of the FIT
>>   * @node:      offset of the DT node describing the image to load (relative
>> - *             to @fit)
>> + *             to @fit)
>>   * @image_info:        will be filled with information about the loaded image
>> - *             If the FIT node does not contain a "load" (address) property,
>> - *             the image gets loaded to the address pointed to by the
>> - *             load_addr member in this struct.
>> + *             If the FIT node does not contain a "load" (address) property,
>> + *             the image gets loaded to the address pointed to by the
>> + *             load_addr member in this struct.
>>   *
>>   * Return:     0 on success or a negative error number.
>>   */
>> @@ -236,6 +266,35 @@ static int spl_fit_append_fdt(struct spl_image_info *spl_image,
>>         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;
>> +
>> +       /* Make the load-address of the FDT available for the SPL framework */
>> +       spl_image->fdt_addr = (void *)image_info.load_addr;
>> +       /* Try to make space, so we can inject details on the loadables */
>> +       ret = fdt_shrink_to_minimum(spl_image->fdt_addr, 8192);
>> +
>> +       return ret;
>> +}
>> +
>> +static int spl_fit_record_loadable(const void *fit, int images, int index,
>> +                                  void *blob, struct spl_image_info *image)
>> +{
>> +       char *name;
>> +       int node, ret;
>> +
>> +       ret = spl_fit_get_image_name(fit, images, "loadables",
>> +                                    index, &name);
>> +       if (ret < 0)
>> +               return ret;
>> +
>> +       node = spl_fit_get_image_node(fit, images, "loadables", index);
>> +
>> +       ret = fdt_record_loadable(blob, index, name, image->load_addr,
>> +                                 image->size, image->entry_point,
>> +                                 fdt_getprop(fit, node, "type", NULL),
>> +                                 fdt_getprop(fit, node, "os", NULL));
>>
>>
>> Calling this fdt_record_loadable is causing compilation issue when CONFIG_ARCH_FIXUP_FDT_MEMORY is disabled.
>>
>> common/spl/built-in.o: In function `spl_fit_record_loadable':
>> /mnt/disk/u-boot/common/spl/spl_fit.c:308: undefined reference to `fdt_record_loadable'
>> /mnt/disk/u-boot/common/spl/spl_fit.c:308:(.text.spl_load_simple_fit+0x2ec): relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol `fdt_record_loadable'
>> make[1]: *** [spl/u-boot-spl] Error 1
>>
>> You can see this issue with xilinx_zynqmp_zcu102_rev1_0_defconfig when you disable that option.
>> Just a note adding dependency to Kconfig is not a proper solution because for this combination I need fit image support.
>>
>> Can you please propose solution?
> 
> Thanks for the heads-up.
> 
> Sloppy editing on my part. The fdt_record_loadable function needs to be moved out of
> the #ifdef CONFIG_ARCH_FIXUP_FDT_MEMORY guard in common/fdt_support.c.
> I’ll send a patch.

Have you sent this patch?

Thanks,
Michal
Philipp Tomsich Feb. 2, 2018, 11:03 a.m. UTC | #7
> On 1 Feb 2018, at 08:51, Michal Simek <monstr@monstr.eu> wrote:
> 
> On 18.1.2018 14:17, Dr. Philipp Tomsich wrote:
>> Michal,
>> 
>>> On 18 Jan 2018, at 13:56, Michal Simek <monstr@monstr.eu> wrote:
>>> 
>>> Hi Philipp,
>>> 
>>> 
>>> 2017-09-13 21:29 GMT+02:00 Philipp Tomsich <philipp.tomsich@theobroma-systems.com>:
>>> If a FDT was loaded (e.g. to append it to U-Boot image), we store it's
>>> address and record information for all loadables into this FDT.  This
>>> allows us to easily keep track of images for multiple privilege levels
>>> (e.g. with ATF) or of firmware images preloaded into temporary
>>> locations (e.g. PMU firmware that may overlap the SPL stage).
>>> 
>>> Signed-off-by: Philipp Tomsich <philipp.tomsich@theobroma-systems.com>
>>> ---
>>> 
>>> common/spl/spl_fit.c | 95 ++++++++++++++++++++++++++++++++++++++++++++--------
>>> 1 file changed, 81 insertions(+), 14 deletions(-)
>>> 
>>> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
>>> index 9f05e1e..6dc0969 100644
>>> --- a/common/spl/spl_fit.c
>>> +++ b/common/spl/spl_fit.c
>>> @@ -2,7 +2,7 @@
>>>  * Copyright (C) 2016 Google, Inc
>>>  * Written by Simon Glass <sjg@chromium.org>
>>>  *
>>> - * SPDX-License-Identifier:     GPL-2.0+
>>> + * SPDX-License-Identifier:    GPL-2.0+
>>>  */
>>> 
>>> #include <common.h>
>>> @@ -16,22 +16,24 @@
>>> #endif
>>> 
>>> /**
>>> - * spl_fit_get_image_node(): By using the matching configuration subnode,
>>> + * spl_fit_get_image_name(): By using the matching configuration subnode,
>>>  * retrieve the name of an image, specified by a property name and an index
>>>  * into that.
>>>  * @fit:       Pointer to the FDT blob.
>>>  * @images:    Offset of the /images subnode.
>>>  * @type:      Name of the property within the configuration subnode.
>>>  * @index:     Index into the list of strings in this property.
>>> + * @outname:   Name of the image
>>>  *
>>> - * Return:     the node offset of the respective image node or a negative
>>> - *             error number.
>>> + * Return:     0 on success, or a negative error number
>>>  */
>>> -static int spl_fit_get_image_node(const void *fit, int images,
>>> -                                 const char *type, int index)
>>> +static int spl_fit_get_image_name(const void *fit, int images,
>>> +                                 const char *type, int index,
>>> +                                 char **outname)
>>> {
>>>        const char *name, *str;
>>> -       int node, conf_node;
>>> +       __maybe_unused int node;
>>> +       int conf_node;
>>>        int len, i;
>>> 
>>>        conf_node = fit_find_config_node(fit);
>>> @@ -63,7 +65,35 @@ static int spl_fit_get_image_node(const void *fit, int images,
>>>                }
>>>        }
>>> 
>>> +       *outname = (char *)str;
>>> +       return 0;
>>> +}
>>> +
>>> +/**
>>> + * spl_fit_get_image_node(): By using the matching configuration subnode,
>>> + * retrieve the name of an image, specified by a property name and an index
>>> + * into that.
>>> + * @fit:       Pointer to the FDT blob.
>>> + * @images:    Offset of the /images subnode.
>>> + * @type:      Name of the property within the configuration subnode.
>>> + * @index:     Index into the list of strings in this property.
>>> + *
>>> + * Return:     the node offset of the respective image node or a negative
>>> + *             error number.
>>> + */
>>> +static int spl_fit_get_image_node(const void *fit, int images,
>>> +                                 const char *type, int index)
>>> +{
>>> +       char *str;
>>> +       int err;
>>> +       int node;
>>> +
>>> +       err = spl_fit_get_image_name(fit, images, type, index, &str);
>>> +       if (err)
>>> +               return err;
>>> +
>>>        debug("%s: '%s'\n", type, str);
>>> +
>>>        node = fdt_subnode_offset(fit, images, str);
>>>        if (node < 0) {
>>>                debug("cannot find image node '%s': %d\n", str, node);
>>> @@ -116,15 +146,15 @@ static int get_aligned_image_size(struct spl_load_info *info, int data_size,
>>>  * @info:      points to information about the device to load data from
>>>  * @sector:    the start sector of the FIT image on the device
>>>  * @fit:       points to the flattened device tree blob describing the FIT
>>> - *             image
>>> + *             image
>>>  * @base_offset: the beginning of the data area containing the actual
>>>  *             image data, relative to the beginning of the FIT
>>>  * @node:      offset of the DT node describing the image to load (relative
>>> - *             to @fit)
>>> + *             to @fit)
>>>  * @image_info:        will be filled with information about the loaded image
>>> - *             If the FIT node does not contain a "load" (address) property,
>>> - *             the image gets loaded to the address pointed to by the
>>> - *             load_addr member in this struct.
>>> + *             If the FIT node does not contain a "load" (address) property,
>>> + *             the image gets loaded to the address pointed to by the
>>> + *             load_addr member in this struct.
>>>  *
>>>  * Return:     0 on success or a negative error number.
>>>  */
>>> @@ -236,6 +266,35 @@ static int spl_fit_append_fdt(struct spl_image_info *spl_image,
>>>        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;
>>> +
>>> +       /* Make the load-address of the FDT available for the SPL framework */
>>> +       spl_image->fdt_addr = (void *)image_info.load_addr;
>>> +       /* Try to make space, so we can inject details on the loadables */
>>> +       ret = fdt_shrink_to_minimum(spl_image->fdt_addr, 8192);
>>> +
>>> +       return ret;
>>> +}
>>> +
>>> +static int spl_fit_record_loadable(const void *fit, int images, int index,
>>> +                                  void *blob, struct spl_image_info *image)
>>> +{
>>> +       char *name;
>>> +       int node, ret;
>>> +
>>> +       ret = spl_fit_get_image_name(fit, images, "loadables",
>>> +                                    index, &name);
>>> +       if (ret < 0)
>>> +               return ret;
>>> +
>>> +       node = spl_fit_get_image_node(fit, images, "loadables", index);
>>> +
>>> +       ret = fdt_record_loadable(blob, index, name, image->load_addr,
>>> +                                 image->size, image->entry_point,
>>> +                                 fdt_getprop(fit, node, "type", NULL),
>>> +                                 fdt_getprop(fit, node, "os", NULL));
>>> 
>>> 
>>> Calling this fdt_record_loadable is causing compilation issue when CONFIG_ARCH_FIXUP_FDT_MEMORY is disabled.
>>> 
>>> common/spl/built-in.o: In function `spl_fit_record_loadable':
>>> /mnt/disk/u-boot/common/spl/spl_fit.c:308: undefined reference to `fdt_record_loadable'
>>> /mnt/disk/u-boot/common/spl/spl_fit.c:308:(.text.spl_load_simple_fit+0x2ec): relocation truncated to fit: R_AARCH64_CALL26 against undefined symbol `fdt_record_loadable'
>>> make[1]: *** [spl/u-boot-spl] Error 1
>>> 
>>> You can see this issue with xilinx_zynqmp_zcu102_rev1_0_defconfig when you disable that option.
>>> Just a note adding dependency to Kconfig is not a proper solution because for this combination I need fit image support.
>>> 
>>> Can you please propose solution?
>> 
>> Thanks for the heads-up.
>> 
>> Sloppy editing on my part. The fdt_record_loadable function needs to be moved out of
>> the #ifdef CONFIG_ARCH_FIXUP_FDT_MEMORY guard in common/fdt_support.c.
>> I’ll send a patch.
> 
> Have you sent this patch?

This had fallen off my radar.
The patch is on the list now.

Regards,
Philipp.
diff mbox series

Patch

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 9f05e1e..6dc0969 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -2,7 +2,7 @@ 
  * Copyright (C) 2016 Google, Inc
  * Written by Simon Glass <sjg@chromium.org>
  *
- * SPDX-License-Identifier:     GPL-2.0+
+ * SPDX-License-Identifier:	GPL-2.0+
  */
 
 #include <common.h>
@@ -16,22 +16,24 @@ 
 #endif
 
 /**
- * spl_fit_get_image_node(): By using the matching configuration subnode,
+ * spl_fit_get_image_name(): By using the matching configuration subnode,
  * retrieve the name of an image, specified by a property name and an index
  * into that.
  * @fit:	Pointer to the FDT blob.
  * @images:	Offset of the /images subnode.
  * @type:	Name of the property within the configuration subnode.
  * @index:	Index into the list of strings in this property.
+ * @outname:	Name of the image
  *
- * Return:	the node offset of the respective image node or a negative
- * 		error number.
+ * Return:	0 on success, or a negative error number
  */
-static int spl_fit_get_image_node(const void *fit, int images,
-				  const char *type, int index)
+static int spl_fit_get_image_name(const void *fit, int images,
+				  const char *type, int index,
+				  char **outname)
 {
 	const char *name, *str;
-	int node, conf_node;
+	__maybe_unused int node;
+	int conf_node;
 	int len, i;
 
 	conf_node = fit_find_config_node(fit);
@@ -63,7 +65,35 @@  static int spl_fit_get_image_node(const void *fit, int images,
 		}
 	}
 
+	*outname = (char *)str;
+	return 0;
+}
+
+/**
+ * spl_fit_get_image_node(): By using the matching configuration subnode,
+ * retrieve the name of an image, specified by a property name and an index
+ * into that.
+ * @fit:	Pointer to the FDT blob.
+ * @images:	Offset of the /images subnode.
+ * @type:	Name of the property within the configuration subnode.
+ * @index:	Index into the list of strings in this property.
+ *
+ * Return:	the node offset of the respective image node or a negative
+ *		error number.
+ */
+static int spl_fit_get_image_node(const void *fit, int images,
+				  const char *type, int index)
+{
+	char *str;
+	int err;
+	int node;
+
+	err = spl_fit_get_image_name(fit, images, type, index, &str);
+	if (err)
+		return err;
+
 	debug("%s: '%s'\n", type, str);
+
 	node = fdt_subnode_offset(fit, images, str);
 	if (node < 0) {
 		debug("cannot find image node '%s': %d\n", str, node);
@@ -116,15 +146,15 @@  static int get_aligned_image_size(struct spl_load_info *info, int data_size,
  * @info:	points to information about the device to load data from
  * @sector:	the start sector of the FIT image on the device
  * @fit:	points to the flattened device tree blob describing the FIT
- * 		image
+ *		image
  * @base_offset: the beginning of the data area containing the actual
  *		image data, relative to the beginning of the FIT
  * @node:	offset of the DT node describing the image to load (relative
- * 		to @fit)
+ *		to @fit)
  * @image_info:	will be filled with information about the loaded image
- * 		If the FIT node does not contain a "load" (address) property,
- * 		the image gets loaded to the address pointed to by the
- * 		load_addr member in this struct.
+ *		If the FIT node does not contain a "load" (address) property,
+ *		the image gets loaded to the address pointed to by the
+ *		load_addr member in this struct.
  *
  * Return:	0 on success or a negative error number.
  */
@@ -236,6 +266,35 @@  static int spl_fit_append_fdt(struct spl_image_info *spl_image,
 	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;
+
+	/* Make the load-address of the FDT available for the SPL framework */
+	spl_image->fdt_addr = (void *)image_info.load_addr;
+	/* Try to make space, so we can inject details on the loadables */
+	ret = fdt_shrink_to_minimum(spl_image->fdt_addr, 8192);
+
+	return ret;
+}
+
+static int spl_fit_record_loadable(const void *fit, int images, int index,
+				   void *blob, struct spl_image_info *image)
+{
+	char *name;
+	int node, ret;
+
+	ret = spl_fit_get_image_name(fit, images, "loadables",
+				     index, &name);
+	if (ret < 0)
+		return ret;
+
+	node = spl_fit_get_image_node(fit, images, "loadables", index);
+
+	ret = fdt_record_loadable(blob, index, name, image->load_addr,
+				  image->size, image->entry_point,
+				  fdt_getprop(fit, node, "type", NULL),
+				  fdt_getprop(fit, node, "os", NULL));
 	return ret;
 }
 
@@ -361,9 +420,11 @@  int spl_load_simple_fit(struct spl_image_info *spl_image,
 		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,
+		if (os_type == IH_OS_U_BOOT) {
+			spl_fit_append_fdt(&image_info, info, sector,
 					   fit, images, base_offset);
+			spl_image->fdt_addr = image_info.fdt_addr;
+		}
 
 		/*
 		 * If the "firmware" image did not provide an entry point,
@@ -372,6 +433,12 @@  int spl_load_simple_fit(struct spl_image_info *spl_image,
 		if (spl_image->entry_point == FDT_ERROR &&
 		    image_info.entry_point != FDT_ERROR)
 			spl_image->entry_point = image_info.entry_point;
+
+		/* Record our loadables into the FDT */
+		if (spl_image->fdt_addr)
+			spl_fit_record_loadable(fit, images, index,
+						spl_image->fdt_addr,
+						&image_info);
 	}
 
 	/*