| Message ID | 20250130072100.27297-4-ilias.apalodimas@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Fix page permission on arm64 architectures | expand |
Hi Ilias, On 1/30/25 08:20, Ilias Apalodimas wrote: > 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 we can use to change the attributes of normal > memory without breaking the pages tables first > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > --- > arch/arm/cpu/armv8/cache_v8.c | 27 ++++++++++++++++++++++++- > arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 10 ++++----- > arch/arm/include/asm/system.h | 3 ++- > arch/arm/mach-snapdragon/board.c | 2 +- > 4 files changed, 34 insertions(+), 8 deletions(-) > > diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c > index a4ca56c8ed42..60d0870013e8 100644 > --- a/arch/arm/cpu/armv8/cache_v8.c > +++ b/arch/arm/cpu/armv8/cache_v8.c > @@ -968,11 +968,14 @@ void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size, > * 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(phys_addr_t addr, size_t siz, u64 attrs, bool bbm) > { > int level; > u64 r, size, start; > > + if (!bbm) > + goto skip_bbm; > + > start = addr; > size = siz; > /* > @@ -997,6 +1000,7 @@ void mmu_change_region_attr(phys_addr_t addr, size_t siz, u64 attrs) > gd->arch.tlb_addr + gd->arch.tlb_size); > __asm_invalidate_tlb_all(); > > +skip_bbm: > /* > * Loop through the address range until we find a page granule that fits > * our alignment constraints, then set it to the new cache attributes > @@ -1020,6 +1024,27 @@ void mmu_change_region_attr(phys_addr_t addr, size_t siz, u64 attrs) > __asm_invalidate_tlb_all(); > } > > +void mmu_set_attrs(phys_addr_t addr, size_t size, int type) type should be an enum > +{ > + u64 attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) | PTE_BLOCK_INNER_SHARE | PTE_TYPE_VALID; > + > + switch (type) { > + case 1: > + /* RO */ > + attrs |= PTE_BLOCK_PXN | PTE_BLOCK_UXN | PTE_BLOCK_RO; > + break; > + case 2: > + /* RX */ > + attrs |= PTE_BLOCK_RO; > + break; > + case 3: > + /* RW */ > + attrs |= PTE_BLOCK_PXN | PTE_BLOCK_UXN; > + } > + > + mmu_change_region_attr(addr, size, attrs, false); > +} > + > #else /* !CONFIG_IS_ENABLED(SYS_DCACHE_OFF) */ > > /* > diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c > index d2d3e346a36f..caf1dab05936 100644 > --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c > +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c > @@ -1573,7 +1573,7 @@ void update_early_mmu_table(void) > PTE_BLOCK_MEMTYPE(MT_NORMAL) | > PTE_BLOCK_OUTER_SHARE | > PTE_BLOCK_NS | > - PTE_TYPE_VALID); > + PTE_TYPE_VALID, true); > } else { > mmu_change_region_attr( > CFG_SYS_SDRAM_BASE, > @@ -1581,7 +1581,7 @@ void update_early_mmu_table(void) > PTE_BLOCK_MEMTYPE(MT_NORMAL) | > PTE_BLOCK_OUTER_SHARE | > PTE_BLOCK_NS | > - PTE_TYPE_VALID); > + PTE_TYPE_VALID, true); > #ifdef CONFIG_SYS_DDR_BLOCK3_BASE > #ifndef CONFIG_SYS_DDR_BLOCK2_SIZE > #error "Missing CONFIG_SYS_DDR_BLOCK2_SIZE" > @@ -1594,7 +1594,7 @@ void update_early_mmu_table(void) > PTE_BLOCK_MEMTYPE(MT_NORMAL) | > PTE_BLOCK_OUTER_SHARE | > PTE_BLOCK_NS | > - PTE_TYPE_VALID); > + PTE_TYPE_VALID, true); > mmu_change_region_attr( > CONFIG_SYS_DDR_BLOCK3_BASE, > gd->ram_size - > @@ -1603,7 +1603,7 @@ void update_early_mmu_table(void) > PTE_BLOCK_MEMTYPE(MT_NORMAL) | > PTE_BLOCK_OUTER_SHARE | > PTE_BLOCK_NS | > - PTE_TYPE_VALID); > + PTE_TYPE_VALID, true); > } else > #endif > { > @@ -1614,7 +1614,7 @@ void update_early_mmu_table(void) > PTE_BLOCK_MEMTYPE(MT_NORMAL) | > PTE_BLOCK_OUTER_SHARE | > PTE_BLOCK_NS | > - PTE_TYPE_VALID); > + PTE_TYPE_VALID, true); > } > } > } > diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h > index dbf9ab43e280..0a8ca19e45e9 100644 > --- a/arch/arm/include/asm/system.h > +++ b/arch/arm/include/asm/system.h > @@ -287,8 +287,9 @@ 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); > -void mmu_change_region_attr(phys_addr_t start, size_t size, u64 attrs); > +void mmu_change_region_attr(phys_addr_t start, size_t size, u64 attrs, bool bbm); > > +void mmu_set_attrs(phys_addr_t addr, size_t size, int type); > /* > * smc_call() - issue a secure monitor call > * > diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c > index 2ef936aab757..13f4e8e640ef 100644 > --- a/arch/arm/mach-snapdragon/board.c > +++ b/arch/arm/mach-snapdragon/board.c > @@ -577,7 +577,7 @@ static void carve_out_reserved_memory(void) > if (i == count || start + size < res[i].start - SZ_2M) { > debug(" 0x%016llx - 0x%016llx: reserved\n", > start, start + size); > - mmu_change_region_attr(start, size, PTE_TYPE_FAULT); > + mmu_change_region_attr(start, size, PTE_TYPE_FAULT, true); > /* If this is the final region then quit here before we index > * out of bounds... > */
Hi Jerome, On Thu, 30 Jan 2025 at 11:11, Jerome Forissier <jerome.forissier@linaro.org> wrote: > > Hi Ilias, > > On 1/30/25 08:20, Ilias Apalodimas wrote: > > 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 we can use to change the attributes of normal > > memory without breaking the pages tables first > > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > --- > > arch/arm/cpu/armv8/cache_v8.c | 27 ++++++++++++++++++++++++- > > arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 10 ++++----- > > arch/arm/include/asm/system.h | 3 ++- > > arch/arm/mach-snapdragon/board.c | 2 +- > > 4 files changed, 34 insertions(+), 8 deletions(-) > > > > diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c > > index a4ca56c8ed42..60d0870013e8 100644 > > --- a/arch/arm/cpu/armv8/cache_v8.c > > +++ b/arch/arm/cpu/armv8/cache_v8.c > > @@ -968,11 +968,14 @@ void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size, > > * 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(phys_addr_t addr, size_t siz, u64 attrs, bool bbm) > > { > > int level; > > u64 r, size, start; > > > > + if (!bbm) > > + goto skip_bbm; > > + > > start = addr; > > size = siz; > > /* > > @@ -997,6 +1000,7 @@ void mmu_change_region_attr(phys_addr_t addr, size_t siz, u64 attrs) > > gd->arch.tlb_addr + gd->arch.tlb_size); > > __asm_invalidate_tlb_all(); > > > > +skip_bbm: > > /* > > * Loop through the address range until we find a page granule that fits > > * our alignment constraints, then set it to the new cache attributes > > @@ -1020,6 +1024,27 @@ void mmu_change_region_attr(phys_addr_t addr, size_t siz, u64 attrs) > > __asm_invalidate_tlb_all(); > > } > > > > +void mmu_set_attrs(phys_addr_t addr, size_t size, int type) > > type should be an enum yea, I mentioned that in the cover letter. Thanks! /Ilias > > > +{ > > + u64 attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) | PTE_BLOCK_INNER_SHARE | PTE_TYPE_VALID; > > + > > + switch (type) { > > + case 1: > > + /* RO */ > > + attrs |= PTE_BLOCK_PXN | PTE_BLOCK_UXN | PTE_BLOCK_RO; > > + break; > > + case 2: > > + /* RX */ > > + attrs |= PTE_BLOCK_RO; > > + break; > > + case 3: > > + /* RW */ > > + attrs |= PTE_BLOCK_PXN | PTE_BLOCK_UXN; > > + } > > + > > + mmu_change_region_attr(addr, size, attrs, false); > > +} > > + > > #else /* !CONFIG_IS_ENABLED(SYS_DCACHE_OFF) */ > > > > /* > > diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c > > index d2d3e346a36f..caf1dab05936 100644 > > --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c > > +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c > > @@ -1573,7 +1573,7 @@ void update_early_mmu_table(void) > > PTE_BLOCK_MEMTYPE(MT_NORMAL) | > > PTE_BLOCK_OUTER_SHARE | > > PTE_BLOCK_NS | > > - PTE_TYPE_VALID); > > + PTE_TYPE_VALID, true); > > } else { > > mmu_change_region_attr( > > CFG_SYS_SDRAM_BASE, > > @@ -1581,7 +1581,7 @@ void update_early_mmu_table(void) > > PTE_BLOCK_MEMTYPE(MT_NORMAL) | > > PTE_BLOCK_OUTER_SHARE | > > PTE_BLOCK_NS | > > - PTE_TYPE_VALID); > > + PTE_TYPE_VALID, true); > > #ifdef CONFIG_SYS_DDR_BLOCK3_BASE > > #ifndef CONFIG_SYS_DDR_BLOCK2_SIZE > > #error "Missing CONFIG_SYS_DDR_BLOCK2_SIZE" > > @@ -1594,7 +1594,7 @@ void update_early_mmu_table(void) > > PTE_BLOCK_MEMTYPE(MT_NORMAL) | > > PTE_BLOCK_OUTER_SHARE | > > PTE_BLOCK_NS | > > - PTE_TYPE_VALID); > > + PTE_TYPE_VALID, true); > > mmu_change_region_attr( > > CONFIG_SYS_DDR_BLOCK3_BASE, > > gd->ram_size - > > @@ -1603,7 +1603,7 @@ void update_early_mmu_table(void) > > PTE_BLOCK_MEMTYPE(MT_NORMAL) | > > PTE_BLOCK_OUTER_SHARE | > > PTE_BLOCK_NS | > > - PTE_TYPE_VALID); > > + PTE_TYPE_VALID, true); > > } else > > #endif > > { > > @@ -1614,7 +1614,7 @@ void update_early_mmu_table(void) > > PTE_BLOCK_MEMTYPE(MT_NORMAL) | > > PTE_BLOCK_OUTER_SHARE | > > PTE_BLOCK_NS | > > - PTE_TYPE_VALID); > > + PTE_TYPE_VALID, true); > > } > > } > > } > > diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h > > index dbf9ab43e280..0a8ca19e45e9 100644 > > --- a/arch/arm/include/asm/system.h > > +++ b/arch/arm/include/asm/system.h > > @@ -287,8 +287,9 @@ 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); > > -void mmu_change_region_attr(phys_addr_t start, size_t size, u64 attrs); > > +void mmu_change_region_attr(phys_addr_t start, size_t size, u64 attrs, bool bbm); > > > > +void mmu_set_attrs(phys_addr_t addr, size_t size, int type); > > /* > > * smc_call() - issue a secure monitor call > > * > > diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c > > index 2ef936aab757..13f4e8e640ef 100644 > > --- a/arch/arm/mach-snapdragon/board.c > > +++ b/arch/arm/mach-snapdragon/board.c > > @@ -577,7 +577,7 @@ static void carve_out_reserved_memory(void) > > if (i == count || start + size < res[i].start - SZ_2M) { > > debug(" 0x%016llx - 0x%016llx: reserved\n", > > start, start + size); > > - mmu_change_region_attr(start, size, PTE_TYPE_FAULT); > > + mmu_change_region_attr(start, size, PTE_TYPE_FAULT, true); > > /* If this is the final region then quit here before we index > > * out of bounds... > > */
On 1/30/25 10:20, Ilias Apalodimas wrote: > Hi Jerome, > > On Thu, 30 Jan 2025 at 11:11, Jerome Forissier > <jerome.forissier@linaro.org> wrote: >> >> Hi Ilias, >> >> On 1/30/25 08:20, Ilias Apalodimas wrote: >>> 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 we can use to change the attributes of normal >>> memory without breaking the pages tables first >>> >>> Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> >>> --- >>> arch/arm/cpu/armv8/cache_v8.c | 27 ++++++++++++++++++++++++- >>> arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 10 ++++----- >>> arch/arm/include/asm/system.h | 3 ++- >>> arch/arm/mach-snapdragon/board.c | 2 +- >>> 4 files changed, 34 insertions(+), 8 deletions(-) >>> >>> diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c >>> index a4ca56c8ed42..60d0870013e8 100644 >>> --- a/arch/arm/cpu/armv8/cache_v8.c >>> +++ b/arch/arm/cpu/armv8/cache_v8.c >>> @@ -968,11 +968,14 @@ void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size, >>> * 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(phys_addr_t addr, size_t siz, u64 attrs, bool bbm) >>> { >>> int level; >>> u64 r, size, start; >>> >>> + if (!bbm) >>> + goto skip_bbm; >>> + >>> start = addr; >>> size = siz; >>> /* >>> @@ -997,6 +1000,7 @@ void mmu_change_region_attr(phys_addr_t addr, size_t siz, u64 attrs) >>> gd->arch.tlb_addr + gd->arch.tlb_size); >>> __asm_invalidate_tlb_all(); >>> >>> +skip_bbm: >>> /* >>> * Loop through the address range until we find a page granule that fits >>> * our alignment constraints, then set it to the new cache attributes >>> @@ -1020,6 +1024,27 @@ void mmu_change_region_attr(phys_addr_t addr, size_t siz, u64 attrs) >>> __asm_invalidate_tlb_all(); >>> } >>> >>> +void mmu_set_attrs(phys_addr_t addr, size_t size, int type) >> >> type should be an enum > > yea, I mentioned that in the cover letter. ...which for some reason I did not get :-/ I found it on the ML archive. Thanks,
On 1/30/25 07:20, Ilias Apalodimas wrote: > 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 we can use to change the attributes of normal > memory without breaking the pages tables first > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > --- > arch/arm/cpu/armv8/cache_v8.c | 27 ++++++++++++++++++++++++- > arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 10 ++++----- > arch/arm/include/asm/system.h | 3 ++- > arch/arm/mach-snapdragon/board.c | 2 +- > 4 files changed, 34 insertions(+), 8 deletions(-) > > diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c > index a4ca56c8ed42..60d0870013e8 100644 > --- a/arch/arm/cpu/armv8/cache_v8.c > +++ b/arch/arm/cpu/armv8/cache_v8.c > @@ -968,11 +968,14 @@ void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size, > * 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(phys_addr_t addr, size_t siz, u64 attrs, bool bbm) The function description should be moved to the include or be completed here. > { > int level; > u64 r, size, start; > > + if (!bbm) > + goto skip_bbm; > + > start = addr; > size = siz; > /* > @@ -997,6 +1000,7 @@ void mmu_change_region_attr(phys_addr_t addr, size_t siz, u64 attrs) > gd->arch.tlb_addr + gd->arch.tlb_size); > __asm_invalidate_tlb_all(); > > +skip_bbm: > /* > * Loop through the address range until we find a page granule that fits > * our alignment constraints, then set it to the new cache attributes > @@ -1020,6 +1024,27 @@ void mmu_change_region_attr(phys_addr_t addr, size_t siz, u64 attrs) > __asm_invalidate_tlb_all(); > } > > +void mmu_set_attrs(phys_addr_t addr, size_t size, int type) Please, add a function description. The function should either be static or added to an include file. > +{ > + u64 attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) | PTE_BLOCK_INNER_SHARE | PTE_TYPE_VALID; > + > + switch (type) { > + case 1: > + /* RO */ > + attrs |= PTE_BLOCK_PXN | PTE_BLOCK_UXN | PTE_BLOCK_RO; > + break; > + case 2: > + /* RX */ > + attrs |= PTE_BLOCK_RO; > + break; > + case 3: > + /* RW */ > + attrs |= PTE_BLOCK_PXN | PTE_BLOCK_UXN; > + } > + > + mmu_change_region_attr(addr, size, attrs, false); > +} > + > #else /* !CONFIG_IS_ENABLED(SYS_DCACHE_OFF) */ > > /* > diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c > index d2d3e346a36f..caf1dab05936 100644 > --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c > +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c > @@ -1573,7 +1573,7 @@ void update_early_mmu_table(void) > PTE_BLOCK_MEMTYPE(MT_NORMAL) | > PTE_BLOCK_OUTER_SHARE | > PTE_BLOCK_NS | > - PTE_TYPE_VALID); > + PTE_TYPE_VALID, true); > } else { > mmu_change_region_attr( > CFG_SYS_SDRAM_BASE, > @@ -1581,7 +1581,7 @@ void update_early_mmu_table(void) > PTE_BLOCK_MEMTYPE(MT_NORMAL) | > PTE_BLOCK_OUTER_SHARE | > PTE_BLOCK_NS | > - PTE_TYPE_VALID); > + PTE_TYPE_VALID, true); > #ifdef CONFIG_SYS_DDR_BLOCK3_BASE > #ifndef CONFIG_SYS_DDR_BLOCK2_SIZE > #error "Missing CONFIG_SYS_DDR_BLOCK2_SIZE" > @@ -1594,7 +1594,7 @@ void update_early_mmu_table(void) > PTE_BLOCK_MEMTYPE(MT_NORMAL) | > PTE_BLOCK_OUTER_SHARE | > PTE_BLOCK_NS | > - PTE_TYPE_VALID); > + PTE_TYPE_VALID, true); > mmu_change_region_attr( > CONFIG_SYS_DDR_BLOCK3_BASE, > gd->ram_size - > @@ -1603,7 +1603,7 @@ void update_early_mmu_table(void) > PTE_BLOCK_MEMTYPE(MT_NORMAL) | > PTE_BLOCK_OUTER_SHARE | > PTE_BLOCK_NS | > - PTE_TYPE_VALID); > + PTE_TYPE_VALID, true); > } else > #endif > { > @@ -1614,7 +1614,7 @@ void update_early_mmu_table(void) > PTE_BLOCK_MEMTYPE(MT_NORMAL) | > PTE_BLOCK_OUTER_SHARE | > PTE_BLOCK_NS | > - PTE_TYPE_VALID); > + PTE_TYPE_VALID, true); > } > } > } > diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h > index dbf9ab43e280..0a8ca19e45e9 100644 > --- a/arch/arm/include/asm/system.h > +++ b/arch/arm/include/asm/system.h > @@ -287,8 +287,9 @@ 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); > -void mmu_change_region_attr(phys_addr_t start, size_t size, u64 attrs); > +void mmu_change_region_attr(phys_addr_t start, size_t size, u64 attrs, bool bbm); Please, add a function description. Best regards Heinrich > > +void mmu_set_attrs(phys_addr_t addr, size_t size, int type); > /* > * smc_call() - issue a secure monitor call > * > diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c > index 2ef936aab757..13f4e8e640ef 100644 > --- a/arch/arm/mach-snapdragon/board.c > +++ b/arch/arm/mach-snapdragon/board.c > @@ -577,7 +577,7 @@ static void carve_out_reserved_memory(void) > if (i == count || start + size < res[i].start - SZ_2M) { > debug(" 0x%016llx - 0x%016llx: reserved\n", > start, start + size); > - mmu_change_region_attr(start, size, PTE_TYPE_FAULT); > + mmu_change_region_attr(start, size, PTE_TYPE_FAULT, true); > /* If this is the final region then quit here before we index > * out of bounds... > */
On Thu, 30 Jan 2025 at 12:13, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 1/30/25 07:20, Ilias Apalodimas wrote: > > 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 we can use to change the attributes of normal > > memory without breaking the pages tables first > > > > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > > --- > > arch/arm/cpu/armv8/cache_v8.c | 27 ++++++++++++++++++++++++- > > arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 10 ++++----- > > arch/arm/include/asm/system.h | 3 ++- > > arch/arm/mach-snapdragon/board.c | 2 +- > > 4 files changed, 34 insertions(+), 8 deletions(-) > > > > diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c > > index a4ca56c8ed42..60d0870013e8 100644 > > --- a/arch/arm/cpu/armv8/cache_v8.c > > +++ b/arch/arm/cpu/armv8/cache_v8.c > > @@ -968,11 +968,14 @@ void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size, > > * 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(phys_addr_t addr, size_t siz, u64 attrs, bool bbm) > > The function description should be moved to the include or be completed > here. > That's a preexisting function, but I can move it around on the next iteration > > { > > int level; > > u64 r, size, start; > > > > + if (!bbm) > > + goto skip_bbm; > > + > > start = addr; > > size = siz; > > /* > > @@ -997,6 +1000,7 @@ void mmu_change_region_attr(phys_addr_t addr, size_t siz, u64 attrs) > > gd->arch.tlb_addr + gd->arch.tlb_size); > > __asm_invalidate_tlb_all(); > > > > +skip_bbm: > > /* > > * Loop through the address range until we find a page granule that fits > > * our alignment constraints, then set it to the new cache attributes > > @@ -1020,6 +1024,27 @@ void mmu_change_region_attr(phys_addr_t addr, size_t siz, u64 attrs) > > __asm_invalidate_tlb_all(); > > } > > > > +void mmu_set_attrs(phys_addr_t addr, size_t size, int type) > > Please, add a function description. > > The function should either be static or added to an include file. Yea I will > > > +{ > > + u64 attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) | PTE_BLOCK_INNER_SHARE | PTE_TYPE_VALID; > > + > > + switch (type) { > > + case 1: > > + /* RO */ > > + attrs |= PTE_BLOCK_PXN | PTE_BLOCK_UXN | PTE_BLOCK_RO; > > + break; > > + case 2: > > + /* RX */ > > + attrs |= PTE_BLOCK_RO; > > + break; > > + case 3: > > + /* RW */ > > + attrs |= PTE_BLOCK_PXN | PTE_BLOCK_UXN; > > + } > > + > > + mmu_change_region_attr(addr, size, attrs, false); > > +} > > + > > #else /* !CONFIG_IS_ENABLED(SYS_DCACHE_OFF) */ > > > > /* > > diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c > > index d2d3e346a36f..caf1dab05936 100644 > > --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c > > +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c > > @@ -1573,7 +1573,7 @@ void update_early_mmu_table(void) > > PTE_BLOCK_MEMTYPE(MT_NORMAL) | > > PTE_BLOCK_OUTER_SHARE | > > PTE_BLOCK_NS | > > - PTE_TYPE_VALID); > > + PTE_TYPE_VALID, true); > > } else { > > mmu_change_region_attr( > > CFG_SYS_SDRAM_BASE, > > @@ -1581,7 +1581,7 @@ void update_early_mmu_table(void) > > PTE_BLOCK_MEMTYPE(MT_NORMAL) | > > PTE_BLOCK_OUTER_SHARE | > > PTE_BLOCK_NS | > > - PTE_TYPE_VALID); > > + PTE_TYPE_VALID, true); > > #ifdef CONFIG_SYS_DDR_BLOCK3_BASE > > #ifndef CONFIG_SYS_DDR_BLOCK2_SIZE > > #error "Missing CONFIG_SYS_DDR_BLOCK2_SIZE" > > @@ -1594,7 +1594,7 @@ void update_early_mmu_table(void) > > PTE_BLOCK_MEMTYPE(MT_NORMAL) | > > PTE_BLOCK_OUTER_SHARE | > > PTE_BLOCK_NS | > > - PTE_TYPE_VALID); > > + PTE_TYPE_VALID, true); > > mmu_change_region_attr( > > CONFIG_SYS_DDR_BLOCK3_BASE, > > gd->ram_size - > > @@ -1603,7 +1603,7 @@ void update_early_mmu_table(void) > > PTE_BLOCK_MEMTYPE(MT_NORMAL) | > > PTE_BLOCK_OUTER_SHARE | > > PTE_BLOCK_NS | > > - PTE_TYPE_VALID); > > + PTE_TYPE_VALID, true); > > } else > > #endif > > { > > @@ -1614,7 +1614,7 @@ void update_early_mmu_table(void) > > PTE_BLOCK_MEMTYPE(MT_NORMAL) | > > PTE_BLOCK_OUTER_SHARE | > > PTE_BLOCK_NS | > > - PTE_TYPE_VALID); > > + PTE_TYPE_VALID, true); > > } > > } > > } > > diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h > > index dbf9ab43e280..0a8ca19e45e9 100644 > > --- a/arch/arm/include/asm/system.h > > +++ b/arch/arm/include/asm/system.h > > @@ -287,8 +287,9 @@ 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); > > -void mmu_change_region_attr(phys_addr_t start, size_t size, u64 attrs); > > +void mmu_change_region_attr(phys_addr_t start, size_t size, u64 attrs, bool bbm); > > Please, add a function description. Thanks /Ilias > > Best regards > > Heinrich > > > > > +void mmu_set_attrs(phys_addr_t addr, size_t size, int type); > > /* > > * smc_call() - issue a secure monitor call > > * > > diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c > > index 2ef936aab757..13f4e8e640ef 100644 > > --- a/arch/arm/mach-snapdragon/board.c > > +++ b/arch/arm/mach-snapdragon/board.c > > @@ -577,7 +577,7 @@ static void carve_out_reserved_memory(void) > > if (i == count || start + size < res[i].start - SZ_2M) { > > debug(" 0x%016llx - 0x%016llx: reserved\n", > > start, start + size); > > - mmu_change_region_attr(start, size, PTE_TYPE_FAULT); > > + mmu_change_region_attr(start, size, PTE_TYPE_FAULT, true); > > /* If this is the final region then quit here before we index > > * out of bounds... > > */ >
diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c index a4ca56c8ed42..60d0870013e8 100644 --- a/arch/arm/cpu/armv8/cache_v8.c +++ b/arch/arm/cpu/armv8/cache_v8.c @@ -968,11 +968,14 @@ void mmu_set_region_dcache_behaviour(phys_addr_t start, size_t size, * 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(phys_addr_t addr, size_t siz, u64 attrs, bool bbm) { int level; u64 r, size, start; + if (!bbm) + goto skip_bbm; + start = addr; size = siz; /* @@ -997,6 +1000,7 @@ void mmu_change_region_attr(phys_addr_t addr, size_t siz, u64 attrs) gd->arch.tlb_addr + gd->arch.tlb_size); __asm_invalidate_tlb_all(); +skip_bbm: /* * Loop through the address range until we find a page granule that fits * our alignment constraints, then set it to the new cache attributes @@ -1020,6 +1024,27 @@ void mmu_change_region_attr(phys_addr_t addr, size_t siz, u64 attrs) __asm_invalidate_tlb_all(); } +void mmu_set_attrs(phys_addr_t addr, size_t size, int type) +{ + u64 attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) | PTE_BLOCK_INNER_SHARE | PTE_TYPE_VALID; + + switch (type) { + case 1: + /* RO */ + attrs |= PTE_BLOCK_PXN | PTE_BLOCK_UXN | PTE_BLOCK_RO; + break; + case 2: + /* RX */ + attrs |= PTE_BLOCK_RO; + break; + case 3: + /* RW */ + attrs |= PTE_BLOCK_PXN | PTE_BLOCK_UXN; + } + + mmu_change_region_attr(addr, size, attrs, false); +} + #else /* !CONFIG_IS_ENABLED(SYS_DCACHE_OFF) */ /* diff --git a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c index d2d3e346a36f..caf1dab05936 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/cpu.c +++ b/arch/arm/cpu/armv8/fsl-layerscape/cpu.c @@ -1573,7 +1573,7 @@ void update_early_mmu_table(void) PTE_BLOCK_MEMTYPE(MT_NORMAL) | PTE_BLOCK_OUTER_SHARE | PTE_BLOCK_NS | - PTE_TYPE_VALID); + PTE_TYPE_VALID, true); } else { mmu_change_region_attr( CFG_SYS_SDRAM_BASE, @@ -1581,7 +1581,7 @@ void update_early_mmu_table(void) PTE_BLOCK_MEMTYPE(MT_NORMAL) | PTE_BLOCK_OUTER_SHARE | PTE_BLOCK_NS | - PTE_TYPE_VALID); + PTE_TYPE_VALID, true); #ifdef CONFIG_SYS_DDR_BLOCK3_BASE #ifndef CONFIG_SYS_DDR_BLOCK2_SIZE #error "Missing CONFIG_SYS_DDR_BLOCK2_SIZE" @@ -1594,7 +1594,7 @@ void update_early_mmu_table(void) PTE_BLOCK_MEMTYPE(MT_NORMAL) | PTE_BLOCK_OUTER_SHARE | PTE_BLOCK_NS | - PTE_TYPE_VALID); + PTE_TYPE_VALID, true); mmu_change_region_attr( CONFIG_SYS_DDR_BLOCK3_BASE, gd->ram_size - @@ -1603,7 +1603,7 @@ void update_early_mmu_table(void) PTE_BLOCK_MEMTYPE(MT_NORMAL) | PTE_BLOCK_OUTER_SHARE | PTE_BLOCK_NS | - PTE_TYPE_VALID); + PTE_TYPE_VALID, true); } else #endif { @@ -1614,7 +1614,7 @@ void update_early_mmu_table(void) PTE_BLOCK_MEMTYPE(MT_NORMAL) | PTE_BLOCK_OUTER_SHARE | PTE_BLOCK_NS | - PTE_TYPE_VALID); + PTE_TYPE_VALID, true); } } } diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h index dbf9ab43e280..0a8ca19e45e9 100644 --- a/arch/arm/include/asm/system.h +++ b/arch/arm/include/asm/system.h @@ -287,8 +287,9 @@ 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); -void mmu_change_region_attr(phys_addr_t start, size_t size, u64 attrs); +void mmu_change_region_attr(phys_addr_t start, size_t size, u64 attrs, bool bbm); +void mmu_set_attrs(phys_addr_t addr, size_t size, int type); /* * smc_call() - issue a secure monitor call * diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c index 2ef936aab757..13f4e8e640ef 100644 --- a/arch/arm/mach-snapdragon/board.c +++ b/arch/arm/mach-snapdragon/board.c @@ -577,7 +577,7 @@ static void carve_out_reserved_memory(void) if (i == count || start + size < res[i].start - SZ_2M) { debug(" 0x%016llx - 0x%016llx: reserved\n", start, start + size); - mmu_change_region_attr(start, size, PTE_TYPE_FAULT); + mmu_change_region_attr(start, size, PTE_TYPE_FAULT, true); /* If this is the final region then quit here before we index * out of bounds... */
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 we can use to change the attributes of normal memory without breaking the pages tables first Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> --- arch/arm/cpu/armv8/cache_v8.c | 27 ++++++++++++++++++++++++- arch/arm/cpu/armv8/fsl-layerscape/cpu.c | 10 ++++----- arch/arm/include/asm/system.h | 3 ++- arch/arm/mach-snapdragon/board.c | 2 +- 4 files changed, 34 insertions(+), 8 deletions(-)