[PATCH] Use CONFIG_VECTORS_BASE instead of hardcoded 0xffff0000.

[PATCH] Use CONFIG_VECTORS_BASE instead of hardcoded 0xffff0000.

[Benoit Fouet]

The configuration option exists but is not used everywhere,
this patch tries to make it the default instead of the
hardcoded value.

Signed-off-by: Benoit Fouet <benoit.fouet@free.fr>
---
 arch/arm/include/asm/system.h |    2 +-
 arch/arm/kernel/traps.c       |    2 +-
 arch/arm/mm/mmu.c             |    6 +++---
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/include/asm/system.h b/arch/arm/include/asm/system.h
index d65b2f5..79dc5da 100644
--- a/arch/arm/include/asm/system.h
+++ b/arch/arm/include/asm/system.h
@@ -30,7 +30,7 @@
 #define CR_F	(1 << 10)	/* Implementation defined		*/
 #define CR_Z	(1 << 11)	/* Implementation defined		*/
 #define CR_I	(1 << 12)	/* Icache enable			*/
-#define CR_V	(1 << 13)	/* Vectors relocated to 0xffff0000	*/
+#define CR_V	(1 << 13)	/* Vectors relocated			*/
 #define CR_RR	(1 << 14)	/* Round Robin cache replacement	*/
 #define CR_L4	(1 << 15)	/* LDR pc can set T bit			*/
 #define CR_DT	(1 << 16)
diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
index f838f36..14669a9 100644
--- a/arch/arm/kernel/traps.c
+++ b/arch/arm/kernel/traps.c
@@ -738,7 +738,7 @@ void __init early_trap_init(void)
 
 	/*
 	 * Copy the vectors, stubs and kuser helpers (in entry-armv.S)
-	 * into the vector page, mapped at 0xffff0000, and ensure these
+	 * into the vector page, mapped at CONFIG_VECTORS_BASE, and ensure these
 	 * are visible to the instruction stream.
 	 */
 	memcpy((void *)vectors, __vectors_start, __vectors_end - __vectors_start);
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index 02243ee..ce4d7cf 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -475,7 +475,7 @@ static void __init build_mem_type_table(void)
 	}
 }
 
