Hi Quentin, On 2024-04-15 16:16, Quentin Schulz wrote: > From: Quentin Schulz <quentin.sch...@theobroma-systems.com> > > Allow RK3568 and RK3588 based boards to get the RAM bank configuration > from the ROCKCHIP_TPL stage instead of the current logic. This fixes > both an issue where 256MB of RAM is blocked for devices with >= 4GB > of RAM and where memory holes need to be defined for devices with > more than 16GB of RAM. In the event that neither SoC is used or the > ROCKCHIP_TPL stage is not used, fall back to existing logic. > > The logic handles creating memory holes from reserved memory areas > defined in mem_map data struct in SoC C files, but only if the DRAM area > overlaps with one reserved memory area. > > Since mem_map data struct is used, it should be rather straightforward > to add support for other SoCs if needed. > > The logic is taken from Rockchip's U-Boot tag linux-5.10-gen-rkr4.1 > (e08e32143dd). > > Note that Rockchip's U-Boot/TF-A/OP-TEE modify the ATAGS at runtime as > well, but the DDR_MEM tag seems to be pretty much stable (though BL31 > seems to be reserving only 1MB for itself at the moment). > > u32 for ATAGS is used because it simplifies the pointer arithmetic and > it's expected that ATAGS are always below the 4GB limit allowed by u32. > > Co-developed-by: Chris Morgan <macromor...@hotmail.com> > Signed-off-by: Chris Morgan <macromor...@hotmail.com> > Signed-off-by: Quentin Schulz <quentin.sch...@theobroma-systems.com> > --- > arch/arm/mach-rockchip/sdram.c | 240 > +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 240 insertions(+) > > diff --git a/arch/arm/mach-rockchip/sdram.c b/arch/arm/mach-rockchip/sdram.c > index 0d9a0aef6f5..5b1ff1e5495 100644 > --- a/arch/arm/mach-rockchip/sdram.c > +++ b/arch/arm/mach-rockchip/sdram.c > @@ -8,6 +8,7 @@ > #include <init.h> > #include <log.h> > #include <ram.h> > +#include <asm/armv8/mmu.h> > #include <asm/global_data.h> > #include <asm/io.h> > #include <asm/arch-rockchip/sdram.h> > @@ -35,12 +36,251 @@ struct tos_parameter_t { > s64 reserve[8]; > }; > > +/* Tag size and offset */ > +#define ATAGS_SIZE SZ_8K > +#define ATAGS_OFFSET (SZ_2M - ATAGS_SIZE) > +#define ATAGS_PHYS_BASE (CFG_SYS_SDRAM_BASE + ATAGS_OFFSET) > +#define ATAGS_PHYS_END (ATAGS_PHYS_BASE + ATAGS_SIZE) > + > +/* ATAGS memory structures */ > + > +enum tag_magic { > + ATAG_NONE, > + ATAG_CORE = 0x54410001, > + ATAG_SERIAL = 0x54410050, > + ATAG_DDR_MEM = 0x54410052, > + ATAG_MAX = 0x544100ff, > +}; > + > +/* > + * An ATAG contains the following data: > + * - header > + * u32 size // sizeof(header + tag data) / sizeof(u32) > + * u32 magic > + * - tag data > + */ > + > +struct tag_header { > + u32 size; > + u32 magic; > +} __packed; > + > +/* > + * DDR_MEM tag bank is storing data this way: > + * - address0 > + * - address1 > + * - [...] > + * - addressX > + * - size0 > + * - size1 > + * - [...] > + * - sizeX > + * > + * with X being tag_ddr_mem.count - 1. > + */ > +struct tag_ddr_mem { > + u32 count; > + u32 version; > + u64 bank[20]; > + u32 flags; > + u32 data[2]; > + u32 hash; > +} __packed; > + > +static u32 js_hash(const void *buf, u32 len) > +{ > + u32 i, hash = 0x47C6A7E6; > + > + if (!buf || !len) > + return hash; > + > + for (i = 0; i < len; i++) > + hash ^= ((hash << 5) + ((const char *)buf)[i] + (hash >> 2)); > + > + return hash; > +} > + > +static int rockchip_dram_init_banksize(void) > +{ > + const struct tag_header *tag_h = NULL; > + u32 *addr = (void *)ATAGS_PHYS_BASE; > + struct tag_ddr_mem *ddr_info; > + u32 calc_hash; > + u8 i, j; > + > + if (!IS_ENABLED(CONFIG_ROCKCHIP_RK3588) && > + !IS_ENABLED(CONFIG_ROCKCHIP_RK3568)) > + return -ENOTSUPP; > + > + if (!IS_ENABLED(CONFIG_ROCKCHIP_EXTERNAL_TPL)) > + return -ENOTSUPP; > + > + /* Find DDR_MEM tag */ > + while (addr < (u32 *)ATAGS_PHYS_END) { > + tag_h = (const struct tag_header *)addr; > + > + if (!tag_h->size) { > + debug("End of ATAGS (0-size tag), no DDR_MEM found\n"); > + return -ENODATA; > + } > + > + if (tag_h->magic == ATAG_DDR_MEM) > + break; > + > + switch (tag_h->magic) { > + case ATAG_NONE: > + case ATAG_CORE: > + case ATAG_SERIAL ... ATAG_MAX: > + addr += tag_h->size; > + continue; > + default: > + debug("Invalid magic (0x%08x) for ATAG at 0x%p\n", > + tag_h->magic, addr); > + return -EINVAL; > + } > + } > + > + if (addr >= (u32 *)ATAGS_PHYS_END || > + (tag_h && (addr + tag_h->size > (u32 *)ATAGS_PHYS_END))) { > + debug("End of ATAGS, no DDR_MEM found\n"); > + return -ENODATA; > + } > + > + /* Data is right after the magic member of the tag_header struct */ > + ddr_info = (struct tag_ddr_mem *)(&tag_h->magic + 1); > + if (!ddr_info->count || ddr_info->count > CONFIG_NR_DRAM_BANKS) { > + debug("Too many ATAG banks, got (%d) but max allowed (%d)\n", > + ddr_info->count, CONFIG_NR_DRAM_BANKS); > + return -ENOMEM; > + } > + > + if (!ddr_info->hash) { > + debug("No hash for tag (0x%08x)\n", tag_h->magic); > + } else { > + calc_hash = js_hash(addr, sizeof(u32) * (tag_h->size - 1)); > + > + if (calc_hash != ddr_info->hash) { > + debug("Incorrect hash for tag (0x%08x), got (0x%08x) > expected (0x%08x)\n", > + tag_h->magic, ddr_info->hash, calc_hash); > + return -EINVAL; > + } > + } > + > + /* > + * Rockchip guaranteed DDR_MEM is ordered so no need to worry about > + * bi_dram order. > + */ > + for (i = 0, j = 0; i < ddr_info->count; i++, j++) { > + phys_size_t size = ddr_info->bank[(i + ddr_info->count)]; > + phys_addr_t start_addr = ddr_info->bank[i]; > + struct mm_region *tmp_mem_map = mem_map; > + phys_addr_t end_addr; > + > + /* > + * BL31 (TF-A) reserves the first 2MB but DDR_MEM tag may not > + * have it, so force this space as reserved. > + */ > + if (start_addr < SZ_2M) { > + size -= SZ_2M - start_addr; > + start_addr = SZ_2M; > + } > + > + /* > + * Put holes for reserved memory areas from mem_map. > + * > + * Only check for at most one overlap with one reserved memory > + * area. > + */ > + while (tmp_mem_map->size) { > + const phys_addr_t rsrv_start = tmp_mem_map->phys; > + const phys_size_t rsrv_size = tmp_mem_map->size; > + const phys_addr_t rsrv_end = rsrv_start + rsrv_size; > + > + if (!(tmp_mem_map->attrs & PTE_BLOCK_NON_SHARE)) {
This check does not seem to work because PTE_BLOCK_NON_SHARE evaluates to 0. Because of this the logic to split the 0-8 GiB bank reported on rk3568 is never split in two. Changing this to e.g. if (tmp_mem_map->attrs & PTE_BLOCK_INNER_SHARE) { seem to split the range in two, and Linux kernel can be booted. Not sure that is a good check or if that has any other unintended consequence. Regards, Jonas > + tmp_mem_map++; > + continue; > + } > + > + /* > + * If the start of the DDR_MEM tag is in a reserved > + * memory area, move start address and resize. > + */ > + if (start_addr >= rsrv_start && start_addr < rsrv_end) { > + if (rsrv_end - start_addr > size) { > + debug("Would be negative memory > size\n"); > + return -EINVAL; > + } > + > + size -= rsrv_end - start_addr; > + start_addr = rsrv_end; > + break; > + } > + > + if (start_addr < rsrv_start) { > + end_addr = start_addr + size; > + > + if (end_addr <= rsrv_start) { > + tmp_mem_map++; > + continue; > + } > + > + /* > + * If the memory area overlaps a reserved memory > + * area with start address outside of reserved > + * memory area and... > + * > + * ... ends in the middle of reserved memory > + * area, resize. > + */ > + if (end_addr <= rsrv_end) { > + size = rsrv_start - start_addr; > + break; > + } > + > + /* > + * ... ends after the reserved memory area, > + * split the region in two, one for before the > + * reserved memory area and one for after. > + */ > + gd->bd->bi_dram[j].start = start_addr; > + gd->bd->bi_dram[j].size = rsrv_start - > start_addr; > + > + j++; > + > + size = end_addr - rsrv_end; > + start_addr = rsrv_end; > + > + break; > + } > + } > + > + if (j > CONFIG_NR_DRAM_BANKS) { > + debug("Too many banks, max allowed (%d)\n", > + CONFIG_NR_DRAM_BANKS); > + return -ENOMEM; > + } > + > + gd->bd->bi_dram[j].start = start_addr; > + gd->bd->bi_dram[j].size = size; > + } > + > + return 0; > +} > + > int dram_init_banksize(void) > { > size_t ram_top = (unsigned long)(gd->ram_size + CFG_SYS_SDRAM_BASE); > size_t top = min((unsigned long)ram_top, (unsigned long)(gd->ram_top)); > > #ifdef CONFIG_ARM64 > + int ret = rockchip_dram_init_banksize(); > + > + if (!ret) > + return ret; > + > + debug("Couldn't use ATAG (%d) to detect DDR layout, falling back...\n", > + ret); > + > /* Reserve 0x200000 for ATF bl31 */ > gd->bd->bi_dram[0].start = 0x200000; > gd->bd->bi_dram[0].size = top - gd->bd->bi_dram[0].start; >