[PATCH] ARM: xip: Use correct symbol for end of ROM marker

[PATCH] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

For an XIP build, _edata_loc, not _etext, represents the end of the
binary image that will be programmed into ROM and mapped into the
MODULES_VADDR area.
With an XIP kernel, nothing is loaded into RAM before boot, meaning
you have to take into account the size of the entire binary image
that was programmed, including the init data values that will be copied
to RAM during kernel boot.

This fixes the bug where you might lose the end of your kernel area
after page table setup is complete.


Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
 arch/arm/mm/mmu.c              |    4 ++--
 include/asm-generic/sections.h |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index 4867f5d..dd5a56b 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1210,7 +1210,7 @@ static inline void prepare_page_table(void)
 
 #ifdef CONFIG_XIP_KERNEL
 	/* The XIP kernel is mapped in the module area -- skip over it */
-	addr = ((unsigned long)_etext + PMD_SIZE - 1) & PMD_MASK;
+	addr = ((unsigned long)_edata_loc + PMD_SIZE - 1) & PMD_MASK;
 #endif
 	for ( ; addr < PAGE_OFFSET; addr += PMD_SIZE)
 		pmd_clear(pmd_off_k(addr));
@@ -1292,7 +1292,7 @@ static void __init devicemaps_init(const struct machine_desc *mdesc)
 #ifdef CONFIG_XIP_KERNEL
 	map.pfn = __phys_to_pfn(CONFIG_XIP_PHYS_ADDR & SECTION_MASK);
 	map.virtual = MODULES_VADDR;
-	map.length = ((unsigned long)_etext - map.virtual + ~SECTION_MASK) & SECTION_MASK;
+	map.length = ((unsigned long)_edata_loc - map.virtual + ~SECTION_MASK) & SECTION_MASK;
 	map.type = MT_ROM;
 	create_mapping(&map);
 #endif
diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index b58fd66..195554d 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -26,7 +26,7 @@
  *	__ctors_start, __ctors_end
  */
 extern char _text[], _stext[], _etext[];
-extern char _data[], _sdata[], _edata[];
+extern char _data[], _sdata[], _edata[], _edata_loc[];
 extern char __bss_start[], __bss_stop[];
 extern char __init_begin[], __init_end[];
 extern char _sinittext[], _einittext[];
-- 
1.7.9.5

[PATCH] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

For an XIP build, _edata_loc, not _etext, represents the end of the
binary image that will be programmed into ROM and mapped into the
MODULES_VADDR area.
With an XIP kernel, nothing is loaded into RAM before boot, meaning
you have to take into account the size of the entire binary image
that was programmed, including the init data values that will be copied
to RAM during kernel boot.

This fixes the bug where you might lose the end of your kernel area
after page table setup is complete.


Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
 arch/arm/mm/mmu.c              |    4 ++--
 include/asm-generic/sections.h |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index 4867f5d..dd5a56b 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1210,7 +1210,7 @@ static inline void prepare_page_table(void)
 
 #ifdef CONFIG_XIP_KERNEL
 	/* The XIP kernel is mapped in the module area -- skip over it */
-	addr = ((unsigned long)_etext + PMD_SIZE - 1) & PMD_MASK;
+	addr = ((unsigned long)_edata_loc + PMD_SIZE - 1) & PMD_MASK;
 #endif
 	for ( ; addr < PAGE_OFFSET; addr += PMD_SIZE)
 		pmd_clear(pmd_off_k(addr));
@@ -1292,7 +1292,7 @@ static void __init devicemaps_init(const struct machine_desc *mdesc)
 #ifdef CONFIG_XIP_KERNEL
 	map.pfn = __phys_to_pfn(CONFIG_XIP_PHYS_ADDR & SECTION_MASK);
 	map.virtual = MODULES_VADDR;
-	map.length = ((unsigned long)_etext - map.virtual + ~SECTION_MASK) & SECTION_MASK;
+	map.length = ((unsigned long)_edata_loc - map.virtual + ~SECTION_MASK) & SECTION_MASK;
 	map.type = MT_ROM;
 	create_mapping(&map);
 #endif
diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index b58fd66..195554d 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -26,7 +26,7 @@
  *	__ctors_start, __ctors_end
  */
 extern char _text[], _stext[], _etext[];
-extern char _data[], _sdata[], _edata[];
+extern char _data[], _sdata[], _edata[], _edata_loc[];
 extern char __bss_start[], __bss_stop[];
 extern char __init_begin[], __init_end[];
 extern char _sinittext[], _einittext[];
-- 
1.7.9.5

[PATCH] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

For an XIP build, _edata_loc, not _etext, represents the end of the
binary image that will be programmed into ROM and mapped into the
MODULES_VADDR area.
With an XIP kernel, nothing is loaded into RAM before boot, meaning
you have to take into account the size of the entire binary image
that was programmed, including the init data values that will be copied
to RAM during kernel boot.

This fixes the bug where you might lose the end of your kernel area
after page table setup is complete.


Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
 arch/arm/mm/mmu.c              |    4 ++--
 include/asm-generic/sections.h |    2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index 4867f5d..dd5a56b 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1210,7 +1210,7 @@ static inline void prepare_page_table(void)
 
 #ifdef CONFIG_XIP_KERNEL
 	/* The XIP kernel is mapped in the module area -- skip over it */
-	addr = ((unsigned long)_etext + PMD_SIZE - 1) & PMD_MASK;
+	addr = ((unsigned long)_edata_loc + PMD_SIZE - 1) & PMD_MASK;
 #endif
 	for ( ; addr < PAGE_OFFSET; addr += PMD_SIZE)
 		pmd_clear(pmd_off_k(addr));
@@ -1292,7 +1292,7 @@ static void __init devicemaps_init(const struct machine_desc *mdesc)
 #ifdef CONFIG_XIP_KERNEL
 	map.pfn = __phys_to_pfn(CONFIG_XIP_PHYS_ADDR & SECTION_MASK);
 	map.virtual = MODULES_VADDR;
-	map.length = ((unsigned long)_etext - map.virtual + ~SECTION_MASK) & SECTION_MASK;
+	map.length = ((unsigned long)_edata_loc - map.virtual + ~SECTION_MASK) & SECTION_MASK;
 	map.type = MT_ROM;
 	create_mapping(&map);
 #endif
diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index b58fd66..195554d 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -26,7 +26,7 @@
  *	__ctors_start, __ctors_end
  */
 extern char _text[], _stext[], _etext[];
-extern char _data[], _sdata[], _edata[];
+extern char _data[], _sdata[], _edata[], _edata_loc[];
 extern char __bss_start[], __bss_stop[];
 extern char __init_begin[], __init_end[];
 extern char _sinittext[], _einittext[];
-- 
1.7.9.5

Re: [PATCH] ARM: xip: Use correct symbol for end of ROM marker

[Peter Hurley]

On 11/11/2015 09:17 AM, Chris Brandt wrote:
> For an XIP build, _edata_loc, not _etext, represents the end of the
> binary image that will be programmed into ROM and mapped into the
> MODULES_VADDR area.
> With an XIP kernel, nothing is loaded into RAM before boot, meaning
> you have to take into account the size of the entire binary image
> that was programmed, including the init data values that will be copied
> to RAM during kernel boot.
> 
> This fixes the bug where you might lose the end of your kernel area
> after page table setup is complete.
> 
> 
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> ---
>  arch/arm/mm/mmu.c              |    4 ++--
>  include/asm-generic/sections.h |    2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index 4867f5d..dd5a56b 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -1210,7 +1210,7 @@ static inline void prepare_page_table(void)
>  
>  #ifdef CONFIG_XIP_KERNEL
>  	/* The XIP kernel is mapped in the module area -- skip over it */
> -	addr = ((unsigned long)_etext + PMD_SIZE - 1) & PMD_MASK;
> +	addr = ((unsigned long)_edata_loc + PMD_SIZE - 1) & PMD_MASK;
>  #endif
>  	for ( ; addr < PAGE_OFFSET; addr += PMD_SIZE)
>  		pmd_clear(pmd_off_k(addr));
> @@ -1292,7 +1292,7 @@ static void __init devicemaps_init(const struct machine_desc *mdesc)
>  #ifdef CONFIG_XIP_KERNEL
>  	map.pfn = __phys_to_pfn(CONFIG_XIP_PHYS_ADDR & SECTION_MASK);
>  	map.virtual = MODULES_VADDR;
> -	map.length = ((unsigned long)_etext - map.virtual + ~SECTION_MASK) & SECTION_MASK;
> +	map.length = ((unsigned long)_edata_loc - map.virtual + ~SECTION_MASK) & SECTION_MASK;
>  	map.type = MT_ROM;
>  	create_mapping(&map);
>  #endif
> diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
> index b58fd66..195554d 100644
> --- a/include/asm-generic/sections.h
> +++ b/include/asm-generic/sections.h
> @@ -26,7 +26,7 @@
>   *	__ctors_start, __ctors_end
>   */
>  extern char _text[], _stext[], _etext[];
> -extern char _data[], _sdata[], _edata[];
> +extern char _data[], _sdata[], _edata[], _edata_loc[];
>  extern char __bss_start[], __bss_stop[];
>  extern char __init_begin[], __init_end[];
>  extern char _sinittext[], _einittext[];
> 

I think below is required as well.

--- a/arch/arm/kernel/module.c
+++ b/arch/arm/kernel/module.c
@@ -34,7 +36,7 @@
  * recompiling the whole kernel when CONFIG_XIP_KERNEL is turned on/off.
  */
 #undef MODULES_VADDR
-#define MODULES_VADDR	(((unsigned long)_etext + ~PMD_MASK) & PMD_MASK)
+#define MODULES_VADDR	(((unsigned long)_edata_loc + ~PMD_MASK) & PMD_MASK)
 #endif
 
Regards,
Peter Hurley

Re: [PATCH] ARM: xip: Use correct symbol for end of ROM marker

[Peter Hurley]

On 11/11/2015 09:17 AM, Chris Brandt wrote:
> For an XIP build, _edata_loc, not _etext, represents the end of the
> binary image that will be programmed into ROM and mapped into the
> MODULES_VADDR area.
> With an XIP kernel, nothing is loaded into RAM before boot, meaning
> you have to take into account the size of the entire binary image
> that was programmed, including the init data values that will be copied
> to RAM during kernel boot.
> 
> This fixes the bug where you might lose the end of your kernel area
> after page table setup is complete.
> 
> 
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> ---
>  arch/arm/mm/mmu.c              |    4 ++--
>  include/asm-generic/sections.h |    2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index 4867f5d..dd5a56b 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -1210,7 +1210,7 @@ static inline void prepare_page_table(void)
>  
>  #ifdef CONFIG_XIP_KERNEL
>  	/* The XIP kernel is mapped in the module area -- skip over it */
> -	addr = ((unsigned long)_etext + PMD_SIZE - 1) & PMD_MASK;
> +	addr = ((unsigned long)_edata_loc + PMD_SIZE - 1) & PMD_MASK;
>  #endif
>  	for ( ; addr < PAGE_OFFSET; addr += PMD_SIZE)
>  		pmd_clear(pmd_off_k(addr));
> @@ -1292,7 +1292,7 @@ static void __init devicemaps_init(const struct machine_desc *mdesc)
>  #ifdef CONFIG_XIP_KERNEL
>  	map.pfn = __phys_to_pfn(CONFIG_XIP_PHYS_ADDR & SECTION_MASK);
>  	map.virtual = MODULES_VADDR;
> -	map.length = ((unsigned long)_etext - map.virtual + ~SECTION_MASK) & SECTION_MASK;
> +	map.length = ((unsigned long)_edata_loc - map.virtual + ~SECTION_MASK) & SECTION_MASK;
>  	map.type = MT_ROM;
>  	create_mapping(&map);
>  #endif
> diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
> index b58fd66..195554d 100644
> --- a/include/asm-generic/sections.h
> +++ b/include/asm-generic/sections.h
> @@ -26,7 +26,7 @@
>   *	__ctors_start, __ctors_end
>   */
>  extern char _text[], _stext[], _etext[];
> -extern char _data[], _sdata[], _edata[];
> +extern char _data[], _sdata[], _edata[], _edata_loc[];
>  extern char __bss_start[], __bss_stop[];
>  extern char __init_begin[], __init_end[];
>  extern char _sinittext[], _einittext[];
> 

I think below is required as well.

--- a/arch/arm/kernel/module.c
+++ b/arch/arm/kernel/module.c
@@ -34,7 +36,7 @@
  * recompiling the whole kernel when CONFIG_XIP_KERNEL is turned on/off.
  */
 #undef MODULES_VADDR
-#define MODULES_VADDR	(((unsigned long)_etext + ~PMD_MASK) & PMD_MASK)
+#define MODULES_VADDR	(((unsigned long)_edata_loc + ~PMD_MASK) & PMD_MASK)
 #endif
 
Regards,
Peter Hurley

[PATCH] ARM: xip: Use correct symbol for end of ROM marker

[Peter Hurley]

On 11/11/2015 09:17 AM, Chris Brandt wrote:
> For an XIP build, _edata_loc, not _etext, represents the end of the
> binary image that will be programmed into ROM and mapped into the
> MODULES_VADDR area.
> With an XIP kernel, nothing is loaded into RAM before boot, meaning
> you have to take into account the size of the entire binary image
> that was programmed, including the init data values that will be copied
> to RAM during kernel boot.
> 
> This fixes the bug where you might lose the end of your kernel area
> after page table setup is complete.
> 
> 
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> ---
>  arch/arm/mm/mmu.c              |    4 ++--
>  include/asm-generic/sections.h |    2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index 4867f5d..dd5a56b 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -1210,7 +1210,7 @@ static inline void prepare_page_table(void)
>  
>  #ifdef CONFIG_XIP_KERNEL
>  	/* The XIP kernel is mapped in the module area -- skip over it */
> -	addr = ((unsigned long)_etext + PMD_SIZE - 1) & PMD_MASK;
> +	addr = ((unsigned long)_edata_loc + PMD_SIZE - 1) & PMD_MASK;
>  #endif
>  	for ( ; addr < PAGE_OFFSET; addr += PMD_SIZE)
>  		pmd_clear(pmd_off_k(addr));
> @@ -1292,7 +1292,7 @@ static void __init devicemaps_init(const struct machine_desc *mdesc)
>  #ifdef CONFIG_XIP_KERNEL
>  	map.pfn = __phys_to_pfn(CONFIG_XIP_PHYS_ADDR & SECTION_MASK);
>  	map.virtual = MODULES_VADDR;
> -	map.length = ((unsigned long)_etext - map.virtual + ~SECTION_MASK) & SECTION_MASK;
> +	map.length = ((unsigned long)_edata_loc - map.virtual + ~SECTION_MASK) & SECTION_MASK;
>  	map.type = MT_ROM;
>  	create_mapping(&map);
>  #endif
> diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
> index b58fd66..195554d 100644
> --- a/include/asm-generic/sections.h
> +++ b/include/asm-generic/sections.h
> @@ -26,7 +26,7 @@
>   *	__ctors_start, __ctors_end
>   */
>  extern char _text[], _stext[], _etext[];
> -extern char _data[], _sdata[], _edata[];
> +extern char _data[], _sdata[], _edata[], _edata_loc[];
>  extern char __bss_start[], __bss_stop[];
>  extern char __init_begin[], __init_end[];
>  extern char _sinittext[], _einittext[];
> 

I think below is required as well.

--- a/arch/arm/kernel/module.c
+++ b/arch/arm/kernel/module.c
@@ -34,7 +36,7 @@
  * recompiling the whole kernel when CONFIG_XIP_KERNEL is turned on/off.
  */
 #undef MODULES_VADDR
-#define MODULES_VADDR	(((unsigned long)_etext + ~PMD_MASK) & PMD_MASK)
+#define MODULES_VADDR	(((unsigned long)_edata_loc + ~PMD_MASK) & PMD_MASK)
 #endif
 
Regards,
Peter Hurley

RE: [PATCH] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

PiBJIHRoaW5rIGJlbG93IGlzIHJlcXVpcmVkIGFzIHdlbGwuDQo+DQo+IC0tLSBhL2FyY2gvYXJt
L2tlcm5lbC9tb2R1bGUuYw0KPiArKysgYi9hcmNoL2FybS9rZXJuZWwvbW9kdWxlLmMNCj4gQEAg
LTM0LDcgKzM2LDcgQEANCj4gICAqIHJlY29tcGlsaW5nIHRoZSB3aG9sZSBrZXJuZWwgd2hlbiBD
T05GSUdfWElQX0tFUk5FTCBpcyB0dXJuZWQgb24vb2ZmLg0KPiAgICovDQo+ICAjdW5kZWYgTU9E
VUxFU19WQUREUg0KPiAtI2RlZmluZSBNT0RVTEVTX1ZBRERSCSgoKHVuc2lnbmVkIGxvbmcpX2V0
ZXh0ICsgflBNRF9NQVNLKSAmIFBNRF9NQVNLKQ0KPiArI2RlZmluZSBNT0RVTEVTX1ZBRERSCSgo
KHVuc2lnbmVkIGxvbmcpX2VkYXRhX2xvYyArIH5QTURfTUFTSykgJiBQTURfTUFTSykNCj4gICNl
bmRpZg0KDQoNClRoYW5rIHlvdSENClNvbWVob3cgSSBtaXNzZWQgdGhhdCBmaWxlIHdoZW4gSSB3
YXMgbW92aW5nIGZyb20gb25lIHRyZWUgdG8gYW5vdGhlciB3aGVuIHByZXBhcmluZyBteSBjb21t
aXRzLg0KDQpJJ2xsIHVwZGF0ZSBhbmQgcmVzdWJtaXQgdGhlIHBhdGNoLg0KDQpDaHJpcw0KDQo

RE: [PATCH] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 709 bytes --]

> I think below is required as well.
>
> --- a/arch/arm/kernel/module.c
> +++ b/arch/arm/kernel/module.c
> @@ -34,7 +36,7 @@
>   * recompiling the whole kernel when CONFIG_XIP_KERNEL is turned on/off.
>   */
>  #undef MODULES_VADDR
> -#define MODULES_VADDR	(((unsigned long)_etext + ~PMD_MASK) & PMD_MASK)
> +#define MODULES_VADDR	(((unsigned long)_edata_loc + ~PMD_MASK) & PMD_MASK)
>  #endif


Thank you!
Somehow I missed that file when I was moving from one tree to another when preparing my commits.

I'll update and resubmit the patch.

Chris

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

[PATCH] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

> I think below is required as well.
>
> --- a/arch/arm/kernel/module.c
> +++ b/arch/arm/kernel/module.c
> @@ -34,7 +36,7 @@
>   * recompiling the whole kernel when CONFIG_XIP_KERNEL is turned on/off.
>   */
>  #undef MODULES_VADDR
> -#define MODULES_VADDR	(((unsigned long)_etext + ~PMD_MASK) & PMD_MASK)
> +#define MODULES_VADDR	(((unsigned long)_edata_loc + ~PMD_MASK) & PMD_MASK)
>  #endif


Thank you!
Somehow I missed that file when I was moving from one tree to another when preparing my commits.

I'll update and resubmit the patch.

Chris

Re: [PATCH] ARM: xip: Use correct symbol for end of ROM marker

[Russell King - ARM Linux]

On Wed, Nov 11, 2015 at 09:17:35AM -0500, Chris Brandt wrote:
> diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
> index b58fd66..195554d 100644
> --- a/include/asm-generic/sections.h
> +++ b/include/asm-generic/sections.h
> @@ -26,7 +26,7 @@
>   *	__ctors_start, __ctors_end
>   */
>  extern char _text[], _stext[], _etext[];
> -extern char _data[], _sdata[], _edata[];
> +extern char _data[], _sdata[], _edata[], _edata_loc[];

It's not good practice to put non-generic symbols in generic header files.
_edata_loc is an ARM specific symbol which no other architecture defines.
Please create an ARM private asm/sections.h header, which should include
the asm-generic version, and also declare the additional symbol.

Thanks.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH] ARM: xip: Use correct symbol for end of ROM marker

[Russell King - ARM Linux]

On Wed, Nov 11, 2015 at 09:17:35AM -0500, Chris Brandt wrote:
> diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
> index b58fd66..195554d 100644
> --- a/include/asm-generic/sections.h
> +++ b/include/asm-generic/sections.h
> @@ -26,7 +26,7 @@
>   *	__ctors_start, __ctors_end
>   */
>  extern char _text[], _stext[], _etext[];
> -extern char _data[], _sdata[], _edata[];
> +extern char _data[], _sdata[], _edata[], _edata_loc[];

It's not good practice to put non-generic symbols in generic header files.
_edata_loc is an ARM specific symbol which no other architecture defines.
Please create an ARM private asm/sections.h header, which should include
the asm-generic version, and also declare the additional symbol.

Thanks.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH] ARM: xip: Use correct symbol for end of ROM marker

[Russell King - ARM Linux]

On Wed, Nov 11, 2015 at 09:17:35AM -0500, Chris Brandt wrote:
> diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
> index b58fd66..195554d 100644
> --- a/include/asm-generic/sections.h
> +++ b/include/asm-generic/sections.h
> @@ -26,7 +26,7 @@
>   *	__ctors_start, __ctors_end
>   */
>  extern char _text[], _stext[], _etext[];
> -extern char _data[], _sdata[], _edata[];
> +extern char _data[], _sdata[], _edata[], _edata_loc[];

It's not good practice to put non-generic symbols in generic header files.
_edata_loc is an ARM specific symbol which no other architecture defines.
Please create an ARM private asm/sections.h header, which should include
the asm-generic version, and also declare the additional symbol.

Thanks.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH v2] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

For an XIP build, _edata_loc, not _etext, represents the end of the
binary image that will be programmed into ROM and mapped into the
MODULES_VADDR area.
With an XIP kernel, nothing is loaded into RAM before boot, meaning
you have to take into account the size of the entire binary image
that was programmed, including the init data values that will be copied
to RAM during kernel boot.

This fixes the bug where you might lose the end of your kernel area
after page table setup is complete.

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
v2
* Added change for MODULES_VADDR
* Moved extern to new file asm/sections.h
---
 arch/arm/include/asm/sections.h |    8 ++++++++
 arch/arm/kernel/module.c        |    2 +-
 arch/arm/mm/mmu.c               |    4 ++--
 3 files changed, 11 insertions(+), 3 deletions(-)
 create mode 100644 arch/arm/include/asm/sections.h

diff --git a/arch/arm/include/asm/sections.h b/arch/arm/include/asm/sections.h
new file mode 100644
index 0000000..401eb3c2
--- /dev/null
+++ b/arch/arm/include/asm/sections.h
@@ -0,0 +1,8 @@
+#ifndef _ASM_ARM_SECTIONS_H
+#define _ASM_ARM_SECTIONS_H
+
+#include <asm-generic/sections.h>
+
+extern char _edata_loc[];
+
+#endif	/* _ASM_ARM_SECTIONS_H */
diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c
index efdddcb..41ae2cc 100644
--- a/arch/arm/kernel/module.c
+++ b/arch/arm/kernel/module.c
@@ -34,7 +34,7 @@
  * recompiling the whole kernel when CONFIG_XIP_KERNEL is turned on/off.
  */
 #undef MODULES_VADDR
-#define MODULES_VADDR	(((unsigned long)_etext + ~PMD_MASK) & PMD_MASK)
+#define MODULES_VADDR	(((unsigned long)_edata_loc + ~PMD_MASK) & PMD_MASK)
 #endif
 
 #ifdef CONFIG_MMU
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index 4867f5d..dd5a56b 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1210,7 +1210,7 @@ static inline void prepare_page_table(void)
 
 #ifdef CONFIG_XIP_KERNEL
 	/* The XIP kernel is mapped in the module area -- skip over it */
-	addr = ((unsigned long)_etext + PMD_SIZE - 1) & PMD_MASK;
+	addr = ((unsigned long)_edata_loc + PMD_SIZE - 1) & PMD_MASK;
 #endif
 	for ( ; addr < PAGE_OFFSET; addr += PMD_SIZE)
 		pmd_clear(pmd_off_k(addr));
@@ -1292,7 +1292,7 @@ static void __init devicemaps_init(const struct machine_desc *mdesc)
 #ifdef CONFIG_XIP_KERNEL
 	map.pfn = __phys_to_pfn(CONFIG_XIP_PHYS_ADDR & SECTION_MASK);
 	map.virtual = MODULES_VADDR;
-	map.length = ((unsigned long)_etext - map.virtual + ~SECTION_MASK) & SECTION_MASK;
+	map.length = ((unsigned long)_edata_loc - map.virtual + ~SECTION_MASK) & SECTION_MASK;
 	map.type = MT_ROM;
 	create_mapping(&map);
 #endif
-- 
1.7.9.5

[PATCH v2] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

For an XIP build, _edata_loc, not _etext, represents the end of the
binary image that will be programmed into ROM and mapped into the
MODULES_VADDR area.
With an XIP kernel, nothing is loaded into RAM before boot, meaning
you have to take into account the size of the entire binary image
that was programmed, including the init data values that will be copied
to RAM during kernel boot.

This fixes the bug where you might lose the end of your kernel area
after page table setup is complete.

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
v2
* Added change for MODULES_VADDR
* Moved extern to new file asm/sections.h
---
 arch/arm/include/asm/sections.h |    8 ++++++++
 arch/arm/kernel/module.c        |    2 +-
 arch/arm/mm/mmu.c               |    4 ++--
 3 files changed, 11 insertions(+), 3 deletions(-)
 create mode 100644 arch/arm/include/asm/sections.h

diff --git a/arch/arm/include/asm/sections.h b/arch/arm/include/asm/sections.h
new file mode 100644
index 0000000..401eb3c2
--- /dev/null
+++ b/arch/arm/include/asm/sections.h
@@ -0,0 +1,8 @@
+#ifndef _ASM_ARM_SECTIONS_H
+#define _ASM_ARM_SECTIONS_H
+
+#include <asm-generic/sections.h>
+
+extern char _edata_loc[];
+
+#endif	/* _ASM_ARM_SECTIONS_H */
diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c
index efdddcb..41ae2cc 100644
--- a/arch/arm/kernel/module.c
+++ b/arch/arm/kernel/module.c
@@ -34,7 +34,7 @@
  * recompiling the whole kernel when CONFIG_XIP_KERNEL is turned on/off.
  */
 #undef MODULES_VADDR
-#define MODULES_VADDR	(((unsigned long)_etext + ~PMD_MASK) & PMD_MASK)
+#define MODULES_VADDR	(((unsigned long)_edata_loc + ~PMD_MASK) & PMD_MASK)
 #endif
 
 #ifdef CONFIG_MMU
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index 4867f5d..dd5a56b 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1210,7 +1210,7 @@ static inline void prepare_page_table(void)
 
 #ifdef CONFIG_XIP_KERNEL
 	/* The XIP kernel is mapped in the module area -- skip over it */
-	addr = ((unsigned long)_etext + PMD_SIZE - 1) & PMD_MASK;
+	addr = ((unsigned long)_edata_loc + PMD_SIZE - 1) & PMD_MASK;
 #endif
 	for ( ; addr < PAGE_OFFSET; addr += PMD_SIZE)
 		pmd_clear(pmd_off_k(addr));
@@ -1292,7 +1292,7 @@ static void __init devicemaps_init(const struct machine_desc *mdesc)
 #ifdef CONFIG_XIP_KERNEL
 	map.pfn = __phys_to_pfn(CONFIG_XIP_PHYS_ADDR & SECTION_MASK);
 	map.virtual = MODULES_VADDR;
-	map.length = ((unsigned long)_etext - map.virtual + ~SECTION_MASK) & SECTION_MASK;
+	map.length = ((unsigned long)_edata_loc - map.virtual + ~SECTION_MASK) & SECTION_MASK;
 	map.type = MT_ROM;
 	create_mapping(&map);
 #endif
-- 
1.7.9.5

Re: [PATCH v2] ARM: xip: Use correct symbol for end of ROM marker

[Geert Uytterhoeven]

Hi Chris,

On Thu, Nov 12, 2015 at 10:01 PM, Chris Brandt <chris.brandt@renesas.com> wrote:
> For an XIP build, _edata_loc, not _etext, represents the end of the
> binary image that will be programmed into ROM and mapped into the
> MODULES_VADDR area.
> With an XIP kernel, nothing is loaded into RAM before boot, meaning
> you have to take into account the size of the entire binary image
> that was programmed, including the init data values that will be copied
> to RAM during kernel boot.
>
> This fixes the bug where you might lose the end of your kernel area
> after page table setup is complete.

Thanks for your patch!

> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> ---
> v2
> * Added change for MODULES_VADDR
> * Moved extern to new file asm/sections.h
> ---
>  arch/arm/include/asm/sections.h |    8 ++++++++
>  arch/arm/kernel/module.c        |    2 +-
>  arch/arm/mm/mmu.c               |    4 ++--
>  3 files changed, 11 insertions(+), 3 deletions(-)
>  create mode 100644 arch/arm/include/asm/sections.h

If you create an arm-specific <asm/sections.h>, you also have to remove the
line

        generic-y += sections.h

from arch/arm/include/asm/Kbuild.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[PATCH v2] ARM: xip: Use correct symbol for end of ROM marker

[Geert Uytterhoeven]

Hi Chris,

On Thu, Nov 12, 2015 at 10:01 PM, Chris Brandt <chris.brandt@renesas.com> wrote:
> For an XIP build, _edata_loc, not _etext, represents the end of the
> binary image that will be programmed into ROM and mapped into the
> MODULES_VADDR area.
> With an XIP kernel, nothing is loaded into RAM before boot, meaning
> you have to take into account the size of the entire binary image
> that was programmed, including the init data values that will be copied
> to RAM during kernel boot.
>
> This fixes the bug where you might lose the end of your kernel area
> after page table setup is complete.

Thanks for your patch!

> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> ---
> v2
> * Added change for MODULES_VADDR
> * Moved extern to new file asm/sections.h
> ---
>  arch/arm/include/asm/sections.h |    8 ++++++++
>  arch/arm/kernel/module.c        |    2 +-
>  arch/arm/mm/mmu.c               |    4 ++--
>  3 files changed, 11 insertions(+), 3 deletions(-)
>  create mode 100644 arch/arm/include/asm/sections.h

If you create an arm-specific <asm/sections.h>, you also have to remove the
line

        generic-y += sections.h

from arch/arm/include/asm/Kbuild.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

RE: [PATCH v2] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

SGkgR2VlcnQsDQoNCj4gSWYgeW91IGNyZWF0ZSBhbiBhcm0tc3BlY2lmaWMgPGFzbS9zZWN0aW9u
cy5oPiwgeW91IGFsc28gaGF2ZSB0byByZW1vdmUgdGhlIGxpbmUNCj4NCj4gICAgICAgICBnZW5l
cmljLXkgKz0gc2VjdGlvbnMuaA0KPg0KPiBmcm9tIGFyY2gvYXJtL2luY2x1ZGUvYXNtL0tidWls
ZC4NCg0KDQpUaGF0LCBJIGRpZG4ndCBrbm93IGFib3V0LiBUaGFuayB5b3UuDQoNCkknbGwgc3Vi
bWl0IGEgbmV3IHBhdGNoIG9uIE1vbmRheS4NCg0KQ2hyaXMNCg0K

[PATCH v2] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

Hi Geert,

> If you create an arm-specific <asm/sections.h>, you also have to remove the line
>
>         generic-y += sections.h
>
> from arch/arm/include/asm/Kbuild.


That, I didn't know about. Thank you.

I'll submit a new patch on Monday.

Chris

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

For an XIP build, _edata_loc, not _etext, represents the end of the
binary image that will be programmed into ROM and mapped into the
MODULES_VADDR area.
With an XIP kernel, nothing is loaded into RAM before boot, meaning
you have to take into account the size of the entire binary image
that was programmed, including the init data values that will be copied
to RAM during kernel boot.

This fixes the bug where you might lose the end of your kernel area
after page table setup is complete.

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
v3:
* Removed sections.h from Kbuild
v2:
* Added change for MODULES_VADDR
* Moved extern to new file asm/sections.h
---
 arch/arm/include/asm/Kbuild     |    1 -
 arch/arm/include/asm/sections.h |    8 ++++++++
 arch/arm/kernel/module.c        |    2 +-
 arch/arm/mm/mmu.c               |    4 ++--
 4 files changed, 11 insertions(+), 4 deletions(-)
 create mode 100644 arch/arm/include/asm/sections.h

diff --git a/arch/arm/include/asm/Kbuild b/arch/arm/include/asm/Kbuild
index be648eb..6d3da22 100644
--- a/arch/arm/include/asm/Kbuild
+++ b/arch/arm/include/asm/Kbuild
@@ -21,7 +21,6 @@ generic-y += preempt.h
 generic-y += resource.h
 generic-y += rwsem.h
 generic-y += seccomp.h
-generic-y += sections.h
 generic-y += segment.h
 generic-y += sembuf.h
 generic-y += serial.h
diff --git a/arch/arm/include/asm/sections.h b/arch/arm/include/asm/sections.h
new file mode 100644
index 0000000..401eb3c2
--- /dev/null
+++ b/arch/arm/include/asm/sections.h
@@ -0,0 +1,8 @@
+#ifndef _ASM_ARM_SECTIONS_H
+#define _ASM_ARM_SECTIONS_H
+
+#include <asm-generic/sections.h>
+
+extern char _edata_loc[];
+
+#endif	/* _ASM_ARM_SECTIONS_H */
diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c
index efdddcb..41ae2cc 100644
--- a/arch/arm/kernel/module.c
+++ b/arch/arm/kernel/module.c
@@ -34,7 +34,7 @@
  * recompiling the whole kernel when CONFIG_XIP_KERNEL is turned on/off.
  */
 #undef MODULES_VADDR
-#define MODULES_VADDR	(((unsigned long)_etext + ~PMD_MASK) & PMD_MASK)
+#define MODULES_VADDR	(((unsigned long)_edata_loc + ~PMD_MASK) & PMD_MASK)
 #endif
 
 #ifdef CONFIG_MMU
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index 4867f5d..dd5a56b 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1210,7 +1210,7 @@ static inline void prepare_page_table(void)
 
 #ifdef CONFIG_XIP_KERNEL
 	/* The XIP kernel is mapped in the module area -- skip over it */
-	addr = ((unsigned long)_etext + PMD_SIZE - 1) & PMD_MASK;
+	addr = ((unsigned long)_edata_loc + PMD_SIZE - 1) & PMD_MASK;
 #endif
 	for ( ; addr < PAGE_OFFSET; addr += PMD_SIZE)
 		pmd_clear(pmd_off_k(addr));
@@ -1292,7 +1292,7 @@ static void __init devicemaps_init(const struct machine_desc *mdesc)
 #ifdef CONFIG_XIP_KERNEL
 	map.pfn = __phys_to_pfn(CONFIG_XIP_PHYS_ADDR & SECTION_MASK);
 	map.virtual = MODULES_VADDR;
-	map.length = ((unsigned long)_etext - map.virtual + ~SECTION_MASK) & SECTION_MASK;
+	map.length = ((unsigned long)_edata_loc - map.virtual + ~SECTION_MASK) & SECTION_MASK;
 	map.type = MT_ROM;
 	create_mapping(&map);
 #endif
-- 
1.7.9.5

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

For an XIP build, _edata_loc, not _etext, represents the end of the
binary image that will be programmed into ROM and mapped into the
MODULES_VADDR area.
With an XIP kernel, nothing is loaded into RAM before boot, meaning
you have to take into account the size of the entire binary image
that was programmed, including the init data values that will be copied
to RAM during kernel boot.

This fixes the bug where you might lose the end of your kernel area
after page table setup is complete.

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
v3:
* Removed sections.h from Kbuild
v2:
* Added change for MODULES_VADDR
* Moved extern to new file asm/sections.h
---
 arch/arm/include/asm/Kbuild     |    1 -
 arch/arm/include/asm/sections.h |    8 ++++++++
 arch/arm/kernel/module.c        |    2 +-
 arch/arm/mm/mmu.c               |    4 ++--
 4 files changed, 11 insertions(+), 4 deletions(-)
 create mode 100644 arch/arm/include/asm/sections.h

diff --git a/arch/arm/include/asm/Kbuild b/arch/arm/include/asm/Kbuild
index be648eb..6d3da22 100644
--- a/arch/arm/include/asm/Kbuild
+++ b/arch/arm/include/asm/Kbuild
@@ -21,7 +21,6 @@ generic-y += preempt.h
 generic-y += resource.h
 generic-y += rwsem.h
 generic-y += seccomp.h
-generic-y += sections.h
 generic-y += segment.h
 generic-y += sembuf.h
 generic-y += serial.h
diff --git a/arch/arm/include/asm/sections.h b/arch/arm/include/asm/sections.h
new file mode 100644
index 0000000..401eb3c2
--- /dev/null
+++ b/arch/arm/include/asm/sections.h
@@ -0,0 +1,8 @@
+#ifndef _ASM_ARM_SECTIONS_H
+#define _ASM_ARM_SECTIONS_H
+
+#include <asm-generic/sections.h>
+
+extern char _edata_loc[];
+
+#endif	/* _ASM_ARM_SECTIONS_H */
diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c
index efdddcb..41ae2cc 100644
--- a/arch/arm/kernel/module.c
+++ b/arch/arm/kernel/module.c
@@ -34,7 +34,7 @@
  * recompiling the whole kernel when CONFIG_XIP_KERNEL is turned on/off.
  */
 #undef MODULES_VADDR
-#define MODULES_VADDR	(((unsigned long)_etext + ~PMD_MASK) & PMD_MASK)
+#define MODULES_VADDR	(((unsigned long)_edata_loc + ~PMD_MASK) & PMD_MASK)
 #endif
 
 #ifdef CONFIG_MMU
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index 4867f5d..dd5a56b 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1210,7 +1210,7 @@ static inline void prepare_page_table(void)
 
 #ifdef CONFIG_XIP_KERNEL
 	/* The XIP kernel is mapped in the module area -- skip over it */
-	addr = ((unsigned long)_etext + PMD_SIZE - 1) & PMD_MASK;
+	addr = ((unsigned long)_edata_loc + PMD_SIZE - 1) & PMD_MASK;
 #endif
 	for ( ; addr < PAGE_OFFSET; addr += PMD_SIZE)
 		pmd_clear(pmd_off_k(addr));
@@ -1292,7 +1292,7 @@ static void __init devicemaps_init(const struct machine_desc *mdesc)
 #ifdef CONFIG_XIP_KERNEL
 	map.pfn = __phys_to_pfn(CONFIG_XIP_PHYS_ADDR & SECTION_MASK);
 	map.virtual = MODULES_VADDR;
-	map.length = ((unsigned long)_etext - map.virtual + ~SECTION_MASK) & SECTION_MASK;
+	map.length = ((unsigned long)_edata_loc - map.virtual + ~SECTION_MASK) & SECTION_MASK;
 	map.type = MT_ROM;
 	create_mapping(&map);
 #endif
-- 
1.7.9.5

Re: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Russell King - ARM Linux]

Nico,

As you originally created the XIP stuff, I hope you can remember
the details - can you check this patch please?

I'm thinking that we need a new symbol around here:

#ifdef CONFIG_XIP_KERNEL
        __data_loc = ALIGN(4);          /* location in binary */
					<== here
        . = PAGE_OFFSET + TEXT_OFFSET;
#else

to denote the end of the XIP kernel image which must remain
accessible after boot.  We don't need the data sections because
they will have been copied to RAM, and we probably don't want to
keep those exposed (it's potentially useful for attackers.)

On Mon, Nov 16, 2015 at 01:05:40PM -0500, Chris Brandt wrote:
> For an XIP build, _edata_loc, not _etext, represents the end of the
> binary image that will be programmed into ROM and mapped into the
> MODULES_VADDR area.
> With an XIP kernel, nothing is loaded into RAM before boot, meaning
> you have to take into account the size of the entire binary image
> that was programmed, including the init data values that will be copied
> to RAM during kernel boot.
> 
> This fixes the bug where you might lose the end of your kernel area
> after page table setup is complete.
> 
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> ---
> v3:
> * Removed sections.h from Kbuild
> v2:
> * Added change for MODULES_VADDR
> * Moved extern to new file asm/sections.h
> ---
>  arch/arm/include/asm/Kbuild     |    1 -
>  arch/arm/include/asm/sections.h |    8 ++++++++
>  arch/arm/kernel/module.c        |    2 +-
>  arch/arm/mm/mmu.c               |    4 ++--
>  4 files changed, 11 insertions(+), 4 deletions(-)
>  create mode 100644 arch/arm/include/asm/sections.h
> 
> diff --git a/arch/arm/include/asm/Kbuild b/arch/arm/include/asm/Kbuild
> index be648eb..6d3da22 100644
> --- a/arch/arm/include/asm/Kbuild
> +++ b/arch/arm/include/asm/Kbuild
> @@ -21,7 +21,6 @@ generic-y += preempt.h
>  generic-y += resource.h
>  generic-y += rwsem.h
>  generic-y += seccomp.h
> -generic-y += sections.h
>  generic-y += segment.h
>  generic-y += sembuf.h
>  generic-y += serial.h
> diff --git a/arch/arm/include/asm/sections.h b/arch/arm/include/asm/sections.h
> new file mode 100644
> index 0000000..401eb3c2
> --- /dev/null
> +++ b/arch/arm/include/asm/sections.h
> @@ -0,0 +1,8 @@
> +#ifndef _ASM_ARM_SECTIONS_H
> +#define _ASM_ARM_SECTIONS_H
> +
> +#include <asm-generic/sections.h>
> +
> +extern char _edata_loc[];
> +
> +#endif	/* _ASM_ARM_SECTIONS_H */
> diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c
> index efdddcb..41ae2cc 100644
> --- a/arch/arm/kernel/module.c
> +++ b/arch/arm/kernel/module.c
> @@ -34,7 +34,7 @@
>   * recompiling the whole kernel when CONFIG_XIP_KERNEL is turned on/off.
>   */
>  #undef MODULES_VADDR
> -#define MODULES_VADDR	(((unsigned long)_etext + ~PMD_MASK) & PMD_MASK)
> +#define MODULES_VADDR	(((unsigned long)_edata_loc + ~PMD_MASK) & PMD_MASK)
>  #endif
>  
>  #ifdef CONFIG_MMU
> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index 4867f5d..dd5a56b 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -1210,7 +1210,7 @@ static inline void prepare_page_table(void)
>  
>  #ifdef CONFIG_XIP_KERNEL
>  	/* The XIP kernel is mapped in the module area -- skip over it */
> -	addr = ((unsigned long)_etext + PMD_SIZE - 1) & PMD_MASK;
> +	addr = ((unsigned long)_edata_loc + PMD_SIZE - 1) & PMD_MASK;
>  #endif
>  	for ( ; addr < PAGE_OFFSET; addr += PMD_SIZE)
>  		pmd_clear(pmd_off_k(addr));
> @@ -1292,7 +1292,7 @@ static void __init devicemaps_init(const struct machine_desc *mdesc)
>  #ifdef CONFIG_XIP_KERNEL
>  	map.pfn = __phys_to_pfn(CONFIG_XIP_PHYS_ADDR & SECTION_MASK);
>  	map.virtual = MODULES_VADDR;
> -	map.length = ((unsigned long)_etext - map.virtual + ~SECTION_MASK) & SECTION_MASK;
> +	map.length = ((unsigned long)_edata_loc - map.virtual + ~SECTION_MASK) & SECTION_MASK;
>  	map.type = MT_ROM;
>  	create_mapping(&map);
>  #endif
> -- 
> 1.7.9.5
> 
> 

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Russell King - ARM Linux]

Nico,

As you originally created the XIP stuff, I hope you can remember
the details - can you check this patch please?

I'm thinking that we need a new symbol around here:

#ifdef CONFIG_XIP_KERNEL
        __data_loc = ALIGN(4);          /* location in binary */
					<=== here
        . = PAGE_OFFSET + TEXT_OFFSET;
#else

