There are a couple of bugs regarding the ROM API.

The first is that the API is entirely undocumented. Why do they need to receive the xor of the preceding arguments as a final argument? And is it really just the lower 32 bits that need to "match", or are we relying on the fact that all the pointer arguments that we cast to (uintptr_t) and mix in to the xor all live in the lower 4G of the address space? What ABI do those functions follow? Presumably mostly the standard aarch64 calling convention, since we do call them as normal C functions (apart from the odd xor argument which is added manually).

There is some effort to save and restore gd, aka r18, presumably because the ROM API functions are not guaranteed to preserve that (but they must then be assumed to preserve the callee-saved registers r19 through r28). This is the second bug: The last invocation of ->download_image in spl_romapi_load_image_stream() fails to do that restore.

Rather than just blindly adding a set_gd(), create wrapper functions that (1) compute the apparently needed xor argument and (2) save/restore gd, instead of open-coding these things everywhere.

I expect the NXP folks will fix the first bug.

Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk --- arch/arm/include/asm/mach-imx/sys_proto.h | 5 ++- arch/arm/mach-imx/imx8m/soc.c | 33 ++++++++++++++--- arch/arm/mach-imx/imx8m/spl_imx_romapi.c | 45 ++++++----------------- 3 files changed, 41 insertions(+), 42 deletions(-)

diff --git a/arch/arm/include/asm/mach-imx/sys_proto.h b/arch/arm/include/asm/mach-imx/sys_proto.h index b612189849..e989982acb 100644 --- a/arch/arm/include/asm/mach-imx/sys_proto.h +++ b/arch/arm/include/asm/mach-imx/sys_proto.h @@ -153,6 +153,9 @@ struct rom_api { u32 (*query_boot_infor)(u32 info_type, u32 *info, u32 xor); };

+u32 rom_api_download_image(u8 *dest, u32 offset, u32 size); +u32 rom_api_query_boot_infor(u32 info_type, u32 *info); + enum boot_dev_type_e { BT_DEV_TYPE_SD = 1, BT_DEV_TYPE_MMC = 2, @@ -173,8 +176,6 @@ enum boot_dev_type_e { #define QUERY_IMG_OFF 6

#define ROM_API_OKAY 0xF0 - -extern struct rom_api *g_rom_api; #endif

u32 get_nr_cpus(void); diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c index 0c44022a6d..c8beb77e1e 100644 --- a/arch/arm/mach-imx/imx8m/soc.c +++ b/arch/arm/mach-imx/imx8m/soc.c @@ -523,21 +523,42 @@ int arch_cpu_init(void) return 0; }

-#if defined(CONFIG_IMX8MN) || defined(CONFIG_IMX8MP) -struct rom_api *g_rom_api = (struct rom_api *)0x980; +u32 rom_api_download_image(u8 *dest, u32 offset, u32 size) +{ + struct rom_api *rom_api = (struct rom_api *)0x980; + u32 xor = (uintptr_t)dest ^ offset ^ size; + volatile gd_t *sgd = gd; + u32 ret; + + ret = rom_api->download_image(dest, offset, size, xor); + set_gd(sgd);

+ return ret; +} + +u32 rom_api_query_boot_infor(u32 info_type, u32 *info) +{ + struct rom_api *rom_api = (struct rom_api *)0x980; + u32 xor = info_type ^ (uintptr_t)info; + volatile gd_t *sgd = gd; + u32 ret; + + ret = rom_api->query_boot_infor(info_type, info, xor); + set_gd(sgd); + + return ret; +} + +#if defined(CONFIG_IMX8MN) || defined(CONFIG_IMX8MP) enum boot_device get_boot_device(void) { - volatile gd_t *pgd = gd; int ret; u32 boot; u16 boot_type; u8 boot_instance; enum boot_device boot_dev = SD1_BOOT;

- ret = g_rom_api->query_boot_infor(QUERY_BT_DEV, &boot, - ((uintptr_t)&boot) ^ QUERY_BT_DEV); - set_gd(pgd); + ret = rom_api_query_boot_infor(QUERY_BT_DEV, &boot);

