[PATCH v5 3/6] arm: provide a function for boards init code to modify MMU virtual-physical map

[Patrick DELAUNAY]

Hi Marek,

> From: U-Boot <u-boot-bounces at lists.denx.de> On Behalf Of Tom Rini
> Sent: vendredi 10 juillet 2020 22:22
> 
> On Wed, Jun 03, 2020 at 02:43:42PM +0200, Marek Szyprowski wrote:
> 
> > Provide function for setting arbitrary virtual-physical MMU mapping
> > and cache settings for the given region.
> >
> > Signed-off-by: Marek Szyprowski <m.szyprowski at samsung.com>
> > Reviewed-by: Tom Rini <trini at konsulko.com>
> 
> Applied to u-boot/master, thanks!

For information, this patch break the SPL boot on stm32mp1 platform in master branch.

It is linked to commit 7e8471cae5c6614c54b9cfae2746d7299bd47a0c
("arm: stm32mp: activate data cache in SPL and before relocation")

For the lines:

+		mmu_set_region_dcache_behaviour(STM32_SYSRAM_BASE,
+						STM32_SYSRAM_SIZE,
+						DCACHE_DEFAULT_OPTION);

In this patch, the STM32MP15x activate the DACACHE on SYSRAM with:

#define STM32_SYSRAM_BASE		0x2FFC0000
#define STM32_SYSRAM_SIZE		SZ_256K

Even is this address is not MMU_SECTION_SIZE aligned, the configuration of the MMU
section was correctly aligned in set_section_dcache

-	value |= ((u32)section << MMU_SECTION_SHIFT); 

With caller :

void mmu_set_region_dcache_behaviour (
.....
	/* div by 2 before start + size to avoid phys_addr_t overflow */
	end = ALIGN((start / 2) + (size / 2), MMU_SECTION_SIZE / 2)
	      >> (MMU_SECTION_SHIFT - 1);
	start = start >> MMU_SECTION_SHIFT;
....
-	for (upto = start; upto < end; upto++)
-		set_section_dcache(upto, option);


But today it it no more the case when the start address is not MMU_SIZE aligned.

void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size,
				     enum dcache_option option)
{
	mmu_set_region_dcache_behaviour_phys(start, start, size, option);
}

'start' parameter  is directly used in 'phys' address in the next function:

void mmu_set_region_dcache_behaviour_phys(phys_addr_t start, phys_addr_t phys,
					size_t size, enum dcache_option option)
{
.....
	for (upto = start; upto < end; upto++, phys += MMU_SECTION_SIZE)
		set_section_phys(upto, phys, option);


and then

static void set_section_phys(int section, phys_addr_t phys,
			     enum dcache_option option)
{
....

	/* Add the page offset */
	value |= phys;


So today I can solve my issue, if I align the section in my code: 

 	if (IS_ENABLED(CONFIG_SPL_BUILD))
-		mmu_set_region_dcache_behaviour(STM32_SYSRAM_BASE,
-						STM32_SYSRAM_SIZE,
-						DCACHE_DEFAULT_OPTION);
+		mmu_set_region_dcache_behaviour(
+			ALIGN(STM32_SYSRAM_BASE, MMU_SECTION_SIZE),
+			round_up(STM32_SYSRAM_SIZE, MMU_SECTION_SIZE),
+			DCACHE_DEFAULT_OPTION);
 	else

But I just concerned that this lib behavior is really unexpected: need to provided MMU_SIZE aligned
start address when mmu_set_region_dcache_behaviour is called.

Do you think we need to restore the previous behavior of the cp15 function mmu_set_region_dcache_behaviour() ?
=> internally aligned DACHE region on MMU_SECTION_SIZE when parameters, start or size, are not aligned.

See also explanation for other use case, and start / end / size usage
http://patchwork.ozlabs.org/project/uboot/patch/20200424201957.v2.3.Ic2c7c6923035711a4c653d52ae7c0f57ca6f9210@changeid/


> --
> Tom

Regards
Patrick