[U-Boot] [PATCH 1/2] image: android: allow booting lz4-compressed kernels

[Eugeniu Rosca]

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.