Hi Marco, On June 17, 2023 12:00:57 AM GMT+02:00, Marco Felsch <m.felsch@xxxxxxxxxxxxxx> wrote: > Hi Jules, > > since I work on the D1 support I also had to port the eGON image support > to barebox ^^ Please see my below comments. > > On 23-05-25, 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, followed by the actual image, must be located at a fixed > > offset of 8192 bytes (4K) from the start of the disk, either SD; or eMMC. > > > > 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 > > Nit: as noted on https://linux-sunxi.org it's not always limited to 32KB. > > > the header itself! The jump instruction in the header needs to be patched > > accordingly with the image size. > > > > Signed-off-by: Jules Maselbas <jmaselbas@xxxxxxxx> > > --- > > rfc->v2: > > - removed arch/arm/mach-sunxi/egon_header.c (unused) > > - reworked egon_mkimage.c to handle images size not multiple of 4bytes > > and added comments > > > > include/mach/sunxi/egon.h | 59 ++++++++++++++++++ > > scripts/Kconfig | 7 +++ > > scripts/Makefile | 1 + > > scripts/egon_mkimage.c | 122 ++++++++++++++++++++++++++++++++++++++ > > 4 files changed, 189 insertions(+) > > create mode 100644 include/mach/sunxi/egon.h > > create mode 100644 scripts/egon_mkimage.c > > > > diff --git a/include/mach/sunxi/egon.h b/include/mach/sunxi/egon.h > > new file mode 100644 > > index 0000000000..e00992eb7d > > --- /dev/null > > +++ b/include/mach/sunxi/egon.h > > @@ -0,0 +1,59 @@ > > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > > +#ifndef MACH_SUNXI_EGON_H > > +#define MACH_SUNXI_EGON_H > > + > > +struct egon_header { > > + uint32_t branch; /* branch instruction to jump over the header */ > > + uint8_t magic[8]; /* "eGON.BT0" or "eGON.BT1" */ > > + uint32_t check_sum; > > + uint32_t length; > > + /* > > + * We use a simplified header, only filling in what is needed > > + * by the boot ROM. To be compatible with Allwinner tools we > > + * would need to implement the proper fields here instead of > > + * padding. > > + * > > + * Actually we want the ability to recognize our "sunxi" variant > > + * of the SPL. To do so, let's place a special signature into the > > + * "pub_head_size" field. We can reasonably expect Allwinner's > > + * boot0 to always have the upper 16 bits of this set to 0 (after > > + * all the value shouldn't be larger than the limit imposed by > > + * SRAM size). > > + * If the signature is present (at 0x14), then we know it's safe > > + * to use the remaining 8 bytes (at 0x18) for our own purposes. > > + * (E.g. sunxi-tools "fel" utility can pass information there.) > > + */ > > + union { > > + uint32_t header_size; > > + uint8_t spl_signature[4]; > > + }; > > + uint32_t fel_script_address;/* since v0.1, set by sunxi-fel */ > > + /* > > + * If the fel_uEnv_length member below is set to a non-zero value, > > + * it specifies the size (byte count) of data at fel_script_address. > > + * At the same time this indicates that the data is in uEnv.txt > > + * compatible format, ready to be imported via "env import -t". > > + */ > > + uint32_t fel_uEnv_length;/* since v0.1, set by sunxi-fel */ > > + /* > > + * Offset of an ASCIIZ string (relative to the SPL header), which > > + * contains the default device tree name (CONFIG_DEFAULT_DEVICE_TREE). > > + * This is optional and may be set to NULL. Is intended to be used > > + * by flash programming tools for providing nice informative messages > > + * to the users. > > + */ > > + uint32_t dt_name_offset;/* since v0.2, set by mksunxiboot */ > > + uint32_t dram_size;/* in MiB, since v0.3, set by SPL */ > > + uint32_t boot_media;/* written here by the boot ROM */ > > + /* A padding area (may be used for storing text strings) */ > > + uint32_t string_pool[13];/* since v0.2, filled by mksunxiboot */ > > + /* The header must be a multiple of 32 bytes (for VBAR alignment) */ > > + /* And at least 64 byte (https://patchwork.ozlabs.org/patch/622173) */ > > +}; > > +#define EGON_HDR_BRANCH (0xea000000 | (sizeof(struct egon_header) / 4 - 2)) > > +#define sunxi_egon_header(section) { \ > > + __section(section) static const struct egon_header hdr= \ > > + { .branch = EGON_HDR_BRANCH, .magic = "eGON" }; \ > > + __keep_symbolref(hdr); \ > > + } > > Using an additional sections seems a bit odd here. We can just write the > header within the image tool. That's what I wanted to do in the first place but I struggled a lot to get barebox relocation working. Having the eGON header embedded in the .text (since the header is loaded by bootrom) is the only solution i found to get the relocation working. I am all for a better way but I really whish to have a first version applied. > > > +#endif > > diff --git a/scripts/Kconfig b/scripts/Kconfig > > index dcd5f32d1d..7517f5b79f 100644 > > --- a/scripts/Kconfig > > +++ b/scripts/Kconfig > > @@ -56,6 +56,13 @@ config RK_IMAGE > > help > > This enables building the image creation tool for Rockchip SoCs > > > > +config EGON_IMAGE > > + bool "Allwinner eGON image tool" if COMPILE_HOST_TOOLS > > + depends on ARCH_SUNXI || COMPILE_HOST_TOOLS > > + default y if ARCH_SUNXI > > + help > > + This enables building the image creation tool for Allwinner sunxi SoCs > > + > > config OMAP_IMAGE > > bool "TI OMAP image tools" if COMPILE_HOST_TOOLS > > depends on ARCH_OMAP || COMPILE_HOST_TOOLS > > diff --git a/scripts/Makefile b/scripts/Makefile > > index 72ad9ad7a6..13e80db7af 100644 > > --- a/scripts/Makefile > > +++ b/scripts/Makefile > > @@ -28,6 +28,7 @@ hostprogs-always-$(CONFIG_LAYERSCAPE_PBLIMAGE) += pblimage > > hostprogs-always-$(CONFIG_STM32_IMAGE) += stm32image > > hostprogs-always-$(CONFIG_RISCV) += prelink-riscv > > hostprogs-always-$(CONFIG_RK_IMAGE) += rkimage > > +hostprogs-always-$(CONFIG_EGON_IMAGE) += egon_mkimage > > HOSTCFLAGS_rkimage = `pkg-config --cflags openssl` > > HOSTLDLIBS_rkimage = `pkg-config --libs openssl` > > KBUILD_HOSTCFLAGS += -I$(srctree)/scripts/include/ > > diff --git a/scripts/egon_mkimage.c b/scripts/egon_mkimage.c > > new file mode 100644 > > index 0000000000..5983bdb28a > > --- /dev/null > > +++ b/scripts/egon_mkimage.c > > @@ -0,0 +1,122 @@ > > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > > + > > +#include <stdio.h> > > +#include <errno.h> > > +#include <stdlib.h> > > +#include <stdint.h> > > +#include <string.h> > > +#include <linux/kernel.h> > > + > > +#include "../include/mach/sunxi/egon.h" > > + > > +#include "compiler.h" > > +#include "common.h" > > +#include "common.c" > > + > > +#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 = sizeof(*hdr); > > + size_t bin_size; > > + size_t img_size; > > + void *bin; > > + int fd, ret; > > + > > + bin = read_file(infile, &bin_size); > > + if (!bin) { > > + perror("read_file"); > > + exit(1); > > + } > > + > > + /* test if the binary has reserved space for the header */ > > + hdr = bin; > > + if (hdr->branch == EGON_HDR_BRANCH && memcmp(hdr->magic, "eGON", 4) == 0) { > > + /* strip/skip existing header */ > > + bin += hdr_size; > > + bin_size -= hdr_size; > > + } > > Hm.. the 'normal' way is to write the header via the image tool, like it > is done for the i.MX. The infile don't need to have reserved space in > front, instead this tool should prepend the header. Yes, thisis only to accomodate for having the header in the .text (see my reply above) > I attached you my two patches adding the eGON image support. Since I > work on the D1 it is RSIC-V related but the eGON image creation should > not differ that much, maybe the offset must be adapted which can be done > via the command line. We could skip this special section handling if my > patches do work for you as well :) Sounds nice, I don't when I will have time to test this. Does the eGON header starts will a risc-v jump instruction ? Or is it still an arm32 insn ? Cheers