Also I just compile tested this. find_pte() isn;t used anywhere anymore. For the next version, just add the argument to find_pte() instead of renaming the functions. It make the whole review process easier.
Thanks /Ilias On Thu, 8 Oct 2026 at 14:00, Ilias Apalodimas <[email protected]> wrote: > > Hi Ashwin, > > > On Sat, 26 Sept 2026 at 12:26, Aswin Murugan > <[email protected]> wrote: > > > > mmu_change_region_attr_nobreak() updates only the primary page > > table, leaving the emergency page table with stale attributes. > > The emergency table is used temporarily while the primary page > > table is being updated. > > > > As a result, speculative accesses can reach regions that were > > intended to be blocked in the primary table, leading to XPU > > violations. > > > > Update both primary and emergency page tables when modifying > > region attributes to keep mappings consistent regardless of the > > active translation table. > > > > Signed-off-by: Aswin Murugan <[email protected]> > > --- > > arch/arm/cpu/armv8/cache_v8.c | 29 +++++++++++++++++++++++------ > > 1 file changed, 23 insertions(+), 6 deletions(-) > > > > diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c > > index ad232435eb2..f351f11007d 100644 > > --- a/arch/arm/cpu/armv8/cache_v8.c > > +++ b/arch/arm/cpu/armv8/cache_v8.c > > @@ -173,7 +173,7 @@ static int level2shift(int level) > > return (12 + 9 * (3 - level)); > > } > > > > -static u64 *find_pte(u64 addr, int level) > > +static u64 *find_pte_tbl(u64 *root, u64 addr, int level) > > { > > int start_level = 0; > > u64 *pte; > > @@ -191,7 +191,7 @@ static u64 *find_pte(u64 addr, int level) > > return NULL; > > > > /* Walk through all page table levels to find our PTE */ > > - pte = (u64*)gd->arch.tlb_addr; > > + pte = root; > > for (i = start_level; i < 4; i++) { > > idx = (addr >> level2shift(i)) & 0x1FF; > > pte += idx; > > @@ -211,6 +211,11 @@ static u64 *find_pte(u64 addr, int level) > > return NULL; > > } > > > > +static u64 *find_pte(u64 addr, int level) > > +{ > > + return find_pte_tbl((u64 *)gd->arch.tlb_addr, addr, level); > > +} > > + > > #ifdef CONFIG_CMO_BY_VA_ONLY > > static void __cmo_on_leaves(void (*cmo_fn)(unsigned long, unsigned long), > > u64 pte, int level, u64 base) > > @@ -998,11 +1003,11 @@ static bool is_aligned(u64 addr, u64 size, u64 align) > > } > > > > /* Use flag to indicate if attrs has more than d-cache attributes */ > > -static u64 set_one_region(u64 start, u64 size, u64 attrs, bool flag, int > > level) > > +static u64 set_one_region_tbl(u64 *root, u64 start, u64 size, u64 attrs, > > bool flag, int level) > > { > > int levelshift = level2shift(level); > > u64 levelsize = 1ULL << levelshift; > > - u64 *pte = find_pte(start, level); > > + u64 *pte = find_pte_tbl(root, start, level); > > > > /* Can we can just modify the current level block/page? */ > > if (is_aligned(start, size, levelsize)) { > > @@ -1039,7 +1044,7 @@ static u64 set_one_region(u64 start, u64 size, u64 > > attrs, bool flag, int level) > > return 0; > > } > > > > -static void set_regions(u64 start, u64 size, u64 attrs, bool flag) > > +static void set_regions_tbl(u64 *root, u64 start, u64 size, u64 attrs, > > bool flag) > > { > > int level; > > u64 r; > > @@ -1050,7 +1055,7 @@ static void set_regions(u64 start, u64 size, u64 > > attrs, bool flag) > > */ > > while (size > 0) { > > for (level = 1; level < 4; level++) { > > - r = set_one_region(start, size, attrs, flag, level); > > + r = set_one_region_tbl(root, start, size, attrs, > > flag, level); > > if (r) { > > /* PTE successfully replaced */ > > size -= r; > > @@ -1061,6 +1066,11 @@ static void set_regions(u64 start, u64 size, u64 > > attrs, bool flag) > > } > > } > > > > +static void set_regions(u64 start, u64 size, u64 attrs, bool flag) > > +{ > > + set_regions_tbl((u64 *)gd->arch.tlb_addr, start, size, attrs, flag); > > +} > > + > > void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size, > > enum dcache_option option) > > { > > @@ -1095,6 +1105,13 @@ void mmu_set_region_dcache_behaviour(phys_addr_t > > start, size_t size, > > void mmu_change_region_attr_nobreak(phys_addr_t addr, size_t size, u64 > > attrs) > > { > > set_regions(addr, size, attrs, true); > > + > > + /* Apply the same change to the emergency table, keeping it in sync > > + * with the primary table. > > + */ > > + if (gd->arch.tlb_emerg) > > + set_regions_tbl((u64 *)gd->arch.tlb_emerg, addr, size, > > attrs, true); > > So the problem I see here is that emergency page table is a snapshot > of the original mem_map, that we only used to update in > mmu_map_region(). We now update it on every change and I think this is > the right approach. But are we sure get_page_table_size() will be > enough? The 4 used to reserve to accommodate dcache settings is a > random pick. What I fear is that some architectures, for example the > ones that enable PGPROT and create even more tables, will require more > space now. I think we should also increase the get_page_table_size > size calculation. > > Thanks > /Ilias > > > + > > flush_dcache_range(gd->arch.tlb_addr, > > gd->arch.tlb_addr + gd->arch.tlb_size); > > __asm_invalidate_tlb_all(); > > -- > > 2.34.1 > >
