On 30/12/19 13:04, Igor Opaniuk wrote: > Hi Stefano, > > On Sat, Dec 28, 2019 at 2:33 PM Stefano Babic <sba...@denx.de> wrote: >> >> >> >> On 28/11/19 14:56, Igor Opaniuk wrote: >>> From: Igor Opaniuk <igor.opan...@toradex.com> >>> >>> Currently imx-specific bootaux command doesn't support ELF format >>> firmware for Cortex-M4 core. >>> >>> This patches introduces a PoC implementation of handling elf firmware >>> (load_elf_image_phdr() was copy-pasted from elf.c just for PoC). >>> >>> This has the advantage that the user does not need to know to which >>> address the binary has been linked to. However, in order to handle >>> and load the elf sections to the right address, we need to translate the >>> Cortex-M4 core memory addresses to primary/host CPU memory >>> addresses (Cortex A7/A9 cores). >>> >>> This allows to boot firmwares from any location with just using >>> bootaux, e.g.: >>>> tftp ${loadaddr} hello_world.elf && bootaux ${loadaddr} >>> >>> Similar translation table can be found in the Linux remoteproc >>> driver [1]. >>> >>> [1] >>> https://elixir.bootlin.com/linux/latest/source/drivers/remoteproc/imx_rproc.c >>> >>> Signed-off-by: Igor Opaniuk <igor.opan...@toradex.com> >>> Signed-off-by: Stefan Agner <stefan.ag...@toradex.com> >>> --- >>> >>> arch/arm/include/asm/mach-imx/sys_proto.h | 7 ++ >>> arch/arm/mach-imx/imx_bootaux.c | 84 +++++++++++++++++++++-- >>> arch/arm/mach-imx/mx7/soc.c | 28 ++++++++ >>> 3 files changed, 115 insertions(+), 4 deletions(-) >>> >>> diff --git a/arch/arm/include/asm/mach-imx/sys_proto.h >>> b/arch/arm/include/asm/mach-imx/sys_proto.h >>> index 1e627c8fc3..ed5d9a1667 100644 >>> --- a/arch/arm/include/asm/mach-imx/sys_proto.h >>> +++ b/arch/arm/include/asm/mach-imx/sys_proto.h >>> @@ -104,6 +104,13 @@ void gpr_init(void); >>> >>> #endif /* CONFIG_MX6 */ >>> >>> +/* address translation table */ >>> +struct rproc_att { >>> + u32 da; /* device address (From Cortex M4 view) */ >>> + u32 sa; /* system bus address */ >>> + u32 size; /* size of reg range */ >>> +}; >>> + >>> u32 get_nr_cpus(void); >>> u32 get_cpu_rev(void); >>> u32 get_cpu_speed_grade_hz(void); >>> diff --git a/arch/arm/mach-imx/imx_bootaux.c >>> b/arch/arm/mach-imx/imx_bootaux.c >>> index c750cee60c..871169e771 100644 >>> --- a/arch/arm/mach-imx/imx_bootaux.c >>> +++ b/arch/arm/mach-imx/imx_bootaux.c >>> @@ -7,18 +7,94 @@ >>> #include <asm/io.h> >>> #include <asm/mach-imx/sys_proto.h> >>> #include <command.h> >>> +#include <elf.h> >>> #include <imx_sip.h> >>> #include <linux/compiler.h> >>> >>> -int arch_auxiliary_core_up(u32 core_id, ulong boot_private_data) >>> +const __weak struct rproc_att hostmap[] = { }; >>> + >>> +static const struct rproc_att *get_host_mapping(unsigned long auxcore) >>> +{ >>> + const struct rproc_att *mmap = hostmap; >>> + >>> + while (mmap && mmap->size) { >>> + if (mmap->da <= auxcore && >>> + mmap->da + mmap->size > auxcore) >>> + return mmap; >>> + mmap++; >>> + } >>> + >>> + return NULL; >>> +} >>> + >>> +/* >>> + * A very simple elf loader, assumes the image is valid, returns the >>> + * entry point address. >>> + */ >>> +static unsigned long load_elf_image_phdr(unsigned long addr) >>> +{ >>> + Elf32_Ehdr *ehdr; /* ELF header structure pointer */ >>> + Elf32_Phdr *phdr; /* Program header structure pointer */ >>> + int i; >>> + >>> + ehdr = (Elf32_Ehdr *)addr; >>> + phdr = (Elf32_Phdr *)(addr + ehdr->e_phoff); >>> + >>> + /* Load each program header */ >>> + for (i = 0; i < ehdr->e_phnum; ++i, ++phdr) { >>> + const struct rproc_att *mmap = >>> get_host_mapping(phdr->p_paddr); >>> + void *dst, *src; >>> + >>> + if (phdr->p_type != PT_LOAD) >>> + continue; >>> + >>> + if (!mmap) { >>> + printf("Invalid aux core address: %08x", >>> + phdr->p_paddr); >>> + return 0; >>> + } >>> + >>> + dst = (void *)(phdr->p_paddr - mmap->da) + mmap->sa; >>> + src = (void *)addr + phdr->p_offset; >>> + >> >> This generates a warning on 64bit targets (like imx8mq_evk): >> >> aarch64: w+ imx8mq_evk >> +WARNING 'signed_hdmi_imx8m.bin' not found, resulting binary is >> not-functional >> + dst = (void *)(phdr->p_paddr) ;//- mmap->da + mmap->sa; >> + ^ >> w+arch/arm/mach-imx/imx_bootaux.c: In function 'load_elf_image_phdr': >> w+arch/arm/mach-imx/imx_bootaux.c:58:9: warning: cast to pointer from >> integer of different size [-Wint-to-pointer-cast] >> >> Can you take a look ? I merge 1/3 and 2/3, no need to repost the whole >> series - thanks ! > > Thanks for reporting the issue. > > I've decided to disable ELF support for now for imx8mq in v3 patch [1], as > it needs a bit different implementation pf loader for ELF64 binaries.
Right, I took a look, too, and changes are not trivial - it is fine for me how to solve it and to enable for mx8m only. > It makes sense to do refactoring of cmd/elf.c and move common code > (ELF loading etc.) to a different file Fully agree. >(I'll do that for the next > U-boot release), ...well, this is already next release. Series flew into my -next branch, I am allowed to push just fixes for 2020.01. I pick up your V3, too, into my -next. Regards, Stefano > which can be then re-used from bootaux command implementation. > > v3 currently builds with one un-related to this patch warning: > u-boot-imx.git$ ./tools/buildman/buildman -f -j10 -d imx8mq_evk -v > ../boards.cfg is up to date. Nothing to do. > Building current source for 1 boards (1 thread, 10 jobs per thread) > aarch64: w+ imx8mq_evk > +WARNING 'signed_hdmi_imx8m.bin' not found, resulting binary is not-functional > 0 1 0 /1 imx8mq_evk > > [1] https://patchwork.ozlabs.org/patch/1216413/ >> >> Best regards, >> Stefano >> >>> + debug("Loading phdr %i to 0x%p (%i bytes)\n", >>> + i, dst, phdr->p_filesz); >>> + >>> + if (phdr->p_filesz) >>> + memcpy(dst, src, phdr->p_filesz); >>> + if (phdr->p_filesz != phdr->p_memsz) >>> + memset(dst + phdr->p_filesz, 0x00, >>> + phdr->p_memsz - phdr->p_filesz); >>> + flush_cache((unsigned long)dst & >>> + ~(CONFIG_SYS_CACHELINE_SIZE - 1), >>> + ALIGN(phdr->p_filesz, CONFIG_SYS_CACHELINE_SIZE)); >>> + } >>> + >>> + return ehdr->e_entry; >>> +} >>> + >>> +int arch_auxiliary_core_up(u32 core_id, ulong addr) >>> { >>> ulong stack, pc; >>> >>> - if (!boot_private_data) >>> + if (!addr) >>> return -EINVAL; >>> >>> - stack = *(u32 *)boot_private_data; >>> - pc = *(u32 *)(boot_private_data + 4); >>> + if (valid_elf_image(addr)) { >>> + stack = 0x0; >>> + pc = load_elf_image_phdr(addr); >>> + if (!pc) >>> + return CMD_RET_FAILURE; >>> + >>> + } else { >>> + /* >>> + * Assume binary file with vector table at the beginning. >>> + * Cortex-M4 vector tables start with the stack pointer (SP) >>> + * and reset vector (initial PC). >>> + */ >>> + stack = *(u32 *)addr; >>> + pc = *(u32 *)(addr + 4); >>> + } >>> >>> printf("## Starting auxiliary core stack = 0x%08lX, pc = >>> 0x%08lX...\n", >>> stack, pc); >>> diff --git a/arch/arm/mach-imx/mx7/soc.c b/arch/arm/mach-imx/mx7/soc.c >>> index 35160f4b37..4aafeed188 100644 >>> --- a/arch/arm/mach-imx/mx7/soc.c >>> +++ b/arch/arm/mach-imx/mx7/soc.c >>> @@ -193,6 +193,34 @@ static void init_cpu_basic(void) >>> #endif >>> } >>> >>> +#ifdef CONFIG_IMX_BOOTAUX >>> +/* >>> + * Table of mappings of physical mem regions in both >>> + * Cortex-A7 and Cortex-M4 address spaces. >>> + * >>> + * For additional details check sections 2.1.2 and 2.1.3 in >>> + * i.MX7Dual Applications Processor Reference Manual >>> + * >>> + */ >>> +const struct rproc_att hostmap[] = { >>> + /* aux core , host core, size */ >>> + { 0x00000000, 0x00180000, 0x8000 }, /* OCRAM_S */ >>> + { 0x00180000, 0x00180000, 0x8000 }, /* OCRAM_S */ >>> + { 0x20180000, 0x00180000, 0x8000 }, /* OCRAM_S */ >>> + { 0x1fff8000, 0x007f8000, 0x8000 }, /* TCML */ >>> + { 0x20000000, 0x00800000, 0x8000 }, /* TCMU */ >>> + { 0x00900000, 0x00900000, 0x20000 }, /* OCRAM_128KB */ >>> + { 0x20200000, 0x00900000, 0x20000 }, /* OCRAM_128KB */ >>> + { 0x00920000, 0x00920000, 0x20000 }, /* OCRAM_EPDC */ >>> + { 0x20220000, 0x00920000, 0x20000 }, /* OCRAM_EPDC */ >>> + { 0x00940000, 0x00940000, 0x20000 }, /* OCRAM_PXP */ >>> + { 0x20240000, 0x00940000, 0x20000 }, /* OCRAM_PXP */ >>> + { 0x10000000, 0x80000000, 0x0fff0000 }, /* DDR Code alias */ >>> + { 0x80000000, 0x80000000, 0xe0000000 }, /* DDRC */ >>> + { /* sentinel */ } >>> +}; >>> +#endif >>> + >>> #ifndef CONFIG_SKIP_LOWLEVEL_INIT >>> /* enable all periherial can be accessed in nosec mode */ >>> static void init_csu(void) >>> >> >> -- >> ===================================================================== >> DENX Software Engineering GmbH, Managing Director: Wolfgang Denk >> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany >> Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de >> ===================================================================== > > > -- ===================================================================== DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sba...@denx.de =====================================================================