[U-Boot] [PATCH V2] spl: Weed out CONFIG_SYS_TEXT_BASE usage
Michal Simek
monstr at monstr.eu
Wed Oct 3 13:39:24 UTC 2018
Hi Marek,
út 14. 8. 2018 v 11:27 odesílatel Marek Vasut <marex at denx.de> napsal:
> 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));
> +
>
I am curious how you have tested this change.
Because I see on my zc706 that this breaks my SPL boot flow.
This should be assigned to u_boot_pos instead of header (which is done 2
line below)
and also here is additional empty line which shouldn't be there.
> }
> 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);
>
And I am also curious why we have 0x40 here.
Thanks,
Michal
--
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Microblaze cpu - http://www.monstr.eu/fdt/
Maintainer of Linux kernel - Xilinx Zynq ARM architecture
Microblaze U-BOOT custodian and responsible for u-boot arm zynq platform
More information about the U-Boot
mailing list