Re: [RFC PATCH 01/11] scripts: Add Allwinner eGON image support

[Sascha Hauer <sha@pengutronix.de>]

On Thu, May 11, 2023 at 01:37:01AM +0200, Jules Maselbas wrote:
> On power-up Allwinner SoC starts in boot ROM, aka BROM, which will search
> for an eGON image: first from the SD card, then from eMMC. If no image is
> found then the BROM will enter into FEL mode that can be used for initial
> programming and recovery of devices using USB.
> 
> The eGON header structure is adapted from u-boot: /include/sunxi_image.h,
> the header structure is also documented on https://linux-sunxi.org/EGON
> 
> BROM will load, at most, the first 32KB of the image into SRAM, including
> the header itself! The jump instruction in the header needs to be patched
> accordingly with the image size.

Do I understand it correctly that all code needed to load the full
barebox image needs to fit into these 32KiB?

Where must this image be placed? Directly at the origin of the SD card?
That would mean the partition table is somewhere inside the binary,
right?

> +
> +#define ALIGN(x, a)		__ALIGN_MASK(x, (typeof(x))(a) - 1)
> +#define __ALIGN_MASK(x, mask)	(((x) + (mask)) & ~(mask))

You can reuse the definition of these in scripts/include/linux/kernel.h

> +#define STAMP_VALUE 0x5f0a6c39
> +
> +static void mkimage(char *infile, char *outfile)
> +{
> +	struct egon_header *hdr;
> +	uint32_t *p32;
> +	uint32_t sum;
> +	int i;
> +	size_t hdr_size;
> +	size_t bin_size;
> +	size_t img_size;
> +	char *bin;
> +	int fd, ret;
> +
> +	bin = read_file(infile, &bin_size);
> +	if (!bin) {
> +		perror("read_file");
> +		exit(1);
> +	}
> +
> +	/* the header must be a multiple of 32 bytes */
> +	hdr_size = sizeof(*hdr);
> +
> +	/* test if the binary has reserved space for the header */
> +	hdr = (void *)bin;

Declare bin as void *, this makes the explicit cast unnecessary.

> +	if (hdr->branch == EGON_HDR_BRANCH && memcmp(hdr->magic, "eGON", 4) == 0) {
> +		/* strip the existing header */
> +		bin += hdr_size;
> +		bin_size -= hdr_size;
> +	}
> +	hdr = calloc(1, hdr_size);
> +	if (!hdr) {
> +		perror("malloc");
> +		exit(1);
> +	}
> +
> +	/* The total image length must be a multiple of 4K bytes */
> +	img_size = ALIGN(hdr_size + bin_size, 4096);
> +
> +	hdr->check_sum = 0;

hdr is already zeroed due to the use of calloc.

> +	hdr->branch = EGON_HDR_BRANCH;
> +	hdr->length = cpu_to_le32(img_size);
> +	memcpy(hdr->magic, "eGON.BT0", 8);
> +	memcpy(hdr->spl_signature, "SPL", 3);
> +	hdr->spl_signature[3] = 0x03; /* version 0.3 */
> +
> +	/* calculate the checksum */
> +	sum = STAMP_VALUE;
> +	for (p32 = (void *) hdr, i = 0; i < hdr_size / sizeof(uint32_t); i++)
> +		sum += le32_to_cpu(p32[i]);
> +	for (p32 = (void *) bin, i = 0; i < bin_size / sizeof(uint32_t); i++)
> +		sum += le32_to_cpu(p32[i]);

Does this work when bin_size is not a multiple of sizeof(uint32_t)?
It likely is, but who knows...

You are calculating the checksum up to bin_size, not including the final
alignment. That works because the alignment is filled with 0x0 which
doesn't add to this checksum. That is ok, but maybe it's worth noting
that in the code.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |