| Message ID | 20250301164922.397441-5-ilias.apalodimas@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Fix page permission on arm64 architectures | expand |
On Sat, 1 Mar 2025 18:49:02 +0200 Ilias Apalodimas <ilias.apalodimas@linaro.org> wrote: Hi Ilias, > The ARM ARM on section 8.17.1 describes the cases where Thanks for referencing the ARM ARM! The section name would be D8.17.1, and please mention the version of the document, since the numbering is not stable across the different releases. So something like: The ARM ARM (Rev L.a) in section D8.17.1 ("Using break-before-make when updating translation table entries") ... > break-before-make is required when changing live page tables. > Since we can use this function to tweak block and page permssions, > where BBM is not required add an extra argument to the function. It looks like one of the two existing users of this function (the snapdragon code) just calls this function with attr=PTE_TYPE_FAULT, so that would not need to call the full BBM version. The Layerscape code indeed requires it, though. > While at it add a function description. I think these sentences describe the v2 and older, but don't fit the current patch anymore. > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > --- > arch/arm/cpu/armv8/cache_v8.c | 48 ++++++++++++++++++++--------------- > arch/arm/include/asm/system.h | 18 +++++++++++++ > 2 files changed, 46 insertions(+), 20 deletions(-) > > diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c > index c4b3da4a8da7..29100913bc82 100644 > --- a/arch/arm/cpu/armv8/cache_v8.c > +++ b/arch/arm/cpu/armv8/cache_v8.c > @@ -967,61 +967,69 @@ void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size, > flush_dcache_range(real_start, real_start + real_size); > } > > -/* > - * Modify MMU table for a region with updated PXN/UXN/Memory type/valid bits. > - * The procecess is break-before-make. The target region will be marked as > - * invalid during the process of changing. > - */ > -void mmu_change_region_attr(phys_addr_t addr, size_t siz, u64 attrs) > +void mmu_change_region_attr_nobreak(phys_addr_t addr, size_t siz, u64 attrs) > { > int level; > u64 r, size, start; > > - start = addr; > - size = siz; > /* > * Loop through the address range until we find a page granule that fits > - * our alignment constraints, then set it to "invalid". > + * our alignment constraints, then set it to the new cache attributes It's not cache attributes (those would require BBM), but "permissions" or just attributes. Otherwise this looks correct, just changing the permission indeed does not require BBM, so it's just the comment and the commit message that might need changing. And clever function split up! Cheers, Andre > */ > + start = addr; > + size = siz; > while (size > 0) { > for (level = 1; level < 4; level++) { > - /* Set PTE to fault */ > - r = set_one_region(start, size, PTE_TYPE_FAULT, true, > - level); > + /* Set PTE to new attributes */ > + r = set_one_region(start, size, attrs, true, level); > if (r) { > - /* PTE successfully invalidated */ > + /* PTE successfully updated */ > size -= r; > start += r; > break; > } > } > } > - > flush_dcache_range(gd->arch.tlb_addr, > gd->arch.tlb_addr + gd->arch.tlb_size); > __asm_invalidate_tlb_all(); > +} > + > +/* > + * Modify MMU table for a region with updated PXN/UXN/Memory type/valid bits. > + * The procecess is break-before-make. The target region will be marked as > + * invalid during the process of changing. > + */ > +void mmu_change_region_attr(phys_addr_t addr, size_t siz, u64 attrs) > +{ > + int level; > + u64 r, size, start; > > + start = addr; > + size = siz; > /* > * Loop through the address range until we find a page granule that fits > - * our alignment constraints, then set it to the new cache attributes > + * our alignment constraints, then set it to "invalid". > */ > - start = addr; > - size = siz; > while (size > 0) { > for (level = 1; level < 4; level++) { > - /* Set PTE to new attributes */ > - r = set_one_region(start, size, attrs, true, level); > + /* Set PTE to fault */ > + r = set_one_region(start, size, PTE_TYPE_FAULT, true, > + level); > if (r) { > - /* PTE successfully updated */ > + /* PTE successfully invalidated */ > size -= r; > start += r; > break; > } > } > } > + > flush_dcache_range(gd->arch.tlb_addr, > gd->arch.tlb_addr + gd->arch.tlb_size); > __asm_invalidate_tlb_all(); > + > + mmu_change_region_attr_nobreak(addr, siz, attrs); > } > > #else /* !CONFIG_IS_ENABLED(SYS_DCACHE_OFF) */ > diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h > index 091082281c73..849b3d0efb7a 100644 > --- a/arch/arm/include/asm/system.h > +++ b/arch/arm/include/asm/system.h > @@ -303,8 +303,26 @@ void flush_l3_cache(void); > * @emerg: Also map the region in the emergency table > */ > void mmu_map_region(phys_addr_t start, u64 size, bool emerg); > + > +/** > + * mmu_change_region_attr() - change a mapped region attributes > + * > + * @start: Start address of the region > + * @size: Size of the region > + * @aatrs: New attributes > + */ > void mmu_change_region_attr(phys_addr_t start, size_t size, u64 attrs); > > +/** > + * mmu_change_region_attr_nobreak() - change a mapped region attributes without doing > + * break-before-make > + * > + * @start: Start address of the region > + * @size: Size of the region > + * @aatrs: New attributes > + */ > +void mmu_change_region_attr_nobreak(phys_addr_t addr, size_t size, u64 attrs); > + > /* > * smc_call() - issue a secure monitor call > *
diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c index c4b3da4a8da7..29100913bc82 100644 --- a/arch/arm/cpu/armv8/cache_v8.c +++ b/arch/arm/cpu/armv8/cache_v8.c @@ -967,61 +967,69 @@ void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size, flush_dcache_range(real_start, real_start + real_size); } -/* - * Modify MMU table for a region with updated PXN/UXN/Memory type/valid bits. - * The procecess is break-before-make. The target region will be marked as - * invalid during the process of changing. - */ -void mmu_change_region_attr(phys_addr_t addr, size_t siz, u64 attrs) +void mmu_change_region_attr_nobreak(phys_addr_t addr, size_t siz, u64 attrs) { int level; u64 r, size, start; - start = addr; - size = siz; /* * Loop through the address range until we find a page granule that fits - * our alignment constraints, then set it to "invalid". + * our alignment constraints, then set it to the new cache attributes */ + start = addr; + size = siz; while (size > 0) { for (level = 1; level < 4; level++) { - /* Set PTE to fault */ - r = set_one_region(start, size, PTE_TYPE_FAULT, true, - level); + /* Set PTE to new attributes */ + r = set_one_region(start, size, attrs, true, level); if (r) { - /* PTE successfully invalidated */ + /* PTE successfully updated */ size -= r; start += r; break; } } } - flush_dcache_range(gd->arch.tlb_addr, gd->arch.tlb_addr + gd->arch.tlb_size); __asm_invalidate_tlb_all(); +} + +/* + * Modify MMU table for a region with updated PXN/UXN/Memory type/valid bits. + * The procecess is break-before-make. The target region will be marked as + * invalid during the process of changing. + */ +void mmu_change_region_attr(phys_addr_t addr, size_t siz, u64 attrs) +{ + int level; + u64 r, size, start; + start = addr; + size = siz; /* * Loop through the address range until we find a page granule that fits - * our alignment constraints, then set it to the new cache attributes + * our alignment constraints, then set it to "invalid". */ - start = addr; - size = siz; while (size > 0) { for (level = 1; level < 4; level++) { - /* Set PTE to new attributes */ - r = set_one_region(start, size, attrs, true, level); + /* Set PTE to fault */ + r = set_one_region(start, size, PTE_TYPE_FAULT, true, + level); if (r) { - /* PTE successfully updated */ + /* PTE successfully invalidated */ size -= r; start += r; break; } } } + flush_dcache_range(gd->arch.tlb_addr, gd->arch.tlb_addr + gd->arch.tlb_size); __asm_invalidate_tlb_all(); + + mmu_change_region_attr_nobreak(addr, siz, attrs); } #else /* !CONFIG_IS_ENABLED(SYS_DCACHE_OFF) */ diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h index 091082281c73..849b3d0efb7a 100644 --- a/arch/arm/include/asm/system.h +++ b/arch/arm/include/asm/system.h @@ -303,8 +303,26 @@ void flush_l3_cache(void); * @emerg: Also map the region in the emergency table */ void mmu_map_region(phys_addr_t start, u64 size, bool emerg); + +/** + * mmu_change_region_attr() - change a mapped region attributes + * + * @start: Start address of the region + * @size: Size of the region + * @aatrs: New attributes + */ void mmu_change_region_attr(phys_addr_t start, size_t size, u64 attrs); +/** + * mmu_change_region_attr_nobreak() - change a mapped region attributes without doing + * break-before-make + * + * @start: Start address of the region + * @size: Size of the region + * @aatrs: New attributes + */ +void mmu_change_region_attr_nobreak(phys_addr_t addr, size_t size, u64 attrs); + /* * smc_call() - issue a secure monitor call *
The ARM ARM on section 8.17.1 describes the cases where break-before-make is required when changing live page tables. Since we can use this function to tweak block and page permssions, where BBM is not required add an extra argument to the function. While at it add a function description. Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> --- arch/arm/cpu/armv8/cache_v8.c | 48 ++++++++++++++++++++--------------- arch/arm/include/asm/system.h | 18 +++++++++++++ 2 files changed, 46 insertions(+), 20 deletions(-)