[U-Boot] [PATCH V2] spl: Weed out CONFIG_SYS_TEXT_BASE usage

Simon Goldschmidt simon.k.r.goldschmidt at gmail.com
Tue Aug 14 09:51:59 UTC 2018


On Tue, Aug 14, 2018 at 11:27 AM Marek Vasut <marex at denx.de> wrote:
>
> The SPL loaders assume that the CONFIG_SYS_TEXT_BASE memory location
> is available and can be corrupted by loading ie. uImage or fitImage
> headers there. Sometimes it could be beneficial to load the headers
> elsewhere, ie. if CONFIG_SYS_TEXT_BASE is not yet writable while we
> still want to parse the image headers in some local onchip memory to
> ie. extract firmware from that image.
>
> Add the possibility to override the location where the headers get
> loaded by introducing new function, spl_get_load_buffer() which takes
> two arguments -- offset from the CONFIG_SYS_TEXT_BASE and size of the
> data that are to be loaded there -- and returns a valid buffer address
> or hangs the system. The default behavior is the same as before, add
> the offset to CONFIG_SYS_TEXT_BASE and return that address. User can
> override the weak spl_get_load_buffer() function though.
>
> Signed-off-by: Marek Vasut <marex at denx.de>
> Cc: Tom Rini <trini at konsulko.com>
> ---
> V2: Fix build issues on multiple boards due to incorrect pointer casting
> ---
>  common/spl/spl.c         | 5 +++++
>  common/spl/spl_ext.c     | 3 +--
>  common/spl/spl_fat.c     | 3 +--
>  common/spl/spl_fit.c     | 6 +++---
>  common/spl/spl_mmc.c     | 6 +++---
>  common/spl/spl_nand.c    | 4 ++--
>  common/spl/spl_onenand.c | 3 +--
>  common/spl/spl_ram.c     | 5 +++--
>  common/spl/spl_spi.c     | 3 +--
>  common/spl/spl_ubi.c     | 3 +--
>  include/spl.h            | 9 +++++++++
>  11 files changed, 30 insertions(+), 20 deletions(-)
>
> diff --git a/common/spl/spl.c b/common/spl/spl.c
> index eda84d0c74..7c22d0f44d 100644
> --- a/common/spl/spl.c
> +++ b/common/spl/spl.c
> @@ -127,6 +127,11 @@ __weak void spl_board_prepare_for_boot(void)
>         /* Nothing to do! */
>  }
>
> +__weak struct image_header *spl_get_load_buffer(ssize_t offset, size_t size)
> +{
> +       return (struct image_header *)(CONFIG_SYS_TEXT_BASE + offset);
> +}
> +
>  void spl_set_header_raw_uboot(struct spl_image_info *spl_image)
>  {
>         ulong u_boot_pos = binman_sym(ulong, u_boot_any, image_pos);
> diff --git a/common/spl/spl_ext.c b/common/spl/spl_ext.c
> index fd30a61f9a..fe05223605 100644
> --- a/common/spl/spl_ext.c
> +++ b/common/spl/spl_ext.c
> @@ -16,8 +16,7 @@ int spl_load_image_ext(struct spl_image_info *spl_image,
>         loff_t filelen, actlen;
>         disk_partition_t part_info = {};
>
> -       header = (struct image_header *)(CONFIG_SYS_TEXT_BASE -
> -                                               sizeof(struct image_header));
> +       header = spl_get_load_buffer(-sizeof(*header), sizeof(*header));
>
>         if (part_get_info(block_dev, partition, &part_info)) {
>                 printf("spl: no partition table found\n");
> diff --git a/common/spl/spl_fat.c b/common/spl/spl_fat.c
> index 0403016bb4..163e540622 100644
> --- a/common/spl/spl_fat.c
> +++ b/common/spl/spl_fat.c
> @@ -63,8 +63,7 @@ int spl_load_image_fat(struct spl_image_info *spl_image,
>         if (err)
>                 goto end;
>
> -       header = (struct image_header *)(CONFIG_SYS_TEXT_BASE -
> -                                               sizeof(struct image_header));
> +       header = spl_get_load_buffer(-sizeof(*header), sizeof(*header));
>
>         err = file_fat_read(filename, header, sizeof(struct image_header));
>         if (err <= 0)
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index 9eabb1c105..f08e5018c3 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -357,7 +357,7 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
>         struct spl_image_info image_info;
>         int node = -1;
>         int images, ret;
> -       int base_offset, align_len = ARCH_DMA_MINALIGN - 1;
> +       int base_offset, hsize, align_len = ARCH_DMA_MINALIGN - 1;
>         int index = 0;
>
>         /*
> @@ -386,8 +386,8 @@ int spl_load_simple_fit(struct spl_image_info *spl_image,
>          * For FIT with data embedded, data is loaded as part of FIT image.
>          * For FIT with external data, data is not loaded in this step.
>          */
> -       fit = (void *)((CONFIG_SYS_TEXT_BASE - size - info->bl_len -
> -                       align_len) & ~align_len);
> +       hsize = (size + info->bl_len + align_len) & ~align_len;
> +       fit = spl_get_load_buffer(-hsize, hsize);
>         sectors = get_aligned_image_size(info, size, 0);
>         count = info->read(info, sector, sectors, fit);
>         debug("fit read sector %lx, sectors=%d, dst=%p, count=%lu\n",
> diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
> index 0b2f059570..75c41598e6 100644
> --- a/common/spl/spl_mmc.c
> +++ b/common/spl/spl_mmc.c
> @@ -55,13 +55,13 @@ int mmc_load_image_raw_sector(struct spl_image_info *spl_image,
>  {
>         unsigned long count;
>         struct image_header *header;
> +       struct blk_desc *bd = mmc_get_blk_desc(mmc);
>         int ret = 0;
>
> -       header = (struct image_header *)(CONFIG_SYS_TEXT_BASE -
> -                                        sizeof(struct image_header));
> +       header = spl_get_load_buffer(-sizeof(*header), bd->blksz);
>
>         /* read image header to find the image size & load address */
> -       count = blk_dread(mmc_get_blk_desc(mmc), sector, 1, header);
> +       count = blk_dread(bd, sector, 1, header);
>         debug("hdr read sector %lx, count=%lu\n", sector, count);
>         if (count == 0) {
>                 ret = -EIO;
> diff --git a/common/spl/spl_nand.c b/common/spl/spl_nand.c
> index 2722fd3860..6eb190f1ea 100644
> --- a/common/spl/spl_nand.c
> +++ b/common/spl/spl_nand.c
> @@ -83,8 +83,8 @@ static int spl_nand_load_image(struct spl_image_info *spl_image,
>  #endif
>         nand_init();
>
> -       /*use CONFIG_SYS_TEXT_BASE as temporary storage area */
> -       header = (struct image_header *)(CONFIG_SYS_TEXT_BASE);
> +       header = spl_get_load_buffer(0, sizeof(*header));
> +
>  #ifdef CONFIG_SPL_OS_BOOT
>         if (!spl_start_uboot()) {
>                 /*
> diff --git a/common/spl/spl_onenand.c b/common/spl/spl_onenand.c
> index d32333935a..ee30f328e6 100644
> --- a/common/spl/spl_onenand.c
> +++ b/common/spl/spl_onenand.c
> @@ -21,8 +21,7 @@ static int spl_onenand_load_image(struct spl_image_info *spl_image,
>
>         debug("spl: onenand\n");
>
> -       /*use CONFIG_SYS_TEXT_BASE as temporary storage area */
> -       header = (struct image_header *)(CONFIG_SYS_TEXT_BASE);
> +       header = spl_get_load_buffer(0, CONFIG_SYS_ONENAND_PAGE_SIZE);
>         /* Load u-boot */
>         onenand_spl_load_image(CONFIG_SYS_ONENAND_U_BOOT_OFFS,
>                 CONFIG_SYS_ONENAND_PAGE_SIZE, (void *)header);
> diff --git a/common/spl/spl_ram.c b/common/spl/spl_ram.c
> index e594beaeaa..619b39a537 100644
> --- a/common/spl/spl_ram.c
> +++ b/common/spl/spl_ram.c
> @@ -63,8 +63,9 @@ static int spl_ram_load_image(struct spl_image_info *spl_image,
>                          * No binman support or no information. For now, fix it
>                          * to the address pointed to by U-Boot.
>                          */
> -                       u_boot_pos = CONFIG_SYS_TEXT_BASE -
> -                                       sizeof(struct image_header);
> +                       header = spl_get_load_buffer(-sizeof(*header),
> +                                                    sizeof(*header));
> +

Using "spl_get_load_buffer()" here seems to be a bit misleading, as
the address is used for "execute-in-place", not for loading.

>                 }
>                 header = (struct image_header *)map_sysmem(u_boot_pos, 0);
>
> diff --git a/common/spl/spl_spi.c b/common/spl/spl_spi.c
> index ba60a3a3c5..e10cf0124f 100644
> --- a/common/spl/spl_spi.c
> +++ b/common/spl/spl_spi.c
> @@ -88,8 +88,7 @@ static int spl_spi_load_image(struct spl_image_info *spl_image,
>                 return -ENODEV;
>         }
>
> -       /* use CONFIG_SYS_TEXT_BASE as temporary storage area */
> -       header = (struct image_header *)(CONFIG_SYS_TEXT_BASE);
> +       header = spl_get_load_buffer(-sizeof(*header), 0x40);

Shouldn't the first argument be 0 here instead of -sizeof(*header)?

>
>  #if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
>         payload_offs = fdtdec_get_config_int(gd->fdt_blob,
> diff --git a/common/spl/spl_ubi.c b/common/spl/spl_ubi.c
> index a7939e9030..67e5fadd7c 100644
> --- a/common/spl/spl_ubi.c
> +++ b/common/spl/spl_ubi.c
> @@ -61,8 +61,7 @@ int spl_ubi_load_image(struct spl_image_info *spl_image,
>                 puts("Loading Linux failed, falling back to U-Boot.\n");
>         }
>  #endif
> -       header = (struct image_header *)
> -               (CONFIG_SYS_TEXT_BASE - sizeof(struct image_header));
> +       header = spl_get_load_buffer(-sizeof(*header), sizeof(header));
>         volumes[0].vol_id = CONFIG_SPL_UBI_LOAD_MONITOR_ID;
>         volumes[0].load_addr = (void *)header;
>
> diff --git a/include/spl.h b/include/spl.h
> index 7fad62c043..b42683c9e7 100644
> --- a/include/spl.h
> +++ b/include/spl.h
> @@ -303,4 +303,13 @@ void board_return_to_bootrom(void);
>   *                        the boot-payload
>   */
>  void spl_perform_fixups(struct spl_image_info *spl_image);
> +
> +/*
> + * spl_get_load_buffer() - get buffer for loading partial image data
> + *
> + * Returns memory area which can be populated by partial image data,
> + * ie. uImage or fitImage header.
> + */
> +struct image_header *spl_get_load_buffer(ssize_t offset, size_t size);
> +
>  #endif
> --
> 2.16.2
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot


More information about the U-Boot mailing list