From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 71BAAC433EF for ; Wed, 6 Apr 2022 05:30:43 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 9BC5C839BA; Wed, 6 Apr 2022 07:30:40 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=denx.de; s=phobos-20191101; t=1649223040; bh=t+NU0kr0uwrLT8Hfhn+tWjZjfrRwwvyoJJmOEiO5vm0=; h=Date:Subject:To:Cc:References:From:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From; b=e95AmsP5LEE0ESIx9XMQbAv2E1kVuCtmFMGWHjVl/3ZjWzbiWLOPF1gpXEf9brC4M 4uihVezm4h/8Y/py0e1B3R0+THzzCtf4IT8Nbi17213hmJytT33mZaOeJmswjriMsx B/7hM23FJca1hxhYDK08cRH/zbTClnijnKnjhG9OGTreCnTmbdMDLJr6XuKbGce9hg 1Odos2N3YQ8+O8Tx8LY3ZGqQuHCUzJWVtX7ihR2zlAs/1/xJVJpV/r+ZTIvf/mXM6M QINUQ1vFerDb4ZTlGxSKSj1bLukrwX+dtuPaR8FhAoRtn/deDLtDxzkBPc8adIBCXY xGOy5MZTIVA2Q== Received: by phobos.denx.de (Postfix, from userid 109) id 5D2778393F; Wed, 6 Apr 2022 07:30:38 +0200 (CEST) Received: from mout-u-107.mailbox.org (mout-u-107.mailbox.org [IPv6:2001:67c:2050:1::465:107]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 894D283C11 for ; Wed, 6 Apr 2022 07:30:34 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=denx.de Authentication-Results: phobos.denx.de; spf=fail smtp.mailfrom=sr@denx.de Received: from smtp2.mailbox.org (unknown [91.198.250.124]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange ECDHE (P-384) server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by mout-u-107.mailbox.org (Postfix) with ESMTPS id 4KYClY6mspz9sSm; Wed, 6 Apr 2022 07:30:33 +0200 (CEST) Message-ID: <0bc5fabd-1af1-05f4-f46b-d74747785b1c@denx.de> Date: Wed, 6 Apr 2022 07:30:28 +0200 MIME-Version: 1.0 Subject: Re: [RFC PATCH 1/7] spl: Add generic spl_load function Content-Language: en-US To: Sean Anderson , Simon Glass Cc: =?UTF-8?Q?Marek_Beh=c3=ban?= , u-boot@lists.denx.de, Marek Vasut , =?UTF-8?Q?Pali_Roh=c3=a1r?= References: <20220401190405.1932697-1-sean.anderson@seco.com> <20220401190405.1932697-2-sean.anderson@seco.com> From: Stefan Roese In-Reply-To: <20220401190405.1932697-2-sean.anderson@seco.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.5 at phobos.denx.de X-Virus-Status: Clean 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 > --- > > 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. Reviewed-by: Stefan Roese 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 -- 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@denx.de