On Wed, Jul 20, 2016 at 09:27:43PM +0900, Masahiro Yamada wrote:

Hi Tom

2016-07-20 21:17 GMT+09:00 Tom Rini trini@konsulko.com:

...

On Wed, Jul 20, 2016 at 08:03:03PM +0900, Masahiro Yamada wrote:

...

Hi.

I found ARM64 Linux fails to boot since commit 555f45d8f916 ("image: Convert the IH_... values to enums"). It claims the ramdisk with uImage header is corrupt or invalid as follow.

## Loading init Ramdisk from Legacy Image at 84a00000 ... Image Name: Created: 2016-06-20 4:41:26 UTC Image Type: ARC Linux RAMDisk Image (uncompressed) Data Size: 1752025 Bytes = 1.7 MiB Load Address: 00000000 Entry Point: 00000000 No Linux AArch64 Ramdisk Image Ramdisk image is corrupt or invalid

Please note the "Image Type" field shows ARC, not AArch64.

The cause of the problem is commit 555f45d8f916 changed IH_ARCH_... values.

Prior to the commit, IH_ARCH_... were defined as follows:

#define IH_ARCH_SH 9 /* SuperH */ #define IH_ARCH_SPARC 10 /* Sparc */ #define IH_ARCH_SPARC64 11 /* Sparc 64 Bit */ #define IH_ARCH_M68K 12 /* M68K */ #define IH_ARCH_MICROBLAZE 14 /* MicroBlaze */ #define IH_ARCH_NIOS2 15 /* Nios-II */ #define IH_ARCH_BLACKFIN 16 /* Blackfin */ #define IH_ARCH_AVR32 17 /* AVR32 */ #define IH_ARCH_ST200 18 /* STMicroelectronics ST200 */ #define IH_ARCH_SANDBOX 19 /* Sandbox architecture (test only) */ #define IH_ARCH_NDS32 20 /* ANDES Technology - NDS32 */ #define IH_ARCH_OPENRISC 21 /* OpenRISC 1000 */ #define IH_ARCH_ARM64 22 /* ARM64 */ #define IH_ARCH_ARC 23 /* Synopsys DesignWare ARC */ #define IH_ARCH_X86_64 24 /* AMD x86_64, Intel and Via */

Please notice 13 is missing!

The enum conversion changed the value of IH_ARCH_ARM64 from 22 to 21.

This broke the compatibility with already existing uImage files.

I think the enum conversion was a bad idea because we tend to assume the order of items in enum is arbitrary, like sorting items alphabetically is allowed.

I think the best thing is to revert 555f45d8f916, but it causes build error:

In file included from tools/common/image.c:1:0: ./tools/../common/image.c:186:20: error: ‘IH_ARCH_COUNT’ undeclared here (not in a function) { "architecture", IH_ARCH_COUNT, uimage_arch }, ^ ./tools/../common/image.c:187:19: error: ‘IH_COMP_COUNT’ undeclared here (not in a function) { "compression", IH_COMP_COUNT, uimage_comp }, ^ ./tools/../common/image.c:188:24: error: ‘IH_OS_COUNT’ undeclared here (not in a function) { "operating system", IH_OS_COUNT, uimage_os },

I am in trouble.

Currently, I patch my local tree, like follows.

@@ -182,7 +188,7 @@ enum { IH_ARCH_SPARC, /* Sparc */ IH_ARCH_SPARC64, /* Sparc 64 Bit */ IH_ARCH_M68K, /* M68K */

• IH_ARCH_MICROBLAZE, /* MicroBlaze */

• IH_ARCH_MICROBLAZE =14, /* MicroBlaze */ IH_ARCH_NIOS2, /* Nios-II */ IH_ARCH_BLACKFIN, /* Blackfin */ IH_ARCH_AVR32, /* AVR32 */

Please formalize this and submit as a real patch, I'll apply it.

As I mentioned, I believe using enum is a bad idea here.

Is it better to convert the code back to #define, fixing the build error?

I think Simon is away this week and that change is in the middle of some others. So I'd like to fix things today and let him weigh in. But I think enums make sense and we've just got a historical oddity to explain that gap.

...

Thanks! And I am, really, getting some aarch64 systems setup such that I can boot them regularly.

But I'm also curious, what is your use case for uImage rather than the kernel generated Image or a FIT image?

I use arch/arm64/boot/Image as is generated in the kernel tree.

I use initramdisk with uImage header generated by Buildroot. (rootfs.cpio.uboot)

If I use a raw cpio, I guess U-Boot cannot know its size. (Perhaps, I am missing something, though. If you know a better idea, please let me know.)

Yes, you would need to pass in the address and size along to 'booti'. When I do this kind of thing locally, I make sure to load the initrd/initramfs last so that $filesize is correct.