to denote the end of the XIP kernel image which must remain
accessible after boot.  We don't need the data sections because
they will have been copied to RAM, and we probably don't want to
keep those exposed (it's potentially useful for attackers.)

On Mon, Nov 16, 2015@01:05:40PM -0500, Chris Brandt wrote:
> For an XIP build, _edata_loc, not _etext, represents the end of the
> binary image that will be programmed into ROM and mapped into the
> MODULES_VADDR area.
> With an XIP kernel, nothing is loaded into RAM before boot, meaning
> you have to take into account the size of the entire binary image
> that was programmed, including the init data values that will be copied
> to RAM during kernel boot.
> 
> This fixes the bug where you might lose the end of your kernel area
> after page table setup is complete.
> 
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> ---
> v3:
> * Removed sections.h from Kbuild
> v2:
> * Added change for MODULES_VADDR
> * Moved extern to new file asm/sections.h
> ---
>  arch/arm/include/asm/Kbuild     |    1 -
>  arch/arm/include/asm/sections.h |    8 ++++++++
>  arch/arm/kernel/module.c        |    2 +-
>  arch/arm/mm/mmu.c               |    4 ++--
>  4 files changed, 11 insertions(+), 4 deletions(-)
>  create mode 100644 arch/arm/include/asm/sections.h
> 
> diff --git a/arch/arm/include/asm/Kbuild b/arch/arm/include/asm/Kbuild
> index be648eb..6d3da22 100644
> --- a/arch/arm/include/asm/Kbuild
> +++ b/arch/arm/include/asm/Kbuild
> @@ -21,7 +21,6 @@ generic-y += preempt.h
>  generic-y += resource.h
>  generic-y += rwsem.h
>  generic-y += seccomp.h
> -generic-y += sections.h
>  generic-y += segment.h
>  generic-y += sembuf.h
>  generic-y += serial.h
> diff --git a/arch/arm/include/asm/sections.h b/arch/arm/include/asm/sections.h
> new file mode 100644
> index 0000000..401eb3c2
> --- /dev/null
> +++ b/arch/arm/include/asm/sections.h
> @@ -0,0 +1,8 @@
> +#ifndef _ASM_ARM_SECTIONS_H
> +#define _ASM_ARM_SECTIONS_H
> +
> +#include <asm-generic/sections.h>
> +
> +extern char _edata_loc[];
> +
> +#endif	/* _ASM_ARM_SECTIONS_H */
> diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c
> index efdddcb..41ae2cc 100644
> --- a/arch/arm/kernel/module.c
> +++ b/arch/arm/kernel/module.c
> @@ -34,7 +34,7 @@
>   * recompiling the whole kernel when CONFIG_XIP_KERNEL is turned on/off.
>   */
>  #undef MODULES_VADDR
> -#define MODULES_VADDR	(((unsigned long)_etext + ~PMD_MASK) & PMD_MASK)
> +#define MODULES_VADDR	(((unsigned long)_edata_loc + ~PMD_MASK) & PMD_MASK)
>  #endif
>  
>  #ifdef CONFIG_MMU
> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index 4867f5d..dd5a56b 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -1210,7 +1210,7 @@ static inline void prepare_page_table(void)
>  
>  #ifdef CONFIG_XIP_KERNEL
>  	/* The XIP kernel is mapped in the module area -- skip over it */
> -	addr = ((unsigned long)_etext + PMD_SIZE - 1) & PMD_MASK;
> +	addr = ((unsigned long)_edata_loc + PMD_SIZE - 1) & PMD_MASK;
>  #endif
>  	for ( ; addr < PAGE_OFFSET; addr += PMD_SIZE)
>  		pmd_clear(pmd_off_k(addr));
> @@ -1292,7 +1292,7 @@ static void __init devicemaps_init(const struct machine_desc *mdesc)
>  #ifdef CONFIG_XIP_KERNEL
>  	map.pfn = __phys_to_pfn(CONFIG_XIP_PHYS_ADDR & SECTION_MASK);
>  	map.virtual = MODULES_VADDR;
> -	map.length = ((unsigned long)_etext - map.virtual + ~SECTION_MASK) & SECTION_MASK;
> +	map.length = ((unsigned long)_edata_loc - map.virtual + ~SECTION_MASK) & SECTION_MASK;
>  	map.type = MT_ROM;
>  	create_mapping(&map);
>  #endif
> -- 
> 1.7.9.5
> 
> 

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

RE: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

> We don't need the data sections because they will have been copied to RAM, and
> we probably don't want to keep those exposed (it's potentially useful for
> attackers.)

The init sections also hang around after boot as well (it's XIP code, so there is nothing to 'free' in terms of executable init code).
Any potential security issues there as well? Should the data and init-text sections be put in a separate section that gets blown away after init-data is freed?

Chris

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

> We don't need the data sections because they will have been copied to RAM, and
> we probably don't want to keep those exposed (it's potentially useful for
> attackers.)

The init sections also hang around after boot as well (it's XIP code, so there is nothing to 'free' in terms of executable init code).
Any potential security issues there as well? Should the data and init-text sections be put in a separate section that gets blown away after init-data is freed?

Chris

Re: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Russell King - ARM Linux]

On Mon, Nov 16, 2015 at 07:46:05PM +0000, Chris Brandt wrote:
> > We don't need the data sections because they will have been copied to RAM, and
> > we probably don't want to keep those exposed (it's potentially useful for
> > attackers.)
> 
> The init sections also hang around after boot as well (it's XIP code, so
> there is nothing to 'free' in terms of executable init code).
> Any potential security issues there as well? Should the data and init-
> text sections be put in a separate section that gets blown away after
> init-data is freed?

That's much harder to do - generally for XIP, people are space limited
(which is why they're using XIP rather than putting the kernel in RAM.)
They won't take kindly to having the kernel image bloated by 1MB just
to pad it out so that the init stuff can be unmapped.

However, from the security point of view, the less that's mapped at
known addresses, the better.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Russell King - ARM Linux]

On Mon, Nov 16, 2015 at 07:46:05PM +0000, Chris Brandt wrote:
> > We don't need the data sections because they will have been copied to RAM, and
> > we probably don't want to keep those exposed (it's potentially useful for
> > attackers.)
> 
> The init sections also hang around after boot as well (it's XIP code, so
> there is nothing to 'free' in terms of executable init code).
> Any potential security issues there as well? Should the data and init-
> text sections be put in a separate section that gets blown away after
> init-data is freed?

