Page MenuHomeFreeBSD

Switch to an empty ttbr0 pagetable when the MMU is enabled
Needs ReviewPublic

Authored by andrew on Fri, Nov 29, 3:51 PM.

Details

Reviewers
manu
alc
markj
mmel
Group Reviewers
arm64
Summary

We don't need these pagetables after the early boot. Remove the chance
we write to memory we didn't expect to and remove architectural undefined
behaviour.

Diff Detail

Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 27978
Build 26144: arc lint + arc unit

Event Timeline

andrew created this revision.Fri, Nov 29, 3:51 PM
alc accepted this revision.Fri, Nov 29, 7:20 PM

Feel free to cross this task off from the list at https://wiki.freebsd.org/PmapReview :-)

This revision is now accepted and ready to land.Fri, Nov 29, 7:20 PM
mmel accepted this revision.Sat, Nov 30, 11:03 AM
mmel added a subscriber: mmel.

Tested on RockPro64 an Tegra TX1 without problems. Unfortunately, it doesn't affect RK3399 issues in either way...

alc added inline comments.Sat, Nov 30, 6:27 PM
sys/arm64/arm64/locore.S
611

Should we have a dsb instruction here to ensure that the invalidation has completed before setting the SCTLR register below?

mmel added a comment.Sun, Dec 1, 10:55 AM

i just realized that this breaks EARLY_UART because SOCDEV_PA mapping is not passed to initarm().
So I think that we should postpone change of kernel ttbt0 until cninit() is processed.

@@ -1164,6 +1164,7 @@ initarm(struct arm64_bootparams *abp)
        valid = bus_probe();

        cninit();
+       set_ttbr0((uint64_t)pagetable_l0_ttbr0 - abp->kern_delta);

        if (!valid)
                panic("Invalid bus configuration: %s",

Also, summary is slightly misleading. This patch doesn't fully remove architectural undefined behavior (and I'm able to demonstrate this on RK3399) because it still exist - same physical memory is mapped in identity map as uncached and in kernel map as cached. This is direct violation of B2.9 in ARMv8 ARM, and it causes lost of coherency visible later in boot process.
Moreover, same problem (Mismatched memory attributes) exist also for DMAPed memory, memory attribute changes are not propagated to it. Worstly, only legal way how to change attribute for page mapped to multiple VA is break before make approach (thus we should unmaps all VA first ...)

alc added a comment.Mon, Dec 2, 7:16 PM
In D22606#494706, @mmel wrote:

i just realized that this breaks EARLY_UART because SOCDEV_PA mapping is not passed to initarm().
So I think that we should postpone change of kernel ttbt0 until cninit() is processed.

@@ -1164,6 +1164,7 @@ initarm(struct arm64_bootparams *abp)
        valid = bus_probe();
        cninit();
+       set_ttbr0((uint64_t)pagetable_l0_ttbr0 - abp->kern_delta);
        if (!valid)
                panic("Invalid bus configuration: %s",

Also, summary is slightly misleading. This patch doesn't fully remove architectural undefined behavior (and I'm able to demonstrate this on RK3399) because it still exist - same physical memory is mapped in identity map as uncached and in kernel map as cached. This is direct violation of B2.9 in ARMv8 ARM, and it causes lost of coherency visible later in boot process.

Why does locore.S set up the identity mapping as uncached? I've never come across an explanation. Does it overlap with one or more devices on some platforms?

andrew updated this revision to Diff 65262.Thu, Dec 5, 1:24 PM

Switch to the empty ttbr0 after cninit is called

This revision now requires review to proceed.Thu, Dec 5, 1:24 PM
alc added inline comments.Fri, Dec 6, 4:46 PM
sys/arm64/arm64/machdep.c
1164

This is replacing the page table that is associated with ASID 0, so it needs to be followed by a complete TLB invalidation.