| Message ID | 20190408153528.23783-1-erosca@de.adit-jv.com |
|---|---|
| State | Accepted |
| Commit | 829ceb28215a1b6178bf690182def5ad56842f49 |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [U-Boot,v2,1/2] image: android: allow booting lz4-compressed kernels | expand |
On 4/8/19 5: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]. H3ULCB Kingfisher is not supported in mainline ;-) [...] > diff --git a/include/image.h b/include/image.h > index 765ffecee0a7..889305cbefdb 100644 > --- a/include/image.h > +++ b/include/image.h > @@ -306,6 +306,7 @@ enum { > IH_COMP_COUNT, > }; > > +#define LZ4F_MAGIC 0x184D2204 /* LZ4 Magic Number */ > #define IH_MAGIC 0x27051956 /* Image Magic Number */ > #define IH_NMLEN 32 /* Image Name Length */ > Keep the list sorted please. [...]
On Tue, Apr 09, 2019 at 01:06:31PM +0200, Marek Vasut wrote: > On 4/8/19 5: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]. > > H3ULCB Kingfisher is not supported in mainline ;-) Well, my H3 ES2.0 ULCB attached to Kingfisher-M06 boots just fine with the recently released v2019.04 U-Boot. I guess U-Boot simply doesn't (need to) care about the fancy peripherals present on the KF extension board (hence no need to spawn another KF defconfig)? > > [...] > > > diff --git a/include/image.h b/include/image.h > > index 765ffecee0a7..889305cbefdb 100644 > > --- a/include/image.h > > +++ b/include/image.h > > @@ -306,6 +306,7 @@ enum { > > IH_COMP_COUNT, > > }; > > > > +#define LZ4F_MAGIC 0x184D2204 /* LZ4 Magic Number */ > > #define IH_MAGIC 0x27051956 /* Image Magic Number */ > > #define IH_NMLEN 32 /* Image Name Length */ > > > > Keep the list sorted please. I hope below looks better? #define IH_MAGIC 0x27051956 /* Image Magic Number */ #define IH_NMLEN 32 /* Image Name Length */ +#define LZ4F_MAGIC 0x184D2204 /* LZ4 Magic Number */ Please, share any other comments/wishes before I push v3 in the next couple of days. TIA! > > [...] > > > -- > Best regards, > Marek Vasut Best regards, Eugeniu.
On 4/9/19 2:29 PM, Eugeniu Rosca wrote: > On Tue, Apr 09, 2019 at 01:06:31PM +0200, Marek Vasut wrote: >> On 4/8/19 5: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]. >> >> H3ULCB Kingfisher is not supported in mainline ;-) > > Well, my H3 ES2.0 ULCB attached to Kingfisher-M06 boots just fine with > the recently released v2019.04 U-Boot. I guess U-Boot simply doesn't > (need to) care about the fancy peripherals present on the KF extension > board (hence no need to spawn another KF defconfig)? Glad to hear it boots well. Is there anything interesting on the KF that'd be worth supporting ? I think it could be handled via DTOs. >> [...] >> >>> diff --git a/include/image.h b/include/image.h >>> index 765ffecee0a7..889305cbefdb 100644 >>> --- a/include/image.h >>> +++ b/include/image.h >>> @@ -306,6 +306,7 @@ enum { >>> IH_COMP_COUNT, >>> }; >>> >>> +#define LZ4F_MAGIC 0x184D2204 /* LZ4 Magic Number */ >>> #define IH_MAGIC 0x27051956 /* Image Magic Number */ >>> #define IH_NMLEN 32 /* Image Name Length */ >>> >> >> Keep the list sorted please. > > I hope below looks better? > > #define IH_MAGIC 0x27051956 /* Image Magic Number */ > #define IH_NMLEN 32 /* Image Name Length */ > +#define LZ4F_MAGIC 0x184D2204 /* LZ4 Magic Number */ > > Please, share any other comments/wishes before I push v3 in the > next couple of days. TIA! I think that's all from my side.
On Tue, Apr 09, 2019 at 02:38:08PM +0200, Marek Vasut wrote: > On 4/9/19 2:29 PM, Eugeniu Rosca wrote: > > On Tue, Apr 09, 2019 at 01:06:31PM +0200, Marek Vasut wrote: > >> On 4/8/19 5: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]. > >> > >> H3ULCB Kingfisher is not supported in mainline ;-) > > > > Well, my H3 ES2.0 ULCB attached to Kingfisher-M06 boots just fine with > > the recently released v2019.04 U-Boot. I guess U-Boot simply doesn't > > (need to) care about the fancy peripherals present on the KF extension > > board (hence no need to spawn another KF defconfig)? > > Glad to hear it boots well. Is there anything interesting on the KF > that'd be worth supporting ? I think it could be handled via DTOs. My vote goes for having a mainline-grade USB gadget/peripheral support needed by fastboot :) simply because we currently rely on a "20 files changed, 6294 insertions(+)" driver from Renesas which didn't undergo any deep review and is unlikely ready for upstreaming. FWIW my other wishes for R-Car3 are: - Add fastboot support, which includes choosing a proper DRAM address/size for the fastboot buffer (currently we make it 16 MiB and place it at 0x4A000000) - Add Android boot image support - Define a flexible unified U-Boot environment which we see on other platforms (e.g. TI), allowing users to boot Linux and Android from various boot media without build-time tunables - [KF-agnostic] Decrease the defconfig maintenance overhead as discussed in a parallel thread - [KF-agnostic] Build/boot U-Boot with UBSAN=y (there are issues) - many others! > > >> [...] > >> > >>> diff --git a/include/image.h b/include/image.h > >>> index 765ffecee0a7..889305cbefdb 100644 > >>> --- a/include/image.h > >>> +++ b/include/image.h > >>> @@ -306,6 +306,7 @@ enum { > >>> IH_COMP_COUNT, > >>> }; > >>> > >>> +#define LZ4F_MAGIC 0x184D2204 /* LZ4 Magic Number */ > >>> #define IH_MAGIC 0x27051956 /* Image Magic Number */ > >>> #define IH_NMLEN 32 /* Image Name Length */ > >>> > >> > >> Keep the list sorted please. > > > > I hope below looks better? > > > > #define IH_MAGIC 0x27051956 /* Image Magic Number */ > > #define IH_NMLEN 32 /* Image Name Length */ > > +#define LZ4F_MAGIC 0x184D2204 /* LZ4 Magic Number */ > > > > Please, share any other comments/wishes before I push v3 in the > > next couple of days. TIA! > > I think that's all from my side. Thanks! > > -- > Best regards, > Marek Vasut Best regards, Eugeniu.
On 4/9/19 4:08 PM, Eugeniu Rosca wrote: > On Tue, Apr 09, 2019 at 02:38:08PM +0200, Marek Vasut wrote: >> On 4/9/19 2:29 PM, Eugeniu Rosca wrote: >>> On Tue, Apr 09, 2019 at 01:06:31PM +0200, Marek Vasut wrote: >>>> On 4/8/19 5: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]. >>>> >>>> H3ULCB Kingfisher is not supported in mainline ;-) >>> >>> Well, my H3 ES2.0 ULCB attached to Kingfisher-M06 boots just fine with >>> the recently released v2019.04 U-Boot. I guess U-Boot simply doesn't >>> (need to) care about the fancy peripherals present on the KF extension >>> board (hence no need to spawn another KF defconfig)? >> >> Glad to hear it boots well. Is there anything interesting on the KF >> that'd be worth supporting ? I think it could be handled via DTOs. > > My vote goes for having a mainline-grade USB gadget/peripheral > support needed by fastboot :) simply because we currently rely on > a "20 files changed, 6294 insertions(+)" driver from Renesas which > didn't undergo any deep review and is unlikely ready for upstreaming. > > FWIW my other wishes for R-Car3 are: > - Add fastboot support, which includes choosing a proper DRAM > address/size for the fastboot buffer (currently we make it > 16 MiB and place it at 0x4A000000) > - Add Android boot image support > - Define a flexible unified U-Boot environment which we see on other > platforms (e.g. TI), allowing users to boot Linux and Android from > various boot media without build-time tunables > - [KF-agnostic] Decrease the defconfig maintenance overhead as > discussed in a parallel thread > - [KF-agnostic] Build/boot U-Boot with UBSAN=y (there are issues) > - many others! Noted
Hello,
I've got some valuable feedback from Roman (cc:) that the magic numbers
of the legacy and the newer LZ4 formats do not match, hence with the
current patch U-Boot would try to handle the legacy "lz4 -l"-compressed
kernel as a raw/uncompressed image.
Since U-Boot itself supports only the newer lz4 format [1], apparently
the best we can do is detect the legacy lz4 format and report it is
unsupported. If no comments, this will be taken care of in v3.
[1] Quote from https://github.com/trini/u-boot/commit/027b728d4aafb5cf97abc804944a77b837b0f07f#diff-df7190887ae0d38ba3f64a4c494d25c0R113
NOTE: This implements the release version of the LZ4 frame
format as generated by default by the 'lz4' command line tool.
This is not the same as the outdated, less efficient legacy
frame format currently (2015) implemented in the Linux kernel
(generated by 'lz4 -l'). The two formats are incompatible.
Best regards,
Eugeniu.
On Mon, Apr 08, 2019 at 05:35:27PM +0200, 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> Applied to u-boot/master, thanks!
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..c31dcd446526 100644 --- a/common/image-android.c +++ b/common/image-android.c @@ -8,6 +8,7 @@ #include <android_image.h> #include <malloc.h> #include <errno.h> +#include <asm/unaligned.h> #define ANDROID_IMAGE_DEFAULT_KERNEL_ADDR 0x10008000 @@ -126,6 +127,16 @@ ulong android_image_get_kload(const struct andr_img_hdr *hdr) return android_image_get_kernel_addr(hdr); } +ulong android_image_get_kcomp(const struct andr_img_hdr *hdr) +{ + const void *p = (void *)((uintptr_t)hdr + hdr->page_size); + + if (get_unaligned_le32(p) == LZ4F_MAGIC) + return IH_COMP_LZ4; + else + 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..889305cbefdb 100644 --- a/include/image.h +++ b/include/image.h @@ -306,6 +306,7 @@ enum { IH_COMP_COUNT, }; +#define LZ4F_MAGIC 0x184D2204 /* LZ4 Magic Number */ #define IH_MAGIC 0x27051956 /* Image Magic Number */ #define IH_NMLEN 32 /* Image Name Length */ @@ -1312,6 +1313,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 */ diff --git a/lib/lz4_wrapper.c b/lib/lz4_wrapper.c index 487d39ef0247..1c68e67452d9 100644 --- a/lib/lz4_wrapper.c +++ b/lib/lz4_wrapper.c @@ -5,6 +5,7 @@ #include <common.h> #include <compiler.h> +#include <image.h> #include <linux/kernel.h> #include <linux/types.h> @@ -23,8 +24,6 @@ typedef uint64_t U64; /* Unaltered (except removing unrelated code) from github.com/Cyan4973/lz4. */ #include "lz4.c" /* #include for inlining, do not link! */ -#define LZ4F_MAGIC 0x184D2204 - struct lz4_frame_header { u32 magic; union {
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> --- v2: - [Marek] Relocate LZ4F_MAGIC from lib/lz4_wrapper.c to include/image.h. This could be factored out into a standalone non-functional change if needed (having it included in this patch reveals its true purpose though) - [Marek] Use get_unaligned() primitive in accordance with doc/README.unaligned-memory-access.txt. NOTE: get_unaligned_le32() looks more appropriate, since the lz4 magic is stored in LE32: https://github.com/lz4/lz4/blob/f66abc45d9b96/lib/lz4frame.c#L659 - Drop switch-case in favor of if-else, since the magic numbers of other compression types are not necessarily 32-bit wide (handling a void pointer looks to be more generic) - Keep android_image_get_kcomp() of type 'ulong' to please the eye (to stay in harmony with other android_image_get_* functions), even if its true return value merely needs a signed/unsigned int - Static analysis: sparse, smatch and `make W=1` are all silent - Runtime: boot-tested on H3ULCB-KF using an Android boot image containing a lz4-compressed kernel v1: - https://patchwork.ozlabs.org/patch/1076477/ --- common/bootm.c | 2 +- common/image-android.c | 11 +++++++++++ include/image.h | 2 ++ lib/lz4_wrapper.c | 3 +-- 4 files changed, 15 insertions(+), 3 deletions(-)