variable PHYS_OFFSET support

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 <[email protected]>

---
 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(-)

arch/arm/Kconfig +5 -0

index d56d21c0..136ed9b 100644
@@ -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"

arch/arm/include/asm/memory.h +41 -10

index 23c2e8e..2783ce2 100644
@@ -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,

arch/arm/kernel/head.S +35 -0

index 6bd82d2..eaaf0ad 100644
@@ -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"

arch/arm/kernel/vmlinux.lds.S +4 -0

index cead889..fb32c9d 100644
@@ -57,6 +57,10 @@ SECTIONS
 		__smpalt_end = .;
 #endif
 
+		__pv_table_begin = .;
+			*(.pv_table)
+		__pv_table_end = .;
+
 		INIT_SETUP(16)
 
 		INIT_CALLS

Signature · 1 line

1.7.3.2.193.g78bbb

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.

Quoted 8 lines · Russell King - ARM Linux

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.

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.

Quoted 10 lines · Nicolas Pitre

Quoted 8 lines · Russell King - ARM Linux

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.

Quoted 4 lines

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.

Quoted 5 lines

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.

Quoted 7 lines · Russell King - ARM Linux

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.

Quoted 6 lines

(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.

Quoted 3 lines

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.

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.

Quoted 7 lines · Russell King - ARM Linux

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?

Quoted 3 lines · David Brown

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.

Quoted 6 lines · Nicolas Pitre

Quoted 3 lines · David Brown

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?

Quoted 3 lines

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.

Quoted 27 lines · Nicolas Pitre

Quoted 7 lines · Russell King - ARM Linux

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.

Quoted 6 lines

(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.

Quoted 3 lines

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.)

Quoted 2 lines

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.

Quoted 4 lines

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.

arch/arm/kernel/setup.c static struct init_tags { · line 709

Quoted 12 lines · Nicolas Pitre

arch/arm/kernel/setup.c +2 -0

index 336f14e..7502bc1 100644
@@ -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.

Thanks to Russell King for his contribution to this patch.

Signed-off-by Nicolas Pitre <[email protected]>

Signed-off-by Nicolas Pitre <[email protected]>

---
 arch/arm/Kconfig              |    3 +--
 arch/arm/include/asm/memory.h |    5 +++--
 arch/arm/kernel/head.S        |   31 +++++++++++++++++++++++++++----
 3 files changed, 31 insertions(+), 8 deletions(-)

arch/arm/Kconfig +1 -2

index 136ed9b..feb374a 100644
@@ -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"
 

arch/arm/include/asm/memory.h +3 -2

index f83be97..80a939d 100644
@@ -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)
 {

arch/arm/kernel/head.S +5 -3

index 621c792..397271d 100644
@@ -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

arch/arm/kernel/head.S +14 -0

+       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)
 
 /*

Hi,

Quoted 17 lines · Nicolas Pitre

arch/arm/kernel/head.S +6 -0

@@ -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

arch/arm/kernel/head.S ENTRY(__fixup_pv_table_loop) · line 482

Quoted 3 lines

arch/arm/kernel/head.S +3 -0

+       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.

I've not been able to test this code yet, but it looks to me like it should work for the Thumb-2 case.

I need to rework this part entirely now anyway to go on top of Russell's latest version.

Quoted 2 lines · Nicolas Pitre

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.

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.

So, I don't think it's ready for this coming merge window.