Re: [PATCH v5 2/4] mach-k3: add runtime configuration of MMU table

[Anshul Dalal <anshuld@ti.com>]

On Tue Jul 8, 2025 at 1:45 AM IST, Ilias Apalodimas wrote:
> Hi Anshul,
>
> On Thu Jul 3, 2025 at 4:35 PM EEST, Anshul Dalal wrote:
>> k3_mem_map is used by u-boot to configure the MMU on k3 devices but
>> currently it's a static array which does not scale for platforms with
>> non-standard load addresses for ATF and OP-TEE. Additionally on systems
>> with limited DRAM, more space is mapped than is available on the device.
>>
>> Therefore this patch adds a new k3_mem_map_init function which can
>> be called from dram_init to configure the table at runtime where we
>> can query the required DDR information and reserved regions from the
>> device-tree.
>
> Is this a problem for k3 only? This kind of code should be aimed at the mmu
> support and improving armv8 overall, not specific platforms
>

In K3 we try to support a wide array of SoCs with a single memory map.
Since the requirements are similar across all K3 devices i.e:

1. Map all DDR banks
2. Create carveouts for ATF and OPTEE
3. At SPL stage, map framebuffer region for VIDEO enabled devices

Of the 3 here, #1 and #3 could be useful for ARMv8 overall but #2 is
quite specific to the platform's boot flow.

As I see it, since most of the existing platforms are using a static map
and thus won't benefit from a generic way to handle #1 and #3. The few
that do have runtime fixups for the memory map, only seem to have #1 in
common (imx and renesas).

>>
>> A dummy implementation is also added in r5/common.c to allow the build
>> to pass without masking each call to k3_mem_map_init behind an ifdef
>> CONFIG_ARM64.
>>
>> Signed-off-by: Anshul Dalal <anshuld@ti.com>
>
> [...]
>
>> +
>> +static void k3_mmu_add_cachable_entry(u64 start, u64 end, unsigned int *map_idx)
>> +{
>> +	if (start >= end)
>> +		return;
>> +
>> +	k3_mem_map[*map_idx].virt = start,
>> +	k3_mem_map[*map_idx].phys = start,
>> +	k3_mem_map[*map_idx].size = end - start,
>> +	k3_mem_map[*map_idx].attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) |
>> +				     PTE_BLOCK_INNER_SHARE;
>> +	(*map_idx)++;
>> +}
>
> Doesn't this need a break-before-make?
> Also why does it have to be a board specific function? Is there something mmu_change_region_attr()
> doesn't do ?
>

Oh, I wasn't aware of that API, I'll update in my next revision.

Thanks for the review,
Anshul