mail archive of the barebox mailing list
 help / color / mirror / Atom feed
From: Ahmad Fatoum <>
To: Sascha Hauer <>
Subject: Re: [PATCH v2 08/10] ARM: mmu: use reserve mem entries to modify maps
Date: Mon, 12 Sep 2022 17:15:45 +0200	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>


On 12.09.22 14:01, Sascha Hauer wrote:
> This patch breaks NAND support on my Phytec i.MX6 board. There are some
> problems with this patch, so I ended up reverting it for now.

I wonder why. I see no memory reserves in imx6q-phytec-phycore-som-nand.dts
and the files it includes.

> On Wed, Aug 17, 2022 at 01:42:42PM +0200, Ahmad Fatoum wrote:
>> @@ -468,11 +469,28 @@ void __mmu_init(bool mmu_on)
>>  	vectors_init();
>>  	for_each_memory_bank(bank) {
>> +		struct resource *rsv;
>> +
>>  		create_sections(ttb, bank->start, bank->start + bank->size - 1,
>> -		__mmu_cache_flush();
>> +
>> +		for_each_reserved_region(bank, rsv) {
>> +			create_sections(ttb, resource_first_page(rsv),
>> +					resource_count_pages(rsv),
>> +					attrs_uncached_mem());
>> +		}
> This operates on 1MiB sections, so everything requiring a finer
> granularity will fail here. Not sure if we currently need that, but not
> even issuing a warning is not good.

At worst it'd needlessly mark some memory uncached/XN. If users are
affected, they will notice and we can revisit this. I could add a debug
print here.

I need to rework this though, because I see now create_sections differs
between ARM64 and ARM32. On ARM64, it accepts the last address as argument,
while on ARM64, it's the size.. resource_count_pages() is not a nice
name either, because it returns bytes (aligned up to PAGE_SIZE).

>>  	}
>> +	/*
>> +	 * We could set_ttbr(ttb) here instead and save on the copy, but
>> +	 * for now we play it safe, so we don't mess with the older ARMs.
>> +	 */
>> +	if (oldttb) {
>> +		memcpy(oldttb, ttb, ARM_TTB_SIZE);
>> +		free(ttb);
>> +	}
> in the early MMU case the MMU still uses 'oldttb' as ttb whereas 'ttb'
> now points to invalid memory. Still functions like arm_create_pte()
> still operate on 'ttb'. It looks like a ttb = oldttb is missing here.

Why would ttb point at invalid memory? It's allocated unconditionally
with xmemalign and freed here.

> Also I wonder when we have to map the reserved regions as execute never,
> then the early MMU setup seems broken as well, as that creates a flat
> mapping without honoring the reserved regions. Shouldn't that be fixed?

Yes, see excerpt from cover letter:

"Note that this doesn't yet solve all problems. For example, PPA secure
 monitor installation on Layerscape may happen with CONFIG_MMU_EARLY=y,
 in which case barebox in EL2 may speculate into the secure memory
 before any device tree reserved-memory settings are considered. For this
 reason, both early MMU and normal MMU setup must be aware of the
 reserved memory regions. The original patch set by Rouven used FDT
 parsing in PBL to achieve this, but this is omitted here to limit
 scope of the patch series. Instead we only handle the CONFIG_OPTEE_SIZE
 case out-of-the-box."

My use case is the CONFIG_OPTEE_SIZE one and at least for ARM64, that looks
resolved now IMO.


> Sascha

Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       |  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

  reply	other threads:[~2022-09-12 15:17 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-17 11:42 [PATCH v2 00/10] ARM: mmu: inhibit speculation into secure memory Ahmad Fatoum
2022-08-17 11:42 ` [PATCH v2 01/10] resource: add flags parameter to __request_region Ahmad Fatoum
2022-08-17 11:42 ` [PATCH v2 02/10] common: allow requesting SDRAM regions with custom flags Ahmad Fatoum
2022-08-17 11:42 ` [PATCH v2 03/10] memory: define reserve_sdram_region helper Ahmad Fatoum
2022-08-17 11:42 ` [PATCH v2 04/10] init: define new postmem_initcall() Ahmad Fatoum
2022-08-17 11:42 ` [PATCH v2 05/10] of: reserved-mem: reserve regions prior to mmu_initcall() Ahmad Fatoum
2022-08-17 11:42 ` [PATCH v2 06/10] ARM: mmu64: map reserved regions uncached Ahmad Fatoum
2022-08-17 11:42 ` [PATCH v2 07/10] ARM: mmu: define attrs_uncached_mem() helper Ahmad Fatoum
2022-08-17 11:42 ` [PATCH v2 08/10] ARM: mmu: use reserve mem entries to modify maps Ahmad Fatoum
2022-09-12 12:01   ` Sascha Hauer
2022-09-12 15:15     ` Ahmad Fatoum [this message]
2022-09-12 16:36       ` Sascha Hauer
2022-08-17 11:42 ` [PATCH v2 09/10] ARM: early-mmu: don't cache/prefetch OPTEE_SIZE bytes from end of memory Ahmad Fatoum
2022-08-17 11:42 ` [PATCH v2 10/10] commands: iomem: point out [R]eserved regions Ahmad Fatoum
2022-08-18 12:39 ` [PATCH v2 00/10] ARM: mmu: inhibit speculation into secure memory Sascha Hauer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox