Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1553698 > unrolled thread

Re: [PATCH RFC 2/2] ARM: nommu: remap exception base address to RAM

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH RFC 2/2] ARM: nommu: remap exception base address to RAM Afzal Mohammed <afzal.mohd.ma@gmail.com> - 2017-01-07 18:20 +0100
    [PATCH WIP 1/4] ARM: nommu: dynamic exception base address setting afzal mohammed <afzal.mohd.ma@gmail.com> - 2017-01-07 18:30 +0100
    [PATCH WIP 2/4] ARM: nommu: remove Hivecs configuration is asm afzal mohammed <afzal.mohd.ma@gmail.com> - 2017-01-07 18:30 +0100
    [PATCH WIP 4/4] ARM: remove compile time vector base for CP15 case afzal mohammed <afzal.mohd.ma@gmail.com> - 2017-01-07 18:30 +0100
      Re: [PATCH WIP 4/4] ARM: remove compile time vector base for CP15  case Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-01-07 18:40 +0100
        Re: [PATCH WIP 4/4] ARM: remove compile time vector base for CP15  case Afzal Mohammed <afzal.mohd.ma@gmail.com> - 2017-01-07 19:10 +0100
          Re: [PATCH WIP 4/4] ARM: remove compile time vector base for CP15  case Afzal Mohammed <afzal.mohd.ma@gmail.com> - 2017-01-07 19:10 +0100
          Re: [PATCH WIP 4/4] ARM: remove compile time vector base for CP15  case Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-01-07 19:30 +0100
            Re: [PATCH WIP 4/4] ARM: remove compile time vector base for CP15  case Afzal Mohammed <afzal.mohd.ma@gmail.com> - 2017-01-08 11:00 +0100
    [PATCH WIP 3/4] ARM: mm: nommu: display dynamic exception base afzal mohammed <afzal.mohd.ma@gmail.com> - 2017-01-07 18:30 +0100
    Re: [PATCH RFC 2/2] ARM: nommu: remap exception base address to RAM Afzal Mohammed <afzal.mohd.ma@gmail.com> - 2017-01-15 12:50 +0100
      Re: [PATCH RFC 2/2] ARM: nommu: remap exception base address to RAM Vladimir Murzin <vladimir.murzin@arm.com> - 2017-01-16 11:00 +0100
        Re: [PATCH RFC 2/2] ARM: nommu: remap exception base address to RAM Afzal Mohammed <afzal.mohd.ma@gmail.com> - 2017-01-16 13:40 +0100

#1553698 — Re: [PATCH RFC 2/2] ARM: nommu: remap exception base address to RAM

Hi,

On Tue, Dec 13, 2016 at 10:02:26AM +0000, Russell King - ARM Linux wrote:

> Is there really any need to do this in head.S ?  I believe it's
> entirely possible to do it later - arch/arm/mm/nommu.c:paging_init().

As memblock_reserve() for exception address was done before
paging_init(), seems it has to be done by arm_mm_memblock_reserve() in
arch/arm/mm/nommu.c, WIP patch follows, but not that happy -
conditional compilation's make it not so readable, still better to
see in C.

> Also, if the region setup for the vectors was moved as well, it would
> then be possible to check the ID registers to determine whether this
> is supported, and make the decision where to locate the vectors base
> more dynamically.

This would affect Cortex-R's, which is a bit concerning due to lack of
those platforms with me, let me try to get it right. Seems
translating __setup_mpu() altogether to C & installing at a later, but
suitable place might be better.

And feeling something strange about Cortex-R support in mainline,
don't know whether it boots out of the box, there are no Cortex-R cpu
compatibles in dts(i), but devicetree documentation documents it.
Still wrecking Cortex-R's could get counted as a regression as dts is
not considered Kernel. Looks like there is a Cortex-R mafia around
mainline ;)

> That leaves one pr_notice() call using the CONFIG_VECTORS_BASE
> constant...

Seems you want to completely kick out CONFIG_VECTORS_BASE.

Saw 2 interesting MMU cases,
1. in devicemaps_init(), if Hivecs is not set, it is being mapped to
virtual address zero, was wondering how MMU Kernel can handle
exceptions with zero address base (& still prints 0xffff0000 as vector
base)
2. One of the platform does a ioremap of CONFIG_VECTORS_BASE

Once i take care of the above, the ugly conditional compilation in
3/4th patch (@arch/arm/mm/init.c) of WIP patch series that follows
will be removed.

Please let know if you have any comments on the above.


