On Thu, 15 Oct 2020 10:14:41 +0200, Ahmad Fatoum wrote:
> On 10/15/20 9:33 AM, Michael Tretter wrote:
> > On Wed, 14 Oct 2020 18:29:47 +0200, Ahmad Fatoum wrote:
> >> On 10/14/20 5:08 PM, Michael Tretter wrote:
> >>> Barebox uses the zero page to trap NULL pointer dereferences. However,
> >>> if the SDRAM starts at address 0x0, this makes the first page of the
> >>> SDRAM inaccessible and makes it impossible to load images to offset 0x0
> >>> in the SDRAM.
> >>>
> >>> Trapping NULL pointer dereferences on such systems is still desirable.
> >>> Therefore, add a function to disable the traps if accessing the zero
> >>> page is necessary and to re-enable the traps after the access is done.
> >>
> >> Can't we map the (phy_addr_t)0 at some higher virtual address and
> >> change uimage to use phys_to_virt() ?
> >
> > To which virt would you map (phy_addr_t)0?
>
> You are on a 64-bit system. Just choose something > 4G that's suitable for
> your ZynqMP?
This would introduce very platform specific handling and diverge from the
shared mmu_64 code. I would rather go for a more general solution.
What about 32 bit ARM systems? Currently, the zero page is simply mapped on
such systems and NULL pointers will never trap.
>
> > This would break the general virt =
> > phys assumption in Barebox. It might work for some cases, but might break in
> > unexpected ways in other. Therefore, I deem it saver to stay with virt = phys
> > and allow to disable the trap, if necessary.
>
> Can you be more specific what kind of problems do you foresee? You map the zero
> page unaccessible already, so any code that would object to having the page 0
> and 1 not be contiguous in memory would already have problems to access page 0.
For example, memtest accesses page 0 and just remaps it.
Michael
>
> >
> > Michael
> >
> >>
> >> Something like
> >>
> >> static inline void *phys_to_virt(unsigned long phys)
> >> {
> >> if (!IS_ENABLED(CONFIG_ARM_MACH_CUSTOM_MAPPING) || !arm_mach_phys_to_virt)
> >> return (void *)phys;
> >> return arm_mach_phys_to_virt(phys);
> >> }
> >>
> >>
> >>>
> >>> Signed-off-by: Michael Tretter <m.tretter@xxxxxxxxxxxxxx>
> >>> ---
> >>> arch/arm/cpu/Kconfig | 1 +
> >>> arch/arm/cpu/mmu_64.c | 13 ++++++++++++-
> >>> include/zero_page.h | 40 ++++++++++++++++++++++++++++++++++++++++
> >>> lib/Kconfig | 3 +++
> >>> 4 files changed, 56 insertions(+), 1 deletion(-)
> >>> create mode 100644 include/zero_page.h
> >>>
> >>> diff --git a/arch/arm/cpu/Kconfig b/arch/arm/cpu/Kconfig
> >>> index f9f52a625260..ca3bd98962e2 100644
> >>> --- a/arch/arm/cpu/Kconfig
> >>> +++ b/arch/arm/cpu/Kconfig
> >>> @@ -89,6 +89,7 @@ config CPU_V8
> >>> select ARM_EXCEPTIONS
> >>> select GENERIC_FIND_NEXT_BIT
> >>> select ARCH_HAS_STACK_DUMP
> >>> + select ARCH_HAS_ZERO_PAGE
> >>>
> >>> config CPU_XSC3
> >>> bool
> >>> diff --git a/arch/arm/cpu/mmu_64.c b/arch/arm/cpu/mmu_64.c
> >>> index 7e9ae84810f6..bd15807f9160 100644
> >>> --- a/arch/arm/cpu/mmu_64.c
> >>> +++ b/arch/arm/cpu/mmu_64.c
> >>> @@ -10,6 +10,7 @@
> >>> #include <init.h>
> >>> #include <mmu.h>
> >>> #include <errno.h>
> >>> +#include <zero_page.h>
> >>> #include <linux/sizes.h>
> >>> #include <asm/memory.h>
> >>> #include <asm/pgtable64.h>
> >>> @@ -168,6 +169,16 @@ static void mmu_enable(void)
> >>> set_cr(get_cr() | CR_M | CR_C | CR_I);
> >>> }
> >>>
> >>> +void zero_page_disable(void)
> >>> +{
> >>> + create_sections(0x0, 0x0, PAGE_SIZE, CACHED_MEM);
> >>> +}
> >>> +
> >>> +void zero_page_enable(void)
> >>> +{
> >>> + create_sections(0x0, 0x0, PAGE_SIZE, 0x0);
> >>> +}
> >>> +
> >>> /*
> >>> * Prepare MMU for usage enable it.
> >>> */
> >>> @@ -194,7 +205,7 @@ void __mmu_init(bool mmu_on)
> >>> create_sections(bank->start, bank->start, bank->size, CACHED_MEM);
> >>>
> >>> /* Make zero page faulting to catch NULL pointer derefs */
> >>> - create_sections(0x0, 0x0, 0x1000, 0x0);
> >>> + zero_page_enable();
> >>>
> >>> mmu_enable();
> >>> }
> >>> diff --git a/include/zero_page.h b/include/zero_page.h
> >>> new file mode 100644
> >>> index 000000000000..d8dd07cfe959
> >>> --- /dev/null
> >>> +++ b/include/zero_page.h
> >>> @@ -0,0 +1,40 @@
> >>> +/* SPDX-License-Identifier: GPL-2.0-only */
> >>> +#ifndef __ZERO_PAGE_H
> >>> +#define __ZERO_PAGE_H
> >>> +
> >>> +#include <common.h>
> >>> +
> >>> +#if defined CONFIG_ARCH_HAS_ZERO_PAGE
> >>> +
> >>> +/*
> >>> + * zero_page_enable - enable null pointer trap
> >>> + */
> >>> +void zero_page_enable(void);
> >>> +
> >>> +/*
> >>> + * zero_page_disable - disable null pointer trap
> >>> + *
> >>> + * Disable the null pointer trap on the zero page if access to the zero page
> >>> + * is actually required. Disable the trap with care and re-enable it
> >>> + * immediately after the access to properly trap null pointers.
> >>> + */
> >>> +void zero_page_disable(void);
> >>> +
> >>> +#else
> >>> +
> >>> +static inline void zero_page_enable(void)
> >>> +{
> >>> +}
> >>> +
> >>> +static inline void zero_page_disable(void)
> >>> +{
> >>> +}
> >>> +
> >>> +#endif
> >>> +
> >>> +static inline bool zero_page_contains(unsigned long addr)
> >>> +{
> >>> + return addr < PAGE_SIZE;
> >>> +}
> >>> +
> >>> +#endif /* __ZERO_PAGE_H */
> >>> diff --git a/lib/Kconfig b/lib/Kconfig
> >>> index 887f50ff003f..e5831ecdb9a7 100644
> >>> --- a/lib/Kconfig
> >>> +++ b/lib/Kconfig
> >>> @@ -182,6 +182,9 @@ config ARCH_HAS_STACK_DUMP
> >>> config ARCH_HAS_DATA_ABORT_MASK
> >>> bool
> >>>
> >>> +config ARCH_HAS_ZERO_PAGE
> >>> + bool
> >>> +
> >>> config HAVE_EFFICIENT_UNALIGNED_ACCESS
> >>> bool
> >>>
> >>>
> >
>
> --
> Pengutronix e.K. | |
> Steuerwalder Str. 21 | http://www.pengutronix.de/ |
> 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
>
--
Pengutronix e.K. | Michael Tretter |
Steuerwalder Str. 21 | https://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
