[PATCH v4 1/9] spl: Add generic spl_load function

[Sean Anderson]

On 7/27/23 02:17, Heinrich Schuchardt wrote:
> On 7/24/23 19:12, 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>
>> Reviewed-by: Stefan Roese <sr at denx.de>
>> ---
>>
>> Changes in v4:
>> - Fix format specifiers in debug prints
>> - Reword/fix some of the doc comments for spl_load
>>
>> Changes in v3:
>> - Fix using ffs instead of fls
>> - Fix using not initializing bl_len when info->filename was NULL
>>
>> Changes in v2:
>> - Use reverse-xmas-tree style for locals in spl_simple_read. This is not
>>    complete, since overhead depends on bl_mask.
>>
>>   common/spl/spl.c | 68 ++++++++++++++++++++++++++++++++++++++++++++++++
>>   include/spl.h    | 29 ++++++++++++++++++++-
>>   2 files changed, 96 insertions(+), 1 deletion(-)
>>
>> diff --git a/common/spl/spl.c b/common/spl/spl.c
>> index f09bb977814..3ef064009e8 100644
>> --- a/common/spl/spl.c
>> +++ b/common/spl/spl.c
>> @@ -450,6 +450,74 @@ 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)
>> +{
>> +    size_t bl_len = info->filename ? ARCH_DMA_MINALIGN : info->bl_len;
>> +    size_t bl_mask = bl_len - 1;
>> +    size_t overhead = offset & bl_mask;
>> +    size_t bl_shift = fls(bl_mask);
>> +    int ret;
>> +
>> +    debug("%s: buf=%p size=%lx offset=%lx\n", __func__, buf, (long)size,
>> +          (long)offset);
>> +    debug("%s: bl_len=%lx bl_mask=%lx bl_shift=%lx\n", __func__, (long)bl_len,
>> +          (long)bl_mask, (long)bl_shift);
>> +
>> +    buf -= overhead;
>> +    size = (size + overhead + bl_mask) >> bl_shift;
>> +    offset = offset >> bl_shift;
>> +
>> +    debug("info->read(info, %lx, %lx, %p)\n", (ulong)offset, (ulong)size,
>> +          buf);
>> +    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 legacy_img_hdr *header, size_t size, size_t sector)
> 
> Hello Sean,
> 
> carving out common functionality is really a good path forward.
> 
> This function spl_load() receives a pointer to a read function in
> info->read() and additionally the file header.
> 
> Why can't we move the reading of the header to spl_load() too?

That looks promising. The only downside is that FAT grows a bit since we can't
determine the file size when we load the whole image.

--Sean

> 
>> +{
>> +    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 92bcaa90a4a..06b34e70a44 100644
>> --- a/include/spl.h
>> +++ b/include/spl.h
>> @@ -268,7 +268,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
>>    */
>> @@ -677,6 +677,33 @@ int spl_blk_load_image(struct spl_image_info *spl_image,
>>                  struct spl_boot_device *bootdev,
>>                  enum uclass_id uclass_id, int devnum, int partnum);
>>
>> +/**
>> + * 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, in bytes, if it is known in advance. Some boot
>> + *        devices (such as filesystems) know how big an image is before parsing
>> + *        the header. If 0, then the size will be determined from the header.
>> + * @sectors: The offset from the start of @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 legacy_img_hdr *header, size_t size, size_t sector);
>> +
>>   /**
>>    * spl_early_init() - Set up device tree and driver model in SPL if enabled
>>    *
>