Re: [PATCH v2 2/4] tools: mkimage: Add Allwinner TOC0 support

["Pali Rohár" <pali@kernel.org>]

On Sunday 22 August 2021 12:44:05 Samuel Holland wrote:
> >> +struct toc0_main_info {
> >> +	uint8_t	name[8];
> >> +	__le32	magic;
> >> +	__le32	checksum;
> >> +	__le32	serial;
> >> +	__le32	status;
> >> +	__le32	num_items;
> >> +	__le32	length;
> >> +	uint8_t	platform[4];
> >> +	uint8_t	reserved[8];
> >> +	uint8_t	end[4];
> >> +};
> >> +
> >> +#define TOC0_MAIN_INFO_NAME		"TOC0.GLH"
> >> +#define TOC0_MAIN_INFO_MAGIC		0x89119800
> >> +#define TOC0_MAIN_INFO_END		"MIE;"
> >> +
> >> +struct toc0_item_info {
> >> +	__le32	name;
> >> +	__le32	offset;
> >> +	__le32	length;
> >> +	__le32	status;
> >> +	__le32	type;
> >> +	__le32	load_addr;
> >> +	uint8_t	reserved[4];
> >> +	uint8_t	end[4];
> >> +};
> > 
> > These structures are raw image structures, right? Therefore they should
> > be marked as __packed?
> 
> I'm not sure why I would want to do that? The structures do not have any
> padding to start with, and the layout of the image format implies that
> they will be naturally aligned (and in fact the boot ROM assumes this).
> Marking them as packed would only cause GCC to generate less efficient code.

Because this code is running on the build machine, not on target
hardware where is the boot ROM. It is compiled by host compiler and
build machine may have totally different padding / layout schematics and
C compiler is free in this case to put any padding between any two
members in that structure.

And if GCC generates different or less efficient code, it means that
__packed is really needed here.