-#define vectors_base()	(vectors_high() ? 0xffff0000 : 0)
+#define vectors_base()	(vectors_high() ? CONFIG_VECTORS_BASE : 0)
 
 static void __init alloc_init_pte(pmd_t *pmd, unsigned long addr,
 				  unsigned long end, unsigned long pfn,
@@ -964,11 +964,11 @@ static void __init devicemaps_init(struct machine_desc *mdesc)
 
 	/*
 	 * Create a mapping for the machine vectors at the high-vectors
-	 * location (0xffff0000).  If we aren't using high-vectors, also
+	 * location (CONFIG_VECTORS_BASE).  If we aren't using high-vectors, also
 	 * create a mapping@the low-vectors virtual address.
 	 */
 	map.pfn = __phys_to_pfn(virt_to_phys(vectors));
-	map.virtual = 0xffff0000;
+	map.virtual = CONFIG_VECTORS_BASE;
 	map.length = PAGE_SIZE;
 	map.type = MT_HIGH_VECTORS;
 	create_mapping(&map);
-- 
1.6.0.4

[PATCH] Use CONFIG_VECTORS_BASE instead of hardcoded 0xffff0000.

[Linus Walleij]

2009/10/20 Benoit Fouet <benoit.fouet@free.fr>:

> The configuration option exists but is not used everywhere,
> this patch tries to make it the default instead of the
> hardcoded value.

Previously IIRC these could only be placed at 0x00000000 or
0xffff0000, and I haven't heard of it being changed, and the
code surrounding this surely does not take into account that
it could be anything that 0 or 0xffff0000.

With a recursive
cd arch/arm/ && grep -r CONFIG_VECTORS_BASE .
you see that everybody and his dog is using 0xffff0000
(high vectors).

I think we'd rather get rid of CONFIG_VECTORS_BASE
with that hex value, this config value could for example
be CONFIG_VECTORS_LOW | CONFIG_VECTORS_HIGH, mutually
exclusive instead, and then _LOW will basically never be used
and things become simpler, I think having a hex value for
CONFIG_VECTORS_BASE is actually confusing since that
may give people the (false) idea that this can be set to anything.

> (...)
> ? ? ? ?/*
> ? ? ? ? * Create a mapping for the machine vectors at the high-vectors
> - ? ? ? ?* location (0xffff0000). ?If we aren't using high-vectors, also
> + ? ? ? ?* location (CONFIG_VECTORS_BASE). ?If we aren't using high-vectors, also
> ? ? ? ? * create a mapping at the low-vectors virtual address.
> ? ? ? ? */
> ? ? ? ?map.pfn = __phys_to_pfn(virt_to_phys(vectors));
> - ? ? ? map.virtual = 0xffff0000;
> + ? ? ? map.virtual = CONFIG_VECTORS_BASE;
> ? ? ? ?map.length = PAGE_SIZE;
> ? ? ? ?map.type = MT_HIGH_VECTORS;
> ? ? ? ?create_mapping(&map);

Speaking of this: for machines with TCM (like the U300) we'd probably
like to move our ITCM to 0xffff0000 (CONFIG_VECTORS_BASE) and alter
the code so that iff CONFIG_VECTORS_BASE == __sitcm_text the
vectors page will not be allocated and mapped, and vectors go into
ITCM instead. The idea is that since TCM fast as cache, it's a pretty
good idea to lock them in there. (Seen on other ARM OS:es...)

I'll try to make a patch for that someday.

Linus Walleij

[PATCH] Use CONFIG_VECTORS_BASE instead of hardcoded 0xffff0000.

[Russell King - ARM Linux]

On Tue, Oct 20, 2009 at 09:51:35PM +0200, Linus Walleij wrote:
> I think we'd rather get rid of CONFIG_VECTORS_BASE
> with that hex value, this config value could for example
> be CONFIG_VECTORS_LOW | CONFIG_VECTORS_HIGH, mutually
> exclusive instead, and then _LOW will basically never be used
> and things become simpler, I think having a hex value for
> CONFIG_VECTORS_BASE is actually confusing since that
> may give people the (false) idea that this can be set to anything.

In the MMU case, there are only two possible addresses for the vectors -
0x00000000 or 0xffff0000.  Which is in play is entirely dependent on the
state of the V bit in the control register.

Since there are only two possibilities, this is hard-coded into the code,
and VECTORS_BASE is not used - it's pointless and its actually got nothing
to do with what VECTORS_BASE is set to.

In the noMMU case, the base for the vectors is configurable.  Consider
that on noMMU, you need the reset vector to be populated by code before
any RAM is available.  One way to do this is to ensure that ROM or flash
is mapped there.  It could be that the hardware CPU vectors are always
directed to ROM.

So, what noMMU implementations do is they load the ROM image up with
a set of instructions to soft-redirect the CPU vectors (except for
reset) into system RAM, which can be at any address.

This is what VECTORS_BASE is doing - it's allowing noMMU systems to
inform the kernel where the redirected vectors are stored in the system
memory map.

VECTORS_BASE really has no meaning for MMU-based systems.

> Speaking of this: for machines with TCM (like the U300) we'd probably
> like to move our ITCM to 0xffff0000 (CONFIG_VECTORS_BASE) and alter
> the code so that iff CONFIG_VECTORS_BASE == __sitcm_text the
> vectors page will not be allocated and mapped, and vectors go into
> ITCM instead. The idea is that since TCM fast as cache, it's a pretty
> good idea to lock them in there. (Seen on other ARM OS:es...)

No can do - the top of the vector page is used for other purposes,
including storing the current TLS value in that page.  It's not used
solely for the vectors.

[PATCH] Use CONFIG_VECTORS_BASE instead of hardcoded 0xffff0000.

[Benoit Fouet]

Hi Russell,

On Tue, 20 Oct 2009 21:10:09 +0100 Russell King - ARM Linux wrote:
> On Tue, Oct 20, 2009 at 09:51:35PM +0200, Linus Walleij wrote:
> > I think we'd rather get rid of CONFIG_VECTORS_BASE
> > with that hex value, this config value could for example
> > be CONFIG_VECTORS_LOW | CONFIG_VECTORS_HIGH, mutually
> > exclusive instead, and then _LOW will basically never be used
> > and things become simpler, I think having a hex value for
> > CONFIG_VECTORS_BASE is actually confusing since that
> > may give people the (false) idea that this can be set to anything.
> 
> In the MMU case, there are only two possible addresses for the vectors -
> 0x00000000 or 0xffff0000.  Which is in play is entirely dependent on the
> state of the V bit in the control register.
> 

Reading the DDI0344D (Cortex A8 r2p1 trm), it seemed to me that this
was configurable with the security extension:
http://infocentre.arm.com/help/topic/com.arm.doc.ddi0344d/Babfbcae.html

although my patch was very likely the wrong approach (changing
VECTORS_BASE does not really make sense, as this is the one that can
not be remapped), there are still processors in the wild in which you
can configure the base address of the exception vectors.

Ben

[PATCH] Use CONFIG_VECTORS_BASE instead of hardcoded 0xffff0000.

[Russell King - ARM Linux]

On Wed, Oct 21, 2009 at 09:04:38AM +0200, Benoit Fouet wrote:
> Reading the DDI0344D (Cortex A8 r2p1 trm), it seemed to me that this
> was configurable with the security extension:
> http://infocentre.arm.com/help/topic/com.arm.doc.ddi0344d/Babfbcae.html
> 
> although my patch was very likely the wrong approach (changing
> VECTORS_BASE does not really make sense, as this is the one that can
> not be remapped), there are still processors in the wild in which you
> can configure the base address of the exception vectors.

However, the page at 0xffff0000 on MMU processors is part of the
kernel<->user API and its address can not be changed.

As I say, the patch does not make any sense.