On 01/02/2023 09:44, Mattijs Korpershoek wrote:
On Thu, Jan 26, 2023 at 17:55, Simon Glass <s...@chromium.org> wrote:

Hi Safae,

On Thu, 26 Jan 2023 at 09:05, Safae Ouajih <soua...@baylibre.com> 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 <soua...@baylibre.com>
---
  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?


Hi Mattijs,

Thank you for the review,

I will update the documentation in a v3.

Best Regards,

Safae

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

Reply via email to