Also !MMU Kernel could boot on 3 ARM v7-A platforms - AM335x Beagle
Bone (A8), AM437x IDK (A9) & Vybrid VF610 (on A5 core, note that it
has M4 core too) with same Kernel image*.

Vybrid did not need any platform specific tweaks, just 1/2th patch
(put in patch system as 8635/1) & WIP series over Vladimir's one,
while TI Sitara AMx3's needed one w.r.t remap.

Please bear my delay - to fill the stomach, work not on Linux and then
the vacations.

Regards
afzal

* Since initramfs was used, tty port had to be changed in initramfs
build for Vybrid, but Kernel except for above initramfs change, was
identical.

[toc] | [next] | [standalone]

#1553700 — [PATCH WIP 1/4] ARM: nommu: dynamic exception base address setting

No-MMU dynamic exception base address configuration on processors
with CP15.

TODO: Handle MMU case as well as ARM_MPU scenario dynamically

Signed-off-by: afzal mohammed <afzal.mohd.ma@gmail.com>
---
 arch/arm/mm/nommu.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 60 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mm/nommu.c b/arch/arm/mm/nommu.c
index 681cec879caf..e82056df0635 100644
--- a/arch/arm/mm/nommu.c
+++ b/arch/arm/mm/nommu.c
@@ -11,6 +11,7 @@
 #include <linux/kernel.h>
 
 #include <asm/cacheflush.h>
+#include <asm/cp15.h>
 #include <asm/sections.h>
 #include <asm/page.h>
 #include <asm/setup.h>
@@ -23,6 +24,8 @@
 
 #include "mm.h"
 
+unsigned long vectors_base;
+
 #ifdef CONFIG_ARM_MPU
 struct mpu_rgn_info mpu_rgn_info;
 
@@ -279,15 +282,70 @@ static void sanity_check_meminfo_mpu(void) {}
 static void __init mpu_setup(void) {}
 #endif /* CONFIG_ARM_MPU */
 
