[RFC PATCH 1/7] spl: Add generic spl_load function
Stefan Roese
sr at denx.de
Fri Apr 8 06:43:38 CEST 2022
On 4/7/22 17:10, Sean Anderson wrote:
>
>
> On 4/6/22 1:30 AM, Stefan Roese wrote:
>> On 4/1/22 21:03, Sean Anderson wrote:
>>> Implementers of SPL_LOAD_IMAGE_METHOD have to correctly determine what
>>> type of image is being loaded and then call the appropriate image load
>>> function correctly. This is tricky, because some image load functions
>>> expect the whole image to already be loaded (CONFIG_SPL_LOAD_FIT_FULL),
>>> some will load the image automatically using spl_load_info.read()
>>> (CONFIG_SPL_LOAD_FIT/CONFIG_SPL_LOAD_IMX_CONTAINER), and some just parse
>>> the header and expect the caller to do the actual loading afterwards
>>> (legacy/raw images). Load methods often only support a subset of the
>>> above methods, meaning that not all image types can be used with all
>>> load methods. Further, the code to invoke these functions is
>>> duplicated between different load functions.
>>>
>>> To address this problem, this commit introduces a "spl_load" function.
>>> It aims to handle image detection and correct invocation of each of the
>>> parse/load functions. spl_simple_read is a wrapper around
>>> spl_load_info.read with get_aligned_image* functions inlined for size
>>> purposes. Additionally, we assume that bl_len is a power of 2 so we can
>>> do bitshifts instead of divisions (which is smaller and faster).
>>>
>>> Signed-off-by: Sean Anderson <sean.anderson at seco.com>
>>> ---
>>>
>>> common/spl/spl.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++
>>> include/spl.h | 30 +++++++++++++++++++++++-
>>> 2 files changed, 90 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/common/spl/spl.c b/common/spl/spl.c
>>> index b452d4feeb..f26df7ac3f 100644
>>> --- a/common/spl/spl.c
>>> +++ b/common/spl/spl.c
>>> @@ -398,6 +398,67 @@ int spl_parse_image_header(struct spl_image_info *spl_image,
>>> return 0;
>>> }
>>> +static int spl_simple_read(struct spl_load_info *info, void *buf, size_t size,
>>> + size_t offset)
>>> +{
>>> + int ret;
>>> + size_t bl_len = info->filename ? ARCH_DMA_MINALIGN : bl_len;
>>> + size_t bl_mask = bl_len - 1;
>>> + size_t bl_shift = ffs(bl_mask);
>>> + size_t overhead = offset & bl_mask;
>>> +
>>
>> Nitpicking comment:
>>
>> It's preferred in general to use the reverse XMAS tree ordering of the
>> declared variables. So I would expect at least to have "int ret" as
>> last statement above. If you address this, then please in the complete
>> series.
>
> I thought only Linux's net subsystem had this requirement. I can reorder
> things for you if you'd like. However, some of these variables have
> dependencies so they cannot all be reordered :)
Thanks. Perhaps it's only my personal preference. I find it more
structured this way. Still I think that I remember having seen this
request of reverse XMAS tree ordering here on the U-Boot list as well.
Nevertheless this is no blocker whatsoever. Please make such changes
only if a v2 of the patches is required.
Thanks,
Stefan
> --Sean
>
>> Reviewed-by: Stefan Roese <sr at denx.de>
>>
>> Thanks,
>> Stefan
>>
>>> + buf -= overhead;
>>> + size = (size + overhead + bl_mask) >> bl_shift;
>>> + offset = offset >> bl_shift;
>>> +
>>> + ret = info->read(info, offset, size, buf);
>>> + return ret == size ? 0 : -EIO;
>>> +}
>>> +
>>> +int spl_load(struct spl_image_info *spl_image,
>>> + const struct spl_boot_device *bootdev, struct spl_load_info *info,
>>> + struct image_header *header, size_t size, size_t sector)
>>> +{
>>> + int ret;
>>> + size_t offset = sector * info->bl_len;
>>> +
>>> + if (image_get_magic(header) == FDT_MAGIC) {
>>> + if (IS_ENABLED(CONFIG_SPL_LOAD_FIT_FULL)) {
>>> + void *buf;
>>> +
>>> + /*
>>> + * In order to support verifying images in the FIT, we
>>> + * need to load the whole FIT into memory. Try and
>>> + * guess how much we need to load by using the total
>>> + * size. This will fail for FITs with external data,
>>> + * but there's not much we can do about that.
>>> + */
>>> + if (!size)
>>> + size = roundup(fdt_totalsize(header), 4);
>>> + buf = spl_get_load_buffer(0, size);
>>> + ret = spl_simple_read(info, buf, size, offset);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + return spl_parse_image_header(spl_image, bootdev, buf);
>>> + }
>>> +
>>> + if (IS_ENABLED(CONFIG_SPL_LOAD_FIT))
>>> + return spl_load_simple_fit(spl_image, info, sector,
>>> + header);
>>> + }
>>> +
>>> + if (IS_ENABLED(CONFIG_SPL_LOAD_IMX_CONTAINER))
>>> + return spl_load_imx_container(spl_image, info, sector);
>>> +
>>> + ret = spl_parse_image_header(spl_image, bootdev, header);
>>> + if (ret)
>>> + return ret;
>>> +
>>> + return spl_simple_read(info, (void *)spl_image->load_addr,
>>> + spl_image->size, offset + spl_image->offset);
>>> +}
>>> +
>>> __weak void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)
>>> {
>>> typedef void __noreturn (*image_entry_noargs_t)(void);
>>> diff --git a/include/spl.h b/include/spl.h
>>> index 8ceb3c0f09..6606f4e5f6 100644
>>> --- a/include/spl.h
>>> +++ b/include/spl.h
>>> @@ -236,7 +236,7 @@ struct spl_image_info {
>>> *
>>> * @dev: Pointer to the device, e.g. struct mmc *
>>> * @priv: Private data for the device
>>> - * @bl_len: Block length for reading in bytes
>>> + * @bl_len: Block length for reading in bytes; must be a power of 2
>>> * @filename: Name of the fit image file.
>>> * @read: Function to call to read from the device
>>> */
>>> @@ -608,6 +608,34 @@ int spl_load_image_ext_os(struct spl_image_info *spl_image,
>>> struct spl_boot_device *bootdev,
>>> struct blk_desc *block_dev, int partition);
>>> +/**
>>> + * spl_load() - Parse a header and load the image
>>> + * @spl_image: Image data which will be filled in by this function
>>> + * @bootdev: The device to load from
>>> + * @info: Describes how to load additional information from @bootdev. At the
>>> + * minimum, read() and bl_len must be populated.
>>> + * @header: The image header. This should already have been loaded. It may be
>>> + * clobbered by the load process (if e.g. the load address overlaps).
>>> + * @size: The size of the image, if it is known in advance. Some boot devices
>>> + * (such as filesystems) know how big an image is before parsing the
>>> + * header. If this information is unknown, then the size will be
>>> + * determined from the header.
>>> + * @sectors: The offset from the start if @bootdev, in units of @info->bl_len.
>>> + * This should have the offset @header was loaded from. It will be
>>> + * added to any offsets passed to @info->read().
>>> + *
>>> + * This function determines the image type (FIT, legacy, i.MX, raw, etc), calls
>>> + * the appropriate parsing function, determines the load address, and the loads
>>> + * the image from storage. It is designed to replace ad-hoc image loading which
>>> + * may not support all image types (especially when config options are
>>> + * involved).
>>> + *
>>> + * Return: 0 on success, or a negative error on failure
>>> + */
>>> +int spl_load(struct spl_image_info *spl_image,
>>> + const struct spl_boot_device *bootdev, struct spl_load_info *info,
>>> + struct image_header *header, size_t size, size_t sector);
>>> +
>>> /**
>>> * spl_early_init() - Set up device tree and driver model in SPL if enabled
>>> *
>>
>> Viele Grüße,
>> Stefan Roese
>>
Viele Grüße,
Stefan Roese
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de
More information about the U-Boot
mailing list