| Message ID | 20240415-rk35xx-dram-atags-v3-1-5bc5475b3c0d@theobroma-systems.com |
|---|---|
| State | Accepted |
| Delegated to: | Kever Yang |
| Headers | show |
| Series | rockchip: sdram: Support getting banks from TPL for rk3568 and rk3588 | expand |
On 2024/4/15 22:16, Quentin Schulz wrote: > From: Quentin Schulz <quentin.schulz@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 <macromorgan@hotmail.com> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com> > Signed-off-by: Quentin Schulz <quentin.schulz@theobroma-systems.com> Reviewed-by: Kever Yang <kever.yang@rock-chips.com> Thanks, - Kever > --- > 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)) { > + 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; >
Hi Quentin, On 2024-04-15 16:16, Quentin Schulz wrote: > From: Quentin Schulz <quentin.schulz@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 <macromorgan@hotmail.com> > Signed-off-by: Chris Morgan <macromorgan@hotmail.com> > Signed-off-by: Quentin Schulz <quentin.schulz@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; >
Hi Jonas, On 4/24/24 00:40, Jonas Karlman wrote: > Hi Quentin, > > On 2024-04-15 16:16, Quentin Schulz wrote: >> From: Quentin Schulz <quentin.schulz@theobroma-systems.com> [...] >> + 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. > Oof, that's a bit oversight, thanks for the catch. Can you test the following please? """ diff --git a/arch/arm/mach-rockchip/sdram.c b/arch/arm/mach-rockchip/sdram.c index 5b1ff1e5495..0492f9b9f41 100644 --- a/arch/arm/mach-rockchip/sdram.c +++ b/arch/arm/mach-rockchip/sdram.c @@ -196,7 +196,23 @@ static int rockchip_dram_init_banksize(void) 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)) { + /* + * DRAM memories are expected by Arm to be marked as + * Normal Write-back cacheable, Inner shareable[1], so + * let's filter on that to put holes in non-DRAM areas. + * + * [1] https://developer.arm.com/documentation/102376/0200/Cacheability-and-shareability-attributes + */ + const u64 dram_attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) | + PTE_BLOCK_INNER_SHARE; + /* + * (AttrIndx | SH) in Lower Attributes of Block + * Descriptor[2]. + * [2] https://developer.arm.com/documentation/102376/0200/Describing-memory-in-AArch64 + */ + const u64 attrs_mask = PMD_ATTRINDX_MASK | GENMASK(9, 8); + + if ((tmp_mem_map->attrs & attrs_mask) == dram_attrs) { tmp_mem_map++; continue; } """ The DRAM mem_map entry for Rockchip devices seems to have those attributes and no other non-DRAM entry seems to have PTE_BLOCK_MEMTYPE(MT_NORMAL). We may have an issue in the future if we also want to mark SRAM, ROM or flash (https://developer.arm.com/documentation/102376/0200/Normal-memory) because it's likely those would also match the same attributes but we would need to put holes for those so that they aren't thought to be DRAM, but I guess we can tackle this the day this happens :) Thanks again for the catch, let me know if this helps and makes sense and I'll send a v4 for it. Cheers, Quentin
Hi Quentin, On 2024-04-24 11:11, Quentin Schulz wrote: > Hi Jonas, > > On 4/24/24 00:40, Jonas Karlman wrote: >> Hi Quentin, >> >> On 2024-04-15 16:16, Quentin Schulz wrote: >>> From: Quentin Schulz <quentin.schulz@theobroma-systems.com> > [...] > >>> + 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. >> > > Oof, that's a bit oversight, thanks for the catch. > > Can you test the following please? > > """ > diff --git a/arch/arm/mach-rockchip/sdram.c b/arch/arm/mach-rockchip/sdram.c > index 5b1ff1e5495..0492f9b9f41 100644 > --- a/arch/arm/mach-rockchip/sdram.c > +++ b/arch/arm/mach-rockchip/sdram.c > @@ -196,7 +196,23 @@ static int rockchip_dram_init_banksize(void) > 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)) { > + /* > + * DRAM memories are expected by Arm to be marked as > + * Normal Write-back cacheable, Inner shareable[1], so > + * let's filter on that to put holes in non-DRAM areas. > + * > + * [1] > https://developer.arm.com/documentation/102376/0200/Cacheability-and-shareability-attributes > + */ > + const u64 dram_attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) | > + PTE_BLOCK_INNER_SHARE; > + /* > + * (AttrIndx | SH) in Lower Attributes of Block > + * Descriptor[2]. > + * [2] > https://developer.arm.com/documentation/102376/0200/Describing-memory-in-AArch64 > + */ > + const u64 attrs_mask = PMD_ATTRINDX_MASK | GENMASK(9, 8); > + > + if ((tmp_mem_map->attrs & attrs_mask) == dram_attrs) { > tmp_mem_map++; > continue; > } > """ > > The DRAM mem_map entry for Rockchip devices seems to have those > attributes and no other non-DRAM entry seems to have > PTE_BLOCK_MEMTYPE(MT_NORMAL). > > We may have an issue in the future if we also want to mark SRAM, ROM or > flash > (https://developer.arm.com/documentation/102376/0200/Normal-memory > because it's likely those would also match the same attributes but we > would need to put holes for those so that they aren't thought to be > DRAM, but I guess we can tackle this the day this happens :) > > Thanks again for the catch, let me know if this helps and makes sense > and I'll send a v4 for it. I think this makes sense and seem to work on at least two of my RK356x 8GB boards using the generic-rk3568 target. U-Boot: => bdinfo boot_params = 0x0000000000000000 DRAM bank = 0x0000000000000000 -> start = 0x0000000000200000 -> size = 0x00000000efe00000 DRAM bank = 0x0000000000000001 -> start = 0x0000000100000000 -> size = 0x0000000100000000 flashstart = 0x0000000000000000 flashsize = 0x0000000000000000 flashoffset = 0x0000000000000000 baudrate = 1500000 bps relocaddr = 0x00000000eff3a000 reloc off = 0x00000000ef53a000 Build = 64-bit fdt_blob = 0x00000000edf0b390 new_fdt = 0x00000000edf0b390 fdt_size = 0x000000000000f980 lmb_dump_all: memory.cnt = 0x2 / max = 0x10 memory[0] [0x200000-0xefffffff], 0xefe00000 bytes flags: 0 memory[1] [0x100000000-0x1ffffffff], 0x100000000 bytes flags: 0 reserved.cnt = 0x2 / max = 0x10 reserved[0] [0xecf07000-0xefffffff], 0x030f9000 bytes flags: 0 reserved[1] [0x100000000-0x1ffffffff], 0x100000000 bytes flags: 0 devicetree = separate serial addr = 0x00000000fe660000 width = 0x0000000000000004 shift = 0x0000000000000002 offset = 0x0000000000000000 clock = 0x00000000016e3600 arch_number = 0x0000000000000000 TLB addr = 0x00000000efff0000 irq_sp = 0x00000000edf0b0c0 sp start = 0x00000000edf0b0c0 Early malloc usage: 2338 / 10000 Linux: [ 0.000000] Machine model: Generic RK3566/RK3568 [ 0.000000] efi: UEFI not found. [ 0.000000] NUMA: No NUMA configuration found [ 0.000000] NUMA: Faking a node at [mem 0x0000000000200000-0x00000001ffffffff] [ 0.000000] NUMA: NODE_DATA [mem 0x1ff0389c0-0x1ff03afff] [ 0.000000] Zone ranges: [ 0.000000] DMA [mem 0x0000000000200000-0x00000000ffffffff] [ 0.000000] DMA32 empty [ 0.000000] Normal [mem 0x0000000100000000-0x00000001ffffffff] [ 0.000000] Movable zone start for each node [ 0.000000] Early memory node ranges [ 0.000000] node 0: [mem 0x0000000000200000-0x00000000efffffff] [ 0.000000] node 0: [mem 0x0000000100000000-0x00000001ffffffff] [ 0.000000] Initmem setup node 0 [mem 0x0000000000200000-0x00000001ffffffff] [ 0.000000] On node 0, zone DMA: 512 pages in unavailable ranges [ 0.000000] cma: Reserved 32 MiB at 0x00000000ee000000 on node -1 Regards, Jonas > > Cheers, > Quentin
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)) { + 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;