* [PATCH 0/4] variable PHYS_OFFSET support
@ 2011-01-04 8:20 Nicolas Pitre
2011-01-04 8:20 ` [PATCH 1/4] ARM: runtime patching of __virt_to_phys() and __phys_to_virt() Nicolas Pitre
` (4 more replies)
0 siblings, 5 replies; 24+ messages in thread
From: Nicolas Pitre @ 2011-01-04 8:20 UTC (permalink / raw)
To: linux-arm-kernel
So here it is, in a form which I think should be ready for merging.
ARM mode is well tested.
Thumb2 mode is only compile tested at the moment.
Nicolas
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/4] ARM: runtime patching of __virt_to_phys() and __phys_to_virt()
2011-01-04 8:20 [PATCH 0/4] variable PHYS_OFFSET support Nicolas Pitre
@ 2011-01-04 8:20 ` Nicolas Pitre
2011-01-04 8:45 ` Russell King - ARM Linux
2011-01-04 8:20 ` [PATCH 2/4] ARM: make PHYS_OFFSET actually variable Nicolas Pitre
` (3 subsequent siblings)
4 siblings, 1 reply; 24+ messages in thread
From: Nicolas Pitre @ 2011-01-04 8:20 UTC (permalink / raw)
To: linux-arm-kernel
On ARM it is common to find different offsets for the location of
physical memory. In order to support multiple machine types with
a single kernel binary, we need to make PHYS_OFFSET a variable.
But turning PHYS_OFFSET into a global variable would impact performance
of many hot paths.
In the context of __virt_to_phys() and __phys_to_virt(), we currently have:
#define __virt_to_phys(x) ((x) - PAGE_OFFSET + PHYS_OFFSET)
This normally translates into the following assembly instruction:
add rx, rx, #(PHYS_OFFSET - PAGE_OFFSET)
If we can assume that the difference between PHYS_OFFSET and PAGE_OFFSET
will always fit into 8 bits shifted to the MSBs, then we can easily patch
this difference into the corresponding assembly instructions at run time.
This is like saying that phys and virt offsets will always be at least
16 MB aligned which is a pretty safe assumption.
So the idea is to create a table of pointers to all those add instructions,
and have the early boot code to walk and patch them up before the kernel
gets to use them. Result is equivalent to a variable PHYS_OFFSET with
next to zero performance impact compared to the constant PHYS_OFFSET.
Right now, the difference between PHYS_OFFSET and PAGE_OFFSET is determined
by the actual physical address the kernel is executing from upon start,
assuming that the kernel is located within the first 16 MB of RAM.
Thanks to Eric Miao and Russell King for their contributions to this patch.
Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>
---
arch/arm/Kconfig | 5 ++++
arch/arm/include/asm/memory.h | 51 +++++++++++++++++++++++++++++++++--------
arch/arm/kernel/head.S | 35 ++++++++++++++++++++++++++++
arch/arm/kernel/vmlinux.lds.S | 4 +++
4 files changed, 85 insertions(+), 10 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index d56d21c0..136ed9b 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -187,6 +187,11 @@ config VECTORS_BASE
help
The base address of exception vectors.
+config ARM_PATCH_PHYS_VIRT
+ bool
+ depends on EXPERIMENTAL
+ depends on !XIP && !THUMB2_KERNEL
+
source "init/Kconfig"
source "kernel/Kconfig.freezer"
diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
index 23c2e8e..2783ce2 100644
--- a/arch/arm/include/asm/memory.h
+++ b/arch/arm/include/asm/memory.h
@@ -133,16 +133,6 @@
#endif
/*
- * Physical vs virtual RAM address space conversion. These are
- * private definitions which should NOT be used outside memory.h
- * files. Use virt_to_phys/phys_to_virt/__pa/__va instead.
- */
-#ifndef __virt_to_phys
-#define __virt_to_phys(x) ((x) - PAGE_OFFSET + PHYS_OFFSET)
-#define __phys_to_virt(x) ((x) - PHYS_OFFSET + PAGE_OFFSET)
-#endif
-
-/*
* Convert a physical address to a Page Frame Number and back
*/
#define __phys_to_pfn(paddr) ((paddr) >> PAGE_SHIFT)
@@ -157,6 +147,47 @@
#ifndef __ASSEMBLY__
/*
+ * Physical vs virtual RAM address space conversion. These are
+ * private definitions which should NOT be used outside memory.h
+ * files. Use virt_to_phys/phys_to_virt/__pa/__va instead.
+ */
+#ifdef CONFIG_ARM_PATCH_PHYS_VIRT
+
+#ifdef __virt_to_phys
+#error "this machine configuration uses complex __virt_to_phys/__phys_to_virt and cannot use CONFIG_ARM_PATCH_PHYS_VIRT"
+#endif
+
+#define __pv_stub(from,to,instr) \
+ __asm__( \
+ "1: " instr "\t%0, %1, %2\n" \
+ " .pushsection .pv_table,\"a\"\n" \
+ " .long 1b\n" \
+ " .popsection" \
+ : "=r" (to) \
+ : "r" (from), "I" (1))
+
+static inline unsigned long __virt_to_phys(unsigned long x)
+{
+ unsigned long t;
+
+ __pv_stub(x, t, "add");
+ return t;
+}
+
+static inline unsigned long __phys_to_virt(unsigned long x)
+{
+ unsigned long t;
+
+ __pv_stub(x, t, "sub");
+ return t;
+}
+
+#else
+#define __virt_to_phys(x) ((x) - PAGE_OFFSET + PHYS_OFFSET)
+#define __phys_to_virt(x) ((x) - PHYS_OFFSET + PAGE_OFFSET)
+#endif
+
+/*
* The DMA mask corresponding to the maximum bus address allocatable
* using GFP_DMA. The default here places no restriction on DMA
* allocations. This must be the smallest DMA mask in the system,
diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
index 6bd82d2..eaaf0ad 100644
--- a/arch/arm/kernel/head.S
+++ b/arch/arm/kernel/head.S
@@ -95,6 +95,9 @@ ENTRY(stext)
#ifdef CONFIG_SMP_ON_UP
bl __fixup_smp
#endif
+#ifdef CONFIG_ARM_PATCH_PHYS_VIRT
+ bl __fixup_pv_table
+#endif
bl __create_page_tables
/*
@@ -433,4 +436,36 @@ smp_on_up:
#endif
+#ifdef CONFIG_ARM_PATCH_PHYS_VIRT
+
+/* __fixup_pv_table - patch the stub instructions with the delta between
+ * PHYS_OFFSET and PAGE_OFFSET, which is assumed to be 16MiB aligned and
+ * can be expressed by an immediate shifter operand. The stub instruction
+ * has a form of '(add|sub) rd, rn, #imm'.
+ */
+__fixup_pv_table:
+ adr r0, 1f
+ ldmia r0, {r3-r5}
+ sub r3, r0, r3 @ PHYS_OFFSET - PAGE_OFFSET
+ mov r6, r3, lsr #24 @ constant for add/sub instructions
+ teq r3, r6, lsl #24 @ must be 16MiB aligned
+ bne __error
+ orr r6, r6, #0x400 @ mask in rotate right 8 bits
+ add r4, r4, r3
+ add r5, r5, r3
+2: cmp r4, r5
+ ldrlo r7, [r4], #4
+ ldrlo ip, [r7, r3]
+ mov ip, ip, lsr #12
+ orr ip, r6, ip, lsl #12
+ strlo ip, [r7, r3]
+ blo 2b
+ mov pc, lr
+ENDPROC(__fixup_phys_virt)
+
+1: .word .
+ .word __pv_table_begin
+ .word __pv_table_end
+#endif
+
#include "head-common.S"
diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index cead889..fb32c9d 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -57,6 +57,10 @@ SECTIONS
__smpalt_end = .;
#endif
+ __pv_table_begin = .;
+ *(.pv_table)
+ __pv_table_end = .;
+
INIT_SETUP(16)
INIT_CALLS
--
1.7.3.2.193.g78bbb
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 2/4] ARM: make PHYS_OFFSET actually variable
2011-01-04 8:20 [PATCH 0/4] variable PHYS_OFFSET support Nicolas Pitre
2011-01-04 8:20 ` [PATCH 1/4] ARM: runtime patching of __virt_to_phys() and __phys_to_virt() Nicolas Pitre
@ 2011-01-04 8:20 ` Nicolas Pitre
2011-01-04 12:30 ` Russell King - ARM Linux
2011-01-04 8:20 ` [PATCH 3/4] ARM: module support for CONFIG_ARM_PATCH_PHYS_VIRT Nicolas Pitre
` (2 subsequent siblings)
4 siblings, 1 reply; 24+ messages in thread
From: Nicolas Pitre @ 2011-01-04 8:20 UTC (permalink / raw)
To: linux-arm-kernel
Previous patch made the implementation of __virt_to_phys() and
__phys_to_virt() assuming PHYS_OFFSET was variable. Let's make
PHYS_OFFSET actually variable by defining it in terms of __virt_to_phys().
There is a catch with this approach, as PHYS_OFFSET cannot be used as
a static initializer anymore. Only one such case was found in generic
code and removed, and that is in the declaration of the init_tags
structure (those who wish to use CONFIG_ARM_PATCH_PHYS_VIRT are better
not to rely on that init_tags default at all anyway, or they'll get
what they deserve).
Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>
PAGE_OFFSET)
---
arch/arm/include/asm/memory.h | 3 +++
arch/arm/kernel/setup.c | 2 ++
2 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
index 2783ce2..acc3126 100644
--- a/arch/arm/include/asm/memory.h
+++ b/arch/arm/include/asm/memory.h
@@ -182,6 +182,9 @@ static inline unsigned long __phys_to_virt(unsigned long x)
return t;
}
+#undef PHYS_OFFSET
+#define PHYS_OFFSET __virt_to_phys(PAGE_OFFSET)
+
#else
#define __virt_to_phys(x) ((x) - PAGE_OFFSET + PHYS_OFFSET)
#define __phys_to_virt(x) ((x) - PHYS_OFFSET + PAGE_OFFSET)
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 336f14e..7502bc1 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -703,8 +703,10 @@ static struct init_tags {
} init_tags __initdata = {
{ tag_size(tag_core), ATAG_CORE },
{ 1, PAGE_SIZE, 0xff },
+#ifndef CONFIG_ARM_PATCH_PHYS_VIRT
{ tag_size(tag_mem32), ATAG_MEM },
{ MEM_SIZE, PHYS_OFFSET },
+#endif
{ 0, ATAG_NONE }
};
--
1.7.3.2.193.g78bbb
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 3/4] ARM: module support for CONFIG_ARM_PATCH_PHYS_VIRT
2011-01-04 8:20 [PATCH 0/4] variable PHYS_OFFSET support Nicolas Pitre
2011-01-04 8:20 ` [PATCH 1/4] ARM: runtime patching of __virt_to_phys() and __phys_to_virt() Nicolas Pitre
2011-01-04 8:20 ` [PATCH 2/4] ARM: make PHYS_OFFSET actually variable Nicolas Pitre
@ 2011-01-04 8:20 ` Nicolas Pitre
2011-01-04 10:06 ` Russell King - ARM Linux
2011-01-04 8:20 ` [PATCH 4/4] ARM: support for Thumb-2 instructions with CONFIG_ARM_PATCH_PHYS_VIRT Nicolas Pitre
2011-01-04 10:41 ` [PATCH 0/4] variable PHYS_OFFSET support Russell King - ARM Linux
4 siblings, 1 reply; 24+ messages in thread
From: Nicolas Pitre @ 2011-01-04 8:20 UTC (permalink / raw)
To: linux-arm-kernel
Thanks to Russell King for his contribution to this patch.
Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>
---
arch/arm/include/asm/memory.h | 4 ++++
arch/arm/kernel/head.S | 35 +++++++++++++++++++++++++++++------
arch/arm/kernel/module.c | 13 +++++++++++++
3 files changed, 46 insertions(+), 6 deletions(-)
diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
index acc3126..f83be97 100644
--- a/arch/arm/include/asm/memory.h
+++ b/arch/arm/include/asm/memory.h
@@ -185,6 +185,10 @@ static inline unsigned long __phys_to_virt(unsigned long x)
#undef PHYS_OFFSET
#define PHYS_OFFSET __virt_to_phys(PAGE_OFFSET)
+extern void fixup_pv_table(void *table_start, void *table_end, long v2p_offset);
+#define fixup_pv_table(start, end) \
+ fixup_pv_table(start, end, PHYS_OFFSET - PAGE_OFFSET)
+
#else
#define __virt_to_phys(x) ((x) - PAGE_OFFSET + PHYS_OFFSET)
#define __phys_to_virt(x) ((x) - PHYS_OFFSET + PAGE_OFFSET)
diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
index eaaf0ad..621c792 100644
--- a/arch/arm/kernel/head.S
+++ b/arch/arm/kernel/head.S
@@ -447,12 +447,22 @@ __fixup_pv_table:
adr r0, 1f
ldmia r0, {r3-r5}
sub r3, r0, r3 @ PHYS_OFFSET - PAGE_OFFSET
+ add r4, r4, r3
+ add r5, r5, r3
mov r6, r3, lsr #24 @ constant for add/sub instructions
teq r3, r6, lsl #24 @ must be 16MiB aligned
bne __error
+ b __fixup_pv_table_loop
+ENDPROC(__fixup_phys_virt)
+
+1: .word .
+ .word __pv_table_begin
+ .word __pv_table_end
+
+ .pushsection .text
+
+ENTRY(__fixup_pv_table_loop)
orr r6, r6, #0x400 @ mask in rotate right 8 bits
- add r4, r4, r3
- add r5, r5, r3
2: cmp r4, r5
ldrlo r7, [r4], #4
ldrlo ip, [r7, r3]
@@ -461,11 +471,24 @@ __fixup_pv_table:
strlo ip, [r7, r3]
blo 2b
mov pc, lr
-ENDPROC(__fixup_phys_virt)
+ENDPROC(__fixup_phys_virt_loop)
+
+/*
+ * This is the C callable version used to fixup modules.
+ * void fixup_pv_table(void *table_start, void *table_end, long v2p_offset)
+ */
+ENTRY(fixup_pv_table)
+ stmfd sp!, {r4 - r7, lr}
+ mov r3, #0 @ offset (zero as we're in virtual space)
+ mov r4, r0 @ loop start
+ mov r5, r1 @ loop end
+ mov r6, r2, lsr #24 @ constant for add/sub instructions
+ bl __fixup_pv_table_loop
+ ldmfd sp!, {r4 - r7, pc}
+ENDPROC(fixup_pv_table)
+
+ .popsection
-1: .word .
- .word __pv_table_begin
- .word __pv_table_end
#endif
#include "head-common.S"
diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c
index d9bd786..6fef8d2 100644
--- a/arch/arm/kernel/module.c
+++ b/arch/arm/kernel/module.c
@@ -329,6 +329,19 @@ int
module_finalize(const Elf32_Ehdr *hdr, const Elf_Shdr *sechdrs,
struct module *module)
{
+#ifdef CONFIG_ARM_PATCH_PHYS_VIRT
+ char *secstrings = (void *)hdr + sechdrs[hdr->e_shstrndx].sh_offset;
+ const Elf_Shdr *s;
+
+ for (s = sechdrs; s < sechdrs + hdr->e_shnum; s++)
+ if (!strcmp(".pv_table", secstrings + s->sh_name)) {
+ void *pv_table_begin = (void *)s->sh_addr;
+ void *pv_table_end = pv_table_begin + s->sh_size;
+ fixup_pv_table(pv_table_begin, pv_table_end);
+ break;
+ }
+#endif
+
register_unwind_tables(module);
return 0;
}
--
1.7.3.2.193.g78bbb
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 4/4] ARM: support for Thumb-2 instructions with CONFIG_ARM_PATCH_PHYS_VIRT
2011-01-04 8:20 [PATCH 0/4] variable PHYS_OFFSET support Nicolas Pitre
` (2 preceding siblings ...)
2011-01-04 8:20 ` [PATCH 3/4] ARM: module support for CONFIG_ARM_PATCH_PHYS_VIRT Nicolas Pitre
@ 2011-01-04 8:20 ` Nicolas Pitre
2011-01-10 22:20 ` Dave Martin
2011-01-04 10:41 ` [PATCH 0/4] variable PHYS_OFFSET support Russell King - ARM Linux
4 siblings, 1 reply; 24+ messages in thread
From: Nicolas Pitre @ 2011-01-04 8:20 UTC (permalink / raw)
To: linux-arm-kernel
Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>
---
arch/arm/Kconfig | 3 +--
arch/arm/include/asm/memory.h | 5 +++--
arch/arm/kernel/head.S | 31 +++++++++++++++++++++++++++----
3 files changed, 31 insertions(+), 8 deletions(-)
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 136ed9b..feb374a 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -189,8 +189,7 @@ config VECTORS_BASE
config ARM_PATCH_PHYS_VIRT
bool
- depends on EXPERIMENTAL
- depends on !XIP && !THUMB2_KERNEL
+ depends on !XIP && EXPERIMENTAL
source "init/Kconfig"
diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
index f83be97..80a939d 100644
--- a/arch/arm/include/asm/memory.h
+++ b/arch/arm/include/asm/memory.h
@@ -157,14 +157,15 @@
#error "this machine configuration uses complex __virt_to_phys/__phys_to_virt and cannot use CONFIG_ARM_PATCH_PHYS_VIRT"
#endif
+/* the stub constant 0xff000000 is used to force the required insn encoding */
#define __pv_stub(from,to,instr) \
__asm__( \
- "1: " instr "\t%0, %1, %2\n" \
+ "1: " instr "\t%0, %1, #0xff000000\n" \
" .pushsection .pv_table,\"a\"\n" \
" .long 1b\n" \
" .popsection" \
: "=r" (to) \
- : "r" (from), "I" (1))
+ : "r" (from))
static inline unsigned long __virt_to_phys(unsigned long x)
{
diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
index 621c792..397271d 100644
--- a/arch/arm/kernel/head.S
+++ b/arch/arm/kernel/head.S
@@ -447,11 +447,11 @@ __fixup_pv_table:
adr r0, 1f
ldmia r0, {r3-r5}
sub r3, r0, r3 @ PHYS_OFFSET - PAGE_OFFSET
+ movs r6, r3, lsl #8 @ must be 16MiB aligned
+ bne __error
add r4, r4, r3
add r5, r5, r3
- mov r6, r3, lsr #24 @ constant for add/sub instructions
- teq r3, r6, lsl #24 @ must be 16MiB aligned
- bne __error
+ mov r6, r3
b __fixup_pv_table_loop
ENDPROC(__fixup_phys_virt)
@@ -462,6 +462,8 @@ ENDPROC(__fixup_phys_virt)
.pushsection .text
ENTRY(__fixup_pv_table_loop)
+#ifndef CONFIG_THUMB2_KERNEL
+ mov r6, r6, lsr #24 @ constant for add/sub instructions
orr r6, r6, #0x400 @ mask in rotate right 8 bits
2: cmp r4, r5
ldrlo r7, [r4], #4
@@ -471,6 +473,27 @@ ENTRY(__fixup_pv_table_loop)
strlo ip, [r7, r3]
blo 2b
mov pc, lr
+#else
+ teq r6, #0
+ beq 2f
+ clz r7, r6
+ lsr r6, #24
+ lsl r6, r7
+ bic r6, r6, #0x3080
+ lsrs r7, #1
+ orrcs r6, r6, #0x80
+ orr r6, r6, r7, lsl #12
+ orr r6, r6, #0x4000
+2: cmp r4, r5
+ bxhs lr
+ ldr r7, [r4], #4
+ add r7, r3
+ ldr ip, [r7, #2]
+ and ip, ip, #0x0f00
+ orr ip, ip, r6
+ str ip, [r7, #2]
+ b 2b
+#endif
ENDPROC(__fixup_phys_virt_loop)
/*
@@ -482,7 +505,7 @@ ENTRY(fixup_pv_table)
mov r3, #0 @ offset (zero as we're in virtual space)
mov r4, r0 @ loop start
mov r5, r1 @ loop end
- mov r6, r2, lsr #24 @ constant for add/sub instructions
+ mov r6, r2 @ PHYS_OFFSET - PAGE_OFFSET
bl __fixup_pv_table_loop
ldmfd sp!, {r4 - r7, pc}
ENDPROC(fixup_pv_table)
--
1.7.3.2.193.g78bbb
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 1/4] ARM: runtime patching of __virt_to_phys() and __phys_to_virt()
2011-01-04 8:20 ` [PATCH 1/4] ARM: runtime patching of __virt_to_phys() and __phys_to_virt() Nicolas Pitre
@ 2011-01-04 8:45 ` Russell King - ARM Linux
2011-01-04 14:32 ` Nicolas Pitre
0 siblings, 1 reply; 24+ messages in thread
From: Russell King - ARM Linux @ 2011-01-04 8:45 UTC (permalink / raw)
To: linux-arm-kernel
This is basically my patch with a few blank lines removed, a couple
of \n's also removed, a #error if __virt_to_phys is defined by a platform,
a minor tweak to the assembly and it being only usable on PXA.
I much prefer my patch over this as anyone can use it. That's one of
the reasons why I arranged the code testing for __virt_to_phys as I
did, so the config option could be offered without having a big long
dependency list attached to it.
On Tue, Jan 04, 2011 at 03:20:05AM -0500, Nicolas Pitre wrote:
> On ARM it is common to find different offsets for the location of
> physical memory. In order to support multiple machine types with
> a single kernel binary, we need to make PHYS_OFFSET a variable.
> But turning PHYS_OFFSET into a global variable would impact performance
> of many hot paths.
>
> In the context of __virt_to_phys() and __phys_to_virt(), we currently have:
>
> #define __virt_to_phys(x) ((x) - PAGE_OFFSET + PHYS_OFFSET)
>
> This normally translates into the following assembly instruction:
>
> add rx, rx, #(PHYS_OFFSET - PAGE_OFFSET)
>
> If we can assume that the difference between PHYS_OFFSET and PAGE_OFFSET
> will always fit into 8 bits shifted to the MSBs, then we can easily patch
> this difference into the corresponding assembly instructions at run time.
> This is like saying that phys and virt offsets will always be at least
> 16 MB aligned which is a pretty safe assumption.
>
> So the idea is to create a table of pointers to all those add instructions,
> and have the early boot code to walk and patch them up before the kernel
> gets to use them. Result is equivalent to a variable PHYS_OFFSET with
> next to zero performance impact compared to the constant PHYS_OFFSET.
>
> Right now, the difference between PHYS_OFFSET and PAGE_OFFSET is determined
> by the actual physical address the kernel is executing from upon start,
> assuming that the kernel is located within the first 16 MB of RAM.
>
> Thanks to Eric Miao and Russell King for their contributions to this patch.
>
> Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>
> ---
> arch/arm/Kconfig | 5 ++++
> arch/arm/include/asm/memory.h | 51 +++++++++++++++++++++++++++++++++--------
> arch/arm/kernel/head.S | 35 ++++++++++++++++++++++++++++
> arch/arm/kernel/vmlinux.lds.S | 4 +++
> 4 files changed, 85 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index d56d21c0..136ed9b 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -187,6 +187,11 @@ config VECTORS_BASE
> help
> The base address of exception vectors.
>
> +config ARM_PATCH_PHYS_VIRT
> + bool
> + depends on EXPERIMENTAL
> + depends on !XIP && !THUMB2_KERNEL
> +
> source "init/Kconfig"
>
> source "kernel/Kconfig.freezer"
> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> index 23c2e8e..2783ce2 100644
> --- a/arch/arm/include/asm/memory.h
> +++ b/arch/arm/include/asm/memory.h
> @@ -133,16 +133,6 @@
> #endif
>
> /*
> - * Physical vs virtual RAM address space conversion. These are
> - * private definitions which should NOT be used outside memory.h
> - * files. Use virt_to_phys/phys_to_virt/__pa/__va instead.
> - */
> -#ifndef __virt_to_phys
> -#define __virt_to_phys(x) ((x) - PAGE_OFFSET + PHYS_OFFSET)
> -#define __phys_to_virt(x) ((x) - PHYS_OFFSET + PAGE_OFFSET)
> -#endif
> -
> -/*
> * Convert a physical address to a Page Frame Number and back
> */
> #define __phys_to_pfn(paddr) ((paddr) >> PAGE_SHIFT)
> @@ -157,6 +147,47 @@
> #ifndef __ASSEMBLY__
>
> /*
> + * Physical vs virtual RAM address space conversion. These are
> + * private definitions which should NOT be used outside memory.h
> + * files. Use virt_to_phys/phys_to_virt/__pa/__va instead.
> + */
> +#ifdef CONFIG_ARM_PATCH_PHYS_VIRT
> +
> +#ifdef __virt_to_phys
> +#error "this machine configuration uses complex __virt_to_phys/__phys_to_virt and cannot use CONFIG_ARM_PATCH_PHYS_VIRT"
> +#endif
> +
> +#define __pv_stub(from,to,instr) \
> + __asm__( \
> + "1: " instr "\t%0, %1, %2\n" \
> + " .pushsection .pv_table,\"a\"\n" \
> + " .long 1b\n" \
> + " .popsection" \
> + : "=r" (to) \
> + : "r" (from), "I" (1))
> +
> +static inline unsigned long __virt_to_phys(unsigned long x)
> +{
> + unsigned long t;
> +
> + __pv_stub(x, t, "add");
> + return t;
> +}
> +
> +static inline unsigned long __phys_to_virt(unsigned long x)
> +{
> + unsigned long t;
> +
> + __pv_stub(x, t, "sub");
> + return t;
> +}
> +
> +#else
> +#define __virt_to_phys(x) ((x) - PAGE_OFFSET + PHYS_OFFSET)
> +#define __phys_to_virt(x) ((x) - PHYS_OFFSET + PAGE_OFFSET)
> +#endif
> +
> +/*
> * The DMA mask corresponding to the maximum bus address allocatable
> * using GFP_DMA. The default here places no restriction on DMA
> * allocations. This must be the smallest DMA mask in the system,
> diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
> index 6bd82d2..eaaf0ad 100644
> --- a/arch/arm/kernel/head.S
> +++ b/arch/arm/kernel/head.S
> @@ -95,6 +95,9 @@ ENTRY(stext)
> #ifdef CONFIG_SMP_ON_UP
> bl __fixup_smp
> #endif
> +#ifdef CONFIG_ARM_PATCH_PHYS_VIRT
> + bl __fixup_pv_table
> +#endif
> bl __create_page_tables
>
> /*
> @@ -433,4 +436,36 @@ smp_on_up:
>
> #endif
>
> +#ifdef CONFIG_ARM_PATCH_PHYS_VIRT
> +
> +/* __fixup_pv_table - patch the stub instructions with the delta between
> + * PHYS_OFFSET and PAGE_OFFSET, which is assumed to be 16MiB aligned and
> + * can be expressed by an immediate shifter operand. The stub instruction
> + * has a form of '(add|sub) rd, rn, #imm'.
> + */
> +__fixup_pv_table:
> + adr r0, 1f
> + ldmia r0, {r3-r5}
> + sub r3, r0, r3 @ PHYS_OFFSET - PAGE_OFFSET
> + mov r6, r3, lsr #24 @ constant for add/sub instructions
> + teq r3, r6, lsl #24 @ must be 16MiB aligned
> + bne __error
> + orr r6, r6, #0x400 @ mask in rotate right 8 bits
> + add r4, r4, r3
> + add r5, r5, r3
> +2: cmp r4, r5
> + ldrlo r7, [r4], #4
> + ldrlo ip, [r7, r3]
> + mov ip, ip, lsr #12
> + orr ip, r6, ip, lsl #12
> + strlo ip, [r7, r3]
> + blo 2b
> + mov pc, lr
> +ENDPROC(__fixup_phys_virt)
> +
> +1: .word .
> + .word __pv_table_begin
> + .word __pv_table_end
> +#endif
> +
> #include "head-common.S"
> diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
> index cead889..fb32c9d 100644
> --- a/arch/arm/kernel/vmlinux.lds.S
> +++ b/arch/arm/kernel/vmlinux.lds.S
> @@ -57,6 +57,10 @@ SECTIONS
> __smpalt_end = .;
> #endif
>
> + __pv_table_begin = .;
> + *(.pv_table)
> + __pv_table_end = .;
> +
> INIT_SETUP(16)
>
> INIT_CALLS
> --
> 1.7.3.2.193.g78bbb
>
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 3/4] ARM: module support for CONFIG_ARM_PATCH_PHYS_VIRT
2011-01-04 8:20 ` [PATCH 3/4] ARM: module support for CONFIG_ARM_PATCH_PHYS_VIRT Nicolas Pitre
@ 2011-01-04 10:06 ` Russell King - ARM Linux
0 siblings, 0 replies; 24+ messages in thread
From: Russell King - ARM Linux @ 2011-01-04 10:06 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jan 04, 2011 at 03:20:07AM -0500, Nicolas Pitre wrote:
> Thanks to Russell King for his contribution to this patch.
>
> Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>
This should all be part of the patch introducing the patching - without
it you will end up with a kernel which has the dynamic P:V translation
support but the modules will be buggered.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 0/4] variable PHYS_OFFSET support
2011-01-04 8:20 [PATCH 0/4] variable PHYS_OFFSET support Nicolas Pitre
` (3 preceding siblings ...)
2011-01-04 8:20 ` [PATCH 4/4] ARM: support for Thumb-2 instructions with CONFIG_ARM_PATCH_PHYS_VIRT Nicolas Pitre
@ 2011-01-04 10:41 ` Russell King - ARM Linux
2011-01-04 14:37 ` Nicolas Pitre
4 siblings, 1 reply; 24+ messages in thread
From: Russell King - ARM Linux @ 2011-01-04 10:41 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jan 04, 2011 at 03:20:04AM -0500, Nicolas Pitre wrote:
> So here it is, in a form which I think should be ready for merging.
>
> ARM mode is well tested.
> Thumb2 mode is only compile tested at the moment.
I don't believe this is ready for merging - there's still work to be
done with the early assembly code using PHYS_OFFSET. That currently
doesn't _appear_ to be a problem because it still uses the old platform
specific setting of PHYS_OFFSET - and that needs careful thought as:
#if (PHYS_OFFSET & 0x001fffff)
#error "PHYS_OFFSET must be at an even 2MiB boundary!"
#endif
has been eroded from 16MB downwards to 2MB over time because of
platform setups - eg, MSM for example.
As we're having to store the p:v offset (it's cheaper to store it than
your way of making PHYS_OFFSET depend on the translation and PAGE_OFFSET)
then we might as well also store what we think is the physical offset
and use that for PHYS_OFFSET too once the assembly code issue has been
sorted.
So, what I suggest is a different patch ordering:
1. Fix the assembly code not to rely upon a fixed PHYS_OFFSET (is this
even possible with the 2MB requirement?)
2. Fix the initializers in platform code (ignoring MSM).
3. Rename PHYS_OFFSET to be PLAT_PHYS_OFFSET in platform memory.h files,
defining PHYS_OFFSET in asm/memory.h to be PLAT_PHYS_OFFSET. (We
could make PHYS_OFFSET at this point be a variable, which'd stop any
new initializers using PHYS_OFFSET - which I think would be beneficial
anyway.)
4. Introduce P2V patching _with_ the module support. With this patch
create __pv_phys_offset, and if P2V patching is enabled, redefine
PHYS_OFFSET to be this variable. Note that __pv_phys_offset would
need to be exported to modules.
So, I don't think it's ready for this coming merge window.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 2/4] ARM: make PHYS_OFFSET actually variable
2011-01-04 8:20 ` [PATCH 2/4] ARM: make PHYS_OFFSET actually variable Nicolas Pitre
@ 2011-01-04 12:30 ` Russell King - ARM Linux
2011-01-04 17:54 ` Nicolas Pitre
0 siblings, 1 reply; 24+ messages in thread
From: Russell King - ARM Linux @ 2011-01-04 12:30 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jan 04, 2011 at 03:20:06AM -0500, Nicolas Pitre wrote:
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index 336f14e..7502bc1 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -703,8 +703,10 @@ static struct init_tags {
> } init_tags __initdata = {
> { tag_size(tag_core), ATAG_CORE },
> { 1, PAGE_SIZE, 0xff },
> +#ifndef CONFIG_ARM_PATCH_PHYS_VIRT
> { tag_size(tag_mem32), ATAG_MEM },
> { MEM_SIZE, PHYS_OFFSET },
> +#endif
Removing the definition is not really on - it's there as a fallback and
removing fallbacks is not a good idea unless you can prove beyond doubt
that it'll never be used.
As that depends on the boot loader, it's something that can never be
proven. So, arrange for it to be initialized at runtime, just like I
will be doing in my revised version I'm working on which I detailed in
a previous email.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/4] ARM: runtime patching of __virt_to_phys() and __phys_to_virt()
2011-01-04 8:45 ` Russell King - ARM Linux
@ 2011-01-04 14:32 ` Nicolas Pitre
2011-01-04 16:53 ` Russell King - ARM Linux
0 siblings, 1 reply; 24+ messages in thread
From: Nicolas Pitre @ 2011-01-04 14:32 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 4 Jan 2011, Russell King - ARM Linux wrote:
> This is basically my patch with a few blank lines removed, a couple
> of \n's also removed, a #error if __virt_to_phys is defined by a platform,
> a minor tweak to the assembly and it being only usable on PXA.
>
> I much prefer my patch over this as anyone can use it. That's one of
> the reasons why I arranged the code testing for __virt_to_phys as I
> did, so the config option could be offered without having a big long
> dependency list attached to it.
I don't think offering the option that people can turn on and not having
the code effectively perform as expected is a good idea. People might
be expecting the feature to be there while in practice it is ignored
which would lead to confusion. Better to offer it so it can be selected
as needed in combination with other features, such as
CONFIG_AUTO_ZRELADDR (which in my opinion would be better hidden from
user selection as well).
As to the authorship, since I drafted the original design, Eric Miao did
the first implementation to validate the concept, and the code surviving
is mostly yours, I didn't know who to singularly attribute the patch to
in the author field. I can put yourself there if you feel this is more
appropriate.
Nicolas
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 0/4] variable PHYS_OFFSET support
2011-01-04 10:41 ` [PATCH 0/4] variable PHYS_OFFSET support Russell King - ARM Linux
@ 2011-01-04 14:37 ` Nicolas Pitre
0 siblings, 0 replies; 24+ messages in thread
From: Nicolas Pitre @ 2011-01-04 14:37 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 4 Jan 2011, Russell King - ARM Linux wrote:
> So, I don't think it's ready for this coming merge window.
OK, agreed.
We could also take the time to make the kdump kernel benefit from this
feature at the same time, removing the need from a special build of the
kernel for this purpose, which would be a direct application.
Nicolas
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/4] ARM: runtime patching of __virt_to_phys() and __phys_to_virt()
2011-01-04 14:32 ` Nicolas Pitre
@ 2011-01-04 16:53 ` Russell King - ARM Linux
2011-01-04 17:50 ` Nicolas Pitre
0 siblings, 1 reply; 24+ messages in thread
From: Russell King - ARM Linux @ 2011-01-04 16:53 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jan 04, 2011 at 09:32:41AM -0500, Nicolas Pitre wrote:
> On Tue, 4 Jan 2011, Russell King - ARM Linux wrote:
> > This is basically my patch with a few blank lines removed, a couple
> > of \n's also removed, a #error if __virt_to_phys is defined by a platform,
> > a minor tweak to the assembly and it being only usable on PXA.
> >
> > I much prefer my patch over this as anyone can use it. That's one of
> > the reasons why I arranged the code testing for __virt_to_phys as I
> > did, so the config option could be offered without having a big long
> > dependency list attached to it.
>
> I don't think offering the option that people can turn on and not having
> the code effectively perform as expected is a good idea. People might
> be expecting the feature to be there while in practice it is ignored
> which would lead to confusion.
Our aims are different then. My aim is to move the code to a point where
it works for _everyone_ it possibly can - and theoretically that's every
platform except:
1. MSM due to their PHYS_OFFSET being 2MB aligned, rather than the more
normal 256MB alignment.
2. Anyone with complex V:P mappings
(1) is dealt with easily by a dependency in the configuration preventing
the option being visible. (2) is dealt with at runtime by ignoring the
configuration option - resulting in the p2v tables being empty. The end
result will still run on the platform, but it won't do the relocation
stuff. (2) could also be dealt with by adding the necessary dependencies
to the configuration option which is the longer term solution.
Lastly, marking the option as 'EXPERIMENTAL' is there to convey that it
may not work for everyone, and people should expect things not to work if
they enable such an option (and report when that's the case.)
Another reason why selecting this option is wrong is that it is incompatible
with XIP. If you're going to unconditionally enable it for platforms like
PXA, make sure you strip out all of PXA's XIP support before you do so,
otherwise you'll build a kernel which has absolutely no way of ever booting.
> As to the authorship, since I drafted the original design, Eric Miao did
> the first implementation to validate the concept, and the code surviving
> is mostly yours, I didn't know who to singularly attribute the patch to
> in the author field. I can put yourself there if you feel this is more
> appropriate.
The correct thing to do is to ensure that it has Eric's and my sign-offs.
If you compare Eric's to my version, mine has a fair amount of changes.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/4] ARM: runtime patching of __virt_to_phys() and __phys_to_virt()
2011-01-04 16:53 ` Russell King - ARM Linux
@ 2011-01-04 17:50 ` Nicolas Pitre
2011-01-04 18:06 ` Russell King - ARM Linux
0 siblings, 1 reply; 24+ messages in thread
From: Nicolas Pitre @ 2011-01-04 17:50 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 4 Jan 2011, Russell King - ARM Linux wrote:
> On Tue, Jan 04, 2011 at 09:32:41AM -0500, Nicolas Pitre wrote:
> > On Tue, 4 Jan 2011, Russell King - ARM Linux wrote:
> > > This is basically my patch with a few blank lines removed, a couple
> > > of \n's also removed, a #error if __virt_to_phys is defined by a platform,
> > > a minor tweak to the assembly and it being only usable on PXA.
> > >
> > > I much prefer my patch over this as anyone can use it. That's one of
> > > the reasons why I arranged the code testing for __virt_to_phys as I
> > > did, so the config option could be offered without having a big long
> > > dependency list attached to it.
> >
> > I don't think offering the option that people can turn on and not having
> > the code effectively perform as expected is a good idea. People might
> > be expecting the feature to be there while in practice it is ignored
> > which would lead to confusion.
>
> Our aims are different then. My aim is to move the code to a point where
> it works for _everyone_ it possibly can - and theoretically that's every
> platform except:
>
> 1. MSM due to their PHYS_OFFSET being 2MB aligned, rather than the more
> normal 256MB alignment.
> 2. Anyone with complex V:P mappings
I completely agree with that goal. But I'd prefer for those platforms
which are not yet supported by this feature not to be able to compile
rather than silently ignore the feature and not behave as expected.
> (1) is dealt with easily by a dependency in the configuration preventing
> the option being visible. (2) is dealt with at runtime by ignoring the
> configuration option - resulting in the p2v tables being empty. The end
> result will still run on the platform, but it won't do the relocation
> stuff. (2) could also be dealt with by adding the necessary dependencies
> to the configuration option which is the longer term solution.
Since (2) is not supported yet with this config option selected, I think
it is best to simply #error the build.
> Lastly, marking the option as 'EXPERIMENTAL' is there to convey that it
> may not work for everyone, and people should expect things not to work if
> they enable such an option (and report when that's the case.)
Sure, hence my #error in the patch which is even easier to diagnose and
self explanatory.
And in fact I think that this would indeed be simpler to just fall back
to a global variable for PHYS_OFFSET when a platform defines its own
p2v/v2p mapping. This way, the goal of this feature would be
universally available.
OTOH, one of the long term goal for this is to be able to support more
than one SOC family with the same kernel binary. I don't think it is
worth extending this support to be able to also include those platforms
with a complex v2p and p2v translation, as the overhead for all included
platforms would simply kill the advantages of having a single kernel
binary. So I still don't see a big compelling reason to support (2) at
all with this, except maybe only for kdump purposes.
> Another reason why selecting this option is wrong is that it is incompatible
> with XIP. If you're going to unconditionally enable it for platforms like
> PXA, make sure you strip out all of PXA's XIP support before you do so,
> otherwise you'll build a kernel which has absolutely no way of ever booting.
Well, my idea is more about a global config option that makes sense for
the user, like "physically position independent kernel binary" or
something like that. This option would of course be dependent on !XIP
and !ZBOOT_ROM, and would in turn select AUTO_ZRELADDR and
ARM_PATCH_PHYS_VIRT. In fact, maybe those options should all be
consolidated under a single generic config option expressing the goal to
be achieved instead of the implementation strategy.
> > As to the authorship, since I drafted the original design, Eric Miao did
> > the first implementation to validate the concept, and the code surviving
> > is mostly yours, I didn't know who to singularly attribute the patch to
> > in the author field. I can put yourself there if you feel this is more
> > appropriate.
>
> The correct thing to do is to ensure that it has Eric's and my
> sign-offs.
Fair enough.
> If you compare Eric's to my version, mine has a fair amount of
> changes.
Yep, and I'm now adding to the mix. So this is a good idea to clarify
this issue.
Nicolas
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 2/4] ARM: make PHYS_OFFSET actually variable
2011-01-04 12:30 ` Russell King - ARM Linux
@ 2011-01-04 17:54 ` Nicolas Pitre
0 siblings, 0 replies; 24+ messages in thread
From: Nicolas Pitre @ 2011-01-04 17:54 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 4 Jan 2011, Russell King - ARM Linux wrote:
> On Tue, Jan 04, 2011 at 03:20:06AM -0500, Nicolas Pitre wrote:
> > diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> > index 336f14e..7502bc1 100644
> > --- a/arch/arm/kernel/setup.c
> > +++ b/arch/arm/kernel/setup.c
> > @@ -703,8 +703,10 @@ static struct init_tags {
> > } init_tags __initdata = {
> > { tag_size(tag_core), ATAG_CORE },
> > { 1, PAGE_SIZE, 0xff },
> > +#ifndef CONFIG_ARM_PATCH_PHYS_VIRT
> > { tag_size(tag_mem32), ATAG_MEM },
> > { MEM_SIZE, PHYS_OFFSET },
> > +#endif
>
> Removing the definition is not really on - it's there as a fallback and
> removing fallbacks is not a good idea unless you can prove beyond doubt
> that it'll never be used.
>
> As that depends on the boot loader, it's something that can never be
> proven. So, arrange for it to be initialized at runtime, just like I
> will be doing in my revised version I'm working on which I detailed in
> a previous email.
Agreed.
I'll wait for that revised version before working on this further.
Nicolas
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/4] ARM: runtime patching of __virt_to_phys() and __phys_to_virt()
2011-01-04 17:50 ` Nicolas Pitre
@ 2011-01-04 18:06 ` Russell King - ARM Linux
2011-01-04 18:25 ` David Brown
2011-01-04 18:29 ` Nicolas Pitre
0 siblings, 2 replies; 24+ messages in thread
From: Russell King - ARM Linux @ 2011-01-04 18:06 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jan 04, 2011 at 12:50:28PM -0500, Nicolas Pitre wrote:
> On Tue, 4 Jan 2011, Russell King - ARM Linux wrote:
> > Our aims are different then. My aim is to move the code to a point where
> > it works for _everyone_ it possibly can - and theoretically that's every
> > platform except:
> >
> > 1. MSM due to their PHYS_OFFSET being 2MB aligned, rather than the more
> > normal 256MB alignment.
> > 2. Anyone with complex V:P mappings
>
> I completely agree with that goal. But I'd prefer for those platforms
> which are not yet supported by this feature not to be able to compile
> rather than silently ignore the feature and not behave as expected.
>
> > (1) is dealt with easily by a dependency in the configuration preventing
> > the option being visible. (2) is dealt with at runtime by ignoring the
> > configuration option - resulting in the p2v tables being empty. The end
> > result will still run on the platform, but it won't do the relocation
> > stuff. (2) could also be dealt with by adding the necessary dependencies
> > to the configuration option which is the longer term solution.
>
> Since (2) is not supported yet with this config option selected, I think
> it is best to simply #error the build.
>
> > Lastly, marking the option as 'EXPERIMENTAL' is there to convey that it
> > may not work for everyone, and people should expect things not to work if
> > they enable such an option (and report when that's the case.)
>
> Sure, hence my #error in the patch which is even easier to diagnose and
> self explanatory.
You're making a mountain out of a mole hill. At present, there is one
platform which defines its own complex v:p mapping and that is Realview,
but only when sparsemem is enabled. As already mentioned, MSM is the
only other platform which can't use this method. So that's a simple
dependency line against the config.
The other breakages are use of PHYS_OFFSET as an initializer which is a
build-error inducing failure, and adopting the approach I outlined in my
4 patch set results in many of those going away before we get support for
this merged - even better, if PHYS_OFFSET were always to be variable-like,
then we'd stop any new uses even appearing.
> And in fact I think that this would indeed be simpler to just fall back
> to a global variable for PHYS_OFFSET when a platform defines its own
> p2v/v2p mapping. This way, the goal of this feature would be
> universally available.
Not really. Platforms define their own mapping because it's not a simple
addition or subtraction, but because it's a complex non-linear conversion.
#define __phys_to_virt(phys) \
((phys) >= 0x80000000 ? (phys) - 0x80000000 + PAGE_OFFSET2 : \
(phys) >= 0x20000000 ? (phys) - 0x20000000 + PAGE_OFFSET1 : \
(phys) + PAGE_OFFSET)
#define __virt_to_phys(virt) \
((virt) >= PAGE_OFFSET2 ? (virt) - PAGE_OFFSET2 + 0x80000000 : \
(virt) >= PAGE_OFFSET1 ? (virt) - PAGE_OFFSET1 + 0x20000000 : \
(virt) - PAGE_OFFSET)
This doesn't lend itself in any way to a variable-based PHYS_OFFSET, and
could never be subsituted code-wise at run time without significant
effort.
In fact, platforms which have complex V:P mappings can _never_ be a part
of a kernel which has this feature enabled.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/4] ARM: runtime patching of __virt_to_phys() and __phys_to_virt()
2011-01-04 18:06 ` Russell King - ARM Linux
@ 2011-01-04 18:25 ` David Brown
2011-01-04 18:33 ` Nicolas Pitre
2011-01-04 18:29 ` Nicolas Pitre
1 sibling, 1 reply; 24+ messages in thread
From: David Brown @ 2011-01-04 18:25 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jan 04 2011, Russell King - ARM Linux wrote:
> On Tue, Jan 04, 2011 at 12:50:28PM -0500, Nicolas Pitre wrote:
> You're making a mountain out of a mole hill. At present, there is one
> platform which defines its own complex v:p mapping and that is Realview,
> but only when sparsemem is enabled. As already mentioned, MSM is the
> only other platform which can't use this method. So that's a simple
> dependency line against the config.
This is unfortunate, however, for us, since it would keep us having all
of our targets having to be built exclusively.
Any idea how much it would hurt other targets to have the
__virt_to_phys() and __phys_to_virt() have a 16-bit fixup, even if only
the upper 8 bits are used?
David
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/4] ARM: runtime patching of __virt_to_phys() and __phys_to_virt()
2011-01-04 18:06 ` Russell King - ARM Linux
2011-01-04 18:25 ` David Brown
@ 2011-01-04 18:29 ` Nicolas Pitre
1 sibling, 0 replies; 24+ messages in thread
From: Nicolas Pitre @ 2011-01-04 18:29 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 4 Jan 2011, Russell King - ARM Linux wrote:
> On Tue, Jan 04, 2011 at 12:50:28PM -0500, Nicolas Pitre wrote:
> > On Tue, 4 Jan 2011, Russell King - ARM Linux wrote:
> > > Our aims are different then. My aim is to move the code to a point where
> > > it works for _everyone_ it possibly can - and theoretically that's every
> > > platform except:
> > >
> > > 1. MSM due to their PHYS_OFFSET being 2MB aligned, rather than the more
> > > normal 256MB alignment.
> > > 2. Anyone with complex V:P mappings
> >
> > I completely agree with that goal. But I'd prefer for those platforms
> > which are not yet supported by this feature not to be able to compile
> > rather than silently ignore the feature and not behave as expected.
> >
> > > (1) is dealt with easily by a dependency in the configuration preventing
> > > the option being visible. (2) is dealt with at runtime by ignoring the
> > > configuration option - resulting in the p2v tables being empty. The end
> > > result will still run on the platform, but it won't do the relocation
> > > stuff. (2) could also be dealt with by adding the necessary dependencies
> > > to the configuration option which is the longer term solution.
> >
> > Since (2) is not supported yet with this config option selected, I think
> > it is best to simply #error the build.
> >
> > > Lastly, marking the option as 'EXPERIMENTAL' is there to convey that it
> > > may not work for everyone, and people should expect things not to work if
> > > they enable such an option (and report when that's the case.)
> >
> > Sure, hence my #error in the patch which is even easier to diagnose and
> > self explanatory.
>
> You're making a mountain out of a mole hill. At present, there is one
> platform which defines its own complex v:p mapping and that is Realview,
> but only when sparsemem is enabled. As already mentioned, MSM is the
> only other platform which can't use this method. So that's a simple
> dependency line against the config.
OK, agreed.
The #error might still be a good idea as a stop gate if some future
platforms also define a complex v:p mapping and the Kbuild dependency is
missed, just like we have things like:
#if __LINUX_ARM_ARCH__ < 6
#error SMP not supported on pre-ARMv6 CPUs
#endif
> The other breakages are use of PHYS_OFFSET as an initializer which is a
> build-error inducing failure, and adopting the approach I outlined in my
> 4 patch set results in many of those going away before we get support for
> this merged - even better, if PHYS_OFFSET were always to be variable-like,
> then we'd stop any new uses even appearing.
Completely agreed.
> > And in fact I think that this would indeed be simpler to just fall back
> > to a global variable for PHYS_OFFSET when a platform defines its own
> > p2v/v2p mapping. This way, the goal of this feature would be
> > universally available.
>
> Not really. Platforms define their own mapping because it's not a simple
> addition or subtraction, but because it's a complex non-linear conversion.
>
> #define __phys_to_virt(phys) \
> ((phys) >= 0x80000000 ? (phys) - 0x80000000 + PAGE_OFFSET2 : \
> (phys) >= 0x20000000 ? (phys) - 0x20000000 + PAGE_OFFSET1 : \
> (phys) + PAGE_OFFSET)
>
> #define __virt_to_phys(virt) \
> ((virt) >= PAGE_OFFSET2 ? (virt) - PAGE_OFFSET2 + 0x80000000 : \
> (virt) >= PAGE_OFFSET1 ? (virt) - PAGE_OFFSET1 + 0x20000000 : \
> (virt) - PAGE_OFFSET)
Indeed, no point trying to support that.
> This doesn't lend itself in any way to a variable-based PHYS_OFFSET, and
> could never be subsituted code-wise at run time without significant
> effort.
>
> In fact, platforms which have complex V:P mappings can _never_ be a part
> of a kernel which has this feature enabled.
I said the same already, perhaps not as strongly.
If we _wanted_ to support such platforms in a common kernel binary, then
we would have to fall back to function pointers. The end goal would be
accomplished through a different implementation. I don't think it is
worth going there though.
Nicolas
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/4] ARM: runtime patching of __virt_to_phys() and __phys_to_virt()
2011-01-04 18:25 ` David Brown
@ 2011-01-04 18:33 ` Nicolas Pitre
2011-01-04 19:00 ` David Brown
0 siblings, 1 reply; 24+ messages in thread
From: Nicolas Pitre @ 2011-01-04 18:33 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 4 Jan 2011, David Brown wrote:
> On Tue, Jan 04 2011, Russell King - ARM Linux wrote:
>
> > On Tue, Jan 04, 2011 at 12:50:28PM -0500, Nicolas Pitre wrote:
>
> > You're making a mountain out of a mole hill. At present, there is one
> > platform which defines its own complex v:p mapping and that is Realview,
> > but only when sparsemem is enabled. As already mentioned, MSM is the
> > only other platform which can't use this method. So that's a simple
> > dependency line against the config.
>
> This is unfortunate, however, for us, since it would keep us having all
> of our targets having to be built exclusively.
>
> Any idea how much it would hurt other targets to have the
> __virt_to_phys() and __phys_to_virt() have a 16-bit fixup, even if only
> the upper 8 bits are used?
Probably not that much. But let's make it work satisfactorily for the
general case first, and then we might consider and test variations that
could accommodate msm.
Nicolas
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/4] ARM: runtime patching of __virt_to_phys() and __phys_to_virt()
2011-01-04 18:33 ` Nicolas Pitre
@ 2011-01-04 19:00 ` David Brown
2011-01-04 20:17 ` Russell King - ARM Linux
0 siblings, 1 reply; 24+ messages in thread
From: David Brown @ 2011-01-04 19:00 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jan 04 2011, Nicolas Pitre wrote:
> On Tue, 4 Jan 2011, David Brown wrote:
>
>> Any idea how much it would hurt other targets to have the
>> __virt_to_phys() and __phys_to_virt() have a 16-bit fixup, even if only
>> the upper 8 bits are used?
>
> Probably not that much. But let's make it work satisfactorily for the
> general case first, and then we might consider and test variations that
> could accommodate msm.
Sounds like a good plan. We've got quite a few other things to clean up
before we can build for more than one arch anyway.
David
--
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/4] ARM: runtime patching of __virt_to_phys() and __phys_to_virt()
2011-01-04 19:00 ` David Brown
@ 2011-01-04 20:17 ` Russell King - ARM Linux
0 siblings, 0 replies; 24+ messages in thread
From: Russell King - ARM Linux @ 2011-01-04 20:17 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Jan 04, 2011 at 11:00:05AM -0800, David Brown wrote:
> On Tue, Jan 04 2011, Nicolas Pitre wrote:
>
> > On Tue, 4 Jan 2011, David Brown wrote:
> >
> >> Any idea how much it would hurt other targets to have the
> >> __virt_to_phys() and __phys_to_virt() have a 16-bit fixup, even if only
> >> the upper 8 bits are used?
> >
> > Probably not that much. But let's make it work satisfactorily for the
> > general case first, and then we might consider and test variations that
> > could accommodate msm.
>
> Sounds like a good plan. We've got quite a few other things to clean up
> before we can build for more than one arch anyway.
Actually, we can do this quite easily. Having a config symbol which
gets enabled when MSM is added, which then enables two consecutive
__pv_fixups gives us 16-bits to play with.
As I originally intended with my implementation, the value field of the
instruction can be used to identify what this fixup is about - and we
can put that to use by selecting either bits 31-24 or 23-16 of the
offset value.
This still allows optimizations to happen with instruction scheduling -
eg, for the MSM kernel I've just built with this enabled:
c000b320: e2844001 add r4, r4, #1 ; 0x1
c000b324: e59f3264 ldr r3, [pc, #612] ; c000b590 <setup_arch+0x690>
c000b328: e2844000 add r4, r4, #0 ; 0x0
c000b32c: e2833001 add r3, r3, #1 ; 0x1
c000b330: e59f5228 ldr r5, [pc, #552] ; c000b560 <setup_arch+0x660>
c000b334: e2833000 add r3, r3, #0 ; 0x0
The first and second adds comprise one translation, the second pair are
a second translation, and as you can see, the compiler scheduled the
loads inbetween.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 4/4] ARM: support for Thumb-2 instructions with CONFIG_ARM_PATCH_PHYS_VIRT
2011-01-04 8:20 ` [PATCH 4/4] ARM: support for Thumb-2 instructions with CONFIG_ARM_PATCH_PHYS_VIRT Nicolas Pitre
@ 2011-01-10 22:20 ` Dave Martin
2011-01-10 22:45 ` Nicolas Pitre
0 siblings, 1 reply; 24+ messages in thread
From: Dave Martin @ 2011-01-10 22:20 UTC (permalink / raw)
To: linux-arm-kernel
Hi,
On Tue, Jan 4, 2011 at 2:20 AM, Nicolas Pitre <nico@fluxnic.net> wrote:
> Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>
> ---
> arch/arm/Kconfig | 3 +--
> arch/arm/include/asm/memory.h | 5 +++--
> arch/arm/kernel/head.S | 31 +++++++++++++++++++++++++++----
> 3 files changed, 31 insertions(+), 8 deletions(-)
>
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 136ed9b..feb374a 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -189,8 +189,7 @@ config VECTORS_BASE
>
> config ARM_PATCH_PHYS_VIRT
> bool
> - depends on EXPERIMENTAL
> - depends on !XIP && !THUMB2_KERNEL
> + depends on !XIP && EXPERIMENTAL
>
> source "init/Kconfig"
>
> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> index f83be97..80a939d 100644
> --- a/arch/arm/include/asm/memory.h
> +++ b/arch/arm/include/asm/memory.h
> @@ -157,14 +157,15 @@
> #error "this machine configuration uses complex __virt_to_phys/__phys_to_virt and cannot use CONFIG_ARM_PATCH_PHYS_VIRT"
> #endif
>
> +/* the stub constant 0xff000000 is used to force the required insn encoding */
> #define __pv_stub(from,to,instr) \
> __asm__( \
> - "1: " instr "\t%0, %1, %2\n" \
> + "1: " instr "\t%0, %1, #0xff000000\n" \
> " .pushsection .pv_table,\"a\"\n" \
> " .long 1b\n" \
> " .popsection" \
> : "=r" (to) \
> - : "r" (from), "I" (1))
> + : "r" (from))
>
> static inline unsigned long __virt_to_phys(unsigned long x)
> {
> diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
> index 621c792..397271d 100644
> --- a/arch/arm/kernel/head.S
> +++ b/arch/arm/kernel/head.S
> @@ -447,11 +447,11 @@ __fixup_pv_table:
> adr r0, 1f
> ldmia r0, {r3-r5}
> sub r3, r0, r3 @ PHYS_OFFSET - PAGE_OFFSET
> + movs r6, r3, lsl #8 @ must be 16MiB aligned
> + bne __error
> add r4, r4, r3
> add r5, r5, r3
> - mov r6, r3, lsr #24 @ constant for add/sub instructions
> - teq r3, r6, lsl #24 @ must be 16MiB aligned
> - bne __error
> + mov r6, r3
> b __fixup_pv_table_loop
> ENDPROC(__fixup_phys_virt)
>
> @@ -462,6 +462,8 @@ ENDPROC(__fixup_phys_virt)
> .pushsection .text
>
> ENTRY(__fixup_pv_table_loop)
> +#ifndef CONFIG_THUMB2_KERNEL
> + mov r6, r6, lsr #24 @ constant for add/sub instructions
> orr r6, r6, #0x400 @ mask in rotate right 8 bits
> 2: cmp r4, r5
> ldrlo r7, [r4], #4
> @@ -471,6 +473,27 @@ ENTRY(__fixup_pv_table_loop)
> strlo ip, [r7, r3]
> blo 2b
> mov pc, lr
> +#else
> + teq r6, #0
> + beq 2f
> + clz r7, r6
> + lsr r6, #24
> + lsl r6, r7
> + bic r6, r6, #0x3080
Should bits 12-13 of r6 ever be nonzero here? The code already throws
an error of the p2v offset is not a multiple of 16 MiB; i.e., (r6 &
~0xff000000) == 0, so r6 >> (24 - clz(r6)) must be in the range
0..0xff.
Of course, clearing the extra bits again doesn't matter...
> + lsrs r7, #1
> + orrcs r6, r6, #0x80
> + orr r6, r6, r7, lsl #12
> + orr r6, r6, #0x4000
> +2: cmp r4, r5
> + bxhs lr
> + ldr r7, [r4], #4
> + add r7, r3
> + ldr ip, [r7, #2]
> + and ip, ip, #0x0f00
> + orr ip, ip, r6
> + str ip, [r7, #2]
> + b 2b
> +#endif
> ENDPROC(__fixup_phys_virt_loop)
>
> /*
> @@ -482,7 +505,7 @@ ENTRY(fixup_pv_table)
> mov r3, #0 @ offset (zero as we're in virtual space)
> mov r4, r0 @ loop start
> mov r5, r1 @ loop end
> - mov r6, r2, lsr #24 @ constant for add/sub instructions
> + mov r6, r2 @ PHYS_OFFSET - PAGE_OFFSET
> bl __fixup_pv_table_loop
> ldmfd sp!, {r4 - r7, pc}
> ENDPROC(fixup_pv_table)
I've not been able to test this code yet, but it looks to me like it
should work for the Thumb-2 case.
Cheers
---Dave
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 4/4] ARM: support for Thumb-2 instructions with CONFIG_ARM_PATCH_PHYS_VIRT
2011-01-10 22:20 ` Dave Martin
@ 2011-01-10 22:45 ` Nicolas Pitre
2011-01-10 23:24 ` Russell King - ARM Linux
0 siblings, 1 reply; 24+ messages in thread
From: Nicolas Pitre @ 2011-01-10 22:45 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, 10 Jan 2011, Dave Martin wrote:
> Hi,
>
> On Tue, Jan 4, 2011 at 2:20 AM, Nicolas Pitre <nico@fluxnic.net> wrote:
> > @@ -462,6 +462,8 @@ ENDPROC(__fixup_phys_virt)
> > .pushsection .text
> >
> > ENTRY(__fixup_pv_table_loop)
> > +#ifndef CONFIG_THUMB2_KERNEL
> > + mov r6, r6, lsr #24 @ constant for add/sub instructions
> > orr r6, r6, #0x400 @ mask in rotate right 8 bits
> > 2: cmp r4, r5
> > ldrlo r7, [r4], #4
> > @@ -471,6 +473,27 @@ ENTRY(__fixup_pv_table_loop)
> > strlo ip, [r7, r3]
> > blo 2b
> > mov pc, lr
> > +#else
> > + teq r6, #0
> > + beq 2f
> > + clz r7, r6
> > + lsr r6, #24
> > + lsl r6, r7
> > + bic r6, r6, #0x3080
>
> Should bits 12-13 of r6 ever be nonzero here? The code already throws
> an error of the p2v offset is not a multiple of 16 MiB; i.e., (r6 &
> ~0xff000000) == 0, so r6 >> (24 - clz(r6)) must be in the range
> 0..0xff.
Yes, they should be cleared already. This is probably a leftover from a
previous version.
I need to rework this part entirely now anyway to go on top of Russell's
latest version.
> I've not been able to test this code yet, but it looks to me like it
> should work for the Thumb-2 case.
Thanks for looking at it.
Nicolas
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 4/4] ARM: support for Thumb-2 instructions with CONFIG_ARM_PATCH_PHYS_VIRT
2011-01-10 22:45 ` Nicolas Pitre
@ 2011-01-10 23:24 ` Russell King - ARM Linux
2011-01-10 23:57 ` Nicolas Pitre
0 siblings, 1 reply; 24+ messages in thread
From: Russell King - ARM Linux @ 2011-01-10 23:24 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, Jan 10, 2011 at 05:45:44PM -0500, Nicolas Pitre wrote:
> I need to rework this part entirely now anyway to go on top of Russell's
> latest version.
I've made a few changes recently to it to fix a few quirks. Once the
merge window is over, I'll push out new patches and tree. Until then
it wouldn't be fair on SFR/linux-next to push new code stuff out during
the merge window.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 4/4] ARM: support for Thumb-2 instructions with CONFIG_ARM_PATCH_PHYS_VIRT
2011-01-10 23:24 ` Russell King - ARM Linux
@ 2011-01-10 23:57 ` Nicolas Pitre
0 siblings, 0 replies; 24+ messages in thread
From: Nicolas Pitre @ 2011-01-10 23:57 UTC (permalink / raw)
To: linux-arm-kernel
On Mon, 10 Jan 2011, Russell King - ARM Linux wrote:
> On Mon, Jan 10, 2011 at 05:45:44PM -0500, Nicolas Pitre wrote:
> > I need to rework this part entirely now anyway to go on top of Russell's
> > latest version.
>
> I've made a few changes recently to it to fix a few quirks. Once the
> merge window is over, I'll push out new patches and tree. Until then
> it wouldn't be fair on SFR/linux-next to push new code stuff out during
> the merge window.
Maybe you could push those into a branch which is not pulled into
linux-next?
Nicolas
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2011-01-10 23:57 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-04 8:20 [PATCH 0/4] variable PHYS_OFFSET support Nicolas Pitre
2011-01-04 8:20 ` [PATCH 1/4] ARM: runtime patching of __virt_to_phys() and __phys_to_virt() Nicolas Pitre
2011-01-04 8:45 ` Russell King - ARM Linux
2011-01-04 14:32 ` Nicolas Pitre
2011-01-04 16:53 ` Russell King - ARM Linux
2011-01-04 17:50 ` Nicolas Pitre
2011-01-04 18:06 ` Russell King - ARM Linux
2011-01-04 18:25 ` David Brown
2011-01-04 18:33 ` Nicolas Pitre
2011-01-04 19:00 ` David Brown
2011-01-04 20:17 ` Russell King - ARM Linux
2011-01-04 18:29 ` Nicolas Pitre
2011-01-04 8:20 ` [PATCH 2/4] ARM: make PHYS_OFFSET actually variable Nicolas Pitre
2011-01-04 12:30 ` Russell King - ARM Linux
2011-01-04 17:54 ` Nicolas Pitre
2011-01-04 8:20 ` [PATCH 3/4] ARM: module support for CONFIG_ARM_PATCH_PHYS_VIRT Nicolas Pitre
2011-01-04 10:06 ` Russell King - ARM Linux
2011-01-04 8:20 ` [PATCH 4/4] ARM: support for Thumb-2 instructions with CONFIG_ARM_PATCH_PHYS_VIRT Nicolas Pitre
2011-01-10 22:20 ` Dave Martin
2011-01-10 22:45 ` Nicolas Pitre
2011-01-10 23:24 ` Russell King - ARM Linux
2011-01-10 23:57 ` Nicolas Pitre
2011-01-04 10:41 ` [PATCH 0/4] variable PHYS_OFFSET support Russell King - ARM Linux
2011-01-04 14:37 ` Nicolas Pitre
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.