if (ret != ROM_API_OKAY) { puts("ROMAPI: failure at query_boot_info\n"); diff --git a/arch/arm/mach-imx/imx8m/spl_imx_romapi.c b/arch/arm/mach-imx/imx8m/spl_imx_romapi.c index d2085dabd3..68a585b52f 100644 --- a/arch/arm/mach-imx/imx8m/spl_imx_romapi.c +++ b/arch/arm/mach-imx/imx8m/spl_imx_romapi.c @@ -34,7 +34,6 @@ static ulong spl_romapi_read_seekable(struct spl_load_info *load, void *buf) { u32 pagesize = *(u32 *)load->priv; - volatile gd_t *pgd = gd; ulong byte = count * pagesize; int ret; u32 offset; @@ -43,9 +42,7 @@ static ulong spl_romapi_read_seekable(struct spl_load_info *load,

debug("ROM API load from 0x%x, size 0x%x\n", offset, (u32)byte);

- ret = g_rom_api->download_image(buf, offset, byte, - ((uintptr_t)buf) ^ offset ^ byte); - set_gd(pgd); + ret = rom_api_download_image(buf, offset, byte);

if (ret == ROM_API_OKAY) return count; @@ -59,21 +56,15 @@ static int spl_romapi_load_image_seekable(struct spl_image_info *spl_image, struct spl_boot_device *bootdev, u32 rom_bt_dev) { - volatile gd_t *pgd = gd; int ret; u32 offset; u32 pagesize, size; struct image_header *header; u32 image_offset;

- ret = g_rom_api->query_boot_infor(QUERY_IVT_OFF, &offset, - ((uintptr_t)&offset) ^ QUERY_IVT_OFF); - ret |= g_rom_api->query_boot_infor(QUERY_PAGE_SZ, &pagesize, - ((uintptr_t)&pagesize) ^ QUERY_PAGE_SZ); - ret |= g_rom_api->query_boot_infor(QUERY_IMG_OFF, &image_offset, - ((uintptr_t)&image_offset) ^ QUERY_IMG_OFF); - - set_gd(pgd); + ret = rom_api_query_boot_infor(QUERY_IVT_OFF, &offset); + ret |= rom_api_query_boot_infor(QUERY_PAGE_SZ, &pagesize); + ret |= rom_api_query_boot_infor(QUERY_IMG_OFF, &image_offset);

if (ret != ROM_API_OKAY) { puts("ROMAPI: Failure query boot infor pagesize/offset\n"); @@ -92,9 +83,7 @@ static int spl_romapi_load_image_seekable(struct spl_image_info *spl_image, CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR * 512 - 0x8000;

size = ALIGN(sizeof(struct image_header), pagesize); - ret = g_rom_api->download_image((u8 *)header, offset, size, - ((uintptr_t)header) ^ offset ^ size); - set_gd(pgd); + ret = rom_api_download_image((u8 *)header, offset, size);

if (ret != ROM_API_OKAY) { printf("ROMAPI: download failure offset 0x%x size 0x%x\n", @@ -169,7 +158,6 @@ static int spl_romapi_load_image_stream(struct spl_image_info *spl_image, struct spl_boot_device *bootdev) { struct spl_load_info load; - volatile gd_t *pgd = gd; u32 pagesize, pg; int ret; int i = 0; @@ -178,9 +166,7 @@ static int spl_romapi_load_image_stream(struct spl_image_info *spl_image, int imagesize; int total;

- ret = g_rom_api->query_boot_infor(QUERY_PAGE_SZ, &pagesize, - ((uintptr_t)&pagesize) ^ QUERY_PAGE_SZ); - set_gd(pgd); + ret = rom_api_query_boot_infor(QUERY_PAGE_SZ, &pagesize);

if (ret != ROM_API_OKAY) puts("failure at query_boot_info\n"); @@ -190,9 +176,7 @@ static int spl_romapi_load_image_stream(struct spl_image_info *spl_image, pg = 1024;

for (i = 0; i < 640; i++) { - ret = g_rom_api->download_image(p, 0, pg, - ((uintptr_t)p) ^ pg); - set_gd(pgd); + ret = rom_api_download_image(p, 0, pg);

if (ret != ROM_API_OKAY) { puts("Steam(USB) download failure\n"); @@ -212,8 +196,7 @@ static int spl_romapi_load_image_stream(struct spl_image_info *spl_image, }

if (p - pfit < sizeof(struct fdt_header)) { - ret = g_rom_api->download_image(p, 0, pg, ((uintptr_t)p) ^ pg); - set_gd(pgd); + ret = rom_api_download_image(p, 0, pg);

if (ret != ROM_API_OKAY) { puts("Steam(USB) download failure\n"); @@ -235,9 +218,7 @@ static int spl_romapi_load_image_stream(struct spl_image_info *spl_image,

printf("Need continue download %d\n", imagesize);

- ret = g_rom_api->download_image(p, 0, imagesize, - ((uintptr_t)p) ^ imagesize); - set_gd(pgd); + ret = rom_api_download_image(p, 0, imagesize);

p += imagesize;

@@ -259,8 +240,7 @@ static int spl_romapi_load_image_stream(struct spl_image_info *spl_image,

printf("Download %d, total fit %d\n", imagesize, total);

- ret = g_rom_api->download_image(p, 0, imagesize, - ((uintptr_t)p) ^ imagesize); + ret = rom_api_download_image(p, 0, imagesize); if (ret != ROM_API_OKAY) printf("ROM download failure %d\n", imagesize);

@@ -274,13 +254,10 @@ static int spl_romapi_load_image_stream(struct spl_image_info *spl_image, int board_return_to_bootrom(struct spl_image_info *spl_image, struct spl_boot_device *bootdev) { - volatile gd_t *pgd = gd; int ret; u32 boot;

- ret = g_rom_api->query_boot_infor(QUERY_BT_DEV, &boot, - ((uintptr_t)&boot) ^ QUERY_BT_DEV); - set_gd(pgd); + ret = rom_api_query_boot_infor(QUERY_BT_DEV, &boot);

if (ret != ROM_API_OKAY) { puts("ROMAPI: failure at query_boot_info\n");