| Message ID | 20190403213504.23681-1-erosca@de.adit-jv.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [U-Boot,1/2] image: android: allow booting lz4-compressed kernels | expand |
On 4/3/19 11:35 PM, Eugeniu Rosca wrote: > According to Android image format [1], kernel image resides at 1 page > offset from the boot image address. Grab the magic number from there > and allow U-Boot to handle LZ4-compressed KNL binaries instead of > hardcoding compression type to IH_COMP_NONE. Other compression types, > if needed, can be added later. > > Tested on H3ULCB-KF using the image detailed in [2]. > > [1] Excerpt from include/android_image.h > +-----------------+ > | boot header | 1 page > +-----------------+ > | kernel | n pages > +-----------------+ > | ramdisk | m pages > +-----------------+ > | second stage | o pages > +-----------------+ > > [2] => iminfo 4c000000 > ## Checking Image at 4c000000 ... > Android image found > kernel size: 85b9d1 > kernel address: 48080000 > ramdisk size: 54ddbc > ramdisk addrress: 4a180000 > second size: 0 > second address: 48000800 > tags address: 48000100 > page size: 800 > os_version: 1200012a (ver: 0.9.0, level: 2018.10) > name: > cmdline: buildvariant=userdebug > > Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> > --- > common/bootm.c | 2 +- > common/image-android.c | 14 ++++++++++++++ > include/image.h | 1 + > 3 files changed, 16 insertions(+), 1 deletion(-) > > diff --git a/common/bootm.c b/common/bootm.c > index 3adbceaa38e3..bbae66df1001 100644 > --- a/common/bootm.c > +++ b/common/bootm.c > @@ -154,7 +154,7 @@ static int bootm_find_os(cmd_tbl_t *cmdtp, int flag, int argc, > #ifdef CONFIG_ANDROID_BOOT_IMAGE > case IMAGE_FORMAT_ANDROID: > images.os.type = IH_TYPE_KERNEL; > - images.os.comp = IH_COMP_NONE; > + images.os.comp = android_image_get_kcomp(os_hdr); > images.os.os = IH_OS_LINUX; > > images.os.end = android_image_get_end(os_hdr); > diff --git a/common/image-android.c b/common/image-android.c > index 2f38c191e911..eec8c4decdbd 100644 > --- a/common/image-android.c > +++ b/common/image-android.c > @@ -126,6 +126,20 @@ ulong android_image_get_kload(const struct andr_img_hdr *hdr) > return android_image_get_kernel_addr(hdr); > } > > +#define LZ4F_MAGIC 0x184D2204 Don't we already have this magic in some common header ? > +ulong android_image_get_kcomp(const struct andr_img_hdr *hdr) > +{ > + u32 *magic = (u32 *)((ulong)hdr + hdr->page_size); Should this be get_unaligned((uintptr_t)hdr + hdr->page_size) ? get_unaligned() because the image may be at unaligned address (although that's unlikely) and uintptr_t to cater for both 32 and 64bit pointers. > + switch (le32_to_cpu(*magic)) { > + case LZ4F_MAGIC: > + return IH_COMP_LZ4; > + default: > + return IH_COMP_NONE; > + } > +} > + > int android_image_get_ramdisk(const struct andr_img_hdr *hdr, > ulong *rd_data, ulong *rd_len) > { > diff --git a/include/image.h b/include/image.h > index 765ffecee0a7..572034afd8c3 100644 > --- a/include/image.h > +++ b/include/image.h > @@ -1312,6 +1312,7 @@ int android_image_get_second(const struct andr_img_hdr *hdr, > ulong *second_data, ulong *second_len); > ulong android_image_get_end(const struct andr_img_hdr *hdr); > ulong android_image_get_kload(const struct andr_img_hdr *hdr); > +ulong android_image_get_kcomp(const struct andr_img_hdr *hdr); > void android_print_contents(const struct andr_img_hdr *hdr); > > #endif /* CONFIG_ANDROID_BOOT_IMAGE */ >
Hi Marek and thanks for your swift comment, On Thu, Apr 04, 2019 at 03:39:30AM +0200, Marek Vasut wrote: > On 4/3/19 11:35 PM, Eugeniu Rosca wrote: > > According to Android image format [1], kernel image resides at 1 page > > offset from the boot image address. Grab the magic number from there > > and allow U-Boot to handle LZ4-compressed KNL binaries instead of > > hardcoding compression type to IH_COMP_NONE. Other compression types, > > if needed, can be added later. > > [..] > > > > +#define LZ4F_MAGIC 0x184D2204 > > Don't we already have this magic in some common header ? Unfortunately not. It is present in lib/lz4_wrapper.c only. Would it be OK to relocate it to include/image.h? > > +ulong android_image_get_kcomp(const struct andr_img_hdr *hdr) > > +{ > > + u32 *magic = (u32 *)((ulong)hdr + hdr->page_size); > > Should this be get_unaligned((uintptr_t)hdr + hdr->page_size) ? > get_unaligned() because the image may be at unaligned address (although > that's unlikely) [..] Just out of curiosity I've copied the Android image to 0x4c000001 instead of 0x4c000000 in RAM and by calling 'bootm 0x4c000001', the compression type is still correctly identified and OS boots properly (w/o get_unaligned). But that's because the data cache is enabled. Booting from 0x4c000001 after calling `dcache off` no longer works and generates a "Synchronous Abort". Actually having dcache enabled is a requirement for LZ4, since it heavily relies on unaligned memory access and produces the same data abort in case dcache is turned off (even if it is passed the image at a properly aligned location in RAM). So, bottom line, even if we use get_unaligned() here, the LZ4 kernel still won't boot with data cache disabled. Anyway, I agree to use an alignment-aware primitive here, as you suggested. > and uintptr_t to cater for both 32 and 64bit pointers. Worked for me. [..] > > @@ -1312,6 +1312,7 @@ int android_image_get_second(const struct andr_img_hdr *hdr, > > ulong *second_data, ulong *second_len); > > ulong android_image_get_end(const struct andr_img_hdr *hdr); > > ulong android_image_get_kload(const struct andr_img_hdr *hdr); > > +ulong android_image_get_kcomp(const struct andr_img_hdr *hdr); Would you like ulong/int here? > > void android_print_contents(const struct andr_img_hdr *hdr); > > > > #endif /* CONFIG_ANDROID_BOOT_IMAGE */ > > > > > -- > Best regards, > Marek Vasut Best regards, Eugeniu.
On 4/4/19 1:14 PM, Eugeniu Rosca wrote: > Hi Marek and thanks for your swift comment, > > On Thu, Apr 04, 2019 at 03:39:30AM +0200, Marek Vasut wrote: >> On 4/3/19 11:35 PM, Eugeniu Rosca wrote: >>> According to Android image format [1], kernel image resides at 1 page >>> offset from the boot image address. Grab the magic number from there >>> and allow U-Boot to handle LZ4-compressed KNL binaries instead of >>> hardcoding compression type to IH_COMP_NONE. Other compression types, >>> if needed, can be added later. >>> > > [..] > >>> >>> +#define LZ4F_MAGIC 0x184D2204 >> >> Don't we already have this magic in some common header ? > > Unfortunately not. It is present in lib/lz4_wrapper.c only. > Would it be OK to relocate it to include/image.h? Yes >>> +ulong android_image_get_kcomp(const struct andr_img_hdr *hdr) >>> +{ >>> + u32 *magic = (u32 *)((ulong)hdr + hdr->page_size); >> >> Should this be get_unaligned((uintptr_t)hdr + hdr->page_size) ? >> get_unaligned() because the image may be at unaligned address (although >> that's unlikely) [..] > > Just out of curiosity I've copied the Android image to 0x4c000001 > instead of 0x4c000000 in RAM and by calling 'bootm 0x4c000001', the > compression type is still correctly identified and OS boots properly > (w/o get_unaligned). It will work on arm64, other platforms might fail. > But that's because the data cache is enabled. Booting from 0x4c000001 > after calling `dcache off` no longer works and generates a > "Synchronous Abort". > > Actually having dcache enabled is a requirement for LZ4, since it > heavily relies on unaligned memory access and produces the same data > abort in case dcache is turned off (even if it is passed the image at > a properly aligned location in RAM). > > So, bottom line, even if we use get_unaligned() here, the LZ4 kernel > still won't boot with data cache disabled. Anyway, I agree to use an > alignment-aware primitive here, as you suggested. > >> and uintptr_t to cater for both 32 and 64bit pointers. > > Worked for me. > > [..] > >>> @@ -1312,6 +1312,7 @@ int android_image_get_second(const struct andr_img_hdr *hdr, >>> ulong *second_data, ulong *second_len); >>> ulong android_image_get_end(const struct andr_img_hdr *hdr); >>> ulong android_image_get_kload(const struct andr_img_hdr *hdr); >>> +ulong android_image_get_kcomp(const struct andr_img_hdr *hdr); > > Would you like ulong/int here? ulong to keep it consistent ?
Superseded by https://patchwork.ozlabs.org/patch/1081176/
("[U-Boot,v2,1/2] image: android: allow booting lz4-compressed kernels")
diff --git a/common/bootm.c b/common/bootm.c index 3adbceaa38e3..bbae66df1001 100644 --- a/common/bootm.c +++ b/common/bootm.c @@ -154,7 +154,7 @@ static int bootm_find_os(cmd_tbl_t *cmdtp, int flag, int argc, #ifdef CONFIG_ANDROID_BOOT_IMAGE case IMAGE_FORMAT_ANDROID: images.os.type = IH_TYPE_KERNEL; - images.os.comp = IH_COMP_NONE; + images.os.comp = android_image_get_kcomp(os_hdr); images.os.os = IH_OS_LINUX; images.os.end = android_image_get_end(os_hdr); diff --git a/common/image-android.c b/common/image-android.c index 2f38c191e911..eec8c4decdbd 100644 --- a/common/image-android.c +++ b/common/image-android.c @@ -126,6 +126,20 @@ ulong android_image_get_kload(const struct andr_img_hdr *hdr) return android_image_get_kernel_addr(hdr); } +#define LZ4F_MAGIC 0x184D2204 + +ulong android_image_get_kcomp(const struct andr_img_hdr *hdr) +{ + u32 *magic = (u32 *)((ulong)hdr + hdr->page_size); + + switch (le32_to_cpu(*magic)) { + case LZ4F_MAGIC: + return IH_COMP_LZ4; + default: + return IH_COMP_NONE; + } +} + int android_image_get_ramdisk(const struct andr_img_hdr *hdr, ulong *rd_data, ulong *rd_len) { diff --git a/include/image.h b/include/image.h index 765ffecee0a7..572034afd8c3 100644 --- a/include/image.h +++ b/include/image.h @@ -1312,6 +1312,7 @@ int android_image_get_second(const struct andr_img_hdr *hdr, ulong *second_data, ulong *second_len); ulong android_image_get_end(const struct andr_img_hdr *hdr); ulong android_image_get_kload(const struct andr_img_hdr *hdr); +ulong android_image_get_kcomp(const struct andr_img_hdr *hdr); void android_print_contents(const struct andr_img_hdr *hdr); #endif /* CONFIG_ANDROID_BOOT_IMAGE */
According to Android image format [1], kernel image resides at 1 page offset from the boot image address. Grab the magic number from there and allow U-Boot to handle LZ4-compressed KNL binaries instead of hardcoding compression type to IH_COMP_NONE. Other compression types, if needed, can be added later. Tested on H3ULCB-KF using the image detailed in [2]. [1] Excerpt from include/android_image.h +-----------------+ | boot header | 1 page +-----------------+ | kernel | n pages +-----------------+ | ramdisk | m pages +-----------------+ | second stage | o pages +-----------------+ [2] => iminfo 4c000000 ## Checking Image at 4c000000 ... Android image found kernel size: 85b9d1 kernel address: 48080000 ramdisk size: 54ddbc ramdisk addrress: 4a180000 second size: 0 second address: 48000800 tags address: 48000100 page size: 800 os_version: 1200012a (ver: 0.9.0, level: 2018.10) name: cmdline: buildvariant=userdebug Signed-off-by: Eugeniu Rosca <erosca@de.adit-jv.com> --- common/bootm.c | 2 +- common/image-android.c | 14 ++++++++++++++ include/image.h | 1 + 3 files changed, 16 insertions(+), 1 deletion(-)