That's much harder to do - generally for XIP, people are space limited
(which is why they're using XIP rather than putting the kernel in RAM.)
They won't take kindly to having the kernel image bloated by 1MB just
to pad it out so that the init stuff can be unmapped.

However, from the security point of view, the less that's mapped at
known addresses, the better.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

RE: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

> That's much harder to do - generally for XIP, people are space limited 
> (which is why they're using XIP rather than putting the kernel in RAM.)
> They won't take kindly to having the kernel image bloated by 1MB just to
> pad it out so that the init stuff can be unmapped.
>
> However, from the security point of view, the less that's mapped at known
> addresses, the better.


Maybe the only true solution is to have some additional XIP Kconfig option that tells the kernel to not copy any of the init or data sections at all at boot, but instead leave it up to pre-boot to preload the physical locations with the correct values. That would get rid of your 1MB bloated boundary and you could even save some valuable XIP-able ROM/Flash space (you could pre-load from serial flash).

However, while that might provide more flexibility and ROM savings, it means now the pre-boot needs to know what values need to get pre-loaded where...adding more complexity to what right now is a very simple boot (set the PC and run).


- Chris

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

> That's much harder to do - generally for XIP, people are space limited 
> (which is why they're using XIP rather than putting the kernel in RAM.)
> They won't take kindly to having the kernel image bloated by 1MB just to
> pad it out so that the init stuff can be unmapped.
>
> However, from the security point of view, the less that's mapped at known
> addresses, the better.


Maybe the only true solution is to have some additional XIP Kconfig option that tells the kernel to not copy any of the init or data sections at all at boot, but instead leave it up to pre-boot to preload the physical locations with the correct values. That would get rid of your 1MB bloated boundary and you could even save some valuable XIP-able ROM/Flash space (you could pre-load from serial flash).

However, while that might provide more flexibility and ROM savings, it means now the pre-boot needs to know what values need to get pre-loaded where...adding more complexity to what right now is a very simple boot (set the PC and run).


- Chris

Re: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Mon, 16 Nov 2015, Chris Brandt wrote:

> For an XIP build, _edata_loc, not _etext, represents the end of the
> binary image that will be programmed into ROM and mapped into the
> MODULES_VADDR area.

This statement is wrong.  The kernel data is copied to RAM before the 
MMU is turned on and that's the copy that the kernel must use.  There is 
no reason having anything past _etext accessible from ROM.

> With an XIP kernel, nothing is loaded into RAM before boot, meaning
> you have to take into account the size of the entire binary image
> that was programmed, including the init data values that will be copied
> to RAM during kernel boot.

But this is done way before the code you're modifying is executed.

> This fixes the bug where you might lose the end of your kernel area
> after page table setup is complete.

Could you elaborate about a possible bug please?

> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> ---
> v3:
> * Removed sections.h from Kbuild
> v2:
> * Added change for MODULES_VADDR
> * Moved extern to new file asm/sections.h
> ---
>  arch/arm/include/asm/Kbuild     |    1 -
>  arch/arm/include/asm/sections.h |    8 ++++++++
>  arch/arm/kernel/module.c        |    2 +-
>  arch/arm/mm/mmu.c               |    4 ++--
>  4 files changed, 11 insertions(+), 4 deletions(-)
>  create mode 100644 arch/arm/include/asm/sections.h
> 
> diff --git a/arch/arm/include/asm/Kbuild b/arch/arm/include/asm/Kbuild
> index be648eb..6d3da22 100644
> --- a/arch/arm/include/asm/Kbuild
> +++ b/arch/arm/include/asm/Kbuild
> @@ -21,7 +21,6 @@ generic-y += preempt.h
>  generic-y += resource.h
>  generic-y += rwsem.h
>  generic-y += seccomp.h
> -generic-y += sections.h
>  generic-y += segment.h
>  generic-y += sembuf.h
>  generic-y += serial.h
> diff --git a/arch/arm/include/asm/sections.h b/arch/arm/include/asm/sections.h
> new file mode 100644
> index 0000000..401eb3c2
> --- /dev/null
> +++ b/arch/arm/include/asm/sections.h
> @@ -0,0 +1,8 @@
> +#ifndef _ASM_ARM_SECTIONS_H
> +#define _ASM_ARM_SECTIONS_H
> +
> +#include <asm-generic/sections.h>
> +
> +extern char _edata_loc[];
> +
> +#endif	/* _ASM_ARM_SECTIONS_H */
> diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c
> index efdddcb..41ae2cc 100644
> --- a/arch/arm/kernel/module.c
> +++ b/arch/arm/kernel/module.c
> @@ -34,7 +34,7 @@
>   * recompiling the whole kernel when CONFIG_XIP_KERNEL is turned on/off.
>   */
>  #undef MODULES_VADDR
> -#define MODULES_VADDR	(((unsigned long)_etext + ~PMD_MASK) & PMD_MASK)
> +#define MODULES_VADDR	(((unsigned long)_edata_loc + ~PMD_MASK) & PMD_MASK)
>  #endif
>  
>  #ifdef CONFIG_MMU
> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index 4867f5d..dd5a56b 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -1210,7 +1210,7 @@ static inline void prepare_page_table(void)
>  
>  #ifdef CONFIG_XIP_KERNEL
>  	/* The XIP kernel is mapped in the module area -- skip over it */
> -	addr = ((unsigned long)_etext + PMD_SIZE - 1) & PMD_MASK;
> +	addr = ((unsigned long)_edata_loc + PMD_SIZE - 1) & PMD_MASK;
>  #endif
>  	for ( ; addr < PAGE_OFFSET; addr += PMD_SIZE)
>  		pmd_clear(pmd_off_k(addr));
> @@ -1292,7 +1292,7 @@ static void __init devicemaps_init(const struct machine_desc *mdesc)
>  #ifdef CONFIG_XIP_KERNEL
>  	map.pfn = __phys_to_pfn(CONFIG_XIP_PHYS_ADDR & SECTION_MASK);
>  	map.virtual = MODULES_VADDR;
> -	map.length = ((unsigned long)_etext - map.virtual + ~SECTION_MASK) & SECTION_MASK;
> +	map.length = ((unsigned long)_edata_loc - map.virtual + ~SECTION_MASK) & SECTION_MASK;
>  	map.type = MT_ROM;
>  	create_mapping(&map);
>  #endif
> -- 
> 1.7.9.5
> 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> 

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Mon, 16 Nov 2015, Chris Brandt wrote:

> For an XIP build, _edata_loc, not _etext, represents the end of the
> binary image that will be programmed into ROM and mapped into the
> MODULES_VADDR area.

This statement is wrong.  The kernel data is copied to RAM before the 
MMU is turned on and that's the copy that the kernel must use.  There is 
no reason having anything past _etext accessible from ROM.

> With an XIP kernel, nothing is loaded into RAM before boot, meaning
> you have to take into account the size of the entire binary image
> that was programmed, including the init data values that will be copied
> to RAM during kernel boot.

But this is done way before the code you're modifying is executed.

> This fixes the bug where you might lose the end of your kernel area
> after page table setup is complete.

Could you elaborate about a possible bug please?

> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
> ---
> v3:
> * Removed sections.h from Kbuild
> v2:
> * Added change for MODULES_VADDR
> * Moved extern to new file asm/sections.h
> ---
>  arch/arm/include/asm/Kbuild     |    1 -
>  arch/arm/include/asm/sections.h |    8 ++++++++
>  arch/arm/kernel/module.c        |    2 +-
>  arch/arm/mm/mmu.c               |    4 ++--
>  4 files changed, 11 insertions(+), 4 deletions(-)
>  create mode 100644 arch/arm/include/asm/sections.h
> 
> diff --git a/arch/arm/include/asm/Kbuild b/arch/arm/include/asm/Kbuild
> index be648eb..6d3da22 100644
> --- a/arch/arm/include/asm/Kbuild
> +++ b/arch/arm/include/asm/Kbuild
> @@ -21,7 +21,6 @@ generic-y += preempt.h
>  generic-y += resource.h
>  generic-y += rwsem.h
>  generic-y += seccomp.h
> -generic-y += sections.h
>  generic-y += segment.h
>  generic-y += sembuf.h
>  generic-y += serial.h
> diff --git a/arch/arm/include/asm/sections.h b/arch/arm/include/asm/sections.h
> new file mode 100644
> index 0000000..401eb3c2
> --- /dev/null
> +++ b/arch/arm/include/asm/sections.h
> @@ -0,0 +1,8 @@
> +#ifndef _ASM_ARM_SECTIONS_H
> +#define _ASM_ARM_SECTIONS_H
> +
> +#include <asm-generic/sections.h>
> +
> +extern char _edata_loc[];
> +
> +#endif	/* _ASM_ARM_SECTIONS_H */
> diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c
> index efdddcb..41ae2cc 100644
> --- a/arch/arm/kernel/module.c
> +++ b/arch/arm/kernel/module.c
> @@ -34,7 +34,7 @@
>   * recompiling the whole kernel when CONFIG_XIP_KERNEL is turned on/off.
>   */
>  #undef MODULES_VADDR
> -#define MODULES_VADDR	(((unsigned long)_etext + ~PMD_MASK) & PMD_MASK)
> +#define MODULES_VADDR	(((unsigned long)_edata_loc + ~PMD_MASK) & PMD_MASK)
>  #endif
>  
>  #ifdef CONFIG_MMU
> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index 4867f5d..dd5a56b 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -1210,7 +1210,7 @@ static inline void prepare_page_table(void)
>  
>  #ifdef CONFIG_XIP_KERNEL
>  	/* The XIP kernel is mapped in the module area -- skip over it */
> -	addr = ((unsigned long)_etext + PMD_SIZE - 1) & PMD_MASK;
> +	addr = ((unsigned long)_edata_loc + PMD_SIZE - 1) & PMD_MASK;
>  #endif
>  	for ( ; addr < PAGE_OFFSET; addr += PMD_SIZE)
>  		pmd_clear(pmd_off_k(addr));
> @@ -1292,7 +1292,7 @@ static void __init devicemaps_init(const struct machine_desc *mdesc)
>  #ifdef CONFIG_XIP_KERNEL
>  	map.pfn = __phys_to_pfn(CONFIG_XIP_PHYS_ADDR & SECTION_MASK);
>  	map.virtual = MODULES_VADDR;
> -	map.length = ((unsigned long)_etext - map.virtual + ~SECTION_MASK) & SECTION_MASK;
> +	map.length = ((unsigned long)_edata_loc - map.virtual + ~SECTION_MASK) & SECTION_MASK;
>  	map.type = MT_ROM;
>  	create_mapping(&map);
>  #endif
> -- 
> 1.7.9.5
> 
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> 

Re: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Russell King - ARM Linux]

Please wrap your messages at or before column 72, thanks.

On Mon, Nov 16, 2015 at 08:18:55PM +0000, Chris Brandt wrote:
> Maybe the only true solution is to have some additional XIP Kconfig
> option that tells the kernel to not copy any of the init or data
> sections at all at boot,

An XIP kernel will copy the .data section from ROM into RAM for you.
You don't need additional code external to the kernel to perfor this.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Russell King - ARM Linux]

Please wrap your messages at or before column 72, thanks.

On Mon, Nov 16, 2015 at 08:18:55PM +0000, Chris Brandt wrote:
> Maybe the only true solution is to have some additional XIP Kconfig
> option that tells the kernel to not copy any of the init or data
> sections at all at boot,

An XIP kernel will copy the .data section from ROM into RAM for you.
You don't need additional code external to the kernel to perfor this.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Mon, 16 Nov 2015, Russell King - ARM Linux wrote:

> Nico,
> 
> As you originally created the XIP stuff, I hope you can remember
> the details - can you check this patch please?
> 
> I'm thinking that we need a new symbol around here:
> 
> #ifdef CONFIG_XIP_KERNEL
>         __data_loc = ALIGN(4);          /* location in binary */
> 					<== here
>         . = PAGE_OFFSET + TEXT_OFFSET;
> #else
> 
> to denote the end of the XIP kernel image which must remain
> accessible after boot.  We don't need the data sections because
> they will have been copied to RAM, and we probably don't want to
> keep those exposed (it's potentially useful for attackers.)

The _etext symbol is already used for that purpose.

Now we round it up to the next section mapping which might leave quite a 
lot of data content exposed in ROM.  But given it is more or less the 
same data present in RAM except for those bits that are modified at run 
time, I don't see what an attacker would gain from the data in ROM   
that cannot already be obtained from kernel RAM.  If the section mapping 
extends to part of the ROM that is no longer kernel data then maybe this 
would expose sensitive data.  Is that what you're worried about?


Nicolas

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Mon, 16 Nov 2015, Russell King - ARM Linux wrote:

> Nico,
> 
> As you originally created the XIP stuff, I hope you can remember
> the details - can you check this patch please?
> 
> I'm thinking that we need a new symbol around here:
> 
> #ifdef CONFIG_XIP_KERNEL
>         __data_loc = ALIGN(4);          /* location in binary */
> 					<=== here
>         . = PAGE_OFFSET + TEXT_OFFSET;
> #else
> 
> to denote the end of the XIP kernel image which must remain
> accessible after boot.  We don't need the data sections because
> they will have been copied to RAM, and we probably don't want to
> keep those exposed (it's potentially useful for attackers.)

The _etext symbol is already used for that purpose.

Now we round it up to the next section mapping which might leave quite a 
lot of data content exposed in ROM.  But given it is more or less the 
same data present in RAM except for those bits that are modified at run 
time, I don't see what an attacker would gain from the data in ROM   
that cannot already be obtained from kernel RAM.  If the section mapping 
extends to part of the ROM that is no longer kernel data then maybe this 
would expose sensitive data.  Is that what you're worried about?


Nicolas

RE: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

>> For an XIP build, _edata_loc, not _etext, represents the end of the 
>> binary image that will be programmed into ROM and mapped into the 
>> MODULES_VADDR area.
>
> This statement is wrong.  The kernel data is copied to RAM before the
> MMU is turned on and that's the copy that the kernel must use.  There
> is no reason having anything past _etext accessible from ROM.

If I remember correctly, when I first ran into an issue was my
initramfs got cut off because it spanned the 1MB boundary after
_etext.

Also, your init code resides after _etext, so that get cut off
too.


>> With an XIP kernel, nothing is loaded into RAM before boot, meaning 
>> you have to take into account the size of the entire binary image that 
>> was programmed, including the init data values that will be copied to 
>> RAM during kernel boot.
>
> But this is done way before the code you're modifying is executed.

If the MMU is set up to cut everything off after _etext early on, then
anything in the init section gets blown away, and there are lots
of init functions in the kernel that get run at various times during
boot.


>> This fixes the bug where you might lose the end of your kernel area 
>> after page table setup is complete.
>
>Could you elaborate about a possible bug please?

During boot, the kernel attempts to execute code that is not mapped
anywhere, and hence crashes.



-Chris

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

>> For an XIP build, _edata_loc, not _etext, represents the end of the 
>> binary image that will be programmed into ROM and mapped into the 
>> MODULES_VADDR area.
>
> This statement is wrong.  The kernel data is copied to RAM before the
> MMU is turned on and that's the copy that the kernel must use.  There
> is no reason having anything past _etext accessible from ROM.

If I remember correctly, when I first ran into an issue was my
initramfs got cut off because it spanned the 1MB boundary after
_etext.

Also, your init code resides after _etext, so that get cut off
too.


>> With an XIP kernel, nothing is loaded into RAM before boot, meaning 
>> you have to take into account the size of the entire binary image that 
>> was programmed, including the init data values that will be copied to 
>> RAM during kernel boot.
>
> But this is done way before the code you're modifying is executed.

If the MMU is set up to cut everything off after _etext early on, then
anything in the init section gets blown away, and there are lots
of init functions in the kernel that get run at various times during
boot.


>> This fixes the bug where you might lose the end of your kernel area 
>> after page table setup is complete.
>
>Could you elaborate about a possible bug please?

During boot, the kernel attempts to execute code that is not mapped
anywhere, and hence crashes.



-Chris

RE: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

> If the section mapping extends to part of the ROM that is no longer
> kernel data then maybe this would expose sensitive data.  Is that
> what you're worried about?

That is a good point that I did not consider before. Thank you.

Chris

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

> If the section mapping extends to part of the ROM that is no longer
> kernel data then maybe this would expose sensitive data.  Is that
> what you're worried about?

That is a good point that I did not consider before. Thank you.

Chris

RE: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Mon, 16 Nov 2015, Chris Brandt wrote:

> >> For an XIP build, _edata_loc, not _etext, represents the end of the 
> >> binary image that will be programmed into ROM and mapped into the 
> >> MODULES_VADDR area.
> >
> > This statement is wrong.  The kernel data is copied to RAM before the
> > MMU is turned on and that's the copy that the kernel must use.  There
> > is no reason having anything past _etext accessible from ROM.
> 
> If I remember correctly, when I first ran into an issue was my
> initramfs got cut off because it spanned the 1MB boundary after
> _etext.

If you are convinced that you need kernel XIP, then you normally would 
want _not_ to use initramfs at all.

> Also, your init code resides after _etext, so that get cut off
> too.

/me looks at the linker script and cry

Forget it.  XIP kernel is simply completely broken at the moment.  

There used to be conditional placement of .init.text that was located in 
the .text section when XIP_KERNEL was selected, or in the .init section 
otherwise.  This particular section was abstracted away into an 
INIT_TEXT_SECTION macro defined in include/asm-generic/vmlinux.lds.h and 
the XIP conditional placement got lost.  New sections (lots of them) 
were added as well since the introduction of XIP kernel with no 
consideration for XIP usage.

The old method with conditional placement in the linker script is broken 
beyond repair.  There is simply way too much stuff these days to do this 
sanely.  I think the only way to fix it properly is to have a separate 
linker script for XIP usage with proper (and obvious) section placement.


Nicolas

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Mon, 16 Nov 2015, Chris Brandt wrote:

> >> For an XIP build, _edata_loc, not _etext, represents the end of the 
> >> binary image that will be programmed into ROM and mapped into the 
> >> MODULES_VADDR area.
> >
> > This statement is wrong.  The kernel data is copied to RAM before the
> > MMU is turned on and that's the copy that the kernel must use.  There
> > is no reason having anything past _etext accessible from ROM.
> 
> If I remember correctly, when I first ran into an issue was my
> initramfs got cut off because it spanned the 1MB boundary after
> _etext.

If you are convinced that you need kernel XIP, then you normally would 
want _not_ to use initramfs at all.

> Also, your init code resides after _etext, so that get cut off
> too.

/me looks at the linker script and cry

Forget it.  XIP kernel is simply completely broken at the moment.  

There used to be conditional placement of .init.text that was located in 
the .text section when XIP_KERNEL was selected, or in the .init section 
otherwise.  This particular section was abstracted away into an 
INIT_TEXT_SECTION macro defined in include/asm-generic/vmlinux.lds.h and 
the XIP conditional placement got lost.  New sections (lots of them) 
were added as well since the introduction of XIP kernel with no 
consideration for XIP usage.

The old method with conditional placement in the linker script is broken 
beyond repair.  There is simply way too much stuff these days to do this 
sanely.  I think the only way to fix it properly is to have a separate 
linker script for XIP usage with proper (and obvious) section placement.


Nicolas

RE: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

> If you are convinced that you need kernel XIP, then you normally would
> want _not_ to use initramfs at all.

True, but in trying to test upstream kernels, initramfs was easier than
fighting with device drivers (although now I've just switched to using
memory mapped mtd-rom).


> Forget it.  XIP kernel is simply completely broken at the moment.  

Yes, there were multiple things wrong that took me a while to find, but
once you get them all working, it's pretty cool. Note, the Kconfig
issues are going to be a bit harder to get approval.


> New sections (lots of them) were added as well since the introduction
> of XIP kernel with no consideration for XIP usage.

So I've noticed. As far as I can tell, the XIP kernel stuff broke years
ago.


> I think the only way to fix it properly is to have a separate linker
> script for XIP usage with proper (and obvious) section placement.

I guess then the parts of the kernel that refer to section locations
need to be looked at again. As you can see from my other patches,
there's a bit of miss-match that I was trying to align.


So at this point...I'm assume all my XIP 'hacks' are being rejected.

As for making a new XIP-only linker script, note that I really only
work with the ARMv7. I know the M4 and M5 guys do XIP_KERNEL builds,
but I've got no way of testing their XIP_KERNEL builds or knowing
what will break.


Chris

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

> If you are convinced that you need kernel XIP, then you normally would
> want _not_ to use initramfs at all.

True, but in trying to test upstream kernels, initramfs was easier than
fighting with device drivers (although now I've just switched to using
memory mapped mtd-rom).


> Forget it.  XIP kernel is simply completely broken at the moment.  

Yes, there were multiple things wrong that took me a while to find, but
once you get them all working, it's pretty cool. Note, the Kconfig
issues are going to be a bit harder to get approval.


> New sections (lots of them) were added as well since the introduction
> of XIP kernel with no consideration for XIP usage.

So I've noticed. As far as I can tell, the XIP kernel stuff broke years
ago.


> I think the only way to fix it properly is to have a separate linker
> script for XIP usage with proper (and obvious) section placement.

I guess then the parts of the kernel that refer to section locations
need to be looked at again. As you can see from my other patches,
there's a bit of miss-match that I was trying to align.


So at this point...I'm assume all my XIP 'hacks' are being rejected.

As for making a new XIP-only linker script, note that I really only
work with the ARMv7. I know the M4 and M5 guys do XIP_KERNEL builds,
but I've got no way of testing their XIP_KERNEL builds or knowing
what will break.


Chris

RE: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Mon, 16 Nov 2015, Chris Brandt wrote:

> > New sections (lots of them) were added as well since the introduction
> > of XIP kernel with no consideration for XIP usage.
> 
> So I've noticed. As far as I can tell, the XIP kernel stuff broke years
> ago.

That, of course, brings the question about keeping XIP around.

> > I think the only way to fix it properly is to have a separate linker
> > script for XIP usage with proper (and obvious) section placement.
> 
> I guess then the parts of the kernel that refer to section locations
> need to be looked at again. As you can see from my other patches,
> there's a bit of miss-match that I was trying to align.

I don't think that's a big issue. Those section locations are obtained 
from symbols that the linker script creates explicitly.

> So at this point...I'm assume all my XIP 'hacks' are being rejected.

Pretty much.  Everything can be fixed at once with a new linker script.

> As for making a new XIP-only linker script, note that I really only
> work with the ARMv7. I know the M4 and M5 guys do XIP_KERNEL builds,
> but I've got no way of testing their XIP_KERNEL builds or knowing
> what will break.

Things can hardly be worse than they are right now.


Nicolas

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Mon, 16 Nov 2015, Chris Brandt wrote:

> > New sections (lots of them) were added as well since the introduction
> > of XIP kernel with no consideration for XIP usage.
> 
> So I've noticed. As far as I can tell, the XIP kernel stuff broke years
> ago.

That, of course, brings the question about keeping XIP around.

> > I think the only way to fix it properly is to have a separate linker
> > script for XIP usage with proper (and obvious) section placement.
> 
> I guess then the parts of the kernel that refer to section locations
> need to be looked at again. As you can see from my other patches,
> there's a bit of miss-match that I was trying to align.

I don't think that's a big issue. Those section locations are obtained 
from symbols that the linker script creates explicitly.

> So at this point...I'm assume all my XIP 'hacks' are being rejected.

Pretty much.  Everything can be fixed at once with a new linker script.

> As for making a new XIP-only linker script, note that I really only
> work with the ARMv7. I know the M4 and M5 guys do XIP_KERNEL builds,
> but I've got no way of testing their XIP_KERNEL builds or knowing
> what will break.

Things can hardly be worse than they are right now.


Nicolas

RE: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

>> So I've noticed. As far as I can tell, the XIP kernel stuff broke 
>> years ago.
>
> That, of course, brings the question about keeping XIP around.

Well, as the email address suggests, I work for Renesas. And since
the ARCH_R7S72100 has up to 10MB internal RAM and can XIP directly from
QSPI Flash, that's a pretty simple BOM.

I also revisted the AXFS file system (that never made it into the
kernel) and like the kernel, required some fixing up.
However, I can get a functioning and usefull ARMv7 system up and
running in under 3MB of total system RAM.
That's got to be worth something to someone ;)


> Everything can be fixed at once with a new linker script.

I do have to agree that a separate linker script would be a cleaner
solution for the section arrangements.
Of course, there are still some other XIP patches needed (like you can't
use text areas as a temporary stack on boot), but the sections are a
pretty logical place to start.


Chris

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

>> So I've noticed. As far as I can tell, the XIP kernel stuff broke 
>> years ago.
>
> That, of course, brings the question about keeping XIP around.

Well, as the email address suggests, I work for Renesas. And since
the ARCH_R7S72100 has up to 10MB internal RAM and can XIP directly from
QSPI Flash, that's a pretty simple BOM.

I also revisted the AXFS file system (that never made it into the
kernel) and like the kernel, required some fixing up.
However, I can get a functioning and usefull ARMv7 system up and
running in under 3MB of total system RAM.
That's got to be worth something to someone ;)


> Everything can be fixed at once with a new linker script.

I do have to agree that a separate linker script would be a cleaner
solution for the section arrangements.
Of course, there are still some other XIP patches needed (like you can't
use text areas as a temporary stack on boot), but the sections are a
pretty logical place to start.


Chris

RE: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Tue, 17 Nov 2015, Chris Brandt wrote:

> >> So I've noticed. As far as I can tell, the XIP kernel stuff broke 
> >> years ago.
> >
> > That, of course, brings the question about keeping XIP around.
> 
> Well, as the email address suggests, I work for Renesas. And since
> the ARCH_R7S72100 has up to 10MB internal RAM and can XIP directly from
> QSPI Flash, that's a pretty simple BOM.
> 
> I also revisted the AXFS file system (that never made it into the
> kernel) and like the kernel, required some fixing up.
> However, I can get a functioning and usefull ARMv7 system up and
> running in under 3MB of total system RAM.
> That's got to be worth something to someone ;)

Absolutely.  I'll be glad to review your patches.

> > Everything can be fixed at once with a new linker script.
> 
> I do have to agree that a separate linker script would be a cleaner
> solution for the section arrangements.
> Of course, there are still some other XIP patches needed (like you can't
> use text areas as a temporary stack on boot), but the sections are a
> pretty logical place to start.

Maybe this can be prevented automatically by the linker with some 
read-only memory regions or the like.


Nicolas

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Tue, 17 Nov 2015, Chris Brandt wrote:

> >> So I've noticed. As far as I can tell, the XIP kernel stuff broke 
> >> years ago.
> >
> > That, of course, brings the question about keeping XIP around.
> 
> Well, as the email address suggests, I work for Renesas. And since
> the ARCH_R7S72100 has up to 10MB internal RAM and can XIP directly from
> QSPI Flash, that's a pretty simple BOM.
> 
> I also revisted the AXFS file system (that never made it into the
> kernel) and like the kernel, required some fixing up.
> However, I can get a functioning and usefull ARMv7 system up and
> running in under 3MB of total system RAM.
> That's got to be worth something to someone ;)

Absolutely.  I'll be glad to review your patches.

> > Everything can be fixed at once with a new linker script.
> 
> I do have to agree that a separate linker script would be a cleaner
> solution for the section arrangements.
> Of course, there are still some other XIP patches needed (like you can't
> use text areas as a temporary stack on boot), but the sections are a
> pretty logical place to start.

Maybe this can be prevented automatically by the linker with some 
read-only memory regions or the like.


Nicolas

RE: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

> The old method with conditional placement in the linker script is
> broken beyond repair.  There is simply way too much stuff these days
> to do this sanely.  I think the only way to fix it properly is to
> have a separate linker script for XIP usage with proper (and obvious)
> section placement.



Step 1: Change the Makefiles to select an alternative linker script.

What do you think about doing the following?


diff --git a/Makefile b/Makefile
index fd46821..956ad21 100644
--- a/Makefile
+++ b/Makefile
@@ -898,7 +898,11 @@ libs-y		:= $(libs-y1) $(libs-y2)
 # Externally visible symbols (used by link-vmlinux.sh)
 export KBUILD_VMLINUX_INIT := $(head-y) $(init-y)
 export KBUILD_VMLINUX_MAIN := $(core-y) $(libs-y) $(drivers-y) $(net-y)
+ifdef CONFIG_XIP_KERNEL
+export KBUILD_LDS          := arch/$(SRCARCH)/kernel/vmlinux-xip.lds
+else
 export KBUILD_LDS          := arch/$(SRCARCH)/kernel/vmlinux.lds
+endif
 export LDFLAGS_vmlinux
 # used by scripts/pacmage/Makefile
 export KBUILD_ALLDIRS := $(sort $(filter-out arch/%,$(vmlinux-alldirs)) arch Documentation include samples scripts tools virt)
 
 

diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index af9e59b..d97fc2e 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -3,6 +3,7 @@
 #
 
 CPPFLAGS_vmlinux.lds := -DTEXT_OFFSET=$(TEXT_OFFSET)
+CPPFLAGS_vmlinux-xip.lds := -DTEXT_OFFSET=$(TEXT_OFFSET)
 AFLAGS_head.o        := -DTEXT_OFFSET=$(TEXT_OFFSET)
 
 ifdef CONFIG_FUNCTION_TRACER
@@ -92,4 +93,8 @@ obj-y				+= psci-call.o
 obj-$(CONFIG_SMP)		+= psci_smp.o
 endif
 
+ifdef CONFIG_XIP_KERNEL
+extra-y := $(head-y) vmlinux-xip.lds
+else
 extra-y := $(head-y) vmlinux.lds
+endif






Chris

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

> The old method with conditional placement in the linker script is
> broken beyond repair.  There is simply way too much stuff these days
> to do this sanely.  I think the only way to fix it properly is to
> have a separate linker script for XIP usage with proper (and obvious)
> section placement.



Step 1: Change the Makefiles to select an alternative linker script.

What do you think about doing the following?


diff --git a/Makefile b/Makefile
index fd46821..956ad21 100644
--- a/Makefile
+++ b/Makefile
@@ -898,7 +898,11 @@ libs-y		:= $(libs-y1) $(libs-y2)
 # Externally visible symbols (used by link-vmlinux.sh)
 export KBUILD_VMLINUX_INIT := $(head-y) $(init-y)
 export KBUILD_VMLINUX_MAIN := $(core-y) $(libs-y) $(drivers-y) $(net-y)
+ifdef CONFIG_XIP_KERNEL
+export KBUILD_LDS          := arch/$(SRCARCH)/kernel/vmlinux-xip.lds
+else
 export KBUILD_LDS          := arch/$(SRCARCH)/kernel/vmlinux.lds
+endif
 export LDFLAGS_vmlinux
 # used by scripts/pacmage/Makefile
 export KBUILD_ALLDIRS := $(sort $(filter-out arch/%,$(vmlinux-alldirs)) arch Documentation include samples scripts tools virt)
 
 

diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index af9e59b..d97fc2e 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -3,6 +3,7 @@
 #
 
 CPPFLAGS_vmlinux.lds := -DTEXT_OFFSET=$(TEXT_OFFSET)
+CPPFLAGS_vmlinux-xip.lds := -DTEXT_OFFSET=$(TEXT_OFFSET)
 AFLAGS_head.o        := -DTEXT_OFFSET=$(TEXT_OFFSET)
 
 ifdef CONFIG_FUNCTION_TRACER
@@ -92,4 +93,8 @@ obj-y				+= psci-call.o
 obj-$(CONFIG_SMP)		+= psci_smp.o
 endif
 
+ifdef CONFIG_XIP_KERNEL
+extra-y := $(head-y) vmlinux-xip.lds
+else
 extra-y := $(head-y) vmlinux.lds
+endif






Chris

RE: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

> Absolutely.  I'll be glad to review your patches.

What do you think about this submission?
I think it still holds true regardless of the incorrect sectioning.

http://lists.infradead.org/pipermail/linux-arm-kernel/2015-November/384354.html



>> Of course, there are still some other XIP patches needed (like you
>> can't use text areas as a temporary stack on boot), but the sections
>> are a pretty logical place to start.
>
> Maybe this can be prevented automatically by the linker with some
> read-only memory regions or the like.

I think this one is more of a coding issue.
These were the attempts to fix the temporary stack issue:

My first patch:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/357106.html


Magnus's try:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-November/383394.html



Chris

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

> Absolutely.  I'll be glad to review your patches.

What do you think about this submission?
I think it still holds true regardless of the incorrect sectioning.

http://lists.infradead.org/pipermail/linux-arm-kernel/2015-November/384354.html



>> Of course, there are still some other XIP patches needed (like you
>> can't use text areas as a temporary stack on boot), but the sections
>> are a pretty logical place to start.
>
> Maybe this can be prevented automatically by the linker with some
> read-only memory regions or the like.

I think this one is more of a coding issue.
These were the attempts to fix the temporary stack issue:

My first patch:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/357106.html


Magnus's try:
http://lists.infradead.org/pipermail/linux-arm-kernel/2015-November/383394.html



Chris

RE: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Tue, 17 Nov 2015, Chris Brandt wrote:

> > The old method with conditional placement in the linker script is
> > broken beyond repair.  There is simply way too much stuff these days
> > to do this sanely.  I think the only way to fix it properly is to
> > have a separate linker script for XIP usage with proper (and obvious)
> > section placement.
> 
> 
> 
> Step 1: Change the Makefiles to select an alternative linker script.
> 
> What do you think about doing the following?




That's fine.  Obviously this is the first step when developing this, but 
logically this will have to be the last patch in the series when you're 
done.

> 
> 
> diff --git a/Makefile b/Makefile
> index fd46821..956ad21 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -898,7 +898,11 @@ libs-y		:= $(libs-y1) $(libs-y2)
>  # Externally visible symbols (used by link-vmlinux.sh)
>  export KBUILD_VMLINUX_INIT := $(head-y) $(init-y)
>  export KBUILD_VMLINUX_MAIN := $(core-y) $(libs-y) $(drivers-y) $(net-y)
> +ifdef CONFIG_XIP_KERNEL
> +export KBUILD_LDS          := arch/$(SRCARCH)/kernel/vmlinux-xip.lds
> +else
>  export KBUILD_LDS          := arch/$(SRCARCH)/kernel/vmlinux.lds
> +endif
>  export LDFLAGS_vmlinux
>  # used by scripts/pacmage/Makefile
>  export KBUILD_ALLDIRS := $(sort $(filter-out arch/%,$(vmlinux-alldirs)) arch Documentation include samples scripts tools virt)
>  
>  
> 
> diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
> index af9e59b..d97fc2e 100644
> --- a/arch/arm/kernel/Makefile
> +++ b/arch/arm/kernel/Makefile
> @@ -3,6 +3,7 @@
>  #
>  
>  CPPFLAGS_vmlinux.lds := -DTEXT_OFFSET=$(TEXT_OFFSET)
> +CPPFLAGS_vmlinux-xip.lds := -DTEXT_OFFSET=$(TEXT_OFFSET)
>  AFLAGS_head.o        := -DTEXT_OFFSET=$(TEXT_OFFSET)
>  
>  ifdef CONFIG_FUNCTION_TRACER
> @@ -92,4 +93,8 @@ obj-y				+= psci-call.o
>  obj-$(CONFIG_SMP)		+= psci_smp.o
>  endif
>  
> +ifdef CONFIG_XIP_KERNEL
> +extra-y := $(head-y) vmlinux-xip.lds
> +else
>  extra-y := $(head-y) vmlinux.lds
> +endif
> 
> 
> 
> 
> 
> 
> Chris
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> 

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Tue, 17 Nov 2015, Chris Brandt wrote:

> > The old method with conditional placement in the linker script is
> > broken beyond repair.  There is simply way too much stuff these days
> > to do this sanely.  I think the only way to fix it properly is to
> > have a separate linker script for XIP usage with proper (and obvious)
> > section placement.
> 
> 
> 
> Step 1: Change the Makefiles to select an alternative linker script.
> 
> What do you think about doing the following?




That's fine.  Obviously this is the first step when developing this, but 
logically this will have to be the last patch in the series when you're 
done.

> 
> 
> diff --git a/Makefile b/Makefile
> index fd46821..956ad21 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -898,7 +898,11 @@ libs-y		:= $(libs-y1) $(libs-y2)
>  # Externally visible symbols (used by link-vmlinux.sh)
>  export KBUILD_VMLINUX_INIT := $(head-y) $(init-y)
>  export KBUILD_VMLINUX_MAIN := $(core-y) $(libs-y) $(drivers-y) $(net-y)
> +ifdef CONFIG_XIP_KERNEL
> +export KBUILD_LDS          := arch/$(SRCARCH)/kernel/vmlinux-xip.lds
> +else
>  export KBUILD_LDS          := arch/$(SRCARCH)/kernel/vmlinux.lds
> +endif
>  export LDFLAGS_vmlinux
>  # used by scripts/pacmage/Makefile
>  export KBUILD_ALLDIRS := $(sort $(filter-out arch/%,$(vmlinux-alldirs)) arch Documentation include samples scripts tools virt)
>  
>  
> 
> diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
> index af9e59b..d97fc2e 100644
> --- a/arch/arm/kernel/Makefile
> +++ b/arch/arm/kernel/Makefile
> @@ -3,6 +3,7 @@
>  #
>  
>  CPPFLAGS_vmlinux.lds := -DTEXT_OFFSET=$(TEXT_OFFSET)
> +CPPFLAGS_vmlinux-xip.lds := -DTEXT_OFFSET=$(TEXT_OFFSET)
>  AFLAGS_head.o        := -DTEXT_OFFSET=$(TEXT_OFFSET)
>  
>  ifdef CONFIG_FUNCTION_TRACER
> @@ -92,4 +93,8 @@ obj-y				+= psci-call.o
>  obj-$(CONFIG_SMP)		+= psci_smp.o
>  endif
>  
> +ifdef CONFIG_XIP_KERNEL
> +extra-y := $(head-y) vmlinux-xip.lds
> +else
>  extra-y := $(head-y) vmlinux.lds
> +endif
> 
> 
> 
> 
> 
> 
> Chris
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 
> 

RE: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Tue, 17 Nov 2015, Chris Brandt wrote:

> > Absolutely.  I'll be glad to review your patches.
> 
> What do you think about this submission?
> I think it still holds true regardless of the incorrect sectioning.
> 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-November/384354.html

This patch is correct.

> >> Of course, there are still some other XIP patches needed (like you
> >> can't use text areas as a temporary stack on boot), but the sections
> >> are a pretty logical place to start.
> >
> > Maybe this can be prevented automatically by the linker with some
> > read-only memory regions or the like.
> 
> I think this one is more of a coding issue.
> These were the attempts to fix the temporary stack issue:
> 
> My first patch:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/357106.html

I think this ought to be fixed regardless of the XIP issue.

> Magnus's try:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-November/383394.html

This one is wrong as it clobbers r11.


Nicolas

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Tue, 17 Nov 2015, Chris Brandt wrote:

> > Absolutely.  I'll be glad to review your patches.
> 
> What do you think about this submission?
> I think it still holds true regardless of the incorrect sectioning.
> 
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-November/384354.html

This patch is correct.

> >> Of course, there are still some other XIP patches needed (like you
> >> can't use text areas as a temporary stack on boot), but the sections
> >> are a pretty logical place to start.
> >
> > Maybe this can be prevented automatically by the linker with some
> > read-only memory regions or the like.
> 
> I think this one is more of a coding issue.
> These were the attempts to fix the temporary stack issue:
> 
> My first patch:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/357106.html

I think this ought to be fixed regardless of the XIP issue.

> Magnus's try:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-November/383394.html

This one is wrong as it clobbers r11.


Nicolas

Re: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Russell King - ARM Linux]

On Mon, Nov 16, 2015 at 04:47:12PM -0500, Nicolas Pitre wrote:
> The old method with conditional placement in the linker script is broken 
> beyond repair.  There is simply way too much stuff these days to do this 
> sanely.  I think the only way to fix it properly is to have a separate 
> linker script for XIP usage with proper (and obvious) section placement.

Absolutely the right thing to do.

The XIP stuff has been broken and getting in the way for a long time
now because of the preprocessor mess.  I've made several changes to
the linker script over the years, and each time I've ended up deciding
to I'll do the best job I can for XIP, but I'm not going to spend
too long making sure that it's correct - especially as there appeared
to be no users of it.  I wouldn't describe the current file as being
"maintainable" in any way - It has devolved into a total mess, and I
would like to see the XIP stuff gone from the non-XIP linker script
- and doing that should mean it's less likely for XIP to break.

I'd also like to see a lot more of the preprocessor gunk gone (or at
least cleaned up) in that file too...

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Russell King - ARM Linux]

On Mon, Nov 16, 2015 at 04:47:12PM -0500, Nicolas Pitre wrote:
> The old method with conditional placement in the linker script is broken 
> beyond repair.  There is simply way too much stuff these days to do this 
> sanely.  I think the only way to fix it properly is to have a separate 
> linker script for XIP usage with proper (and obvious) section placement.

Absolutely the right thing to do.

The XIP stuff has been broken and getting in the way for a long time
now because of the preprocessor mess.  I've made several changes to
the linker script over the years, and each time I've ended up deciding
to I'll do the best job I can for XIP, but I'm not going to spend
too long making sure that it's correct - especially as there appeared
to be no users of it.  I wouldn't describe the current file as being
"maintainable" in any way - It has devolved into a total mess, and I
would like to see the XIP stuff gone from the non-XIP linker script
- and doing that should mean it's less likely for XIP to break.

I'd also like to see a lot more of the preprocessor gunk gone (or at
least cleaned up) in that file too...

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

RE: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

So after looking back in history, I think I now understand how/when the XIP stuff broke.

It looks like originally, the init section came first, followed by text and data.

This is why all the XIP code used _etext as its marker for the end of what needed to be mapped.
Like you said, the initialized data values get copied from ROM to RAM early in boot so it's not an issue if the data section (that comes after text) gets blown away when the full MMU setup is done.


However, after the init section was moved from the first section to almost the last, all the _etext references scatter through the kernel were never updated.


So basically, even though I've now got a separate linker script for XIP builds, it really hasn't changed much because the real issue all along was that the XIP_KERNEL references to _etext should have been changed to _data_loc.

With that said, I still think the correct way to fix this issue is to export _data_loc and use that as the end marker. (notice I changed my mind and now say _data_loc instead of _edata_loc because there's no reason to map the full data section anymore).

The other option, if we really wanted to keep the references to _etext as they are, is to simply move the one line " _etext = .;" to the bottom of the file....right before _data_loc is declared.
But...that would be a bit misleading if you ask me.


It sounds like there would still be a request to split up the linker scripts in order to remove the 6 XIP_KERNEL references scattered  throughout vmlinux.lds.S. However, it would still be nice to keep the two versions looking as close as they can to each other with only the obvious changes (like removing XIP_KERNEL from the non-XIP version, and removing SMP_ON_UP from the XIP version)


Thoughts?


Chris

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

So after looking back in history, I think I now understand how/when the XIP stuff broke.

It looks like originally, the init section came first, followed by text and data.

This is why all the XIP code used _etext as its marker for the end of what needed to be mapped.
Like you said, the initialized data values get copied from ROM to RAM early in boot so it's not an issue if the data section (that comes after text) gets blown away when the full MMU setup is done.


However, after the init section was moved from the first section to almost the last, all the _etext references scatter through the kernel were never updated.


So basically, even though I've now got a separate linker script for XIP builds, it really hasn't changed much because the real issue all along was that the XIP_KERNEL references to _etext should have been changed to _data_loc.

With that said, I still think the correct way to fix this issue is to export _data_loc and use that as the end marker. (notice I changed my mind and now say _data_loc instead of _edata_loc because there's no reason to map the full data section anymore).

The other option, if we really wanted to keep the references to _etext as they are, is to simply move the one line " _etext = .;" to the bottom of the file....right before _data_loc is declared.
But...that would be a bit misleading if you ask me.


It sounds like there would still be a request to split up the linker scripts in order to remove the 6 XIP_KERNEL references scattered  throughout vmlinux.lds.S. However, it would still be nice to keep the two versions looking as close as they can to each other with only the obvious changes (like removing XIP_KERNEL from the non-XIP version, and removing SMP_ON_UP from the XIP version)


Thoughts?


Chris

RE: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Wed, 18 Nov 2015, Chris Brandt wrote:

> It sounds like there would still be a request to split up the linker 
> scripts in order to remove the 6 XIP_KERNEL references scattered 
> throughout vmlinux.lds.S. However, it would still be nice to keep the 
> two versions looking as close as they can to each other with only the 
> obvious changes (like removing XIP_KERNEL from the non-XIP version, 
> and removing SMP_ON_UP from the XIP version)

I don't think so. XIP and non-XIP are fundamentally looking different 
these days.  Trying to keep them as similar as possible is rather an 
impediment to both cases and brings no real benefits.

Not only should .init.smpalt be gone from the XIP version, but it might 
be safer to put that into a special "should_be_empty" section and bail 
out with ASSERT() if that section size isn't zero. Same thing for 
pv_table. This way we'll be sure that nothing in the kernel config 
expects self-modified code to be present.

With XIP there is still an init section, but it is pointless to store 
.init.text stuff in there. That should instead stay in ROM and never be 
discarded (obviously). No need to page align it either at that point 
which should save some ROM space.

Things like .kprobes.text should be in ROM, but the entire executable in 
ROM should be surrounded by __kprobes_text_start and __kprobes_text_end 
to prevent any attempt to put kprobes there. This way, kprobes could 
still work for modules.

Etc. Etc.

There are simply too many things these days that require special 
considerations with XIP. And freeing the main script from XIP 
considerations will ease maintenance as well, more than the added 
maintenance from the duplicated parts.


Nicolas

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Wed, 18 Nov 2015, Chris Brandt wrote:

> It sounds like there would still be a request to split up the linker 
> scripts in order to remove the 6 XIP_KERNEL references scattered 
> throughout vmlinux.lds.S. However, it would still be nice to keep the 
> two versions looking as close as they can to each other with only the 
> obvious changes (like removing XIP_KERNEL from the non-XIP version, 
> and removing SMP_ON_UP from the XIP version)

I don't think so. XIP and non-XIP are fundamentally looking different 
these days.  Trying to keep them as similar as possible is rather an 
impediment to both cases and brings no real benefits.

Not only should .init.smpalt be gone from the XIP version, but it might 
be safer to put that into a special "should_be_empty" section and bail 
out with ASSERT() if that section size isn't zero. Same thing for 
pv_table. This way we'll be sure that nothing in the kernel config 
expects self-modified code to be present.

With XIP there is still an init section, but it is pointless to store 
.init.text stuff in there. That should instead stay in ROM and never be 
discarded (obviously). No need to page align it either at that point 
which should save some ROM space.

Things like .kprobes.text should be in ROM, but the entire executable in 
ROM should be surrounded by __kprobes_text_start and __kprobes_text_end 
to prevent any attempt to put kprobes there. This way, kprobes could 
still work for modules.

Etc. Etc.

There are simply too many things these days that require special 
considerations with XIP. And freeing the main script from XIP 
considerations will ease maintenance as well, more than the added 
maintenance from the duplicated parts.


Nicolas

RE: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Tue, 17 Nov 2015, Chris Brandt wrote:

> I think this one is more of a coding issue.
> These were the attempts to fix the temporary stack issue:
> 
> My first patch:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/357106.html
> 
> Magnus's try:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-November/383394.html

Here's my proposal:

diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
index de2b246fed..2d0ac32320 100644
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
@@ -274,10 +274,12 @@ __v7_ca15mp_setup:
 __v7_b15mp_setup:
 __v7_ca17mp_setup:
 	mov	r10, #0
-1:	adr	r12, __v7_setup_stack		@ the local stack
-	stmia	r12, {r0-r5, lr}		@ v7_invalidate_l1 touches r0-r6
+1:	adr	r0, __v7_setup_stack_ptr
+	ldr	r12, [r0]
+	add	r12, r12, r0			@ the local stack
+	stmia	r12, {r1-r6, lr}		@ v7_invalidate_l1 touches r0-r6
 	bl      v7_invalidate_l1
-	ldmia	r12, {r0-r5, lr}
+	ldmia	r12, {r1-r6, lr}
 #ifdef CONFIG_SMP
 	ALT_SMP(mrc	p15, 0, r0, c1, c0, 1)
 	ALT_UP(mov	r0, #(1 << 6))		@ fake it for UP
@@ -415,10 +417,12 @@ __v7_pj4b_setup:
 #endif /* CONFIG_CPU_PJ4B */
 
 __v7_setup:
-	adr	r12, __v7_setup_stack		@ the local stack
-	stmia	r12, {r0-r5, lr}		@ v7_invalidate_l1 touches r0-r6
+	adr	r0, __v7_setup_stack_ptr
+	ldr	r12, [r0]
+	add	r12, r12, r0			@ the local stack
+	stmia	r12, {r1-r6, lr}		@ v7_invalidate_l1 touches r0-r6
 	bl      v7_invalidate_l1
-	ldmia	r12, {r0-r5, lr}
+	ldmia	r12, {r1-r6, lr}
 
 __v7_setup_cont:
 	and	r0, r9, #0xff000000		@ ARM?
@@ -480,11 +484,16 @@ __errata_finish:
 	orr	r0, r0, r6			@ set them
  THUMB(	orr	r0, r0, #1 << 30	)	@ Thumb exceptions
 	ret	lr				@ return to head.S:__ret
+
+	.align	2
+__v7_setup_stack_ptr:
+	.word	__v7_setup_stack - .
 ENDPROC(__v7_setup)
 
+	.bss
 	.align	2
 __v7_setup_stack:
-	.space	4 * 7				@ 12 registers
+	.space	4 * 7				@ 7 registers
 
 	__INITDATA
 
diff --git a/arch/arm/mm/proc-v7m.S b/arch/arm/mm/proc-v7m.S
index 67d9209077..8cd465927a 100644
--- a/arch/arm/mm/proc-v7m.S
+++ b/arch/arm/mm/proc-v7m.S
@@ -123,10 +123,12 @@ __v7m_setup:
 	ret	lr
 ENDPROC(__v7m_setup)
 
+	.pushsection .bss
 	.align 2
 __v7m_setup_stack:
 	.space	4 * 8				@ 8 registers
 __v7m_setup_stack_top:
+	.previous
 
 	define_processor_functions v7m, dabort=nommu_early_abort, pabort=legacy_pabort, nommu=1
 

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Tue, 17 Nov 2015, Chris Brandt wrote:

> I think this one is more of a coding issue.
> These were the attempts to fix the temporary stack issue:
> 
> My first patch:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/357106.html
> 
> Magnus's try:
> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-November/383394.html

Here's my proposal:

diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
index de2b246fed..2d0ac32320 100644
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
@@ -274,10 +274,12 @@ __v7_ca15mp_setup:
 __v7_b15mp_setup:
 __v7_ca17mp_setup:
 	mov	r10, #0
-1:	adr	r12, __v7_setup_stack		@ the local stack
-	stmia	r12, {r0-r5, lr}		@ v7_invalidate_l1 touches r0-r6
+1:	adr	r0, __v7_setup_stack_ptr
+	ldr	r12, [r0]
+	add	r12, r12, r0			@ the local stack
+	stmia	r12, {r1-r6, lr}		@ v7_invalidate_l1 touches r0-r6
 	bl      v7_invalidate_l1
-	ldmia	r12, {r0-r5, lr}
+	ldmia	r12, {r1-r6, lr}
 #ifdef CONFIG_SMP
 	ALT_SMP(mrc	p15, 0, r0, c1, c0, 1)
 	ALT_UP(mov	r0, #(1 << 6))		@ fake it for UP
@@ -415,10 +417,12 @@ __v7_pj4b_setup:
 #endif /* CONFIG_CPU_PJ4B */
 
 __v7_setup:
-	adr	r12, __v7_setup_stack		@ the local stack
-	stmia	r12, {r0-r5, lr}		@ v7_invalidate_l1 touches r0-r6
+	adr	r0, __v7_setup_stack_ptr
+	ldr	r12, [r0]
+	add	r12, r12, r0			@ the local stack
+	stmia	r12, {r1-r6, lr}		@ v7_invalidate_l1 touches r0-r6
 	bl      v7_invalidate_l1
-	ldmia	r12, {r0-r5, lr}
+	ldmia	r12, {r1-r6, lr}
 
 __v7_setup_cont:
 	and	r0, r9, #0xff000000		@ ARM?
@@ -480,11 +484,16 @@ __errata_finish:
 	orr	r0, r0, r6			@ set them
  THUMB(	orr	r0, r0, #1 << 30	)	@ Thumb exceptions
 	ret	lr				@ return to head.S:__ret
+
+	.align	2
+__v7_setup_stack_ptr:
+	.word	__v7_setup_stack - .
 ENDPROC(__v7_setup)
 
+	.bss
 	.align	2
 __v7_setup_stack:
-	.space	4 * 7				@ 12 registers
+	.space	4 * 7				@ 7 registers
 
 	__INITDATA
 
diff --git a/arch/arm/mm/proc-v7m.S b/arch/arm/mm/proc-v7m.S
index 67d9209077..8cd465927a 100644
--- a/arch/arm/mm/proc-v7m.S
+++ b/arch/arm/mm/proc-v7m.S
@@ -123,10 +123,12 @@ __v7m_setup:
 	ret	lr
 ENDPROC(__v7m_setup)
 
+	.pushsection .bss
 	.align 2
 __v7m_setup_stack:
 	.space	4 * 8				@ 8 registers
 __v7m_setup_stack_top:
+	.previous
 
 	define_processor_functions v7m, dabort=nommu_early_abort, pabort=legacy_pabort, nommu=1
 

Re: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Magnus Damm]

Hi Nicolas,

On Wed, Nov 18, 2015 at 12:58 PM, Nicolas Pitre <nico@fluxnic.net> wrote:
> On Tue, 17 Nov 2015, Chris Brandt wrote:
>
>> I think this one is more of a coding issue.
>> These were the attempts to fix the temporary stack issue:
>>
>> My first patch:
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/357106.html
>>
>> Magnus's try:
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-November/383394.html
>
> Here's my proposal:
>
> diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
> index de2b246fed..2d0ac32320 100644
> --- a/arch/arm/mm/proc-v7.S
> +++ b/arch/arm/mm/proc-v7.S
> @@ -274,10 +274,12 @@ __v7_ca15mp_setup:
>  __v7_b15mp_setup:
>  __v7_ca17mp_setup:
>         mov     r10, #0
> -1:     adr     r12, __v7_setup_stack           @ the local stack
> -       stmia   r12, {r0-r5, lr}                @ v7_invalidate_l1 touches r0-r6
> +1:     adr     r0, __v7_setup_stack_ptr
> +       ldr     r12, [r0]
> +       add     r12, r12, r0                    @ the local stack
> +       stmia   r12, {r1-r6, lr}                @ v7_invalidate_l1 touches r0-r6
>         bl      v7_invalidate_l1
> -       ldmia   r12, {r0-r5, lr}
> +       ldmia   r12, {r1-r6, lr}
>  #ifdef CONFIG_SMP
>         ALT_SMP(mrc     p15, 0, r0, c1, c0, 1)
>         ALT_UP(mov      r0, #(1 << 6))          @ fake it for UP

[snip]

> @@ -480,11 +484,16 @@ __errata_finish:
>         orr     r0, r0, r6                      @ set them
>   THUMB(        orr     r0, r0, #1 << 30        )       @ Thumb exceptions
>         ret     lr                              @ return to head.S:__ret
> +
> +       .align  2
> +__v7_setup_stack_ptr:
> +       .word   __v7_setup_stack - .
>  ENDPROC(__v7_setup)

Thanks for your take on this. I did a couple of local implementations
before submitting, and one of the issues I ran into was the need to
get rid of PAGE_OFFSET due to the code running without MMU enabled. I
suppose that is taken care of the "__v7_setup_stack - ." calculation
above?

Cheers,

/ magnus

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Magnus Damm]

Hi Nicolas,

On Wed, Nov 18, 2015 at 12:58 PM, Nicolas Pitre <nico@fluxnic.net> wrote:
> On Tue, 17 Nov 2015, Chris Brandt wrote:
>
>> I think this one is more of a coding issue.
>> These were the attempts to fix the temporary stack issue:
>>
>> My first patch:
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/357106.html
>>
>> Magnus's try:
>> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-November/383394.html
>
> Here's my proposal:
>
> diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
> index de2b246fed..2d0ac32320 100644
> --- a/arch/arm/mm/proc-v7.S
> +++ b/arch/arm/mm/proc-v7.S
> @@ -274,10 +274,12 @@ __v7_ca15mp_setup:
>  __v7_b15mp_setup:
>  __v7_ca17mp_setup:
>         mov     r10, #0
> -1:     adr     r12, __v7_setup_stack           @ the local stack
> -       stmia   r12, {r0-r5, lr}                @ v7_invalidate_l1 touches r0-r6
> +1:     adr     r0, __v7_setup_stack_ptr
> +       ldr     r12, [r0]
> +       add     r12, r12, r0                    @ the local stack
> +       stmia   r12, {r1-r6, lr}                @ v7_invalidate_l1 touches r0-r6
>         bl      v7_invalidate_l1
> -       ldmia   r12, {r0-r5, lr}
> +       ldmia   r12, {r1-r6, lr}
>  #ifdef CONFIG_SMP
>         ALT_SMP(mrc     p15, 0, r0, c1, c0, 1)
>         ALT_UP(mov      r0, #(1 << 6))          @ fake it for UP

[snip]

> @@ -480,11 +484,16 @@ __errata_finish:
>         orr     r0, r0, r6                      @ set them
>   THUMB(        orr     r0, r0, #1 << 30        )       @ Thumb exceptions
>         ret     lr                              @ return to head.S:__ret
> +
> +       .align  2
> +__v7_setup_stack_ptr:
> +       .word   __v7_setup_stack - .
>  ENDPROC(__v7_setup)

Thanks for your take on this. I did a couple of local implementations
before submitting, and one of the issues I ran into was the need to
get rid of PAGE_OFFSET due to the code running without MMU enabled. I
suppose that is taken care of the "__v7_setup_stack - ." calculation
above?

Cheers,

/ magnus

Re: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Arnd Bergmann]

On Tuesday 17 November 2015 22:17:41 Nicolas Pitre wrote:
> On Wed, 18 Nov 2015, Chris Brandt wrote:
> 
> > It sounds like there would still be a request to split up the linker 
> > scripts in order to remove the 6 XIP_KERNEL references scattered 
> > throughout vmlinux.lds.S. However, it would still be nice to keep the 
> > two versions looking as close as they can to each other with only the 
> > obvious changes (like removing XIP_KERNEL from the non-XIP version, 
> > and removing SMP_ON_UP from the XIP version)
> 
> I don't think so. XIP and non-XIP are fundamentally looking different 
> these days.  Trying to keep them as similar as possible is rather an 
> impediment to both cases and brings no real benefits.
> 
> Not only should .init.smpalt be gone from the XIP version, but it might 
> be safer to put that into a special "should_be_empty" section and bail 
> out with ASSERT() if that section size isn't zero. Same thing for 
> pv_table. This way we'll be sure that nothing in the kernel config 
> expects self-modified code to be present.
> 
> With XIP there is still an init section, but it is pointless to store 
> .init.text stuff in there. That should instead stay in ROM and never be 
> discarded (obviously). No need to page align it either at that point 
> which should save some ROM space.
> 
> Things like .kprobes.text should be in ROM, but the entire executable in 
> ROM should be surrounded by __kprobes_text_start and __kprobes_text_end 
> to prevent any attempt to put kprobes there. This way, kprobes could 
> still work for modules.
> 
> Etc. Etc.
> 
> There are simply too many things these days that require special 
> considerations with XIP. And freeing the main script from XIP 
> considerations will ease maintenance as well, more than the added 
> maintenance from the duplicated parts.

[Adding Uwe and Maxime to Cc]

I believe Uwe has been running XIP_KERNEL on efm32 for a while now,
but he also mentioned that he needed some extra hacks in the linker
script and elsewhere to get it to work.

It would be good to coordinate this, he may have found additional
problems, or he can maybe help test the new approach on his hardware.

	Arnd

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Arnd Bergmann]

On Tuesday 17 November 2015 22:17:41 Nicolas Pitre wrote:
> On Wed, 18 Nov 2015, Chris Brandt wrote:
> 
> > It sounds like there would still be a request to split up the linker 
> > scripts in order to remove the 6 XIP_KERNEL references scattered 
> > throughout vmlinux.lds.S. However, it would still be nice to keep the 
> > two versions looking as close as they can to each other with only the 
> > obvious changes (like removing XIP_KERNEL from the non-XIP version, 
> > and removing SMP_ON_UP from the XIP version)
> 
> I don't think so. XIP and non-XIP are fundamentally looking different 
> these days.  Trying to keep them as similar as possible is rather an 
> impediment to both cases and brings no real benefits.
> 
> Not only should .init.smpalt be gone from the XIP version, but it might 
> be safer to put that into a special "should_be_empty" section and bail 
> out with ASSERT() if that section size isn't zero. Same thing for 
> pv_table. This way we'll be sure that nothing in the kernel config 
> expects self-modified code to be present.
> 
> With XIP there is still an init section, but it is pointless to store 
> .init.text stuff in there. That should instead stay in ROM and never be 
> discarded (obviously). No need to page align it either at that point 
> which should save some ROM space.
> 
> Things like .kprobes.text should be in ROM, but the entire executable in 
> ROM should be surrounded by __kprobes_text_start and __kprobes_text_end 
> to prevent any attempt to put kprobes there. This way, kprobes could 
> still work for modules.
> 
> Etc. Etc.
> 
> There are simply too many things these days that require special 
> considerations with XIP. And freeing the main script from XIP 
> considerations will ease maintenance as well, more than the added 
> maintenance from the duplicated parts.

[Adding Uwe and Maxime to Cc]

I believe Uwe has been running XIP_KERNEL on efm32 for a while now,
but he also mentioned that he needed some extra hacks in the linker
script and elsewhere to get it to work.

It would be good to coordinate this, he may have found additional
problems, or he can maybe help test the new approach on his hardware.

	Arnd

Re: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Wed, 18 Nov 2015, Magnus Damm wrote:

> Hi Nicolas,
> 
> On Wed, Nov 18, 2015 at 12:58 PM, Nicolas Pitre <nico@fluxnic.net> wrote:
> > On Tue, 17 Nov 2015, Chris Brandt wrote:
> >
> >> I think this one is more of a coding issue.
> >> These were the attempts to fix the temporary stack issue:
> >>
> >> My first patch:
> >> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/357106.html
> >>
> >> Magnus's try:
> >> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-November/383394.html
> >
> > Here's my proposal:
> >
> > diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
> > index de2b246fed..2d0ac32320 100644
> > --- a/arch/arm/mm/proc-v7.S
> > +++ b/arch/arm/mm/proc-v7.S
> > @@ -274,10 +274,12 @@ __v7_ca15mp_setup:
> >  __v7_b15mp_setup:
> >  __v7_ca17mp_setup:
> >         mov     r10, #0
> > -1:     adr     r12, __v7_setup_stack           @ the local stack
> > -       stmia   r12, {r0-r5, lr}                @ v7_invalidate_l1 touches r0-r6
> > +1:     adr     r0, __v7_setup_stack_ptr
> > +       ldr     r12, [r0]
> > +       add     r12, r12, r0                    @ the local stack
> > +       stmia   r12, {r1-r6, lr}                @ v7_invalidate_l1 touches r0-r6
> >         bl      v7_invalidate_l1
> > -       ldmia   r12, {r0-r5, lr}
> > +       ldmia   r12, {r1-r6, lr}
> >  #ifdef CONFIG_SMP
> >         ALT_SMP(mrc     p15, 0, r0, c1, c0, 1)
> >         ALT_UP(mov      r0, #(1 << 6))          @ fake it for UP
> 
> [snip]
> 
> > @@ -480,11 +484,16 @@ __errata_finish:
> >         orr     r0, r0, r6                      @ set them
> >   THUMB(        orr     r0, r0, #1 << 30        )       @ Thumb exceptions
> >         ret     lr                              @ return to head.S:__ret
> > +
> > +       .align  2
> > +__v7_setup_stack_ptr:
> > +       .word   __v7_setup_stack - .
> >  ENDPROC(__v7_setup)
> 
> Thanks for your take on this. I did a couple of local implementations
> before submitting, and one of the issues I ran into was the need to
> get rid of PAGE_OFFSET due to the code running without MMU enabled. I
> suppose that is taken care of the "__v7_setup_stack - ." calculation
> above?

Yes.  That provides the offset from __v7_setup_stack_ptr to reach 
__v7_setup_stack. And __v7_setup_stack_ptr is obtained with adr which is 
relative to the current pc. So this works whether or not the MMU is 
enabled.


Nicolas

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Wed, 18 Nov 2015, Magnus Damm wrote:

> Hi Nicolas,
> 
> On Wed, Nov 18, 2015 at 12:58 PM, Nicolas Pitre <nico@fluxnic.net> wrote:
> > On Tue, 17 Nov 2015, Chris Brandt wrote:
> >
> >> I think this one is more of a coding issue.
> >> These were the attempts to fix the temporary stack issue:
> >>
> >> My first patch:
> >> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-July/357106.html
> >>
> >> Magnus's try:
> >> http://lists.infradead.org/pipermail/linux-arm-kernel/2015-November/383394.html
> >
> > Here's my proposal:
> >
> > diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
> > index de2b246fed..2d0ac32320 100644
> > --- a/arch/arm/mm/proc-v7.S
> > +++ b/arch/arm/mm/proc-v7.S
> > @@ -274,10 +274,12 @@ __v7_ca15mp_setup:
> >  __v7_b15mp_setup:
> >  __v7_ca17mp_setup:
> >         mov     r10, #0
> > -1:     adr     r12, __v7_setup_stack           @ the local stack
> > -       stmia   r12, {r0-r5, lr}                @ v7_invalidate_l1 touches r0-r6
> > +1:     adr     r0, __v7_setup_stack_ptr
> > +       ldr     r12, [r0]
> > +       add     r12, r12, r0                    @ the local stack
> > +       stmia   r12, {r1-r6, lr}                @ v7_invalidate_l1 touches r0-r6
> >         bl      v7_invalidate_l1
> > -       ldmia   r12, {r0-r5, lr}
> > +       ldmia   r12, {r1-r6, lr}
> >  #ifdef CONFIG_SMP
> >         ALT_SMP(mrc     p15, 0, r0, c1, c0, 1)
> >         ALT_UP(mov      r0, #(1 << 6))          @ fake it for UP
> 
> [snip]
> 
> > @@ -480,11 +484,16 @@ __errata_finish:
> >         orr     r0, r0, r6                      @ set them
> >   THUMB(        orr     r0, r0, #1 << 30        )       @ Thumb exceptions
> >         ret     lr                              @ return to head.S:__ret
> > +
> > +       .align  2
> > +__v7_setup_stack_ptr:
> > +       .word   __v7_setup_stack - .
> >  ENDPROC(__v7_setup)
> 
> Thanks for your take on this. I did a couple of local implementations
> before submitting, and one of the issues I ran into was the need to
> get rid of PAGE_OFFSET due to the code running without MMU enabled. I
> suppose that is taken care of the "__v7_setup_stack - ." calculation
> above?

Yes.  That provides the offset from __v7_setup_stack_ptr to reach 
__v7_setup_stack. And __v7_setup_stack_ptr is obtained with adr which is 
relative to the current pc. So this works whether or not the MMU is 
enabled.


Nicolas

RE: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

> With XIP there is still an init section, [snip]
> No need to page align it either at that point which should save some
> ROM space.

Good point.

> Things like .kprobes.text should be in ROM, [snip]
>
> Etc. Etc.

Here's the thing...I'm probably out of my depth on most of this. But, I'd be happy to post up a RFC patch and then start collecting the feedback on what can be removed. I can at least test it on a real XIP system.


Chris

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

> With XIP there is still an init section, [snip]
> No need to page align it either at that point which should save some
> ROM space.

Good point.

> Things like .kprobes.text should be in ROM, [snip]
>
> Etc. Etc.

Here's the thing...I'm probably out of my depth on most of this. But, I'd be happy to post up a RFC patch and then start collecting the feedback on what can be removed. I can at least test it on a real XIP system.


Chris

RE: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

> [Adding Uwe and Maxime to Cc]
> 
> I believe Uwe has been running XIP_KERNEL on efm32 for a while now,
> but he also mentioned that he needed some extra hacks in the linker
> script and elsewhere to get it to work.


Yes, Maxime got the XIP fix for scripts/link-vmlinux.sh into the kernel ([PATCH v6 01/15] scripts: link-vmlinux: Don't pass page offset to kallsyms if XIP Kernel), which of course made one less file I have to patch.


> It would be good to coordinate this, he may have found additional
> problems, or he can maybe help test the new approach on his hardware.

The part I'm using (RZ/A1H) is a Cortex A9 and the EFM32 is a Cortex M3, so we'd be able to test both MMU and non-MMU XIP systems.


Chris

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

> [Adding Uwe and Maxime to Cc]
> 
> I believe Uwe has been running XIP_KERNEL on efm32 for a while now,
> but he also mentioned that he needed some extra hacks in the linker
> script and elsewhere to get it to work.


Yes, Maxime got the XIP fix for scripts/link-vmlinux.sh into the kernel ([PATCH v6 01/15] scripts: link-vmlinux: Don't pass page offset to kallsyms if XIP Kernel), which of course made one less file I have to patch.


> It would be good to coordinate this, he may have found additional
> problems, or he can maybe help test the new approach on his hardware.

The part I'm using (RZ/A1H) is a Cortex A9 and the EFM32 is a Cortex M3, so we'd be able to test both MMU and non-MMU XIP systems.


Chris

Re: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Wed, 18 Nov 2015, Nicolas Pitre wrote:

> On Wed, 18 Nov 2015, Magnus Damm wrote:
> 
> > Thanks for your take on this. I did a couple of local implementations
> > before submitting, and one of the issues I ran into was the need to
> > get rid of PAGE_OFFSET due to the code running without MMU enabled. I
> > suppose that is taken care of the "__v7_setup_stack - ." calculation
> > above?
> 
> Yes.  That provides the offset from __v7_setup_stack_ptr to reach 
> __v7_setup_stack. And __v7_setup_stack_ptr is obtained with adr which is 
> relative to the current pc. So this works whether or not the MMU is 
> enabled.

Here's the patch with proper changelog, etc.  I don't have XIP capable 
hardware to test it with though.

------ >8
From: Nicolas Pitre <nicolas.pitre@linaro.org>
Subject: [PATCH] ARM: proc-v7*.S: don't locate temporary stack space in .text section

Both proc-v7.S and proc-v7m.S are using a small temporary stack to
preserve register content within their respective setup code. This
stack is located in the .text section which is normally meant to be
read-only.  This is even more true in the context of an XIP kernel
where .text is accessed from ROM directly.

Move that temporary stack to the .bss section and get its address in
a position independent way.

While at it, one comments was updated to reflect reality, and the list
of saved registers in the proc-v7.S case is updated to match the comment
next to it for coherency.

Signed-off-by: Nicolas Pitre <nico@linaro.org>

diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
index de2b246fed..2d0ac32320 100644
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
@@ -274,10 +274,12 @@ __v7_ca15mp_setup:
 __v7_b15mp_setup:
 __v7_ca17mp_setup:
 	mov	r10, #0
-1:	adr	r12, __v7_setup_stack		@ the local stack
-	stmia	r12, {r0-r5, lr}		@ v7_invalidate_l1 touches r0-r6
+1:	adr	r0, __v7_setup_stack_ptr
+	ldr	r12, [r0]
+	add	r12, r12, r0			@ the local stack
+	stmia	r12, {r1-r6, lr}		@ v7_invalidate_l1 touches r0-r6
 	bl      v7_invalidate_l1
-	ldmia	r12, {r0-r5, lr}
+	ldmia	r12, {r1-r6, lr}
 #ifdef CONFIG_SMP
 	ALT_SMP(mrc	p15, 0, r0, c1, c0, 1)
 	ALT_UP(mov	r0, #(1 << 6))		@ fake it for UP
@@ -415,10 +417,12 @@ __v7_pj4b_setup:
 #endif /* CONFIG_CPU_PJ4B */
 
 __v7_setup:
-	adr	r12, __v7_setup_stack		@ the local stack
-	stmia	r12, {r0-r5, lr}		@ v7_invalidate_l1 touches r0-r6
+	adr	r0, __v7_setup_stack_ptr
+	ldr	r12, [r0]
+	add	r12, r12, r0			@ the local stack
+	stmia	r12, {r1-r6, lr}		@ v7_invalidate_l1 touches r0-r6
 	bl      v7_invalidate_l1
-	ldmia	r12, {r0-r5, lr}
+	ldmia	r12, {r1-r6, lr}
 
 __v7_setup_cont:
 	and	r0, r9, #0xff000000		@ ARM?
@@ -480,11 +484,16 @@ __errata_finish:
 	orr	r0, r0, r6			@ set them
  THUMB(	orr	r0, r0, #1 << 30	)	@ Thumb exceptions
 	ret	lr				@ return to head.S:__ret
+
+	.align	2
+__v7_setup_stack_ptr:
+	.word	__v7_setup_stack - .
 ENDPROC(__v7_setup)
 
+	.bss
 	.align	2
 __v7_setup_stack:
-	.space	4 * 7				@ 12 registers
+	.space	4 * 7				@ 7 registers
 
 	__INITDATA
 
diff --git a/arch/arm/mm/proc-v7m.S b/arch/arm/mm/proc-v7m.S
index 67d9209077..8cd465927a 100644
--- a/arch/arm/mm/proc-v7m.S
+++ b/arch/arm/mm/proc-v7m.S
@@ -123,10 +123,12 @@ __v7m_setup:
 	ret	lr
 ENDPROC(__v7m_setup)
 
+	.pushsection .bss
 	.align 2
 __v7m_setup_stack:
 	.space	4 * 8				@ 8 registers
 __v7m_setup_stack_top:
+	.previous
 
 	define_processor_functions v7m, dabort=nommu_early_abort, pabort=legacy_pabort, nommu=1
 

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Wed, 18 Nov 2015, Nicolas Pitre wrote:

> On Wed, 18 Nov 2015, Magnus Damm wrote:
> 
> > Thanks for your take on this. I did a couple of local implementations
> > before submitting, and one of the issues I ran into was the need to
> > get rid of PAGE_OFFSET due to the code running without MMU enabled. I
> > suppose that is taken care of the "__v7_setup_stack - ." calculation
> > above?
> 
> Yes.  That provides the offset from __v7_setup_stack_ptr to reach 
> __v7_setup_stack. And __v7_setup_stack_ptr is obtained with adr which is 
> relative to the current pc. So this works whether or not the MMU is 
> enabled.

Here's the patch with proper changelog, etc.  I don't have XIP capable 
hardware to test it with though.

------ >8
From: Nicolas Pitre <nicolas.pitre@linaro.org>
Subject: [PATCH] ARM: proc-v7*.S: don't locate temporary stack space in .text section

Both proc-v7.S and proc-v7m.S are using a small temporary stack to
preserve register content within their respective setup code. This
stack is located in the .text section which is normally meant to be
read-only.  This is even more true in the context of an XIP kernel
where .text is accessed from ROM directly.

Move that temporary stack to the .bss section and get its address in
a position independent way.

While at it, one comments was updated to reflect reality, and the list
of saved registers in the proc-v7.S case is updated to match the comment
next to it for coherency.

Signed-off-by: Nicolas Pitre <nico@linaro.org>

diff --git a/arch/arm/mm/proc-v7.S b/arch/arm/mm/proc-v7.S
index de2b246fed..2d0ac32320 100644
--- a/arch/arm/mm/proc-v7.S
+++ b/arch/arm/mm/proc-v7.S
@@ -274,10 +274,12 @@ __v7_ca15mp_setup:
 __v7_b15mp_setup:
 __v7_ca17mp_setup:
 	mov	r10, #0
-1:	adr	r12, __v7_setup_stack		@ the local stack
-	stmia	r12, {r0-r5, lr}		@ v7_invalidate_l1 touches r0-r6
+1:	adr	r0, __v7_setup_stack_ptr
+	ldr	r12, [r0]
+	add	r12, r12, r0			@ the local stack
+	stmia	r12, {r1-r6, lr}		@ v7_invalidate_l1 touches r0-r6
 	bl      v7_invalidate_l1
-	ldmia	r12, {r0-r5, lr}
+	ldmia	r12, {r1-r6, lr}
 #ifdef CONFIG_SMP
 	ALT_SMP(mrc	p15, 0, r0, c1, c0, 1)
 	ALT_UP(mov	r0, #(1 << 6))		@ fake it for UP
@@ -415,10 +417,12 @@ __v7_pj4b_setup:
 #endif /* CONFIG_CPU_PJ4B */
 
 __v7_setup:
-	adr	r12, __v7_setup_stack		@ the local stack
-	stmia	r12, {r0-r5, lr}		@ v7_invalidate_l1 touches r0-r6
+	adr	r0, __v7_setup_stack_ptr
+	ldr	r12, [r0]
+	add	r12, r12, r0			@ the local stack
+	stmia	r12, {r1-r6, lr}		@ v7_invalidate_l1 touches r0-r6
 	bl      v7_invalidate_l1
-	ldmia	r12, {r0-r5, lr}
+	ldmia	r12, {r1-r6, lr}
 
 __v7_setup_cont:
 	and	r0, r9, #0xff000000		@ ARM?
@@ -480,11 +484,16 @@ __errata_finish:
 	orr	r0, r0, r6			@ set them
  THUMB(	orr	r0, r0, #1 << 30	)	@ Thumb exceptions
 	ret	lr				@ return to head.S:__ret
+
+	.align	2
+__v7_setup_stack_ptr:
+	.word	__v7_setup_stack - .
 ENDPROC(__v7_setup)
 
+	.bss
 	.align	2
 __v7_setup_stack:
-	.space	4 * 7				@ 12 registers
+	.space	4 * 7				@ 7 registers
 
 	__INITDATA
 
diff --git a/arch/arm/mm/proc-v7m.S b/arch/arm/mm/proc-v7m.S
index 67d9209077..8cd465927a 100644
--- a/arch/arm/mm/proc-v7m.S
+++ b/arch/arm/mm/proc-v7m.S
@@ -123,10 +123,12 @@ __v7m_setup:
 	ret	lr
 ENDPROC(__v7m_setup)
 
+	.pushsection .bss
 	.align 2
 __v7m_setup_stack:
 	.space	4 * 8				@ 8 registers
 __v7m_setup_stack_top:
+	.previous
 
 	define_processor_functions v7m, dabort=nommu_early_abort, pabort=legacy_pabort, nommu=1
 

RE: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Wed, 18 Nov 2015, Chris Brandt wrote:

> > With XIP there is still an init section, [snip]
> > No need to page align it either at that point which should save some
> > ROM space.
> 
> Good point.
> 
> > Things like .kprobes.text should be in ROM, [snip]
> >
> > Etc. Etc.
> 
> Here's the thing...I'm probably out of my depth on most of this. But, 
> I'd be happy to post up a RFC patch and then start collecting the 
> feedback on what can be removed. I can at least test it on a real XIP 
> system.

Sure.  You should start by splitting the XIP stuff out to a separate 
script so functionally it is the same (broken) end result.  And from 
there adding incremental fixes to the XIP script.


Nicolas

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Wed, 18 Nov 2015, Chris Brandt wrote:

> > With XIP there is still an init section, [snip]
> > No need to page align it either at that point which should save some
> > ROM space.
> 
> Good point.
> 
> > Things like .kprobes.text should be in ROM, [snip]
> >
> > Etc. Etc.
> 
> Here's the thing...I'm probably out of my depth on most of this. But, 
> I'd be happy to post up a RFC patch and then start collecting the 
> feedback on what can be removed. I can at least test it on a real XIP 
> system.

Sure.  You should start by splitting the XIP stuff out to a separate 
script so functionally it is the same (broken) end result.  And from 
there adding incremental fixes to the XIP script.


Nicolas

RE: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

Hi Nicolas,

> Here's the patch with proper changelog, etc.  I don't have XIP
> capable hardware to test it with though.


I'm testing it now...but it's crashing.

I fired up GDB, so here the reason:


__v7_ca17mp_setup:
	mov	r10, #0
1:	adr	r0, __v7_setup_stack_ptr
r0=0x18213df4

	ldr	r12, [r0]
r12=0x10174cc

	add	r12, r12, r0			@ the local stack
r12=0x1922b2c0

	stmia	r12, {r1-r6, lr}		@ v7_invalidate_l1 touches r0-r6
	bl      v7_invalidate_l1


0x1922b2c0 is NOT RAM....it's nothing.


As point of reference, here's the memory map of my XIP system:
  Physical ROM address: 0x18000000 (I have my XIP kernel starting at 0x18200000)
  Physical RAM address: 0x20000000
  Virtual ROM  address: 0xBF0000000
  Virtual RAM  address: 0xC00000000


Basically, you made the same mistake that Magnus first did: You can't rely on the current PC address to obtain an address in physical RAM because the ROM virt-to-phys relationship is different than the RAM virt-to-phys relationship.

No matter how you look at it, this early in boot, the only 'thing' that really knows where physical RAM is located is PLAT_PHYS_OFFSET.

That's why both Magnus's an my patches (that I both tested) contain PLAT_PHYS_OFFSET.


The only other 'slight' piece of info would be the DTB pointer is still sitting in R2 from boot, and at least for my system, that is in RAM. However, the DTB could be in ROM if the user wanted it to be, and I also think that this code might run twice in a SMP system...so even R2 would not be reliable. So...forget I mentioned it...


I don't see any way around not having a "#ifdef XIP_KERNEL" statement in order to expose PLAT_PHYS_OFFSET so you can properly do your translation math.


Chris


ps, I will add that I appreciate your efforts with this.

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

Hi Nicolas,

> Here's the patch with proper changelog, etc.  I don't have XIP
> capable hardware to test it with though.


I'm testing it now...but it's crashing.

I fired up GDB, so here the reason:


__v7_ca17mp_setup:
	mov	r10, #0
1:	adr	r0, __v7_setup_stack_ptr
r0=0x18213df4

	ldr	r12, [r0]
r12=0x10174cc

	add	r12, r12, r0			@ the local stack
r12=0x1922b2c0

	stmia	r12, {r1-r6, lr}		@ v7_invalidate_l1 touches r0-r6
	bl      v7_invalidate_l1


0x1922b2c0 is NOT RAM....it's nothing.


As point of reference, here's the memory map of my XIP system:
  Physical ROM address: 0x18000000 (I have my XIP kernel starting at 0x18200000)
  Physical RAM address: 0x20000000
  Virtual ROM  address: 0xBF0000000
  Virtual RAM  address: 0xC00000000


Basically, you made the same mistake that Magnus first did: You can't rely on the current PC address to obtain an address in physical RAM because the ROM virt-to-phys relationship is different than the RAM virt-to-phys relationship.

No matter how you look at it, this early in boot, the only 'thing' that really knows where physical RAM is located is PLAT_PHYS_OFFSET.

That's why both Magnus's an my patches (that I both tested) contain PLAT_PHYS_OFFSET.


The only other 'slight' piece of info would be the DTB pointer is still sitting in R2 from boot, and at least for my system, that is in RAM. However, the DTB could be in ROM if the user wanted it to be, and I also think that this code might run twice in a SMP system...so even R2 would not be reliable. So...forget I mentioned it...


I don't see any way around not having a "#ifdef XIP_KERNEL" statement in order to expose PLAT_PHYS_OFFSET so you can properly do your translation math.


Chris


ps, I will add that I appreciate your efforts with this.

RE: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

> Sure.  You should start by splitting the XIP stuff out to a separate
> script so functionally it is the same (broken) end result.  And from
> there adding incremental fixes to the XIP script.

More or less that's what I'm doing now on my system. You can diff the 2 in order to see what I've removed.

But wait...you said 'the last step' was to add a 2nd linker file and change the Makefiles to use it.


Are you saying I should first submit a patch that simply adds the new file, but not hook it up to any Makefiles yet?
Then, once it has been updated a few times and is in better shape, I submit another patch that hooks it up with the Makefiles so I can then finally remove the XIP_KERNEL parts from the current vmlinux.lds.S?

Chris

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

> Sure.  You should start by splitting the XIP stuff out to a separate
> script so functionally it is the same (broken) end result.  And from
> there adding incremental fixes to the XIP script.

More or less that's what I'm doing now on my system. You can diff the 2 in order to see what I've removed.

But wait...you said 'the last step' was to add a 2nd linker file and change the Makefiles to use it.


Are you saying I should first submit a patch that simply adds the new file, but not hook it up to any Makefiles yet?
Then, once it has been updated a few times and is in better shape, I submit another patch that hooks it up with the Makefiles so I can then finally remove the XIP_KERNEL parts from the current vmlinux.lds.S?

Chris

RE: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Wed, 18 Nov 2015, Chris Brandt wrote:

> > Sure.  You should start by splitting the XIP stuff out to a separate
> > script so functionally it is the same (broken) end result.  And from
> > there adding incremental fixes to the XIP script.
> 
> More or less that's what I'm doing now on my system. You can diff the 2 in order to see what I've removed.
> 
> But wait...you said 'the last step' was to add a 2nd linker file and change the Makefiles to use it.
> 
> 
> Are you saying I should first submit a patch that simply adds the new file, but not hook it up to any Makefiles yet?
> Then, once it has been updated a few times and is in better shape, I submit another patch that hooks it up with the Makefiles so I can then finally remove the XIP_KERNEL parts from the current vmlinux.lds.S?

Well... I'm of the opinion now that the first patch should be the 
creation of the new script being a copy of the original with the XIP 
conditionals removed from both, plus the makefile changes. That should 
be easy to verify that nothing changed result wise, and then additional 
patches could bring XIP efinements on top without disturbing the non-XIP 
case.


Nicolas

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Wed, 18 Nov 2015, Chris Brandt wrote:

> > Sure.  You should start by splitting the XIP stuff out to a separate
> > script so functionally it is the same (broken) end result.  And from
> > there adding incremental fixes to the XIP script.
> 
> More or less that's what I'm doing now on my system. You can diff the 2 in order to see what I've removed.
> 
> But wait...you said 'the last step' was to add a 2nd linker file and change the Makefiles to use it.
> 
> 
> Are you saying I should first submit a patch that simply adds the new file, but not hook it up to any Makefiles yet?
> Then, once it has been updated a few times and is in better shape, I submit another patch that hooks it up with the Makefiles so I can then finally remove the XIP_KERNEL parts from the current vmlinux.lds.S?

Well... I'm of the opinion now that the first patch should be the 
creation of the new script being a copy of the original with the XIP 
conditionals removed from both, plus the makefile changes. That should 
be easy to verify that nothing changed result wise, and then additional 
patches could bring XIP efinements on top without disturbing the non-XIP 
case.


Nicolas

RE: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

>> Are you saying I should first submit a patch that simply adds the
>> new file, but not hook it up to any Makefiles yet?
>> Then, once it has been updated a few times and is in better shape,
>> I submit another patch that hooks it up with the Makefiles so I can
>> then finally remove the XIP_KERNEL parts from the current
>> vmlinux.lds.S?
>
> Well... I'm of the opinion now that the first patch should be the
> creation of the new script being a copy of the original with the
> XIP conditionals removed from both, plus the makefile changes. That
> should be easy to verify that nothing changed result wise, and then
> additional patches could bring XIP efinements on top without
> disturbing the non-XIP case.
>
> Nicolas

OK. I'm good with that. I'll submit a patch and see what happens.

Chris

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

>> Are you saying I should first submit a patch that simply adds the
>> new file, but not hook it up to any Makefiles yet?
>> Then, once it has been updated a few times and is in better shape,
>> I submit another patch that hooks it up with the Makefiles so I can
>> then finally remove the XIP_KERNEL parts from the current
>> vmlinux.lds.S?
>
> Well... I'm of the opinion now that the first patch should be the
> creation of the new script being a copy of the original with the
> XIP conditionals removed from both, plus the makefile changes. That
> should be easy to verify that nothing changed result wise, and then
> additional patches could bring XIP efinements on top without
> disturbing the non-XIP case.
>
> Nicolas

OK. I'm good with that. I'll submit a patch and see what happens.

Chris

RE: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Wed, 18 Nov 2015, Chris Brandt wrote:

> Hi Nicolas,
> 
> > Here's the patch with proper changelog, etc.  I don't have XIP
> > capable hardware to test it with though.
> 
> 
> I'm testing it now...but it's crashing.
> 
> I fired up GDB, so here the reason:
> 
> 
> __v7_ca17mp_setup:
> 	mov	r10, #0
> 1:	adr	r0, __v7_setup_stack_ptr
> r0=0x18213df4
> 
> 	ldr	r12, [r0]
> r12=0x10174cc
> 
> 	add	r12, r12, r0			@ the local stack
> r12=0x1922b2c0
> 
> 	stmia	r12, {r1-r6, lr}		@ v7_invalidate_l1 touches r0-r6
> 	bl      v7_invalidate_l1
> 
> 
> 0x1922b2c0 is NOT RAM....it's nothing.
> 
> 
> As point of reference, here's the memory map of my XIP system:
>   Physical ROM address: 0x18000000 (I have my XIP kernel starting at 0x18200000)
>   Physical RAM address: 0x20000000
>   Virtual ROM  address: 0xBF0000000
>   Virtual RAM  address: 0xC00000000
> 
> 
> Basically, you made the same mistake that Magnus first did: You can't 
> rely on the current PC address to obtain an address in physical RAM 
> because the ROM virt-to-phys relationship is different than the RAM 
> virt-to-phys relationship.

Crap... you're right of course.  I suspect a couple other places might 
have problems as they use similar constructs. See kernel/sleep.S for 
example.

Probably the  best way to fix it would be something like:

in asm/memory.h or similar:

#ifdef CONFIG_XIP_KERNEL
#define PHYS_OFFSET_FIXUP \
	( XIP_VIRT_ADDR(CONFIG_XIP_PHYS_ADDR) - PAGE_OFFSET + \
	  PLAT_PHYS_OFFSET - CONFIG_XIP_PHYS_ADDR )
#else
#define PHYS_OFFSET_FIXUP 0
#endif

And then, after my patch is applied, changing:

__v7_setup_stack_ptr:
        .word   __v7_setup_stack - .

into:

__v7_setup_stack_ptr:
        .word   __v7_setup_stack - . + PHYS_OFFSET_FIXUP

should do the trick. This way it'll work for all those places where the 
code is getting at the data area when the MMU is off with no XIP 
conditionals in the code.

I think my patch should be applied as is (minus the mention of XIP) to 
remove the write access to the .text area for the general case which is 
a worthy goal in itself.  We did a bunch of similar cleanups a while 
ago.

Then another patch could bring all those places XIP compatible with the 
simple addition of that PHYS_OFFSET_FIXUP constant.


Nicolas

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Wed, 18 Nov 2015, Chris Brandt wrote:

> Hi Nicolas,
> 
> > Here's the patch with proper changelog, etc.  I don't have XIP
> > capable hardware to test it with though.
> 
> 
> I'm testing it now...but it's crashing.
> 
> I fired up GDB, so here the reason:
> 
> 
> __v7_ca17mp_setup:
> 	mov	r10, #0
> 1:	adr	r0, __v7_setup_stack_ptr
> r0=0x18213df4
> 
> 	ldr	r12, [r0]
> r12=0x10174cc
> 
> 	add	r12, r12, r0			@ the local stack
> r12=0x1922b2c0
> 
> 	stmia	r12, {r1-r6, lr}		@ v7_invalidate_l1 touches r0-r6
> 	bl      v7_invalidate_l1
> 
> 
> 0x1922b2c0 is NOT RAM....it's nothing.
> 
> 
> As point of reference, here's the memory map of my XIP system:
>   Physical ROM address: 0x18000000 (I have my XIP kernel starting at 0x18200000)
>   Physical RAM address: 0x20000000
>   Virtual ROM  address: 0xBF0000000
>   Virtual RAM  address: 0xC00000000
> 
> 
> Basically, you made the same mistake that Magnus first did: You can't 
> rely on the current PC address to obtain an address in physical RAM 
> because the ROM virt-to-phys relationship is different than the RAM 
> virt-to-phys relationship.

Crap... you're right of course.  I suspect a couple other places might 
have problems as they use similar constructs. See kernel/sleep.S for 
example.

Probably the  best way to fix it would be something like:

in asm/memory.h or similar:

#ifdef CONFIG_XIP_KERNEL
#define PHYS_OFFSET_FIXUP \
	( XIP_VIRT_ADDR(CONFIG_XIP_PHYS_ADDR) - PAGE_OFFSET + \
	  PLAT_PHYS_OFFSET - CONFIG_XIP_PHYS_ADDR )
#else
#define PHYS_OFFSET_FIXUP 0
#endif

And then, after my patch is applied, changing:

__v7_setup_stack_ptr:
        .word   __v7_setup_stack - .

into:

__v7_setup_stack_ptr:
        .word   __v7_setup_stack - . + PHYS_OFFSET_FIXUP

should do the trick. This way it'll work for all those places where the 
code is getting at the data area when the MMU is off with no XIP 
conditionals in the code.

I think my patch should be applied as is (minus the mention of XIP) to 
remove the write access to the .text area for the general case which is 
a worthy goal in itself.  We did a bunch of similar cleanups a while 
ago.

Then another patch could bring all those places XIP compatible with the 
simple addition of that PHYS_OFFSET_FIXUP constant.


Nicolas

RE: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

> Probably the best way to fix it would be something like:
>
> in asm/memory.h or similar:
>
> #ifdef CONFIG_XIP_KERNEL
> #define PHYS_OFFSET_FIXUP \
>	( XIP_VIRT_ADDR(CONFIG_XIP_PHYS_ADDR) - PAGE_OFFSET + \
>	  PLAT_PHYS_OFFSET - CONFIG_XIP_PHYS_ADDR ) #else #define
> PHYS_OFFSET_FIXUP 0 #endif
>
> And then, after my patch is applied, changing:
>
> __v7_setup_stack_ptr:
>        .word   __v7_setup_stack - .
>
> into:
>
> __v7_setup_stack_ptr:
>        .word   __v7_setup_stack - . + PHYS_OFFSET_FIXUP
>
> should do the trick. This way it'll work for all those places where
> the code is getting at the data area when the MMU is off with no XIP
> conditionals in the code.


Tested....it works!



Here's your new results:

__v7_ca17mp_setup:
	mov	r10, #0
1:	adr	r0, __v7_setup_stack_ptr
r0=0x18213df4

	ldr	r12, [r0]
r12=0x7e174cc

	add	r12, r12, r0			@ the local stack
r12=0x2002b2c0

	stmia	r12, {r1-r6, lr}		@ v7_invalidate_l1 touches r0-r6
	bl      v7_invalidate_l1


0x2002b2c0 is indeed the correct physical address of __v7_setup_stack_ptr.

So, you still need to use #ifdef XIP_KERNEL and PLAT_PHYS_OFFSET....but...you bury it in another file. I'm good with that!!


> Then another patch could bring all those places XIP compatible with
> the simple addition of that PHYS_OFFSET_FIXUP constant.

Yes, the less CONFIG_XIP_KERNEL is sprinkled around the arm tree code, the better.

Chris

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

> Probably the best way to fix it would be something like:
>
> in asm/memory.h or similar:
>
> #ifdef CONFIG_XIP_KERNEL
> #define PHYS_OFFSET_FIXUP \
>	( XIP_VIRT_ADDR(CONFIG_XIP_PHYS_ADDR) - PAGE_OFFSET + \
>	  PLAT_PHYS_OFFSET - CONFIG_XIP_PHYS_ADDR ) #else #define
> PHYS_OFFSET_FIXUP 0 #endif
>
> And then, after my patch is applied, changing:
>
> __v7_setup_stack_ptr:
>        .word   __v7_setup_stack - .
>
> into:
>
> __v7_setup_stack_ptr:
>        .word   __v7_setup_stack - . + PHYS_OFFSET_FIXUP
>
> should do the trick. This way it'll work for all those places where
> the code is getting at the data area when the MMU is off with no XIP
> conditionals in the code.


Tested....it works!



Here's your new results:

__v7_ca17mp_setup:
	mov	r10, #0
1:	adr	r0, __v7_setup_stack_ptr
r0=0x18213df4

	ldr	r12, [r0]
r12=0x7e174cc

	add	r12, r12, r0			@ the local stack
r12=0x2002b2c0

	stmia	r12, {r1-r6, lr}		@ v7_invalidate_l1 touches r0-r6
	bl      v7_invalidate_l1


0x2002b2c0 is indeed the correct physical address of __v7_setup_stack_ptr.

So, you still need to use #ifdef XIP_KERNEL and PLAT_PHYS_OFFSET....but...you bury it in another file. I'm good with that!!


> Then another patch could bring all those places XIP compatible with
> the simple addition of that PHYS_OFFSET_FIXUP constant.

Yes, the less CONFIG_XIP_KERNEL is sprinkled around the arm tree code, the better.

Chris

RE: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Wed, 18 Nov 2015, Chris Brandt wrote:

> > Probably the best way to fix it would be something like:
> >
> > in asm/memory.h or similar:
> >
> > #ifdef CONFIG_XIP_KERNEL
> > #define PHYS_OFFSET_FIXUP \
> >	( XIP_VIRT_ADDR(CONFIG_XIP_PHYS_ADDR) - PAGE_OFFSET + \
> >	  PLAT_PHYS_OFFSET - CONFIG_XIP_PHYS_ADDR ) #else #define
> > PHYS_OFFSET_FIXUP 0 #endif
> >
> > And then, after my patch is applied, changing:
> >
> > __v7_setup_stack_ptr:
> >        .word   __v7_setup_stack - .
> >
> > into:
> >
> > __v7_setup_stack_ptr:
> >        .word   __v7_setup_stack - . + PHYS_OFFSET_FIXUP
> >
> > should do the trick. This way it'll work for all those places where
> > the code is getting at the data area when the MMU is off with no XIP
> > conditionals in the code.
> 
> 
> Tested....it works!

Excellent.


Nicolas

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Wed, 18 Nov 2015, Chris Brandt wrote:

> > Probably the best way to fix it would be something like:
> >
> > in asm/memory.h or similar:
> >
> > #ifdef CONFIG_XIP_KERNEL
> > #define PHYS_OFFSET_FIXUP \
> >	( XIP_VIRT_ADDR(CONFIG_XIP_PHYS_ADDR) - PAGE_OFFSET + \
> >	  PLAT_PHYS_OFFSET - CONFIG_XIP_PHYS_ADDR ) #else #define
> > PHYS_OFFSET_FIXUP 0 #endif
> >
> > And then, after my patch is applied, changing:
> >
> > __v7_setup_stack_ptr:
> >        .word   __v7_setup_stack - .
> >
> > into:
> >
> > __v7_setup_stack_ptr:
> >        .word   __v7_setup_stack - . + PHYS_OFFSET_FIXUP
> >
> > should do the trick. This way it'll work for all those places where
> > the code is getting at the data area when the MMU is off with no XIP
> > conditionals in the code.
> 
> 
> Tested....it works!

Excellent.


Nicolas

RE: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

On Wed, 18 Nov 2015, Nicolas Pitre wrote:

> On Wed, 18 Nov 2015, Chris Brandt wrote:
>
> > > Probably the best way to fix it would be something like:
> > >
> > > in asm/memory.h or similar:
> > >
> > > #ifdef CONFIG_XIP_KERNEL
> > > #define PHYS_OFFSET_FIXUP \
> > >	( XIP_VIRT_ADDR(CONFIG_XIP_PHYS_ADDR) - PAGE_OFFSET + \
> > >	  PLAT_PHYS_OFFSET - CONFIG_XIP_PHYS_ADDR ) #else #define  
> > >PHYS_OFFSET_FIXUP 0 #endif
> > >
> > > And then, after my patch is applied, changing:
> > >
> > > __v7_setup_stack_ptr:
> > >        .word   __v7_setup_stack - .
> > >
> > > into:
> > >
> > > __v7_setup_stack_ptr:
> > >        .word   __v7_setup_stack - . + PHYS_OFFSET_FIXUP
> > >
> > > should do the trick. This way it'll work for all those places where 
> > > the code is getting at the data area when the MMU is off with no XIP 
> > > conditionals in the code.
> > 
> > 
> > Tested....it works!
>
> Excellent.
>
>
> Nicolas


Nicolas,

I applied your PHYS_OFFSET_FIXUP macro code (shown above) to the current tree which already has your commit "ARM: 8453/2: proc-v7.S: don't locate temporary stack space in .text section".

After testing, it shows the same results for an XIP_KERNEL build as it did in November:

   No PHYS_OFFSET_FIXUP = crash

   with PHYS_OFFSET_FIXUP = good


Can I submit a patch with your code for review and inclusion to the kernel?


Chris

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

On Wed, 18 Nov 2015, Nicolas Pitre wrote:

> On Wed, 18 Nov 2015, Chris Brandt wrote:
>
> > > Probably the best way to fix it would be something like:
> > >
> > > in asm/memory.h or similar:
> > >
> > > #ifdef CONFIG_XIP_KERNEL
> > > #define PHYS_OFFSET_FIXUP \
> > >	( XIP_VIRT_ADDR(CONFIG_XIP_PHYS_ADDR) - PAGE_OFFSET + \
> > >	  PLAT_PHYS_OFFSET - CONFIG_XIP_PHYS_ADDR ) #else #define  
> > >PHYS_OFFSET_FIXUP 0 #endif
> > >
> > > And then, after my patch is applied, changing:
> > >
> > > __v7_setup_stack_ptr:
> > >        .word   __v7_setup_stack - .
> > >
> > > into:
> > >
> > > __v7_setup_stack_ptr:
> > >        .word   __v7_setup_stack - . + PHYS_OFFSET_FIXUP
> > >
> > > should do the trick. This way it'll work for all those places where 
> > > the code is getting at the data area when the MMU is off with no XIP 
> > > conditionals in the code.
> > 
> > 
> > Tested....it works!
>
> Excellent.
>
>
> Nicolas


Nicolas,

I applied your PHYS_OFFSET_FIXUP macro code (shown above) to the current tree which already has your commit "ARM: 8453/2: proc-v7.S: don't locate temporary stack space in .text section".

After testing, it shows the same results for an XIP_KERNEL build as it did in November:

   No PHYS_OFFSET_FIXUP = crash

   with PHYS_OFFSET_FIXUP = good


Can I submit a patch with your code for review and inclusion to the kernel?


Chris

RE: [PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Fri, 29 Jan 2016, Chris Brandt wrote:

> On Wed, 18 Nov 2015, Nicolas Pitre wrote:
> 
> > On Wed, 18 Nov 2015, Chris Brandt wrote:
> >
> > > > Probably the best way to fix it would be something like:
> > > >
> > > > in asm/memory.h or similar:
> > > >
> > > > #ifdef CONFIG_XIP_KERNEL
> > > > #define PHYS_OFFSET_FIXUP \
> > > >	( XIP_VIRT_ADDR(CONFIG_XIP_PHYS_ADDR) - PAGE_OFFSET + \
> > > >	  PLAT_PHYS_OFFSET - CONFIG_XIP_PHYS_ADDR ) #else #define  
> > > >PHYS_OFFSET_FIXUP 0 #endif
> > > >
> > > > And then, after my patch is applied, changing:
> > > >
> > > > __v7_setup_stack_ptr:
> > > >        .word   __v7_setup_stack - .
> > > >
> > > > into:
> > > >
> > > > __v7_setup_stack_ptr:
> > > >        .word   __v7_setup_stack - . + PHYS_OFFSET_FIXUP
> > > >
> > > > should do the trick. This way it'll work for all those places where 
> > > > the code is getting at the data area when the MMU is off with no XIP 
> > > > conditionals in the code.
> > > 
> > > 
> > > Tested....it works!
> >
> > Excellent.
> >
> >
> > Nicolas
> 
> 
> Nicolas,
> 
> I applied your PHYS_OFFSET_FIXUP macro code (shown above) to the current tree which already has your commit "ARM: 8453/2: proc-v7.S: don't locate temporary stack space in .text section".
> 
> After testing, it shows the same results for an XIP_KERNEL build as it did in November:
> 
>    No PHYS_OFFSET_FIXUP = crash
> 
>    with PHYS_OFFSET_FIXUP = good
> 
> 
> Can I submit a patch with your code for review and inclusion to the kernel?

Sure.

Signed-off-by: Nicolas Pitre <nico@linaro.org>


Nicolas

[PATCH v3] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Fri, 29 Jan 2016, Chris Brandt wrote:

> On Wed, 18 Nov 2015, Nicolas Pitre wrote:
> 
> > On Wed, 18 Nov 2015, Chris Brandt wrote:
> >
> > > > Probably the best way to fix it would be something like:
> > > >
> > > > in asm/memory.h or similar:
> > > >
> > > > #ifdef CONFIG_XIP_KERNEL
> > > > #define PHYS_OFFSET_FIXUP \
> > > >	( XIP_VIRT_ADDR(CONFIG_XIP_PHYS_ADDR) - PAGE_OFFSET + \
> > > >	  PLAT_PHYS_OFFSET - CONFIG_XIP_PHYS_ADDR ) #else #define  
> > > >PHYS_OFFSET_FIXUP 0 #endif
> > > >
> > > > And then, after my patch is applied, changing:
> > > >
> > > > __v7_setup_stack_ptr:
> > > >        .word   __v7_setup_stack - .
> > > >
> > > > into:
> > > >
> > > > __v7_setup_stack_ptr:
> > > >        .word   __v7_setup_stack - . + PHYS_OFFSET_FIXUP
> > > >
> > > > should do the trick. This way it'll work for all those places where 
> > > > the code is getting at the data area when the MMU is off with no XIP 
> > > > conditionals in the code.
> > > 
> > > 
> > > Tested....it works!
> >
> > Excellent.
> >
> >
> > Nicolas
> 
> 
> Nicolas,
> 
> I applied your PHYS_OFFSET_FIXUP macro code (shown above) to the current tree which already has your commit "ARM: 8453/2: proc-v7.S: don't locate temporary stack space in .text section".
> 
> After testing, it shows the same results for an XIP_KERNEL build as it did in November:
> 
>    No PHYS_OFFSET_FIXUP = crash
> 
>    with PHYS_OFFSET_FIXUP = good
> 
> 
> Can I submit a patch with your code for review and inclusion to the kernel?

Sure.

Signed-off-by: Nicolas Pitre <nico@linaro.org>


Nicolas

[PATCH v4] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

For an XIP build, __data_loc, not _etext, represents the end of the
binary image that needs to stay mapped into the MODULES_VADDR area.
Years ago, data came before text in the memory map. However,
now that the order is text/init/data, an XIP_KERNEL needs to map
up to the __data_loc symbol in order to keep from cutting off
parts of the kernel that are needed.
We only map up to the beginning of data because data has already been
copied, so there's no reason to keep it around anymore.

This fixes the bug where you might lose the end of your kernel area
after page table setup is complete.

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
v4:
* Switched from using _edata_loc __data_loc.
  Made commit text more accurate.
v3:
* Removed sections.h from Kbuild
v2:
* Added change for MODULES_VADDR
* Moved extern to new file asm/sections.h
---
 arch/arm/include/asm/Kbuild     | 1 -
 arch/arm/include/asm/sections.h | 8 ++++++++
 arch/arm/kernel/module.c        | 2 +-
 arch/arm/mm/mmu.c               | 8 ++++++--
 4 files changed, 15 insertions(+), 4 deletions(-)
 create mode 100644 arch/arm/include/asm/sections.h

diff --git a/arch/arm/include/asm/Kbuild b/arch/arm/include/asm/Kbuild
index 16da638..3f6616b 100644
--- a/arch/arm/include/asm/Kbuild
+++ b/arch/arm/include/asm/Kbuild
@@ -23,7 +23,6 @@ generic-y += preempt.h
 generic-y += resource.h
 generic-y += rwsem.h
 generic-y += seccomp.h
-generic-y += sections.h
 generic-y += segment.h
 generic-y += sembuf.h
 generic-y += serial.h
diff --git a/arch/arm/include/asm/sections.h b/arch/arm/include/asm/sections.h
new file mode 100644
index 0000000..8d50cce
--- /dev/null
+++ b/arch/arm/include/asm/sections.h
@@ -0,0 +1,8 @@
+#ifndef _ASM_ARM_SECTIONS_H
+#define _ASM_ARM_SECTIONS_H
+
+#include <asm-generic/sections.h>
+
+extern char __data_loc[];
+
+#endif	/* _ASM_ARM_SECTIONS_H */
diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c
index efdddcb..b48e3bc 100644
--- a/arch/arm/kernel/module.c
+++ b/arch/arm/kernel/module.c
@@ -34,7 +34,7 @@
  * recompiling the whole kernel when CONFIG_XIP_KERNEL is turned on/off.
  */
 #undef MODULES_VADDR
-#define MODULES_VADDR	(((unsigned long)_etext + ~PMD_MASK) & PMD_MASK)
+#define MODULES_VADDR	(((unsigned long)__data_loc + ~PMD_MASK) & PMD_MASK)
 #endif
 
 #ifdef CONFIG_MMU
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index 434d76f..53d7a88 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1253,7 +1253,7 @@ static inline void prepare_page_table(void)
 
 #ifdef CONFIG_XIP_KERNEL
 	/* The XIP kernel is mapped in the module area -- skip over it */
-	addr = ((unsigned long)_etext + PMD_SIZE - 1) & PMD_MASK;
+	addr = ((unsigned long)__data_loc + PMD_SIZE - 1) & PMD_MASK;
 #endif
 	for ( ; addr < PAGE_OFFSET; addr += PMD_SIZE)
 		pmd_clear(pmd_off_k(addr));
@@ -1335,7 +1335,7 @@ static void __init devicemaps_init(const struct machine_desc *mdesc)
 #ifdef CONFIG_XIP_KERNEL
 	map.pfn = __phys_to_pfn(CONFIG_XIP_PHYS_ADDR & SECTION_MASK);
 	map.virtual = MODULES_VADDR;
-	map.length = ((unsigned long)_etext - map.virtual + ~SECTION_MASK) & SECTION_MASK;
+	map.length = ((unsigned long)__data_loc - map.virtual + ~SECTION_MASK) & SECTION_MASK;
 	map.type = MT_ROM;
 	create_mapping(&map);
 #endif
@@ -1426,7 +1426,11 @@ static void __init kmap_init(void)
 static void __init map_lowmem(void)
 {
 	struct memblock_region *reg;
+#ifdef CONFIG_XIP_KERNEL
+	phys_addr_t kernel_x_start = round_down(__pa(_sdata), SECTION_SIZE);
+#else
 	phys_addr_t kernel_x_start = round_down(__pa(_stext), SECTION_SIZE);
+#endif
 	phys_addr_t kernel_x_end = round_up(__pa(__init_end), SECTION_SIZE);
 
 	/* Map all the lowmem memory banks. */
-- 
1.9.1

[PATCH v4] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Mon, 1 Feb 2016, Chris Brandt wrote:

> For an XIP build, __data_loc, not _etext, represents the end of the
> binary image that needs to stay mapped into the MODULES_VADDR area.
> Years ago, data came before text in the memory map. However,
> now that the order is text/init/data, an XIP_KERNEL needs to map
> up to the __data_loc symbol in order to keep from cutting off
> parts of the kernel that are needed.
> We only map up to the beginning of data because data has already been
> copied, so there's no reason to keep it around anymore.
> 
> This fixes the bug where you might lose the end of your kernel area
> after page table setup is complete.
> 
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>

The __data_loc symbol doesn't convey much to say with regards to XIP 
mapping.  It would be better to dedicate another symbol with a proper 
name instead.  Something like xiprom_start and xiprom_end perhaps.  
This way, someone looking at the linker script won't be fooled by a 
hidden meaning tied to _etext or __data_loc.


> ---
> v4:
> * Switched from using _edata_loc __data_loc.
>   Made commit text more accurate.
> v3:
> * Removed sections.h from Kbuild
> v2:
> * Added change for MODULES_VADDR
> * Moved extern to new file asm/sections.h
> ---
>  arch/arm/include/asm/Kbuild     | 1 -
>  arch/arm/include/asm/sections.h | 8 ++++++++
>  arch/arm/kernel/module.c        | 2 +-
>  arch/arm/mm/mmu.c               | 8 ++++++--
>  4 files changed, 15 insertions(+), 4 deletions(-)
>  create mode 100644 arch/arm/include/asm/sections.h
> 
> diff --git a/arch/arm/include/asm/Kbuild b/arch/arm/include/asm/Kbuild
> index 16da638..3f6616b 100644
> --- a/arch/arm/include/asm/Kbuild
> +++ b/arch/arm/include/asm/Kbuild
> @@ -23,7 +23,6 @@ generic-y += preempt.h
>  generic-y += resource.h
>  generic-y += rwsem.h
>  generic-y += seccomp.h
> -generic-y += sections.h
>  generic-y += segment.h
>  generic-y += sembuf.h
>  generic-y += serial.h
> diff --git a/arch/arm/include/asm/sections.h b/arch/arm/include/asm/sections.h
> new file mode 100644
> index 0000000..8d50cce
> --- /dev/null
> +++ b/arch/arm/include/asm/sections.h
> @@ -0,0 +1,8 @@
> +#ifndef _ASM_ARM_SECTIONS_H
> +#define _ASM_ARM_SECTIONS_H
> +
> +#include <asm-generic/sections.h>
> +
> +extern char __data_loc[];
> +
> +#endif	/* _ASM_ARM_SECTIONS_H */
> diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c
> index efdddcb..b48e3bc 100644
> --- a/arch/arm/kernel/module.c
> +++ b/arch/arm/kernel/module.c
> @@ -34,7 +34,7 @@
>   * recompiling the whole kernel when CONFIG_XIP_KERNEL is turned on/off.
>   */
>  #undef MODULES_VADDR
> -#define MODULES_VADDR	(((unsigned long)_etext + ~PMD_MASK) & PMD_MASK)
> +#define MODULES_VADDR	(((unsigned long)__data_loc + ~PMD_MASK) & PMD_MASK)
>  #endif
>  
>  #ifdef CONFIG_MMU
> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index 434d76f..53d7a88 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -1253,7 +1253,7 @@ static inline void prepare_page_table(void)
>  
>  #ifdef CONFIG_XIP_KERNEL
>  	/* The XIP kernel is mapped in the module area -- skip over it */
> -	addr = ((unsigned long)_etext + PMD_SIZE - 1) & PMD_MASK;
> +	addr = ((unsigned long)__data_loc + PMD_SIZE - 1) & PMD_MASK;
>  #endif
>  	for ( ; addr < PAGE_OFFSET; addr += PMD_SIZE)
>  		pmd_clear(pmd_off_k(addr));
> @@ -1335,7 +1335,7 @@ static void __init devicemaps_init(const struct machine_desc *mdesc)
>  #ifdef CONFIG_XIP_KERNEL
>  	map.pfn = __phys_to_pfn(CONFIG_XIP_PHYS_ADDR & SECTION_MASK);
>  	map.virtual = MODULES_VADDR;
> -	map.length = ((unsigned long)_etext - map.virtual + ~SECTION_MASK) & SECTION_MASK;
> +	map.length = ((unsigned long)__data_loc - map.virtual + ~SECTION_MASK) & SECTION_MASK;
>  	map.type = MT_ROM;
>  	create_mapping(&map);
>  #endif
> @@ -1426,7 +1426,11 @@ static void __init kmap_init(void)
>  static void __init map_lowmem(void)
>  {
>  	struct memblock_region *reg;
> +#ifdef CONFIG_XIP_KERNEL
> +	phys_addr_t kernel_x_start = round_down(__pa(_sdata), SECTION_SIZE);
> +#else
>  	phys_addr_t kernel_x_start = round_down(__pa(_stext), SECTION_SIZE);
> +#endif
>  	phys_addr_t kernel_x_end = round_up(__pa(__init_end), SECTION_SIZE);
>  
>  	/* Map all the lowmem memory banks. */
> -- 
> 1.9.1
> 
> 
> 

[PATCH v4] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

On Mon, 1 Feb 2016, Nicolas Pitre wrote:

> The __data_loc symbol doesn't convey much to say with regards to XIP
> mapping.  It would be better to dedicate another symbol with a proper
> name instead.  Something like xiprom_start and xiprom_end perhaps.  
> This way, someone looking at the linker script won't be fooled by a
> hidden meaning tied to _etext or __data_loc.

That's fine by me.


However, I just saw the email "[PATCH] ARM: vmlinux.lds: assert that ROM and RAM don't overlap when XIP_KERNEL=y" sent in a little while ago.

So, do we first want to reconsider splitting out the XIP stuff into a separate linker script? Hence my proposal form November "[PATCH v2] ARM: xip: Move XIP linking to a separate file".

Or, should we keep piling more stuff in vmlinux.lds.S?


Chris

[PATCH v4] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Mon, 1 Feb 2016, Chris Brandt wrote:

> On Mon, 1 Feb 2016, Nicolas Pitre wrote:
> 
> > The __data_loc symbol doesn't convey much to say with regards to XIP
> > mapping.  It would be better to dedicate another symbol with a proper
> > name instead.  Something like xiprom_start and xiprom_end perhaps.  
> > This way, someone looking at the linker script won't be fooled by a
> > hidden meaning tied to _etext or __data_loc.
> 
> That's fine by me.
> 
> 
> However, I just saw the email "[PATCH] ARM: vmlinux.lds: assert that 
> ROM and RAM don't overlap when XIP_KERNEL=y" sent in a little while 
> ago.
> 
> So, do we first want to reconsider splitting out the XIP stuff into a 
> separate linker script? Hence my proposal form November "[PATCH v2] 
> ARM: xip: Move XIP linking to a separate file".
> 
> Or, should we keep piling more stuff in vmlinux.lds.S?

I still think the split linker file is the way to go.


Nicolas

[PATCH v4] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

On Mon, 1 Feb 2016, Nicolas Pitre wrote:
> The __data_loc symbol doesn't convey much to say with regards to XIP 
> mapping.  It would be better to dedicate another symbol with a 
> proper name instead.  Something like xiprom_start and xiprom_end perhaps.
> This way, someone looking at the linker script won't be fooled by a 
> hidden meaning tied to _etext or __data_loc.

In keeping with the convention that's already in the vmlinux.lds, I'm going to go with "_xiprom" and "_exiprom"


> > Or, should we keep piling more stuff in vmlinux.lds.S?
>
> I still think the split linker file is the way to go.

So...since I didn't hear any clapping and cheering, first I'd like to fix what I know is broken and get that in. Then go back to the splitting linker script idea.


Chris

[PATCH v5] ARM: xip: Use correct symbol for end of ROM marker

[Chris Brandt]

For an XIP build, _etext does not represent the end of the
binary image that needs to stay mapped into the MODULES_VADDR area.
Years ago, data came before text in the memory map. However,
now that the order is text/init/data, an XIP_KERNEL needs to map
up to the data location in order to keep from cutting off
parts of the kernel that are needed.
We only map up to the beginning of data because data has already been
copied, so there's no reason to keep it around anymore.

This fixes the bug where you might lose the end of your kernel area
after page table setup is complete.

Signed-off-by: Chris Brandt <chris.brandt@renesas.com>
---
v5:
* Switched from using __data_loc to _exiprom (new symbol)
v4:
* Switched from using _edata_loc __data_loc.
* Made commit text more accurate.
v3:
* Removed sections.h from Kbuild
v2:
* Added change for MODULES_VADDR
* Moved extern to new file asm/sections.h
---
 arch/arm/include/asm/Kbuild     | 1 -
 arch/arm/include/asm/sections.h | 8 ++++++++
 arch/arm/kernel/module.c        | 2 +-
 arch/arm/kernel/vmlinux.lds.S   | 2 ++
 arch/arm/mm/mmu.c               | 8 ++++++--
 5 files changed, 17 insertions(+), 4 deletions(-)
 create mode 100644 arch/arm/include/asm/sections.h

diff --git a/arch/arm/include/asm/Kbuild b/arch/arm/include/asm/Kbuild
index 16da638..3f6616b 100644
--- a/arch/arm/include/asm/Kbuild
+++ b/arch/arm/include/asm/Kbuild
@@ -23,7 +23,6 @@ generic-y += preempt.h
 generic-y += resource.h
 generic-y += rwsem.h
 generic-y += seccomp.h
-generic-y += sections.h
 generic-y += segment.h
 generic-y += sembuf.h
 generic-y += serial.h
diff --git a/arch/arm/include/asm/sections.h b/arch/arm/include/asm/sections.h
new file mode 100644
index 0000000..803bbf2
--- /dev/null
+++ b/arch/arm/include/asm/sections.h
@@ -0,0 +1,8 @@
+#ifndef _ASM_ARM_SECTIONS_H
+#define _ASM_ARM_SECTIONS_H
+
+#include <asm-generic/sections.h>
+
+extern char _exiprom[];
+
+#endif	/* _ASM_ARM_SECTIONS_H */
diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c
index efdddcb..4f14b5c 100644
--- a/arch/arm/kernel/module.c
+++ b/arch/arm/kernel/module.c
@@ -34,7 +34,7 @@
  * recompiling the whole kernel when CONFIG_XIP_KERNEL is turned on/off.
  */
 #undef MODULES_VADDR
-#define MODULES_VADDR	(((unsigned long)_etext + ~PMD_MASK) & PMD_MASK)
+#define MODULES_VADDR	(((unsigned long)_exiprom + ~PMD_MASK) & PMD_MASK)
 #endif
 
 #ifdef CONFIG_MMU
diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index 8b60fde..d18e4e5 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -86,6 +86,7 @@ SECTIONS
 
 #ifdef CONFIG_XIP_KERNEL
 	. = XIP_VIRT_ADDR(CONFIG_XIP_PHYS_ADDR);
+	_xiprom = .;			/* XIP ROM area to be mapped */
 #else
 	. = PAGE_OFFSET + TEXT_OFFSET;
 #endif
@@ -228,6 +229,7 @@ SECTIONS
 #endif
 
 #ifdef CONFIG_XIP_KERNEL
+	_exiprom = .;			/* End of XIP ROM area */
 	__data_loc = ALIGN(4);		/* location in binary */
 	. = PAGE_OFFSET + TEXT_OFFSET;
 #else
diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
index 434d76f..e4b681a 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1253,7 +1253,7 @@ static inline void prepare_page_table(void)
 
 #ifdef CONFIG_XIP_KERNEL
 	/* The XIP kernel is mapped in the module area -- skip over it */
-	addr = ((unsigned long)_etext + PMD_SIZE - 1) & PMD_MASK;
+	addr = ((unsigned long)_exiprom + PMD_SIZE - 1) & PMD_MASK;
 #endif
 	for ( ; addr < PAGE_OFFSET; addr += PMD_SIZE)
 		pmd_clear(pmd_off_k(addr));
@@ -1335,7 +1335,7 @@ static void __init devicemaps_init(const struct machine_desc *mdesc)
 #ifdef CONFIG_XIP_KERNEL
 	map.pfn = __phys_to_pfn(CONFIG_XIP_PHYS_ADDR & SECTION_MASK);
 	map.virtual = MODULES_VADDR;
-	map.length = ((unsigned long)_etext - map.virtual + ~SECTION_MASK) & SECTION_MASK;
+	map.length = ((unsigned long)_exiprom - map.virtual + ~SECTION_MASK) & SECTION_MASK;
 	map.type = MT_ROM;
 	create_mapping(&map);
 #endif
@@ -1426,7 +1426,11 @@ static void __init kmap_init(void)
 static void __init map_lowmem(void)
 {
 	struct memblock_region *reg;
+#ifdef CONFIG_XIP_KERNEL
+	phys_addr_t kernel_x_start = round_down(__pa(_sdata), SECTION_SIZE);
+#else
 	phys_addr_t kernel_x_start = round_down(__pa(_stext), SECTION_SIZE);
+#endif
 	phys_addr_t kernel_x_end = round_up(__pa(__init_end), SECTION_SIZE);
 
 	/* Map all the lowmem memory banks. */
-- 
1.9.1

[PATCH v5] ARM: xip: Use correct symbol for end of ROM marker

[Nicolas Pitre]

On Tue, 2 Feb 2016, Chris Brandt wrote:

> For an XIP build, _etext does not represent the end of the
> binary image that needs to stay mapped into the MODULES_VADDR area.
> Years ago, data came before text in the memory map. However,
> now that the order is text/init/data, an XIP_KERNEL needs to map
> up to the data location in order to keep from cutting off
> parts of the kernel that are needed.
> We only map up to the beginning of data because data has already been
> copied, so there's no reason to keep it around anymore.
> 
> This fixes the bug where you might lose the end of your kernel area
> after page table setup is complete.
> 
> Signed-off-by: Chris Brandt <chris.brandt@renesas.com>

Acked-by: Nicolas Pitre <nico@linaro.org>


> ---
> v5:
> * Switched from using __data_loc to _exiprom (new symbol)
> v4:
> * Switched from using _edata_loc __data_loc.
> * Made commit text more accurate.
> v3:
> * Removed sections.h from Kbuild
> v2:
> * Added change for MODULES_VADDR
> * Moved extern to new file asm/sections.h
> ---
>  arch/arm/include/asm/Kbuild     | 1 -
>  arch/arm/include/asm/sections.h | 8 ++++++++
>  arch/arm/kernel/module.c        | 2 +-
>  arch/arm/kernel/vmlinux.lds.S   | 2 ++
>  arch/arm/mm/mmu.c               | 8 ++++++--
>  5 files changed, 17 insertions(+), 4 deletions(-)
>  create mode 100644 arch/arm/include/asm/sections.h
> 
> diff --git a/arch/arm/include/asm/Kbuild b/arch/arm/include/asm/Kbuild
> index 16da638..3f6616b 100644
> --- a/arch/arm/include/asm/Kbuild
> +++ b/arch/arm/include/asm/Kbuild
> @@ -23,7 +23,6 @@ generic-y += preempt.h
>  generic-y += resource.h
>  generic-y += rwsem.h
>  generic-y += seccomp.h
> -generic-y += sections.h
>  generic-y += segment.h
>  generic-y += sembuf.h
>  generic-y += serial.h
> diff --git a/arch/arm/include/asm/sections.h b/arch/arm/include/asm/sections.h
> new file mode 100644
> index 0000000..803bbf2
> --- /dev/null
> +++ b/arch/arm/include/asm/sections.h
> @@ -0,0 +1,8 @@
> +#ifndef _ASM_ARM_SECTIONS_H
> +#define _ASM_ARM_SECTIONS_H
> +
> +#include <asm-generic/sections.h>
> +
> +extern char _exiprom[];
> +
> +#endif	/* _ASM_ARM_SECTIONS_H */
> diff --git a/arch/arm/kernel/module.c b/arch/arm/kernel/module.c
> index efdddcb..4f14b5c 100644
> --- a/arch/arm/kernel/module.c
> +++ b/arch/arm/kernel/module.c
> @@ -34,7 +34,7 @@
>   * recompiling the whole kernel when CONFIG_XIP_KERNEL is turned on/off.
>   */
>  #undef MODULES_VADDR
> -#define MODULES_VADDR	(((unsigned long)_etext + ~PMD_MASK) & PMD_MASK)
> +#define MODULES_VADDR	(((unsigned long)_exiprom + ~PMD_MASK) & PMD_MASK)
>  #endif
>  
>  #ifdef CONFIG_MMU
> diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
> index 8b60fde..d18e4e5 100644
> --- a/arch/arm/kernel/vmlinux.lds.S
> +++ b/arch/arm/kernel/vmlinux.lds.S
> @@ -86,6 +86,7 @@ SECTIONS
>  
>  #ifdef CONFIG_XIP_KERNEL
>  	. = XIP_VIRT_ADDR(CONFIG_XIP_PHYS_ADDR);
> +	_xiprom = .;			/* XIP ROM area to be mapped */
>  #else
>  	. = PAGE_OFFSET + TEXT_OFFSET;
>  #endif
> @@ -228,6 +229,7 @@ SECTIONS
>  #endif
>  
>  #ifdef CONFIG_XIP_KERNEL
> +	_exiprom = .;			/* End of XIP ROM area */
>  	__data_loc = ALIGN(4);		/* location in binary */
>  	. = PAGE_OFFSET + TEXT_OFFSET;
>  #else
> diff --git a/arch/arm/mm/mmu.c b/arch/arm/mm/mmu.c
> index 434d76f..e4b681a 100644
> --- a/arch/arm/mm/mmu.c
> +++ b/arch/arm/mm/mmu.c
> @@ -1253,7 +1253,7 @@ static inline void prepare_page_table(void)
>  
>  #ifdef CONFIG_XIP_KERNEL
>  	/* The XIP kernel is mapped in the module area -- skip over it */
> -	addr = ((unsigned long)_etext + PMD_SIZE - 1) & PMD_MASK;
> +	addr = ((unsigned long)_exiprom + PMD_SIZE - 1) & PMD_MASK;
>  #endif
>  	for ( ; addr < PAGE_OFFSET; addr += PMD_SIZE)
>  		pmd_clear(pmd_off_k(addr));
> @@ -1335,7 +1335,7 @@ static void __init devicemaps_init(const struct machine_desc *mdesc)
>  #ifdef CONFIG_XIP_KERNEL
>  	map.pfn = __phys_to_pfn(CONFIG_XIP_PHYS_ADDR & SECTION_MASK);
>  	map.virtual = MODULES_VADDR;
> -	map.length = ((unsigned long)_etext - map.virtual + ~SECTION_MASK) & SECTION_MASK;
> +	map.length = ((unsigned long)_exiprom - map.virtual + ~SECTION_MASK) & SECTION_MASK;
>  	map.type = MT_ROM;
>  	create_mapping(&map);
>  #endif
> @@ -1426,7 +1426,11 @@ static void __init kmap_init(void)
>  static void __init map_lowmem(void)
>  {
>  	struct memblock_region *reg;
> +#ifdef CONFIG_XIP_KERNEL
> +	phys_addr_t kernel_x_start = round_down(__pa(_sdata), SECTION_SIZE);
> +#else
>  	phys_addr_t kernel_x_start = round_down(__pa(_stext), SECTION_SIZE);
> +#endif
>  	phys_addr_t kernel_x_end = round_up(__pa(__init_end), SECTION_SIZE);
>  
>  	/* Map all the lowmem memory banks. */
> -- 
> 1.9.1
> 
> 
>