diff mbox

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

Message ID 1437057434-1616-3-git-send-email-chris.brandt@renesas.com (mailing list archive)
State Not Applicable
Headers show

Commit Message

Chris Brandt July 16, 2015, 2:37 p.m. UTC
For an XIP build, _edata_loc, not _etext, is the end of constant R/O
memory that needs to be mapped into the MODULES_VADDR area. This fixes
the bug where you might loose part of your R/O data after page table
setup is complete.

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

Comments

Russell King - ARM Linux July 16, 2015, 4:12 p.m. UTC | #1
On Thu, Jul 16, 2015 at 10:37:14AM -0400, Chris Brandt wrote:
> For an XIP build, _edata_loc, not _etext, is the end of constant R/O
> memory that needs to be mapped into the MODULES_VADDR area. This fixes
> the bug where you might loose part of your R/O data after page table
> setup is complete.

That's not correct.  _etext is set in the linker script to be the end of
the text + readonly data + exception table + unwinder tables + notes.
_edata_loc is the end address of the read/write data section stored in
read-only memory, which exists to be copied by the early kernel assembly
code to RAM.  There's no need for that to remain mapped.

I think you need to either provide more details of the problem you're
seeing, or further reasoning why this is a correct change.
Chris Brandt July 16, 2015, 4:23 p.m. UTC | #2
> I think you need to either provide more details of the problem you're seeing, or further reasoning why this is a correct change.


When I look at my System.map, it's clear that _edata_loc is the very last thing of ROM, and hence programmed into Flash.
_etext is farther back.


Example:

00000000 t __vectors_start
00000024 A cpu_ca8_suspend_size
00000024 A cpu_v7_suspend_size
0000002c A cpu_ca9mp_suspend_size
00001000 t __stubs_start
00001004 t vector_rst
00001020 t vector_irq
000010a0 t vector_dabt
00001120 t vector_pabt
000011a0 t vector_und
00001220 t vector_addrexcptn
00001240 t vector_fiq
00001240 T vector_fiq_offset
bf000000 T _text
bf000000 T stext
bf000070 t __create_page_tables
bf000180 t __turn_mmu_on_loc
bf00018c t __enable_mmu
bf0001c0 t __vet_atags
bf000240 T __idmap_text_start
bf000240 T __turn_mmu_on
bf000240 T _stext
bf000260 t __turn_mmu_on_end
bf000260 T cpu_resume_mmu
bf000284 T cpu_ca8_reset
bf000284 T cpu_ca9mp_reset
bf000284 T cpu_v7_reset
~ ~ ~
bf370f30 T __start_notes
bf370f30 R __stop___ex_table
bf370f54 T __stop_notes
bf370f54 A __vectors_start
bf370f54 A _etext                  <<<<<<<<<<<<<<<<<<<<<
bf370f74 A __stubs_start
bf370f74 A __vectors_end
bf371234 A __stubs_end
bf371240 t __mmap_switched
~ ~ ~
bf38ca74 T __security_initcall_end
bf38ca74 T __security_initcall_start
bf428a4a t __irf_end
bf428a50 T __initramfs_size
bf428a54 A __data_loc
bf445314 A _edata_loc
c0004000 A swapper_pg_dir
c0008000 D _data                  <<<<<<<<<<<<<<<<<<<<<
c0008000 D _sdata
c0008000 D init_thread_union
c000a000 D __init_begin
c000a000 d kthreadd_done
c000a00c d done.42220
~ ~ ~


The issue is basically early in boot when the MMU is being set up, it is mapping (in 1MB chucks) your ROM kernel to 0xBF000000.
If the size of you kernel happens to be that _etext is on 1 side of a 1MB boundary and the actual end of ROM (marked by _edata_loc) is on the other side of the 1MB boundry, it doesn't get mapped and you lose the end of your ROM as soon as the MMU is turned on.


Chris

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

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 6ca7d9a..57456df 100644
--- a/arch/arm/mm/mmu.c
+++ b/arch/arm/mm/mmu.c
@@ -1162,7 +1162,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));
@@ -1241,7 +1241,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[];