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 2BCFFC05027 for ; Wed, 1 Feb 2023 08:44:39 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 630B885C30; Wed, 1 Feb 2023 09:44:37 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=baylibre.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=baylibre-com.20210112.gappssmtp.com header.i=@baylibre-com.20210112.gappssmtp.com header.b="QBEQDYbW"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id B20F985C33; Wed, 1 Feb 2023 09:44:35 +0100 (CET) Received: from mail-wm1-x32b.google.com (mail-wm1-x32b.google.com [IPv6:2a00:1450:4864:20::32b]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 8378085C1B for ; Wed, 1 Feb 2023 09:44:32 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=baylibre.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=mkorpershoek@baylibre.com Received: by mail-wm1-x32b.google.com with SMTP id o36so6036992wms.1 for ; Wed, 01 Feb 2023 00:44:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre-com.20210112.gappssmtp.com; s=20210112; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:from:to:cc:subject:date:message-id:reply-to; bh=uLAbtE2ZFvQJGY+kajAGTw9T8qC4NWAxYBAqz9l3QtU=; b=QBEQDYbWwIpUiOKdCSQSa+RnDO+eM7BALIHZOR3Jl1hnJtLxOgNlE8q/dOhXYGw9hZ KJaHm8OiP1rwHJNuhiTW6yLoPY+YqGGk30UiBp/A0i37zPoYvi6YdQeOFifVudGKkmKU kPQrsu9VOKWnJ0NtOU9cPvO9e+00+KemaGJ+UoivFQIN2VEh9rXl6q8QJTBMQlfyOMzx 6yMgYW1L8CgvGMbTCOOo2rgjTBodtiiM4nvhoFPq0JO4VLe5/9RCew1d/XphDcwL44o0 anLvfb6y1THyVTOTriB/Da3weby5P2loAvsaJzpJOTURr3FLeDFOjDSqZWcCJVnv7mfa ddHQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:message-id:date:references:in-reply-to:subject:cc:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=uLAbtE2ZFvQJGY+kajAGTw9T8qC4NWAxYBAqz9l3QtU=; b=SkIFb2AHFzWdCU3Ga0MMLT23StQPVTOr04cBT1fMLc5jVlVZ4jEKwGPguyX+OhhaXz t1F/8ght5nFfqanTG1c06tbDu754Hvsa+kMXrzDQ1QfxVz2HMTsxq1aWe3FXK/Rm0sg9 on3CaJmOJ4hTYgY4FQBcB68UAndmbaC3EEcGdXK1ocZKyRPPVKCnF+6BRtwMubnCGbkV 67Kd5D1EPIZrqxS/K89Ij7kw70iPSpYQbXChbfRVuKF0z3fFB2Ye9k+v2YxxlfMzLQwS v3aKZwIYESUrbFS6evC1rFzWtWTC1l8qTxRyYFDKxgfipsAGaFJp+be/bChq63liqzjk AwSw== X-Gm-Message-State: AO0yUKWrKx88kFo4bS2r+4Bak8chTTP4Dwy0wjQDao5wGOkfde5vbZqe 8wFc8evC7jywl5v+rSv2Ryop7Q== X-Google-Smtp-Source: AK7set+ngEatW/DnrnhjeCmaTYpACgzC0DDqFjs2VwMNgSyhTGDcngK8os6eErHUeshsdR+VC9PZWw== X-Received: by 2002:a05:600c:4f07:b0:3da:fd06:a6f1 with SMTP id l7-20020a05600c4f0700b003dafd06a6f1mr1148189wmq.31.1675241072000; Wed, 01 Feb 2023 00:44:32 -0800 (PST) Received: from localhost ([82.66.159.240]) by smtp.gmail.com with ESMTPSA id m13-20020a05600c3b0d00b003dc51c48f0bsm1138708wms.19.2023.02.01.00.44.29 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 01 Feb 2023 00:44:30 -0800 (PST) From: Mattijs Korpershoek To: Simon Glass , Safae Ouajih Cc: u-boot@lists.denx.de, sean.anderson@seco.com, r.stratiienko@gmail.com, glaroque@baylibre.com, khilman@baylibre.com Subject: Re: [PATCH v2 15/17] android: boot: support boot image header version 3 and 4 In-Reply-To: References: <20230126160506.1026643-1-souajih@baylibre.com> <20230126160506.1026643-16-souajih@baylibre.com> Date: Wed, 01 Feb 2023 09:44:26 +0100 Message-ID: <87h6w5ixlh.fsf@baylibre.com> MIME-Version: 1.0 Content-Type: text/plain 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.6 at phobos.denx.de X-Virus-Status: Clean On Thu, Jan 26, 2023 at 17:55, Simon Glass wrote: > Hi Safae, > > On Thu, 26 Jan 2023 at 09:05, Safae Ouajih wrote: >> >> This enables the support for boot image header version 3 and 4 >> using abootimg command. >> >> In order to use version 3 or 4: >> >> 1- Vendor boot image address should be given to abootimg cmd. >> >> abootimg addr $1 $vendor_boot_load_addr >> >> 2- "ramdisk_addr_r" env variable (ramdisk address) should be set to host >> the ramdisk : generic ramdisk + vendor ramdisk >> >> "struct andr_boot_img_hdr_v0*" is replaced by "void *" in >> some functions since v3 and v4 are now supported as well. >> >> Signed-off-by: Safae Ouajih >> --- >> boot/bootm.c | 29 ++++++++++++++++++++++++----- >> boot/image-android.c | 16 ++++++++++------ >> boot/image-board.c | 14 +++++++++++--- >> boot/image-fdt.c | 2 +- >> cmd/abootimg.c | 24 ++++++++++++++++++++++-- >> include/image.h | 14 ++++++++------ >> 6 files changed, 76 insertions(+), 23 deletions(-) Since this introduces the new formats (v3 and v4), can we also update the doc/android/boot-image.rst files which lists all the boot formats for android? >> >> diff --git a/boot/bootm.c b/boot/bootm.c >> index a58e6f391e..3e130c175c 100644 >> --- a/boot/bootm.c >> +++ b/boot/bootm.c >> @@ -113,6 +113,10 @@ static int bootm_find_os(struct cmd_tbl *cmdtp, int flag, int argc, >> char *const argv[]) >> { >> const void *os_hdr; >> +#ifdef CONFIG_ANDROID_BOOT_IMAGE >> + const void *vendor_boot_img; >> + const void *boot_img; >> +#endif >> bool ep_found = false; >> int ret; >> >> @@ -181,12 +185,17 @@ static int bootm_find_os(struct cmd_tbl *cmdtp, int flag, int argc, >> #endif >> #ifdef CONFIG_ANDROID_BOOT_IMAGE >> case IMAGE_FORMAT_ANDROID: >> + boot_img = os_hdr; >> + vendor_boot_img = NULL; >> + if (IS_ENABLED(CONFIG_CMD_ABOOTIMG)) { >> + boot_img = (void *)get_abootimg_addr(); >> + vendor_boot_img = (void *)get_avendor_bootimg_addr(); > > map_sysmem() so it owrks on sandbox > >> + } >> images.os.type = IH_TYPE_KERNEL; >> - images.os.comp = android_image_get_kcomp(os_hdr, NULL); >> + images.os.comp = android_image_get_kcomp(boot_img, vendor_boot_img); >> images.os.os = IH_OS_LINUX; >> - >> - images.os.end = android_image_get_end(os_hdr, NULL); >> - images.os.load = android_image_get_kload(os_hdr, NULL); >> + images.os.end = android_image_get_end(boot_img, vendor_boot_img); >> + images.os.load = android_image_get_kload(boot_img, vendor_boot_img); >> images.ep = images.os.load; >> ep_found = true; >> break; >> @@ -889,6 +898,10 @@ static const void *boot_get_kernel(struct cmd_tbl *cmdtp, int flag, int argc, >> int os_noffset; >> #endif >> >> +#ifdef CONFIG_ANDROID_BOOT_IMAGE >> + const void *boot_img; >> + const void *vendor_boot_img; >> +#endif >> img_addr = genimg_get_kernel_addr_fit(argc < 1 ? NULL : argv[0], >> &fit_uname_config, >> &fit_uname_kernel); >> @@ -964,8 +977,14 @@ static const void *boot_get_kernel(struct cmd_tbl *cmdtp, int flag, int argc, >> #endif >> #ifdef CONFIG_ANDROID_BOOT_IMAGE >> case IMAGE_FORMAT_ANDROID: >> + boot_img = buf; >> + vendor_boot_img = NULL; >> + if (IS_ENABLED(CONFIG_CMD_ABOOTIMG)) { >> + boot_img = (void *)get_abootimg_addr(); >> + vendor_boot_img = (void *)get_avendor_bootimg_addr(); > > and here > >> + } >> printf("## Booting Android Image at 0x%08lx ...\n", img_addr); >> - if (android_image_get_kernel(buf, NULL, images->verify, >> + if (android_image_get_kernel(boot_img, vendor_boot_img, images->verify, >> os_data, os_len)) >> return NULL; >> break; >> diff --git a/boot/image-android.c b/boot/image-android.c >> index edeeaaaee0..dd06c09279 100644 >> --- a/boot/image-android.c >> +++ b/boot/image-android.c >> @@ -201,7 +201,7 @@ static ulong android_image_get_kernel_addr(struct andr_image_data *img_data) >> * Return: Zero, os start address and length on success, >> * otherwise on failure. >> */ >> -int android_image_get_kernel(const struct andr_boot_img_hdr_v0 *hdr, >> +int android_image_get_kernel(const void *hdr, >> const void *vendor_boot_img, int verify, >> ulong *os_data, ulong *os_len) >> { >> @@ -286,7 +286,7 @@ bool is_android_vendor_boot_image_header(const void *vendor_boot_img) >> return !memcmp(VENDOR_BOOT_MAGIC, vendor_boot_img, ANDR_VENDOR_BOOT_MAGIC_SIZE); >> } >> >> -bool is_android_boot_image_header(const struct andr_boot_img_hdr_v0 *hdr) >> +bool is_android_boot_image_header(const void *hdr) >> { >> return !memcmp(ANDR_BOOT_MAGIC, hdr, ANDR_BOOT_MAGIC_SIZE); >> } >> @@ -305,7 +305,7 @@ ulong android_image_get_end(const struct andr_boot_img_hdr_v0 *hdr, >> return img_data.boot_img_total_size; >> } >> >> -ulong android_image_get_kload(const struct andr_boot_img_hdr_v0 *hdr, >> +ulong android_image_get_kload(const void *hdr, >> const void *vendor_boot_img) >> { >> struct andr_image_data img_data; >> @@ -316,7 +316,7 @@ ulong android_image_get_kload(const struct andr_boot_img_hdr_v0 *hdr, >> return android_image_get_kernel_addr(&img_data); >> } >> >> -ulong android_image_get_kcomp(const struct andr_boot_img_hdr_v0 *hdr, >> +ulong android_image_get_kcomp(const void *hdr, >> const void *vendor_boot_img) >> { >> struct andr_image_data img_data; >> @@ -364,14 +364,18 @@ int android_image_get_ramdisk(const void *hdr, const void *vendor_boot_img, >> return 0; >> } >> >> -int android_image_get_second(const struct andr_boot_img_hdr_v0 *hdr, >> - ulong *second_data, ulong *second_len) >> +int android_image_get_second(const void *hdr, ulong *second_data, ulong *second_len) >> { >> struct andr_image_data img_data; >> >> if (!android_image_get_data(hdr, NULL, &img_data)) >> return -EINVAL; >> >> + if (img_data.header_version > 2) { >> + printf("Second stage bootloader is only supported for boot image version <= 2\n"); >> + return -1; > > Use proper error number. This is -EPERM which seems wrong. Perhaps -ENOTSUPP ? > >> + } >> + >> if (!img_data.second_size) { >> *second_data = *second_len = 0; >> return -1; >> diff --git a/boot/image-board.c b/boot/image-board.c >> index b6c9bbe8f5..a20b57900e 100644 >> --- a/boot/image-board.c >> +++ b/boot/image-board.c >> @@ -426,10 +426,18 @@ static int select_ramdisk(struct bootm_headers *images, const char *select, u8 a >> break; >> case IMAGE_FORMAT_ANDROID: >> if (IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE)) { >> - void *ptr = map_sysmem(images->os.start, 0); >> int ret; >> - ret = android_image_get_ramdisk(ptr, NULL, rd_datap, rd_lenp); >> - unmap_sysmem(ptr); >> + if (IS_ENABLED(CONFIG_CMD_ABOOTIMG)) { >> + ret = android_image_get_ramdisk((void *)get_abootimg_addr(), > > map_sysmem() again. Or perhaps update that function to take an address? > >> + (void *)get_avendor_bootimg_addr(), >> + rd_datap, rd_lenp); >> + } else { >> + void *ptr = map_sysmem(images->os.start, 0); >> + >> + ret = android_image_get_ramdisk(ptr, NULL, rd_datap, rd_lenp); >> + unmap_sysmem(ptr); >> + } >> + >> if (ret) >> return ret; >> done = true; >> diff --git a/boot/image-fdt.c b/boot/image-fdt.c >> index f270d00a5f..5f9cf7b4e6 100644 >> --- a/boot/image-fdt.c >> +++ b/boot/image-fdt.c >> @@ -529,7 +529,7 @@ int boot_get_fdt(int flag, int argc, char *const argv[], uint8_t arch, >> } >> #ifdef CONFIG_ANDROID_BOOT_IMAGE >> } else if (genimg_get_format(buf) == IMAGE_FORMAT_ANDROID) { >> - struct andr_boot_img_hdr_v0 *hdr = buf; >> + void *hdr = buf; >> ulong fdt_data, fdt_len; >> u32 fdt_size, dtb_idx; >> /* >> diff --git a/cmd/abootimg.c b/cmd/abootimg.c >> index 4d6cf0fa3e..ef7130da0b 100644 >> --- a/cmd/abootimg.c >> +++ b/cmd/abootimg.c >> @@ -17,6 +17,16 @@ >> static ulong _abootimg_addr = -1; >> static ulong _avendor_bootimg_addr = -1; >> >> +ulong get_abootimg_addr(void) >> +{ >> + return (_abootimg_addr == -1 ? image_load_addr : _abootimg_addr); >> +} >> + >> +ulong get_avendor_bootimg_addr(void) >> +{ >> + return _avendor_bootimg_addr; >> +} >> + >> static int abootimg_get_ver(int argc, char *const argv[]) >> { >> const struct andr_boot_img_hdr_v0 *hdr; >> @@ -70,12 +80,21 @@ static int abootimg_get_dtb_load_addr(int argc, char *const argv[]) >> return CMD_RET_USAGE; >> struct andr_image_data img_data = {0}; >> const struct andr_boot_img_hdr_v0 *hdr; >> + const struct andr_vendor_img_hdr *vhdr; >> >> hdr = map_sysmem(abootimg_addr(), sizeof(*hdr)); >> - if (!android_image_get_data(hdr, NULL, &img_data)) { >> + if (get_avendor_bootimg_addr() != -1) >> + vhdr = map_sysmem(get_avendor_bootimg_addr(), sizeof(*vhdr)); >> + >> + if (!android_image_get_data(hdr, vhdr, &img_data)) { >> + if (get_avendor_bootimg_addr() != -1) >> + unmap_sysmem(vhdr); >> unmap_sysmem(hdr); >> return CMD_RET_FAILURE; >> } >> + >> + if (get_avendor_bootimg_addr() != -1) >> + unmap_sysmem(vhdr); >> unmap_sysmem(hdr); >> >> if (img_data.header_version < 2) { >> @@ -119,7 +138,8 @@ static int abootimg_get_dtb_by_index(int argc, char *const argv[]) >> return CMD_RET_FAILURE; >> } >> >> - if (!android_image_get_dtb_by_index(abootimg_addr(), 0, num, >> + if (!android_image_get_dtb_by_index(abootimg_addr(), >> + get_avendor_bootimg_addr(), num, >> &addr, &size)) { >> return CMD_RET_FAILURE; >> } >> diff --git a/include/image.h b/include/image.h >> index ac94ecaa14..5bd4bf5d57 100644 >> --- a/include/image.h >> +++ b/include/image.h >> @@ -1739,29 +1739,31 @@ bool android_image_get_data(const void *boot_hdr, const void *vendor_boot_hdr, >> struct andr_image_data *data); >> >> struct andr_boot_img_hdr_v0; >> -int android_image_get_kernel(const struct andr_boot_img_hdr_v0 *hdr, >> +int android_image_get_kernel(const void *hdr, >> const void *vendor_boot_img, int verify, >> ulong *os_data, ulong *os_len); >> int android_image_get_ramdisk(const void *hdr, const void *vendor_boot_img, >> ulong *rd_data, ulong *rd_len); >> -int android_image_get_second(const struct andr_boot_img_hdr_v0 *hdr, >> - ulong *second_data, ulong *second_len); >> +int android_image_get_second(const void *hdr, ulong *second_data, ulong *second_len); >> bool android_image_get_dtbo(ulong hdr_addr, ulong *addr, u32 *size); >> bool android_image_get_dtb_by_index(ulong hdr_addr, ulong vendor_boot_img, >> u32 index, ulong *addr, u32 *size); >> ulong android_image_get_end(const struct andr_boot_img_hdr_v0 *hdr, >> const void *vendor_boot_img); >> -ulong android_image_get_kload(const struct andr_boot_img_hdr_v0 *hdr, >> +ulong android_image_get_kload(const void *hdr, >> const void *vendor_boot_img); >> -ulong android_image_get_kcomp(const struct andr_boot_img_hdr_v0 *hdr, >> +ulong android_image_get_kcomp(const void *hdr, >> const void *vendor_boot_img); >> void android_print_contents(const struct andr_boot_img_hdr_v0 *hdr); >> #ifdef CONFIG_CMD_ABOOTIMG >> bool android_image_print_dtb_contents(ulong hdr_addr); >> #endif >> -bool is_android_boot_image_header(const struct andr_boot_img_hdr_v0 *hdr); >> +bool is_android_boot_image_header(const void *hdr); >> bool is_android_vendor_boot_image_header(const void *vendor_boot_img); >> >> +ulong get_abootimg_addr(void); >> +ulong get_avendor_bootimg_addr(void); > > again please add comments to all of these. > >> + >> /** >> * board_fit_config_name_match() - Check for a matching board name >> * >> -- >> 2.34.1 >> > > Regards, > Simon