Skip to content

core: mm: core_mmu_map_pages() walks the translation tables once per 4kB page #7946

Description

@Edward-HP

Environment

OP-TEE OS 4.7.0, Armv8-A / LPAE (CORE_MMU_PGDIR_SHIFT == 21). The code
is arch-independent, so v7 and RISC-V are affected the same way.

Problem

core_mmu_map_pages(), core_mmu_map_contiguous_pages() and
core_mmu_unmap_pages() in core/mm/core_mmu.c each loop over small
pages and call core_mmu_find_table() for every single one:

    for (i = 0; i < num_pages; i++) {
        ...
        while (true) {
            if (!core_mmu_find_table(NULL, vaddr, UINT_MAX,
                         &tbl_info))
                panic("Can't find pagetable for vaddr ");

            idx = core_mmu_va2idx(&tbl_info, vaddr);
            if (tbl_info.shift == SMALL_PAGE_SHIFT)
                break;
            /* This is supertable. Need to divide it. */
            ...
        }

The leaf translation table covering a VA only changes every
CORE_MMU_PGDIR_SIZE. For a 2MB pgdir that means 511 out of every 512
walks re-derive the exact same table. The while (true) loop only needs
to iterate when core_mmu_entry_to_finer_grained() had to split a
supertable and the new leaf table has to be reached; re-walking for every
page is a side effect of the loop's placement, not its purpose.

Why the walk is not cheap

Per call to core_mmu_find_table()
(core/arch/arm/mm/core_mmu_lpae.c):

  • thread_mask_exceptions(THREAD_EXCP_ALL) / thread_unmask_exceptions()
    — redundant here, since mmu_lock() uses cpu_spin_lock_xsave() and
    has already masked everything
  • get_core_pos(), i.e. an mpidr_el1 system register read
  • one core_mmu_xlat_table_entry_pa2va() per level descended, each doing
    one or two phys_to_virt() calls, each a linear scan of
    static_memory_map[]

With CFG_TEE_CORE_DEBUG=y every one of those phys_to_virt() calls also
runs check_va_matches_pa() -> virt_to_phys() -> arch_va2pa_helper(),
which issues an AT S1E1R plus an isb(). Mapping a single page then
costs several pipeline flushes.

All of this happens with mmu_spinlock held and all exceptions masked, so
it is an interrupt-latency problem for the whole system, not only a
throughput problem for the calling thread.

The user mapping path already avoids this

set_um_region() in core/mm/vm.c and set_pg_region() in
core/mm/core_mmu.c walk per CORE_MMU_PGDIR_SIZE and never call
core_mmu_find_table() in the page loop at all — they take the leaf table
from the pgt cache and then write entries with core_mmu_va2idx():

    for (ti.va_base = ROUNDDOWN(r->va, CORE_MMU_PGDIR_SIZE);
         ti.va_base < r->va + r->size;
         ti.va_base += CORE_MMU_PGDIR_SIZE) {

So the inefficiency is an inconsistency between the two mapping paths
rather than a missing optimisation.

Impact

Registering an 8MB non-secure shared buffer is 2048 pages, so 2048 table
walks, and the same again on unmap. Only 4 distinct leaf tables are
involved (8 walks if each pgdir entry still has to be split the first
time).

Affected callers:

  • mobj_ffa.c — every FF-A shared memory register / unregister
  • mobj_dyn_shm.c — dynamic shared memory registration
  • page_alloc.ccore_mmu_map_contiguous_pages() for core allocations
  • boot_mem.ccore_mmu_unmap_pages() at boot

Proposed fix

Keep the struct core_mmu_table_info across loop iterations and only walk
when the VA has left the cached table:

static bool tbl_info_covers_va(struct core_mmu_table_info *ti, vaddr_t va)
{
    return ti->table &&
           va - ti->va_base < BIT64(ti->shift) * ti->num_entries;
}

and turn while (true) into while (!tbl_info_covers_va(&tbl_info, vaddr)),
clearing tbl_info.table after a successful
core_mmu_entry_to_finer_grained() so the divided table is reached by a
fresh walk. The cached table is always a small-page leaf table, because
that is the only way the loop can exit, so no supertable is ever reused.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions