[PATCH v1 5/6] treewide: Add a function to change page permissions
Ilias Apalodimas
ilias.apalodimas at linaro.org
Wed Feb 5 17:54:22 CET 2025
Hi Heinrich,
On Wed, 5 Feb 2025 at 18:48, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 2/5/25 08:16, Ilias Apalodimas wrote:
> > For armv8 we are adding proper page permissions for the relocated U-Boot
> > binary. Add a weak function that can be used across architectures to change
> > the page permissions
> >
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > ---
> > arch/arc/lib/cache.c | 2 ++
> > arch/arm/cpu/arm926ejs/cache.c | 2 ++
> > arch/arm/cpu/armv7/cache_v7.c | 1 +
> > arch/arm/cpu/armv7m/cache.c | 2 ++
> > arch/arm/cpu/armv8/cache_v8.c | 22 ++++++++++++++++++++++
> > arch/arm/lib/cache.c | 2 ++
> > arch/m68k/lib/cache.c | 2 ++
> > arch/nios2/lib/cache.c | 2 ++
> > arch/powerpc/lib/cache.c | 2 ++
> > arch/riscv/lib/cache.c | 2 ++
> > arch/sh/cpu/sh4/cache.c | 2 ++
> > arch/xtensa/lib/cache.c | 2 ++
> > include/cpu_func.h | 16 ++++++++++++++++
> > 13 files changed, 59 insertions(+)
> >
> > diff --git a/arch/arc/lib/cache.c b/arch/arc/lib/cache.c
> > index 5169fc627fa5..5c79243d7223 100644
> > --- a/arch/arc/lib/cache.c
> > +++ b/arch/arc/lib/cache.c
> > @@ -819,3 +819,5 @@ void sync_n_cleanup_cache_all(void)
> >
> > __ic_entire_invalidate();
> > }
> > +
> > +void __weak pgprot_set_attrs(phys_addr_t addr, size_t size, u64 perm) {}
> > diff --git a/arch/arm/cpu/arm926ejs/cache.c b/arch/arm/cpu/arm926ejs/cache.c
> > index 5b87a3af91b2..857311b3dfad 100644
> > --- a/arch/arm/cpu/arm926ejs/cache.c
> > +++ b/arch/arm/cpu/arm926ejs/cache.c
> > @@ -88,3 +88,5 @@ void enable_caches(void)
> > dcache_enable();
> > #endif
> > }
> > +
> > +void __weak pgprot_set_attrs(phys_addr_t addr, size_t size, u64 perm) {}
> > diff --git a/arch/arm/cpu/armv7/cache_v7.c b/arch/arm/cpu/armv7/cache_v7.c
> > index d11420d2fdd0..14c9be77db8d 100644
> > --- a/arch/arm/cpu/armv7/cache_v7.c
> > +++ b/arch/arm/cpu/armv7/cache_v7.c
> > @@ -209,3 +209,4 @@ __weak void v7_outer_cache_flush_all(void) {}
> > __weak void v7_outer_cache_inval_all(void) {}
> > __weak void v7_outer_cache_flush_range(u32 start, u32 end) {}
> > __weak void v7_outer_cache_inval_range(u32 start, u32 end) {}
> > +__weak void pgprot_set_attrs(phys_addr_t addr, size_t size, u64 perm) {}
> > diff --git a/arch/arm/cpu/armv7m/cache.c b/arch/arm/cpu/armv7m/cache.c
> > index b6d08b7aad73..458a214e9577 100644
> > --- a/arch/arm/cpu/armv7m/cache.c
> > +++ b/arch/arm/cpu/armv7m/cache.c
> > @@ -370,3 +370,5 @@ void enable_caches(void)
> > dcache_enable();
> > #endif
> > }
> > +
> > +void __weak pgprot_set_attrs(phys_addr_t addr, size_t size, u64 perm) {}
> > diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c
> > index 670379e17b7a..1cf3870177ee 100644
> > --- a/arch/arm/cpu/armv8/cache_v8.c
> > +++ b/arch/arm/cpu/armv8/cache_v8.c
> > @@ -1028,6 +1028,28 @@ skip_break:
> > __asm_invalidate_tlb_all();
> > }
> >
> > +void pgprot_set_attrs(phys_addr_t addr, size_t size, u64 perm)
> > +{
> > + u64 attrs = PTE_BLOCK_MEMTYPE(MT_NORMAL) | PTE_BLOCK_INNER_SHARE | PTE_TYPE_VALID;
> > +
> > + switch (perm) {
> > + case MMU_ATTR_RO:
> > + attrs |= PTE_BLOCK_PXN | PTE_BLOCK_UXN | PTE_BLOCK_RO;
> > + break;
> > + case MMU_ATTR_RX:
> > + attrs |= PTE_BLOCK_RO;
> > + break;
> > + case MMU_ATTR_RW:
> > + attrs |= PTE_BLOCK_PXN | PTE_BLOCK_UXN;
> > + break;
> > + default:
> > + log_err("Unknown attribute %llx\n", perm);
> > + return;
> > + }
> > +
> > + mmu_change_region_attr(addr, size, attrs, false);
> > +}
> > +
> > #else /* !CONFIG_IS_ENABLED(SYS_DCACHE_OFF) */
> >
> > /*
> > diff --git a/arch/arm/lib/cache.c b/arch/arm/lib/cache.c
> > index 516754caeaf9..c7704d8ee354 100644
> > --- a/arch/arm/lib/cache.c
> > +++ b/arch/arm/lib/cache.c
> > @@ -170,3 +170,5 @@ __weak int arm_reserve_mmu(void)
> >
> > return 0;
> > }
> > +
> > +void __weak pgprot_set_attrs(phys_addr_t addr, size_t size, u64 perm) {}
>
> I would prefer if the weak function would return -ENOSYS indicating the
> missing implementation and the real function would return 0 in case of
> success or an error code on failure. This way the EFI protocol could set
> the return code if the architecture does not provide support for setting
> the attributes the passed addresses are invalid.
Sure I'll change that in v2
Cheers
/Ilias
>
> Best regards
>
> Heinrich
>
> > diff --git a/arch/m68k/lib/cache.c b/arch/m68k/lib/cache.c
> > index 370ad40f1423..b275646384a5 100644
> > --- a/arch/m68k/lib/cache.c
> > +++ b/arch/m68k/lib/cache.c
> > @@ -151,3 +151,5 @@ __weak void flush_dcache_range(unsigned long start, unsigned long stop)
> > {
> > /* An empty stub, real implementation should be in platform code */
> > }
> > +
> > +void __weak pgprot_set_attrs(phys_addr_t addr, size_t size, u64 perm) {}
> > diff --git a/arch/nios2/lib/cache.c b/arch/nios2/lib/cache.c
> > index 8f543f2a2f26..7a93a8fcc6a7 100644
> > --- a/arch/nios2/lib/cache.c
> > +++ b/arch/nios2/lib/cache.c
> > @@ -127,3 +127,5 @@ void dcache_disable(void)
> > {
> > flush_dcache_all();
> > }
> > +
> > +void __weak pgprot_set_attrs(phys_addr_t addr, size_t size, u64 perm) {}
> > diff --git a/arch/powerpc/lib/cache.c b/arch/powerpc/lib/cache.c
> > index a9cd7b8d30ac..3d0536caccde 100644
> > --- a/arch/powerpc/lib/cache.c
> > +++ b/arch/powerpc/lib/cache.c
> > @@ -58,3 +58,5 @@ void invalidate_icache_all(void)
> > {
> > puts("No arch specific invalidate_icache_all available!\n");
> > }
> > +
> > +void __weak pgprot_set_attrs(phys_addr_t addr, size_t size, u64 perm) {}
> > diff --git a/arch/riscv/lib/cache.c b/arch/riscv/lib/cache.c
> > index 71e4937ab542..1c751e562157 100644
> > --- a/arch/riscv/lib/cache.c
> > +++ b/arch/riscv/lib/cache.c
> > @@ -151,3 +151,5 @@ __weak void enable_caches(void)
> > if (!zicbom_block_size)
> > log_debug("Zicbom not initialized.\n");
> > }
> > +
> > +void __weak pgprot_set_attrs(phys_addr_t addr, size_t size, u64 perm) {}
> > diff --git a/arch/sh/cpu/sh4/cache.c b/arch/sh/cpu/sh4/cache.c
> > index 99acc5999652..22e0f1484a33 100644
> > --- a/arch/sh/cpu/sh4/cache.c
> > +++ b/arch/sh/cpu/sh4/cache.c
> > @@ -126,3 +126,5 @@ int dcache_status(void)
> > {
> > return 0;
> > }
> > +
> > +void __weak pgprot_set_attrs(phys_addr_t addr, size_t size, u64 perm) {}
> > diff --git a/arch/xtensa/lib/cache.c b/arch/xtensa/lib/cache.c
> > index e6a7f6827fc2..aacc2d2627d6 100644
> > --- a/arch/xtensa/lib/cache.c
> > +++ b/arch/xtensa/lib/cache.c
> > @@ -57,3 +57,5 @@ void invalidate_icache_all(void)
> > {
> > __invalidate_icache_all();
> > }
> > +
> > +void __weak pgprot_set_attrs(phys_addr_t addr, size_t size, u64 perm) {}
> > diff --git a/include/cpu_func.h b/include/cpu_func.h
> > index 7e81c4364a73..17b6d199302a 100644
> > --- a/include/cpu_func.h
> > +++ b/include/cpu_func.h
> > @@ -69,6 +69,22 @@ void flush_dcache_range(unsigned long start, unsigned long stop);
> > void invalidate_dcache_range(unsigned long start, unsigned long stop);
> > void invalidate_dcache_all(void);
> > void invalidate_icache_all(void);
> > +
> > +enum pgprot_attrs {
> > + MMU_ATTR_RO,
> > + MMU_ATTR_RX,
> > + MMU_ATTR_RW,
> > +};
> > +
> > +/** pgprot_set_attrs() - Set page table permissions
> > + *
> > + * @addr: Physical address start
> > + * @size: size of memory to change
> > + * @perm: New permissions
> > + *
> > + **/
> > +void pgprot_set_attrs(phys_addr_t addr, size_t size, u64 perm);
> > +
> > /**
> > * noncached_init() - Initialize non-cached memory region
> > *
>
More information about the U-Boot
mailing list