+#ifdef CONFIG_CPU_CP15
+/*
+ * ID_PRF1 bits (CP#15 ID_PFR1)
+ */
+#define ID_PFR1_SE (0x3 << 4)	/* Security extension enable bits */
+
+#ifndef CONFIG_CPU_HIGH_VECTOR
+static inline unsigned long get_id_pfr1(void)
+{
+	unsigned long val;
+	asm("mrc p15, 0, %0, c0, c1, 1" : "=r" (val) : : "cc");
+	return val;
+}
+
+static inline void set_vbar(unsigned long val)
+{
+	asm("mcr p15, 0, %0, c12, c0, 0" : : "r" (val) : "cc");
+}
+
+static bool __init security_extensions_enabled(void)
+{
+	return !!(get_id_pfr1() & ID_PFR1_SE);
+}
+#endif
+
+static unsigned long __init setup_vector_base(void)
+{
+	unsigned long reg, base;
+
+	reg = get_cr();
+
+#ifdef CONFIG_CPU_HIGH_VECTOR
+	set_cr(reg | CR_V);
+	base = 0xFFFF0000;
+#else
+	set_cr(reg & ~CR_V);
+	base = 0;
+	if (security_extensions_enabled()) {
+#ifdef CONFIG_REMAP_VECTORS_TO_RAM
+		base = CONFIG_DRAM_BASE;
+#endif
+		set_vbar(base);
+	}
+#endif /* CONFIG_CPU_HIGH_VECTOR */
+
+	return base;
+}
+#endif /* CONFIG_CPU_CP15 */
+
 void __init arm_mm_memblock_reserve(void)
 {
 #ifndef CONFIG_CPU_V7M
+
+#ifdef CONFIG_CPU_CP15
+	vectors_base = setup_vector_base();
+#else
+	vectors_base = CONFIG_VECTORS_BASE;
+#endif
 	/*
 	 * Register the exception vector page.
 	 * some architectures which the DRAM is the exception vector to trap,
 	 * alloc_page breaks with error, although it is not NULL, but "0."
 	 */
-	memblock_reserve(CONFIG_VECTORS_BASE, 2 * PAGE_SIZE);
+	memblock_reserve(vectors_base, 2 * PAGE_SIZE);
 #else /* ifndef CONFIG_CPU_V7M */
 	/*
 	 * There is no dedicated vector page on V7-M. So nothing needs to be
@@ -311,7 +369,7 @@ void __init sanity_check_meminfo(void)
  */
 void __init paging_init(const struct machine_desc *mdesc)
 {
-	early_trap_init((void *)CONFIG_VECTORS_BASE);
+	early_trap_init((void *)vectors_base);
 	mpu_setup();
 	bootmem_init();
 }
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1553701 — [PATCH WIP 2/4] ARM: nommu: remove Hivecs configuration is asm

Now that exception based address is handled dynamically for
processors with CP15, remove Hivecs configuration in assembly.

Signed-off-by: afzal mohammed <afzal.mohd.ma@gmail.com>
---
 arch/arm/kernel/head-nommu.S | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/arch/arm/kernel/head-nommu.S b/arch/arm/kernel/head-nommu.S
index 2ab026ffc270..e0565d73e49e 100644
--- a/arch/arm/kernel/head-nommu.S
+++ b/arch/arm/kernel/head-nommu.S
@@ -162,11 +162,6 @@ ENDPROC(secondary_startup_arm)
 #ifdef CONFIG_CPU_ICACHE_DISABLE
 	bic	r0, r0, #CR_I
 #endif
-#ifdef CONFIG_CPU_HIGH_VECTOR
-	orr	r0, r0, #CR_V
-#else
-	bic	r0, r0, #CR_V
-#endif
 	mcr	p15, 0, r0, c1, c0, 0		@ write control reg
 #elif defined (CONFIG_CPU_V7M)
 	/* For V7M systems we want to modify the CCR similarly to the SCTLR */
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1553702 — [PATCH WIP 4/4] ARM: remove compile time vector base for CP15 case

vectors base is now dynamically updated for Hivecs as well as for
REMAP_VECTORS_TO_RAM case to DRAM_START. Hence remove these CP15
cases.

TODO:
Kill off VECTORS_BASE completely - this would require to handle MMU
 case as well as ARM_MPU scenario dynamically.

Signed-off-by: afzal mohammed <afzal.mohd.ma@gmail.com>
---
 arch/arm/Kconfig | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index bc6f4065840e..720ee62b4955 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -232,8 +232,7 @@ config ARCH_MTD_XIP
 
 config VECTORS_BASE
 	hex
-	default 0xffff0000 if MMU || CPU_HIGH_VECTOR
-	default DRAM_BASE if REMAP_VECTORS_TO_RAM
+	default 0xffff0000 if MMU
 	default 0x00000000
 	help
 	  The base address of exception vectors.  This must be two pages
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1553704 — Re: [PATCH WIP 4/4] ARM: remove compile time vector base for CP15 case

On Sat, Jan 07, 2017 at 10:52:28PM +0530, afzal mohammed wrote:
> vectors base is now dynamically updated for Hivecs as well as for
> REMAP_VECTORS_TO_RAM case to DRAM_START. Hence remove these CP15
> cases.
> 
> TODO:
> Kill off VECTORS_BASE completely - this would require to handle MMU
>  case as well as ARM_MPU scenario dynamically.

Why do you think MMU doesn't already handle it?

>  config VECTORS_BASE
>  	hex
> -	default 0xffff0000 if MMU || CPU_HIGH_VECTOR
> -	default DRAM_BASE if REMAP_VECTORS_TO_RAM
> +	default 0xffff0000 if MMU
>  	default 0x00000000

When MMU=y, the resulting VECTORS_BASE is always 0xffff0000.  The only
case where this ends up zero after your change is when MMU=n.

In any case here's the places it's used:

For nommu:
arch/arm/kernel/head-nommu.S:       mov     r0, #CONFIG_VECTORS_BASE   @ Cover from VECTORS_BASE
arch/arm/kernel/head-nommu.S:       setup_region r0, r5, r6, MPU_DATA_SIDE  @ VECTORS_BASE, PL0 NA, enabled
arch/arm/kernel/head-nommu.S:       setup_region r0, r5, r6, MPU_INSTR_SIDE @ VECTORS_BASE, PL0 NA, enabled
arch/arm/mm/nommu.c:        memblock_reserve(CONFIG_VECTORS_BASE, 2 * PAGE_SIZE);
arch/arm/mm/nommu.c:        early_trap_init((void *)CONFIG_VECTORS_BASE);

To intercept the reset vector for secondary CPUs (because we hide it
away from platforms, so platforms end up hacking around to get at it.)
arch/arm/mach-berlin/platsmp.c:     vectors_base = ioremap(CONFIG_VECTORS_BASE, SZ_32K);

For printing:
arch/arm/mm/init.c:                 MLK(UL(CONFIG_VECTORS_BASE), UL(CONFIG_VECTORS_BASE) +

For dumping the page tables (but since this is only built for MMU=y,
we know what CONFIG_VECTORS_BASE is here.)
arch/arm/mm/dump.c: { CONFIG_VECTORS_BASE,     "Vectors" },
arch/arm/mm/dump.c: { CONFIG_VECTORS_BASE + PAGE_SIZE * 2, "Vectors End" },

For the Berlin and mm/dump code, we could very easily just have a
#define VECTORS_BASE 0xffff0000 in a header file and drop the CONFIG_
prefix.

The MMU case does have to cater for CPUs wanting vectors at 0xffff0000
and at 0x00000000, and this is handled via the page tables - but this
has nothing to do with CONFIG_VECTORS_BASE.  CONFIG_VECTORS_BASE
exists primarily for noMMU.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[toc] | [prev] | [next] | [standalone]

#1553709 — Re: [PATCH WIP 4/4] ARM: remove compile time vector base for CP15 case

Hi,

On Sat, Jan 07, 2017 at 05:38:32PM +0000, Russell King - ARM Linux wrote:
> On Sat, Jan 07, 2017 at 10:52:28PM +0530, afzal mohammed wrote:

> > TODO:
> > Kill off VECTORS_BASE completely - this would require to handle MMU
> >  case as well as ARM_MPU scenario dynamically.

> Why do you think MMU doesn't already handle it?

i meant here w.r.t displaying vector base address in
arch/arm/mm/init.c, i.e. dynamically get it based on Hivecs setting as
either 0xffff0000 or 0x00000000
> 
> >  config VECTORS_BASE
> >  	hex
> > -	default 0xffff0000 if MMU || CPU_HIGH_VECTOR
> > -	default DRAM_BASE if REMAP_VECTORS_TO_RAM
> > +	default 0xffff0000 if MMU
> >  	default 0x00000000
> 
> When MMU=y, the resulting VECTORS_BASE is always 0xffff0000.  The only
> case where this ends up zero after your change is when MMU=n.

> The MMU case does have to cater for CPUs wanting vectors at 0xffff0000
> and at 0x00000000, and this is handled via the page tables - but this
> has nothing to do with CONFIG_VECTORS_BASE.  CONFIG_VECTORS_BASE
> exists primarily for noMMU.

i had thought that for MMU case if Hivecs is not enabled,
CONFIG_VECTOR_BASE has to be considered as 0x00000000 at least for the
purpose of displaying exception base address.

One thing i have not yet understood is how CPU can take exception with
it base address as 0x00000000 (for Hivecs not enabled case) virtual
address as it is below Kernel memory map.

> For the Berlin and mm/dump code, we could very easily just have a
> #define VECTORS_BASE 0xffff0000 in a header file and drop the CONFIG_
> prefix.

Okay, thanks for the tip.

Regards
afzal

[toc] | [prev] | [next] | [standalone]

#1553710 — Re: [PATCH WIP 4/4] ARM: remove compile time vector base for CP15 case

Hi,

On Sat, Jan 07, 2017 at 11:32:27PM +0530, Afzal Mohammed wrote:

> i had thought that for MMU case if Hivecs is not enabled,
> CONFIG_VECTOR_BASE has to be considered as 0x00000000 at least for the

s/CONFIG_VECTOR_BASE/exception base address

> purpose of displaying exception base address.

Regards
afzal

[toc] | [prev] | [next] | [standalone]

#1553712 — Re: [PATCH WIP 4/4] ARM: remove compile time vector base for CP15 case

On Sat, Jan 07, 2017 at 11:32:27PM +0530, Afzal Mohammed wrote:
> Hi,
> 
> On Sat, Jan 07, 2017 at 05:38:32PM +0000, Russell King - ARM Linux wrote:
> > On Sat, Jan 07, 2017 at 10:52:28PM +0530, afzal mohammed wrote:
> 
> > > TODO:
> > > Kill off VECTORS_BASE completely - this would require to handle MMU
> > >  case as well as ARM_MPU scenario dynamically.
> 
> > Why do you think MMU doesn't already handle it?
> 
> i meant here w.r.t displaying vector base address in
> arch/arm/mm/init.c, i.e. dynamically get it based on Hivecs setting as
> either 0xffff0000 or 0x00000000

You mean:

        pr_notice("Virtual kernel memory layout:\n"
                        "    vector  : 0x%08lx - 0x%08lx   (%4ld kB)\n"
...
                        MLK(UL(CONFIG_VECTORS_BASE), UL(CONFIG_VECTORS_BASE) +
                                (PAGE_SIZE)),

As I've said, CONFIG_VECTORS_BASE is _always_ 0xffff0000 on MMU, so
this always displays 0xffff0000 - 0xffff1000 here.

> i had thought that for MMU case if Hivecs is not enabled,
> CONFIG_VECTOR_BASE has to be considered as 0x00000000 at least for the
> purpose of displaying exception base address.
> 
> One thing i have not yet understood is how CPU can take exception with
> it base address as 0x00000000 (for Hivecs not enabled case) virtual
> address as it is below Kernel memory map.

Older ARM CPUs without the V bit (ARMv3 and early ARMv4) expect the
vectors to be at virtual address zero.

Most of these systems place ROM at physical address 0, so when the CPU
starts from reset (with the MMU off) it starts executing from ROM.  Once
the MMU is initialised, RAM can be placed there and the ROM vectors
replaced.  The side effect of this is that NULL pointer dereferences
are not always caught... of course, it makes sense that the page at
address 0 is write protected even from the kernel, so a NULL pointer
write dereference doesn't corrupt the vectors.

How we handle it in Linux is that we always map the page for the vectors
at 0xffff0000, and then only map that same page at 0x00000000 if we have
a CPU that needs it there.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[toc] | [prev] | [next] | [standalone]

#1553824 — Re: [PATCH WIP 4/4] ARM: remove compile time vector base for CP15 case

Hi,

On Sat, Jan 07, 2017 at 06:24:15PM +0000, Russell King - ARM Linux wrote:

> As I've said, CONFIG_VECTORS_BASE is _always_ 0xffff0000 on MMU, so
> this always displays 0xffff0000 - 0xffff1000 here.

> Older ARM CPUs without the V bit (ARMv3 and early ARMv4) expect the
> vectors to be at virtual address zero.
> 
> Most of these systems place ROM at physical address 0, so when the CPU
> starts from reset (with the MMU off) it starts executing from ROM.  Once
> the MMU is initialised, RAM can be placed there and the ROM vectors
> replaced.  The side effect of this is that NULL pointer dereferences
> are not always caught... of course, it makes sense that the page at
> address 0 is write protected even from the kernel, so a NULL pointer
> write dereference doesn't corrupt the vectors.
> 
> How we handle it in Linux is that we always map the page for the vectors
> at 0xffff0000, and then only map that same page at 0x00000000 if we have
> a CPU that needs it there.

Thanks for the information, i was not aware, seems that simplifies MMU
case handling.

arch/arm/mm/mmu.c:

	if (!vectors_high()) {
		map.virtual = 0;
		map.length = PAGE_SIZE * 2;
		map.type = MT_LOW_VECTORS;
		create_mapping(&map);
	}



arch/arm/include/asm/cp15.h:

#if __LINUX_ARM_ARCH__ >= 4
#define vectors_high()	(get_cr() & CR_V)
#else
#define vectors_high()	(0)
#endif

Deducing from your reply & above code snippets that for
__LINUX_ARM_ARCH__ >= 4, in all practical cases, vector_high() returns
true

Regards
afzal

[toc] | [prev] | [next] | [standalone]

#1553703 — [PATCH WIP 3/4] ARM: mm: nommu: display dynamic exception base

Display dynamically estimated nommu exception base.

TODO: Dynamically update MMU case too.

Signed-off-by: afzal mohammed <afzal.mohd.ma@gmail.com>
---
 arch/arm/mm/init.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/arm/mm/init.c b/arch/arm/mm/init.c
index 370581aeb871..1777ee23a6a2 100644
--- a/arch/arm/mm/init.c
+++ b/arch/arm/mm/init.c
@@ -39,6 +39,10 @@
 
 #include "mm.h"
 
+#ifndef CONFIG_MMU
+extern unsigned long vectors_base;
+#endif
+
 #ifdef CONFIG_CPU_CP15_MMU
 unsigned long __init __clear_cr(unsigned long mask)
 {
@@ -521,8 +525,13 @@ void __init mem_init(void)
 			"      .data : 0x%p" " - 0x%p" "   (%4td kB)\n"
 			"       .bss : 0x%p" " - 0x%p" "   (%4td kB)\n",
 
+#ifdef CONFIG_MMU
 			MLK(UL(CONFIG_VECTORS_BASE), UL(CONFIG_VECTORS_BASE) +
 				(PAGE_SIZE)),
+#else
+			MLK_ROUNDUP(vectors_base, vectors_base + PAGE_SIZE),
+#endif
+
 #ifdef CONFIG_HAVE_TCM
 			MLK(DTCM_OFFSET, (unsigned long) dtcm_end),
 			MLK(ITCM_OFFSET, (unsigned long) itcm_end),
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1559210

Hi,

On Sat, Jan 07, 2017 at 10:43:39PM +0530, Afzal Mohammed wrote:
> On Tue, Dec 13, 2016 at 10:02:26AM +0000, Russell King - ARM Linux wrote:

> > Also, if the region setup for the vectors was moved as well, it would
> > then be possible to check the ID registers to determine whether this
> > is supported, and make the decision where to locate the vectors base
> > more dynamically.
> 
> This would affect Cortex-R's, which is a bit concerning due to lack of
> those platforms with me, let me try to get it right.

QEMU too doesn't seem to provide a Cortex-R target

> Seems translating __setup_mpu() altogether to C

afaics, a kind of C translation is already present as
mpu_setup_region() in arch/arm/mm/nommu.c that takes care of
MPU_RAM_REGION only. And that seems to be a kind of redundant as it is
also done in asm at __setup_mpu(). Git blames asm & C to consecutive
commits, that makes me a little shaky about the conclusion on it being
redundant.

> & installing at a later, but suitable place might be better.

But looks like enabling MPU can't be moved to C & that would
necessitate keeping at least some portion of__setu_mpu() in asm.

Instead, moving region setup only for vectors to C as Russell
suggested at first would have to be done.

A kind of diff at the end is in my mind, with additional changes to
handle the similar during secondary cpu bringup too.

Thinking of invoking mpu_setup() from secondary_start_kernel() in
arch/arm/kernel/smp.c, with mpu_setup() being slightly modified to
avoid storing region details again when invoked by secondary cpu's.

Vladimir, once changes are done after a revisit, i would need your
help to test on Cortex-R.

As an aside, wasn't aware of the fact that Cortex-R supports SMP
Linux, had thought that, of !MMU one's, only Blackfin & J2 had it.


> Also !MMU Kernel could boot on 3 ARM v7-A platforms - AM335x Beagle
> Bone (A8), AM437x IDK (A9) & Vybrid VF610 (on A5 core, note that it
> has M4 core too)

Talking about Cortex-M, AMx3's too have it, to be specific M3, but
they are not Linux-able unlike the one in VF610.

Regards
afzal

--->8---

diff --git a/arch/arm/kernel/head-nommu.S b/arch/arm/kernel/head-nommu.S
index e0565d73e49e..f8ac79b6136d 100644
--- a/arch/arm/kernel/head-nommu.S
+++ b/arch/arm/kernel/head-nommu.S
@@ -249,20 +249,6 @@ ENTRY(__setup_mpu)
 	setup_region r0, r5, r6, MPU_INSTR_SIDE @ 0x0, BG region, enabled
 2:	isb
 
-	/* Vectors region */
-	set_region_nr r0, #MPU_VECTORS_REGION
-	isb
-	/* Shared, inaccessible to PL0, rw PL1 */
-	mov	r0, #CONFIG_VECTORS_BASE	@ Cover from VECTORS_BASE
-	ldr	r5,=(MPU_AP_PL1RW_PL0NA | MPU_RGN_NORMAL)
-	/* Writing N to bits 5:1 (RSR_SZ) --> region size 2^N+1 */
-	mov	r6, #(((2 * PAGE_SHIFT - 1) << MPU_RSR_SZ) | 1 << MPU_RSR_EN)
-
-	setup_region r0, r5, r6, MPU_DATA_SIDE	@ VECTORS_BASE, PL0 NA, enabled
-	beq	3f				@ Memory-map not unified
-	setup_region r0, r5, r6, MPU_INSTR_SIDE	@ VECTORS_BASE, PL0 NA, enabled
-3:	isb
-
 	/* Enable the MPU */
 	mrc	p15, 0, r0, c1, c0, 0		@ Read SCTLR
 	bic     r0, r0, #CR_BR			@ Disable the 'default mem-map'
diff --git a/arch/arm/mm/nommu.c b/arch/arm/mm/nommu.c
index e82056df0635..7fe8906322d5 100644
--- a/arch/arm/mm/nommu.c
+++ b/arch/arm/mm/nommu.c
@@ -269,12 +269,19 @@ void __init mpu_setup(void)
 					ilog2(memblock.memory.regions[0].size),
 					MPU_AP_PL1RW_PL0RW | MPU_RGN_NORMAL);
 	if (region_err) {
-		panic("MPU region initialization failure! %d", region_err);
+		panic("MPU RAM region initialization failure! %d", region_err);
 	} else {
-		pr_info("Using ARMv7 PMSA Compliant MPU. "
-			 "Region independence: %s, Max regions: %d\n",
-			mpu_iside_independent() ? "Yes" : "No",
-			mpu_max_regions());
+		region_err = mpu_setup_region(MPU_VECTORS_REGION, vectors_base,
+					ilog2(memblock.memory.regions[0].size),
+					MPU_AP_PL1RW_PL0NA | MPU_RGN_NORMAL);
+		if (region_err) {
+			panic("MPU VECTOR region initialization failure! %d",
+			      region_err);
+		} else {
+			pr_info("Using ARMv7 PMSA Compliant MPU. "
+				"Region independence: %s, Max regions: %d\n",
+				mpu_iside_independent() ? "Yes" : "No",
+				mpu_max_regions());
 	}
 }
 #else

[toc] | [prev] | [next] | [standalone]

#1559567

Hi,

On 15/01/17 11:47, Afzal Mohammed wrote:
> Hi,
> 
> On Sat, Jan 07, 2017 at 10:43:39PM +0530, Afzal Mohammed wrote:
>> On Tue, Dec 13, 2016 at 10:02:26AM +0000, Russell King - ARM Linux wrote:
> 
>>> Also, if the region setup for the vectors was moved as well, it would
>>> then be possible to check the ID registers to determine whether this
>>> is supported, and make the decision where to locate the vectors base
>>> more dynamically.
>>
>> This would affect Cortex-R's, which is a bit concerning due to lack of
>> those platforms with me, let me try to get it right.
> 
> QEMU too doesn't seem to provide a Cortex-R target
> 
>> Seems translating __setup_mpu() altogether to C
> 
> afaics, a kind of C translation is already present as
> mpu_setup_region() in arch/arm/mm/nommu.c that takes care of
> MPU_RAM_REGION only. And that seems to be a kind of redundant as it is
> also done in asm at __setup_mpu(). Git blames asm & C to consecutive
> commits, that makes me a little shaky about the conclusion on it being
> redundant.
> 

It is not redundant. MPU setup is done it two steps. The first step done in
asm to enable caches, there only kernel image is covered; the second step takes
care on the whole RAM given via dt or "mem=" parameter.

I think other regions are kept there to avoid C side dancing in case of SMP.

>> & installing at a later, but suitable place might be better.
> 
> But looks like enabling MPU can't be moved to C & that would
> necessitate keeping at least some portion of__setu_mpu() in asm.
> 
> Instead, moving region setup only for vectors to C as Russell
> suggested at first would have to be done.
> 
> A kind of diff at the end is in my mind, with additional changes to
> handle the similar during secondary cpu bringup too.
> 
> Thinking of invoking mpu_setup() from secondary_start_kernel() in
> arch/arm/kernel/smp.c, with mpu_setup() being slightly modified to
> avoid storing region details again when invoked by secondary cpu's.

I have wip patches on reworking MPU setup code. The idea is to start using
mpu_rgn_info[] actively, so asm part for secondariness would just sync-up
content of that array. Additionally, it seems that we can reuse free MPU slots
to cover memory which is discarded due to MPU alignment restrictions... 

> 
> Vladimir, once changes are done after a revisit, i would need your
> help to test on Cortex-R.

I'm more than happy to help, but currently I have limited bandwidth, so if it
can wait till the next dev cycle I'd try to make MPU rework finished by that
time.

> 
> As an aside, wasn't aware of the fact that Cortex-R supports SMP
> Linux, had thought that, of !MMU one's, only Blackfin & J2 had it.
> 
> 
>> Also !MMU Kernel could boot on 3 ARM v7-A platforms - AM335x Beagle
>> Bone (A8), AM437x IDK (A9) & Vybrid VF610 (on A5 core, note that it
>> has M4 core too)
> 
> Talking about Cortex-M, AMx3's too have it, to be specific M3, but
> they are not Linux-able unlike the one in VF610.
> 

Thanks!

Vladimir

> Regards
> afzal
> 
> --->8---
> 
> diff --git a/arch/arm/kernel/head-nommu.S b/arch/arm/kernel/head-nommu.S
> index e0565d73e49e..f8ac79b6136d 100644
> --- a/arch/arm/kernel/head-nommu.S
> +++ b/arch/arm/kernel/head-nommu.S
> @@ -249,20 +249,6 @@ ENTRY(__setup_mpu)
>  	setup_region r0, r5, r6, MPU_INSTR_SIDE @ 0x0, BG region, enabled
>  2:	isb
>  
> -	/* Vectors region */
> -	set_region_nr r0, #MPU_VECTORS_REGION
> -	isb
> -	/* Shared, inaccessible to PL0, rw PL1 */
> -	mov	r0, #CONFIG_VECTORS_BASE	@ Cover from VECTORS_BASE
> -	ldr	r5,=(MPU_AP_PL1RW_PL0NA | MPU_RGN_NORMAL)
> -	/* Writing N to bits 5:1 (RSR_SZ) --> region size 2^N+1 */
> -	mov	r6, #(((2 * PAGE_SHIFT - 1) << MPU_RSR_SZ) | 1 << MPU_RSR_EN)
> -
> -	setup_region r0, r5, r6, MPU_DATA_SIDE	@ VECTORS_BASE, PL0 NA, enabled
> -	beq	3f				@ Memory-map not unified
> -	setup_region r0, r5, r6, MPU_INSTR_SIDE	@ VECTORS_BASE, PL0 NA, enabled
> -3:	isb
> -
>  	/* Enable the MPU */
>  	mrc	p15, 0, r0, c1, c0, 0		@ Read SCTLR
>  	bic     r0, r0, #CR_BR			@ Disable the 'default mem-map'
> diff --git a/arch/arm/mm/nommu.c b/arch/arm/mm/nommu.c
> index e82056df0635..7fe8906322d5 100644
> --- a/arch/arm/mm/nommu.c
> +++ b/arch/arm/mm/nommu.c
> @@ -269,12 +269,19 @@ void __init mpu_setup(void)
>  					ilog2(memblock.memory.regions[0].size),
>  					MPU_AP_PL1RW_PL0RW | MPU_RGN_NORMAL);
>  	if (region_err) {
> -		panic("MPU region initialization failure! %d", region_err);
> +		panic("MPU RAM region initialization failure! %d", region_err);
>  	} else {
> -		pr_info("Using ARMv7 PMSA Compliant MPU. "
> -			 "Region independence: %s, Max regions: %d\n",
> -			mpu_iside_independent() ? "Yes" : "No",
> -			mpu_max_regions());
> +		region_err = mpu_setup_region(MPU_VECTORS_REGION, vectors_base,
> +					ilog2(memblock.memory.regions[0].size),
> +					MPU_AP_PL1RW_PL0NA | MPU_RGN_NORMAL);
> +		if (region_err) {
> +			panic("MPU VECTOR region initialization failure! %d",
> +			      region_err);
> +		} else {
> +			pr_info("Using ARMv7 PMSA Compliant MPU. "
> +				"Region independence: %s, Max regions: %d\n",
> +				mpu_iside_independent() ? "Yes" : "No",
> +				mpu_max_regions());
>  	}
>  }
>  #else
> 

[toc] | [prev] | [next] | [standalone]

#1559693

Hi,

On Mon, Jan 16, 2017 at 09:53:41AM +0000, Vladimir Murzin wrote:
> On 15/01/17 11:47, Afzal Mohammed wrote:

> > mpu_setup_region() in arch/arm/mm/nommu.c that takes care of
> > MPU_RAM_REGION only. And that seems to be a kind of redundant as it is
> > also done in asm at __setup_mpu(). Git blames asm & C to consecutive
> > commits, that makes me a little shaky about the conclusion on it being
> > redundant.
> 
> It is not redundant. MPU setup is done it two steps. The first step done in
> asm to enable caches, there only kernel image is covered; the second step takes
> care on the whole RAM given via dt or "mem=" parameter.

Okay, thanks for the details.

> > Thinking of invoking mpu_setup() from secondary_start_kernel() in
> > arch/arm/kernel/smp.c, with mpu_setup() being slightly modified to
> > avoid storing region details again when invoked by secondary cpu's.
> 
> I have wip patches on reworking MPU setup code. The idea is to start using
> mpu_rgn_info[] actively, so asm part for secondariness would just sync-up
> content of that array. Additionally, it seems that we can reuse free MPU slots
> to cover memory which is discarded due to MPU alignment restrictions... 
> 
> > Vladimir, once changes are done after a revisit, i would need your
> > help to test on Cortex-R.
> 
> I'm more than happy to help, but currently I have limited bandwidth, so if it
> can wait till the next dev cycle I'd try to make MPU rework finished by that
> time.

Okay, please feel free to do MPU rework the way you were planning, you
know more details & have the platform to achieve it with much higher
efficiency than me.

Regards
afzal

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web