All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] kallsyms: remove special handling for CONFIG_ARM
@ 2016-02-02 13:19 Ard Biesheuvel
  2016-02-02 13:19 ` [PATCH 1/3] ARM: move .vectors and .stubs sections back into the kernel VMA Ard Biesheuvel
                   ` (5 more replies)
  0 siblings, 6 replies; 29+ messages in thread
From: Ard Biesheuvel @ 2016-02-02 13:19 UTC (permalink / raw)
  To: linux-arm-kernel

This series applies on top of today's -next, and addresses an issue with
the new kallsyms code that is queued there, that enables base relative
kallsyms tables for all architectures (except IA-64)

Two issues have surfaced on ARM with the new kallsyms code:
a) CONFIG_HAVE_TCM creates a virtual region that is too far away for the
   relative kallsyms code to reach it;
b) CONFIG_XIP_KERNEL=y kernel symbols are not filtered against PAGE_OFFSET,
   as is the case for kernels that execute from RAM, resulting in symbols
   that are out of range.

Since the way kallsyms deals with XIP kernels on ARM leaves some room for
improvement regardless of the base relative changes, this series proposes
a fix that allows the special case to be removed from the kallsyms handling
entirely.

Patch #1 moves the .stubs and .vectors section back into the kernel VMA, while
preserving the guaranteed virtual offset of 4 KB. This results in all symbols
that kallsyms sees to be in a reasonable interval.

Patch #2 removes the special case for CONFIG_ARM && !CONFIG_XIP_KERNEL in the
invocation of scripts/kallsyms

Patch #3 removes the now unused --page-offset command line argument handling
from scripts/kallsyms.c

Note that we may still need to remove ARM from the list of architectures that
support base relative kallsyms tables if we cannot fix issue a) above, but
removing this special case seemed like an obvious improvement to me.

Ard Biesheuvel (3):
  ARM: move .vectors and .stubs sections back into the kernel VMA
  kallsyms: remove special lower address limit for CONFIG_ARM
  kallsyms: remove --page-offset command line option

 arch/arm/kernel/entry-armv.S  |  7 +++--
 arch/arm/kernel/vmlinux.lds.S | 15 ++++++-----
 scripts/kallsyms.c            | 27 ++++----------------
 scripts/link-vmlinux.sh       |  4 ---
 4 files changed, 16 insertions(+), 37 deletions(-)

-- 
2.5.0

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 1/3] ARM: move .vectors and .stubs sections back into the kernel VMA
  2016-02-02 13:19 [PATCH 0/3] kallsyms: remove special handling for CONFIG_ARM Ard Biesheuvel
@ 2016-02-02 13:19 ` Ard Biesheuvel
  2016-02-03  0:03   ` Russell King - ARM Linux
  2016-02-02 13:19 ` [PATCH 2/3] kallsyms: remove special lower address limit for CONFIG_ARM Ard Biesheuvel
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 29+ messages in thread
From: Ard Biesheuvel @ 2016-02-02 13:19 UTC (permalink / raw)
  To: linux-arm-kernel

Commit b9b32bf70f2f ("ARM: use linker magic for vectors and vector stubs")
updated the linker script to emit the .vectors and .stubs sections into a
VMA range that is zero based and disjoint from the normal static kernel
region. The reason for that was that this way, the sections can be placed
exactly 4 KB apart, while the payload of the .vectors section is only 32
bytes.

Since the symbols that are part of the .stubs section are emitted into the
kallsyms table, they appear with zero based addresses as well, e.g.,

  00000000 t __vectors_start
  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

As this confuses perf when it accesses the kallsyms tables, commit
7122c3e9154b ("scripts/link-vmlinux.sh: only filter kernel symbols for
arm") implemented a somewhat ugly special case for ARM, where the value
of CONFIG_PAGE_OFFSET is passed to scripts/kallsyms, and symbols whose
address is below it are filtered out. Note that this special case only
applies to CONFIG_XIP_KERNEL=n, not because the issue the patch addresses
exists only in that case, but because finding a limit below which to apply
the filtering is too difficult.

Since the constraint we are trying to meet here is that the .vectors
section lives exactly 4 KB before the .stubs section, regardless of the
absolute addresses of either (since relative branches are used to jump from
the vector table to the stubs), we can simply emit the .stubs section as
part of the kernel VMA, and place the .vectors section 4 KB before it using
an explicit VMA override. By doing that, and renaming the __vectors_start
symbol that is local to arch/arm/kernel/entry-armv.S (not the one in the
linker script), the kallsyms table looks somewhat sane, regardless of
whether CONFIG_XIP_KERNEL is set, and we can drop the special case in
scripts/kallsyms entirely. E.g.,

  00001240 A vector_fiq_offset
  ...
  c0c35000 T __init_begin
  c0c35000 t __stubs_start
  c0c35000 T __stubs_start
  c0c35004 t vector_rst
  c0c35020 t vector_irq
  c0c350a0 t vector_dabt
  c0c35120 t vector_pabt
  c0c351a0 t vector_und
  c0c35220 t vector_addrexcptn
  c0c35240 T vector_fiq
  c0c352c0 T __stubs_end
  c0c352c0 T __vectors_start
  c0c352e0 t __mmap_switched
  c0c352e0 T _sinittext
  c0c352e0 T __vectors_end

(Note that vector_fiq_offset is now an absolute symbol, which kallsyms
already ignores by default)

The sections themselves are emitted 4 KB apart, as required:
  ...
  [16] .stubs       PROGBITS   c0c35000 a35000 0002c0 00  AX  0   0 32
  [17] .vectors     PROGBITS   c0c34000 a44000 000020 00  AX  0   0  2
  ...

and the relative branches in the .vectors section still point to the
right place:

  c0c34000 <.vectors>:
  c0c34000:       f001 b800       b.w     c0c35004 <vector_rst>
  c0c34004:       f001 b8cc       b.w     c0c351a0 <vector_und>
  c0c34008:       f8df fff4       ldr.w   pc, [pc, #4084] ; c0c35000
  c0c3400c:       f001 b888       b.w     c0c35120 <vector_pabt>
  c0c34010:       f001 b846       b.w     c0c350a0 <vector_dabt>
  c0c34014:       f001 b904       b.w     c0c35220 <vector_addrexcptn>
  c0c34018:       f001 b802       b.w     c0c35020 <vector_irq>
  c0c3401c:       f001 b910       b.w     c0c35240 <vector_fiq>

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm/kernel/entry-armv.S  |  7 +++----
 arch/arm/kernel/vmlinux.lds.S | 15 ++++++++-------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index 3ce377f7251f..8575ff42c0d4 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -1202,14 +1202,13 @@ vector_addrexcptn:
 	.long	__fiq_svc			@  e
 	.long	__fiq_svc			@  f
 
-	.globl	vector_fiq_offset
-	.equ	vector_fiq_offset, vector_fiq
+	.globl	vector_fiq
 
 	.section .vectors, "ax", %progbits
-__vectors_start:
+.L__vectors_start:
 	W(b)	vector_rst
 	W(b)	vector_und
-	W(ldr)	pc, __vectors_start + 0x1000
+	W(ldr)	pc, .L__vectors_start + 0x1000
 	W(b)	vector_pabt
 	W(b)	vector_dabt
 	W(b)	vector_addrexcptn
diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index 8b60fde5ce48..7160658fd5d4 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -164,19 +164,20 @@ SECTIONS
 	 * The vectors and stubs are relocatable code, and the
 	 * only thing that matters is their relative offsets
 	 */
+	__stubs_start = .;
+	.stubs : {
+		*(.stubs)
+	}
+	__stubs_end = .;
+
 	__vectors_start = .;
-	.vectors 0 : AT(__vectors_start) {
+	.vectors ADDR(.stubs) - 0x1000 : AT(__vectors_start) {
 		*(.vectors)
 	}
 	. = __vectors_start + SIZEOF(.vectors);
 	__vectors_end = .;
 
-	__stubs_start = .;
-	.stubs 0x1000 : AT(__stubs_start) {
-		*(.stubs)
-	}
-	. = __stubs_start + SIZEOF(.stubs);
-	__stubs_end = .;
+	vector_fiq_offset = vector_fiq - ADDR(.vectors);
 
 	INIT_TEXT_SECTION(8)
 	.exit.text : {
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 2/3] kallsyms: remove special lower address limit for CONFIG_ARM
  2016-02-02 13:19 [PATCH 0/3] kallsyms: remove special handling for CONFIG_ARM Ard Biesheuvel
  2016-02-02 13:19 ` [PATCH 1/3] ARM: move .vectors and .stubs sections back into the kernel VMA Ard Biesheuvel
@ 2016-02-02 13:19 ` Ard Biesheuvel
  2016-02-02 13:19 ` [PATCH 3/3] kallsyms: remove --page-offset command line option Ard Biesheuvel
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 29+ messages in thread
From: Ard Biesheuvel @ 2016-02-02 13:19 UTC (permalink / raw)
  To: linux-arm-kernel

Now that we no longer emit .stubs symbols into a section VMA loaded
at absolute address 0x1000, we can drop the ARM-specific override that
sets a lower limit based on CONFIG_PAGE_OFFSET, below which symbols are
filtered from the kallsyms output.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 scripts/link-vmlinux.sh | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index aae2473301ea..b83f918c7830 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -82,10 +82,6 @@ kallsyms()
 		kallsymopt="${kallsymopt} --all-symbols"
 	fi
 
-	if [ -n "${CONFIG_ARM}" ] && [ -z "${CONFIG_XIP_KERNEL}" ] && [ -n "${CONFIG_PAGE_OFFSET}" ]; then
-		kallsymopt="${kallsymopt} --page-offset=$CONFIG_PAGE_OFFSET"
-	fi
-
 	if [ -n "${CONFIG_X86_64}" ] && [ -n "${CONFIG_SMP}" ]; then
 		kallsymopt="${kallsymopt} --absolute-percpu"
 	fi
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 3/3] kallsyms: remove --page-offset command line option
  2016-02-02 13:19 [PATCH 0/3] kallsyms: remove special handling for CONFIG_ARM Ard Biesheuvel
  2016-02-02 13:19 ` [PATCH 1/3] ARM: move .vectors and .stubs sections back into the kernel VMA Ard Biesheuvel
  2016-02-02 13:19 ` [PATCH 2/3] kallsyms: remove special lower address limit for CONFIG_ARM Ard Biesheuvel
@ 2016-02-02 13:19 ` Ard Biesheuvel
  2016-02-03  0:05   ` Russell King - ARM Linux
  2016-02-02 17:51 ` [PATCH 0/3] kallsyms: remove special handling for CONFIG_ARM Nicolas Pitre
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 29+ messages in thread
From: Ard Biesheuvel @ 2016-02-02 13:19 UTC (permalink / raw)
  To: linux-arm-kernel

The --page-offset command line option was only used for ARM, to filter
symbol addresses below CONFIG_PAGE_OFFSET. This is no longer needed, so
remove the functionality altogether.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 scripts/kallsyms.c | 27 ++++----------------
 1 file changed, 5 insertions(+), 22 deletions(-)

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 02473b71643b..ce36fc02fe8c 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -63,7 +63,6 @@ static unsigned int table_size, table_cnt;
 static int all_symbols = 0;
 static int absolute_percpu = 0;
 static char symbol_prefix_char = '\0';
-static unsigned long long kernel_start_addr = 0;
 static int base_relative = 0;
 
 int token_profit[0x10000];
@@ -229,9 +228,6 @@ static int symbol_valid(struct sym_entry *s)
 	char *sym_name = (char *)s->sym + 1;
 
 
-	if (s->addr < kernel_start_addr)
-		return 0;
-
 	/* skip prefix char */
 	if (symbol_prefix_char && *sym_name == symbol_prefix_char)
 		sym_name++;
@@ -742,21 +738,11 @@ static void record_relative_base(void)
 {
 	unsigned int i;
 
-	if (kernel_start_addr > 0) {
-		/*
-		 * If the kernel start address was specified, use that as
-		 * the relative base rather than going through the table,
-		 * since it should be a reasonable default, and values below
-		 * it will be ignored anyway.
-		 */
-		relative_base = kernel_start_addr;
-	} else {
-		relative_base = -1ULL;
-		for (i = 0; i < table_cnt; i++)
-			if (!symbol_absolute(&table[i]) &&
-			    table[i].addr < relative_base)
-				relative_base = table[i].addr;
-	}
+	relative_base = -1ULL;
+	for (i = 0; i < table_cnt; i++)
+		if (!symbol_absolute(&table[i]) &&
+		    table[i].addr < relative_base)
+			relative_base = table[i].addr;
 }
 
 int main(int argc, char **argv)
@@ -774,9 +760,6 @@ int main(int argc, char **argv)
 				if ((*p == '"' && *(p+2) == '"') || (*p == '\'' && *(p+2) == '\''))
 					p++;
 				symbol_prefix_char = *p;
-			} else if (strncmp(argv[i], "--page-offset=", 14) == 0) {
-				const char *p = &argv[i][14];
-				kernel_start_addr = strtoull(p, NULL, 16);
 			} else if (strcmp(argv[i], "--base-relative") == 0)
 				base_relative = 1;
 			else
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 0/3] kallsyms: remove special handling for CONFIG_ARM
  2016-02-02 13:19 [PATCH 0/3] kallsyms: remove special handling for CONFIG_ARM Ard Biesheuvel
                   ` (2 preceding siblings ...)
  2016-02-02 13:19 ` [PATCH 3/3] kallsyms: remove --page-offset command line option Ard Biesheuvel
@ 2016-02-02 17:51 ` Nicolas Pitre
  2016-02-02 18:59 ` Chris Brandt
  2016-02-03 20:14 ` Linus Walleij
  5 siblings, 0 replies; 29+ messages in thread
From: Nicolas Pitre @ 2016-02-02 17:51 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 2 Feb 2016, Ard Biesheuvel wrote:

> This series applies on top of today's -next, and addresses an issue with
> the new kallsyms code that is queued there, that enables base relative
> kallsyms tables for all architectures (except IA-64)
> 
> Two issues have surfaced on ARM with the new kallsyms code:
> a) CONFIG_HAVE_TCM creates a virtual region that is too far away for the
>    relative kallsyms code to reach it;
> b) CONFIG_XIP_KERNEL=y kernel symbols are not filtered against PAGE_OFFSET,
>    as is the case for kernels that execute from RAM, resulting in symbols
>    that are out of range.
> 
> Since the way kallsyms deals with XIP kernels on ARM leaves some room for
> improvement regardless of the base relative changes, this series proposes
> a fix that allows the special case to be removed from the kallsyms handling
> entirely.
> 
> Patch #1 moves the .stubs and .vectors section back into the kernel VMA, while
> preserving the guaranteed virtual offset of 4 KB. This results in all symbols
> that kallsyms sees to be in a reasonable interval.
> 
> Patch #2 removes the special case for CONFIG_ARM && !CONFIG_XIP_KERNEL in the
> invocation of scripts/kallsyms
> 
> Patch #3 removes the now unused --page-offset command line argument handling
> from scripts/kallsyms.c
> 
> Note that we may still need to remove ARM from the list of architectures that
> support base relative kallsyms tables if we cannot fix issue a) above, but
> removing this special case seemed like an obvious improvement to me.

For those 3 patches:

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



> 
> Ard Biesheuvel (3):
>   ARM: move .vectors and .stubs sections back into the kernel VMA
>   kallsyms: remove special lower address limit for CONFIG_ARM
>   kallsyms: remove --page-offset command line option
> 
>  arch/arm/kernel/entry-armv.S  |  7 +++--
>  arch/arm/kernel/vmlinux.lds.S | 15 ++++++-----
>  scripts/kallsyms.c            | 27 ++++----------------
>  scripts/link-vmlinux.sh       |  4 ---
>  4 files changed, 16 insertions(+), 37 deletions(-)
> 
> -- 
> 2.5.0
> 
> 

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 0/3] kallsyms: remove special handling for CONFIG_ARM
  2016-02-02 13:19 [PATCH 0/3] kallsyms: remove special handling for CONFIG_ARM Ard Biesheuvel
                   ` (3 preceding siblings ...)
  2016-02-02 17:51 ` [PATCH 0/3] kallsyms: remove special handling for CONFIG_ARM Nicolas Pitre
@ 2016-02-02 18:59 ` Chris Brandt
  2016-02-02 19:00   ` Ard Biesheuvel
  2016-02-03 20:14 ` Linus Walleij
  5 siblings, 1 reply; 29+ messages in thread
From: Chris Brandt @ 2016-02-02 18:59 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, 2 Feb 2016, Ard Biesheuvel wrote:

> This series applies on top of today's -next, and addresses an issue 
> with the new kallsyms code that is queued there, that enables base 
> relative kallsyms tables for all architectures (except IA-64)
> 
> Two issues have surfaced on ARM with the new kallsyms code:
> a) CONFIG_HAVE_TCM creates a virtual region that is too far away for the
>    relative kallsyms code to reach it;
> b) CONFIG_XIP_KERNEL=y kernel symbols are not filtered against PAGE_OFFSET,
>    as is the case for kernels that execute from RAM, resulting in symbols
>    that are out of range.


For whatever it's worth:


I applied my XIP_KERNEL fixes (for ARMv7) on top of -next and attempted to boot which resulted in a flood of these:

  "Unable to handle kernel NULL pointer dereference at virtual address 00000000"

and of course it dies.


I then applied the 3 patches and tried again and this time it booted up....almost.
It looks like it makes it all the way up to when it is going to mount my rootfs, but then dies.

My kernel (.text) starts at 0xbf000000 (MODULES_VADDR), so the "LR is at 0xb6f87e88" is obviously bad.


Chris



================ boot log ======================


VFS: Mounted root (squashfs filesystem) readonly on device 31:0.
devtmpfs: mounted
Freeing unused kernel memory: 36K (c000a000 - c0013000)
Unable to handle kernel NULL pointer dereference at virtual address 00000000
pgd = c05f0000
[00000000] *pgd=205f5831, *pte=00000000, *ppte=00000000
Internal error: Oops: 80000007 [#1] ARM
Modules linked in:
CPU: 0 PID: 1 Comm: init Not tainted 4.5.0-rc2-next-20160202-dirty #3
Hardware name: Generic R7S72100 (Flattened Device Tree)
task: c0613000 ti: c0614000 task.ti: c0614000
PC is at 0x0
LR is at 0xb6f87e88
pc : [<00000000>]    lr : [<b6f87e88>]    psr: 80000093
sp : c0615ff8  ip : 00000054  fp : beca8ba4
r10: b6f8b9ab  r9 : 00000000  r8 : 00000014
r7 : 000000c0  r6 : b6f93f74  r5 : 00000000  r4 : ffffffff
r3 : 04000022  r2 : 00000003  r1 : 00001000  r0 : 00000000
Flags: Nzcv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment user
Control: 18c5347d  Table: 205f0059  DAC: 00000055
Process init (pid: 1, stack limit = 0xc0614208)
Stack: (0xc0615ff8 to 0xc0616000)
5fe0:                                                       fde4be6f 89031a8e
Backtrace: invalid frame pointer 0xbeca8ba4
Code: bad PC value
---[ end trace ed7071b866b0f003 ]---
Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b

---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 0/3] kallsyms: remove special handling for CONFIG_ARM
  2016-02-02 18:59 ` Chris Brandt
@ 2016-02-02 19:00   ` Ard Biesheuvel
  2016-02-02 19:13     ` Chris Brandt
  2016-02-03 13:33     ` Chris Brandt
  0 siblings, 2 replies; 29+ messages in thread
From: Ard Biesheuvel @ 2016-02-02 19:00 UTC (permalink / raw)
  To: linux-arm-kernel

On 2 February 2016 at 19:59, Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On Tue, 2 Feb 2016, Ard Biesheuvel wrote:
>
>> This series applies on top of today's -next, and addresses an issue
>> with the new kallsyms code that is queued there, that enables base
>> relative kallsyms tables for all architectures (except IA-64)
>>
>> Two issues have surfaced on ARM with the new kallsyms code:
>> a) CONFIG_HAVE_TCM creates a virtual region that is too far away for the
>>    relative kallsyms code to reach it;
>> b) CONFIG_XIP_KERNEL=y kernel symbols are not filtered against PAGE_OFFSET,
>>    as is the case for kernels that execute from RAM, resulting in symbols
>>    that are out of range.
>
>
> For whatever it's worth:
>
>
> I applied my XIP_KERNEL fixes (for ARMv7) on top of -next and attempted to boot which resulted in a flood of these:
>
>   "Unable to handle kernel NULL pointer dereference at virtual address 00000000"
>
> and of course it dies.
>
>
> I then applied the 3 patches and tried again and this time it booted up....almost.
> It looks like it makes it all the way up to when it is going to mount my rootfs, but then dies.
>
> My kernel (.text) starts at 0xbf000000 (MODULES_VADDR), so the "LR is at 0xb6f87e88" is obviously bad.
>
>
> ================ boot log ======================
>
>
> VFS: Mounted root (squashfs filesystem) readonly on device 31:0.
> devtmpfs: mounted
> Freeing unused kernel memory: 36K (c000a000 - c0013000)
> Unable to handle kernel NULL pointer dereference at virtual address 00000000
> pgd = c05f0000
> [00000000] *pgd=205f5831, *pte=00000000, *ppte=00000000
> Internal error: Oops: 80000007 [#1] ARM
> Modules linked in:
> CPU: 0 PID: 1 Comm: init Not tainted 4.5.0-rc2-next-20160202-dirty #3
> Hardware name: Generic R7S72100 (Flattened Device Tree)
> task: c0613000 ti: c0614000 task.ti: c0614000
> PC is at 0x0
> LR is at 0xb6f87e88
> pc : [<00000000>]    lr : [<b6f87e88>]    psr: 80000093
> sp : c0615ff8  ip : 00000054  fp : beca8ba4
> r10: b6f8b9ab  r9 : 00000000  r8 : 00000014
> r7 : 000000c0  r6 : b6f93f74  r5 : 00000000  r4 : ffffffff
> r3 : 04000022  r2 : 00000003  r1 : 00001000  r0 : 00000000
> Flags: Nzcv  IRQs off  FIQs on  Mode SVC_32  ISA ARM  Segment user
> Control: 18c5347d  Table: 205f0059  DAC: 00000055
> Process init (pid: 1, stack limit = 0xc0614208)
> Stack: (0xc0615ff8 to 0xc0616000)
> 5fe0:                                                       fde4be6f 89031a8e
> Backtrace: invalid frame pointer 0xbeca8ba4
> Code: bad PC value
> ---[ end trace ed7071b866b0f003 ]---
> Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
>
> ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b
>
>

Are you getting any warnings/errors during the build?

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 0/3] kallsyms: remove special handling for CONFIG_ARM
  2016-02-02 19:00   ` Ard Biesheuvel
@ 2016-02-02 19:13     ` Chris Brandt
  2016-02-03 13:33     ` Chris Brandt
  1 sibling, 0 replies; 29+ messages in thread
From: Chris Brandt @ 2016-02-02 19:13 UTC (permalink / raw)
  To: linux-arm-kernel

> Are you getting any warnings/errors during the build?

Nothing. The build was clean.


And just to be safe, I did make clean, then make xipImage and re-flashed again.
Same result.


I'll see if I can step through and see exactly where it's dying.
I realize that no one really care about or tests XIP_KERNEL with ARMv7 at the moment, so I'll have to figure it out that some point. I just thought I'd let you know.


Chris

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 1/3] ARM: move .vectors and .stubs sections back into the kernel VMA
  2016-02-02 13:19 ` [PATCH 1/3] ARM: move .vectors and .stubs sections back into the kernel VMA Ard Biesheuvel
@ 2016-02-03  0:03   ` Russell King - ARM Linux
  2016-02-03  0:20     ` Nicolas Pitre
  2016-02-03  7:56     ` Ard Biesheuvel
  0 siblings, 2 replies; 29+ messages in thread
From: Russell King - ARM Linux @ 2016-02-03  0:03 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Feb 02, 2016 at 02:19:32PM +0100, Ard Biesheuvel wrote:
> The sections themselves are emitted 4 KB apart, as required:
>   ...
>   [16] .stubs       PROGBITS   c0c35000 a35000 0002c0 00  AX  0   0 32
>   [17] .vectors     PROGBITS   c0c34000 a44000 000020 00  AX  0   0  2
>   ...

... which means we end up wasting most of a page for absolutely no
purpose what so ever.  Sorry, I'm really not happy about this
gratuitous wastage.

We changed to the existing solution to get rid of the VMA offset
to make the code easier to read, but if we're going to get rid of
that improvement, just revert the change.  (Check the kernel history.)

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 3/3] kallsyms: remove --page-offset command line option
  2016-02-02 13:19 ` [PATCH 3/3] kallsyms: remove --page-offset command line option Ard Biesheuvel
@ 2016-02-03  0:05   ` Russell King - ARM Linux
  2016-02-03  8:16     ` Ard Biesheuvel
  0 siblings, 1 reply; 29+ messages in thread
From: Russell King - ARM Linux @ 2016-02-03  0:05 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Feb 02, 2016 at 02:19:34PM +0100, Ard Biesheuvel wrote:
> The --page-offset command line option was only used for ARM, to filter
> symbol addresses below CONFIG_PAGE_OFFSET. This is no longer needed, so
> remove the functionality altogether.

>From what I remember, this option has nothing to do with MMU-ful ARM,
but noMMU ARM, where the .text segment can be significantly different
from the .data segment.

I think you need to talk to noMMU people about this patch.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 1/3] ARM: move .vectors and .stubs sections back into the kernel VMA
  2016-02-03  0:03   ` Russell King - ARM Linux
@ 2016-02-03  0:20     ` Nicolas Pitre
  2016-02-03  7:56     ` Ard Biesheuvel
  1 sibling, 0 replies; 29+ messages in thread
From: Nicolas Pitre @ 2016-02-03  0:20 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, 3 Feb 2016, Russell King - ARM Linux wrote:

> On Tue, Feb 02, 2016 at 02:19:32PM +0100, Ard Biesheuvel wrote:
> > The sections themselves are emitted 4 KB apart, as required:
> >   ...
> >   [16] .stubs       PROGBITS   c0c35000 a35000 0002c0 00  AX  0   0 32
> >   [17] .vectors     PROGBITS   c0c34000 a44000 000020 00  AX  0   0  2
> >   ...
> 
> ... which means we end up wasting most of a page for absolutely no
> purpose what so ever.  Sorry, I'm really not happy about this
> gratuitous wastage.

In practice this shouldn't make much of a difference given zImage is 
compressed and large zero filled areas compress very well.

But this is not nice for XIP though.


Nicolas

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 1/3] ARM: move .vectors and .stubs sections back into the kernel VMA
  2016-02-03  0:03   ` Russell King - ARM Linux
  2016-02-03  0:20     ` Nicolas Pitre
@ 2016-02-03  7:56     ` Ard Biesheuvel
  2016-02-03  9:13       ` Russell King - ARM Linux
  1 sibling, 1 reply; 29+ messages in thread
From: Ard Biesheuvel @ 2016-02-03  7:56 UTC (permalink / raw)
  To: linux-arm-kernel

On 3 February 2016 at 01:03, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Tue, Feb 02, 2016 at 02:19:32PM +0100, Ard Biesheuvel wrote:
>> The sections themselves are emitted 4 KB apart, as required:
>>   ...
>>   [16] .stubs       PROGBITS   c0c35000 a35000 0002c0 00  AX  0   0 32
>>   [17] .vectors     PROGBITS   c0c34000 a44000 000020 00  AX  0   0  2
>>   ...
>
> ... which means we end up wasting most of a page for absolutely no
> purpose what so ever.  Sorry, I'm really not happy about this
> gratuitous wastage.
>

No, we don't. Here's some more context:

Before:
  [14] .ARM.unwind_tab   PROGBITS        c0c2ef98 a2ef98 005088 00   A  0   0  4
  [15] .notes            NOTE            c0c34020 a34020 000024 00  AX  0   0  4
  [16] .vectors          PROGBITS        00000000 a40000 000020 00  AX  0   0  2
  [17] .stubs            PROGBITS        00001000 a41000 0002c0 00  AX  0   0 32
  [18] .init.text        PROGBITS        c0c352e0 a452e0 06b1b8 00  AX  0   0 32

After
  [14] .ARM.unwind_tab   PROGBITS        c0c2ef98 a2ef98 005088 00   A  0   0  4
  [15] .notes            NOTE            c0c34020 a34020 000024 00  AX  0   0  4
  [16] .stubs            PROGBITS        c0c35000 a35000 0002c0 00  AX  0   0 32
  [17] .vectors          PROGBITS        c0c34000 a44000 000020 00  AX  0   0  2
  [18] .init.text        PROGBITS        c0c352e0 a452e0 06b1b8 00  AX  0   0 32

As you can see, the LMA footprint has not increased, only .stubs and
.vectors have been reordered. The apparent rounding after the .notes
section occurs in both cases, and is due to the fact that __init_begin
is rounded to page size, and has nothing to do with the VMAs chosen
for the .stubs and .vectors section.

> We changed to the existing solution to get rid of the VMA offset
> to make the code easier to read, but if we're going to get rid of
> that improvement, just revert the change.  (Check the kernel history.)
>

I think that change was an improvement. It makes the code itself more
readable, but also results in correct references in the object files
rather than opaque expressions, i.e.,

  c0c34000 <.vectors>:
  c0c34000:       f001 b800       b.w     c0c35004 <vector_rst>
  c0c34004:       f001 b8cc       b.w     c0c351a0 <vector_und>
  c0c34008:       f8df fff4       ldr.w   pc, [pc, #4084] ; c0c35000
  c0c3400c:       f001 b888       b.w     c0c35120 <vector_pabt>
  c0c34010:       f001 b846       b.w     c0c350a0 <vector_dabt>
  c0c34014:       f001 b904       b.w     c0c35220 <vector_addrexcptn>
  c0c34018:       f001 b802       b.w     c0c35020 <vector_irq>
  c0c3401c:       f001 b910       b.w     c0c35240 <vector_fiq>

-- 
Ard.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 3/3] kallsyms: remove --page-offset command line option
  2016-02-03  0:05   ` Russell King - ARM Linux
@ 2016-02-03  8:16     ` Ard Biesheuvel
  2016-02-03  9:53       ` Maxime Coquelin
  0 siblings, 1 reply; 29+ messages in thread
From: Ard Biesheuvel @ 2016-02-03  8:16 UTC (permalink / raw)
  To: linux-arm-kernel

(+ Uwe, Maxime)

Complete thread here:
http://thread.gmane.org/gmane.linux.ports.arm.kernel/474400

On 3 February 2016 at 01:05, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Tue, Feb 02, 2016 at 02:19:34PM +0100, Ard Biesheuvel wrote:
>> The --page-offset command line option was only used for ARM, to filter
>> symbol addresses below CONFIG_PAGE_OFFSET. This is no longer needed, so
>> remove the functionality altogether.
>
> From what I remember, this option has nothing to do with MMU-ful ARM,
> but noMMU ARM, where the .text segment can be significantly different
> from the .data segment.
>
> I think you need to talk to noMMU people about this patch.
>

We have

commit f6537f2f0eba4eba3354e48dbe3047db6d8b6254
Author: Ming Lei <tom.leiming@gmail.com>
Date:   Sat Nov 2 09:11:33 2013 +1030

    scripts/kallsyms: filter symbols not in kernel address space

    This patch uses CONFIG_PAGE_OFFSET to filter symbols which
    are not in kernel address space because these symbols are
    generally for generating code purpose and can't be run at
    kernel mode, so we needn't keep them in /proc/kallsyms.

    For example, on ARM there are some symbols which may be
    linked in relocatable code section, then perf can't parse
    symbols any more from /proc/kallsyms, this patch fixes the
    problem (introduced b9b32bf70f2fb710b07c94e13afbc729afe221da)

which introduces the --page-offset option to kallsyms, in response to
your patch that moves .vectors and .stubs to VMA 0x0. Then we have

commit 7122c3e9154b5d9a7422f68f02d8acf050fad2b0
Author: Ming Lei <tom.leiming@gmail.com>
Date:   Tue Dec 10 16:46:29 2013 +1030

    scripts/link-vmlinux.sh: only filter kernel symbols for arm

    Actually CONFIG_PAGE_OFFSET isn't same with PAGE_OFFSET, so
    it isn't easy to figue out PAGE_OFFSET defined in header
    file from scripts.

    Because CONFIG_PAGE_OFFSET may not be defined in some ARCHs(
    64bit ARCH), or defined as bogus value in !MMU case, so
    this patch only applys the filter on ARM when CONFIG_PAGE_OFFSET
    is defined as the original problem is only on ARM.

which restricts the use of --page-offset to CONFIG_ARM, and

commit cc8475305203ddfd117b81e2e732194b67d8f310
Author: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Date:   Thu May 21 19:17:44 2015 +0800

    scripts: link-vmlinux: Don't pass page offset to kallsyms if XIP Kernel

    When Kernel is executed in place from ROM, the symbol addresses can be
    lower than the page offset.

which restricts the use of --page-offset to CONFIG_ARM && !CONFIG_XIP_KERNEL

So it looks like the Dec 10 patch fixes some fallout related to !MMU
that was caused by the Nov 2 patch, but the original patch has nothing
to do with !MMU. Furthermore, 3 out of 4 !MMU configs we have in the
tree (efm32, stm32 and vf610m4) are also XIP kernel, for which
--page-offset isn't even used to begin with after Maxime's patch.
Hopefully Uwe and/or Maxime can confirm, but removing the PAGE_OFFSET
lower limit should not affect those platforms at all afaict,
especially since, after patch 1/3, there are no symbols left to filter
below that limit in the first place.

Thanks,
Ard.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 1/3] ARM: move .vectors and .stubs sections back into the kernel VMA
  2016-02-03  7:56     ` Ard Biesheuvel
@ 2016-02-03  9:13       ` Russell King - ARM Linux
  2016-02-03  9:16         ` Ard Biesheuvel
  0 siblings, 1 reply; 29+ messages in thread
From: Russell King - ARM Linux @ 2016-02-03  9:13 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Feb 03, 2016 at 08:56:35AM +0100, Ard Biesheuvel wrote:
> On 3 February 2016 at 01:03, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Tue, Feb 02, 2016 at 02:19:32PM +0100, Ard Biesheuvel wrote:
> >> The sections themselves are emitted 4 KB apart, as required:
> >>   ...
> >>   [16] .stubs       PROGBITS   c0c35000 a35000 0002c0 00  AX  0   0 32
> >>   [17] .vectors     PROGBITS   c0c34000 a44000 000020 00  AX  0   0  2
> >>   ...
> >
> > ... which means we end up wasting most of a page for absolutely no
> > purpose what so ever.  Sorry, I'm really not happy about this
> > gratuitous wastage.
> >
> 
> No, we don't. Here's some more context:
> 
> Before:
>   [14] .ARM.unwind_tab   PROGBITS        c0c2ef98 a2ef98 005088 00   A  0   0  4
>   [15] .notes            NOTE            c0c34020 a34020 000024 00  AX  0   0  4
>   [16] .vectors          PROGBITS        00000000 a40000 000020 00  AX  0   0  2
>   [17] .stubs            PROGBITS        00001000 a41000 0002c0 00  AX  0   0 32
>   [18] .init.text        PROGBITS        c0c352e0 a452e0 06b1b8 00  AX  0   0 32
> 
> After
>   [14] .ARM.unwind_tab   PROGBITS        c0c2ef98 a2ef98 005088 00   A  0   0  4
>   [15] .notes            NOTE            c0c34020 a34020 000024 00  AX  0   0  4
>   [16] .stubs            PROGBITS        c0c35000 a35000 0002c0 00  AX  0   0 32
>   [17] .vectors          PROGBITS        c0c34000 a44000 000020 00  AX  0   0  2
>   [18] .init.text        PROGBITS        c0c352e0 a452e0 06b1b8 00  AX  0   0 32

Meaningless numbers... Do you think you can quote with the header
please?  I don't tend to carry around the objdump headers in my head.
Thanks.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 1/3] ARM: move .vectors and .stubs sections back into the kernel VMA
  2016-02-03  9:13       ` Russell King - ARM Linux
@ 2016-02-03  9:16         ` Ard Biesheuvel
  2016-02-08 16:39           ` Russell King - ARM Linux
  0 siblings, 1 reply; 29+ messages in thread
From: Ard Biesheuvel @ 2016-02-03  9:16 UTC (permalink / raw)
  To: linux-arm-kernel

On 3 February 2016 at 10:13, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Wed, Feb 03, 2016 at 08:56:35AM +0100, Ard Biesheuvel wrote:
>> On 3 February 2016 at 01:03, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > On Tue, Feb 02, 2016 at 02:19:32PM +0100, Ard Biesheuvel wrote:
>> >> The sections themselves are emitted 4 KB apart, as required:
>> >>   ...
>> >>   [16] .stubs       PROGBITS   c0c35000 a35000 0002c0 00  AX  0   0 32
>> >>   [17] .vectors     PROGBITS   c0c34000 a44000 000020 00  AX  0   0  2
>> >>   ...
>> >
>> > ... which means we end up wasting most of a page for absolutely no
>> > purpose what so ever.  Sorry, I'm really not happy about this
>> > gratuitous wastage.
>> >
>>
>> No, we don't. Here's some more context:
>>
>> Before:
>>   [14] .ARM.unwind_tab   PROGBITS        c0c2ef98 a2ef98 005088 00   A  0   0  4
>>   [15] .notes            NOTE            c0c34020 a34020 000024 00  AX  0   0  4
>>   [16] .vectors          PROGBITS        00000000 a40000 000020 00  AX  0   0  2
>>   [17] .stubs            PROGBITS        00001000 a41000 0002c0 00  AX  0   0 32
>>   [18] .init.text        PROGBITS        c0c352e0 a452e0 06b1b8 00  AX  0   0 32
>>
>> After
>>   [14] .ARM.unwind_tab   PROGBITS        c0c2ef98 a2ef98 005088 00   A  0   0  4
>>   [15] .notes            NOTE            c0c34020 a34020 000024 00  AX  0   0  4
>>   [16] .stubs            PROGBITS        c0c35000 a35000 0002c0 00  AX  0   0 32
>>   [17] .vectors          PROGBITS        c0c34000 a44000 000020 00  AX  0   0  2
>>   [18] .init.text        PROGBITS        c0c352e0 a452e0 06b1b8 00  AX  0   0 32
>
> Meaningless numbers... Do you think you can quote with the header
> please?  I don't tend to carry around the objdump headers in my head.
> Thanks.
>

BEFORE

$ readelf -S vmlinux
There are 34 section headers, starting at offset 0x10a6e60:

Section Headers:
  [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
  [ 0]                   NULL            00000000 000000 000000 00      0   0  0
  [ 1] .head.text        PROGBITS        c0208000 008000 000268 00  AX  0   0  4
  [ 2] .text             PROGBITS        c0209000 009000 5d6e7c 00  AX
 0   0 4096
  [ 3] .fixup            PROGBITS        c07dfe7c 5dfe7c 000024 00  AX  0   0  4
  [ 4] .rodata           PROGBITS        c07e0000 5e0000 3ca310 00   A  0   0 64
  [ 5] __bug_table       PROGBITS        c0baa310 9aa310 0082c8 00   A  0   0  4
  [ 6] .pci_fixup        PROGBITS        c0bb25d8 9b25d8 001910 00   A  0   0  4
  [ 7] __ksymtab         PROGBITS        c0bb3ee8 9b3ee8 008a70 00   A  0   0  4
  [ 8] __ksymtab_gpl     PROGBITS        c0bbc958 9bc958 007d60 00   A  0   0  4
  [ 9] __ksymtab_strings PROGBITS        c0bc46b8 9c46b8 027c07 00   A  0   0  1
  [10] __param           PROGBITS        c0bec2c0 9ec2c0 0015b8 00   A  0   0  4
  [11] __modver          PROGBITS        c0bed878 9ed878 000788 00   A  0   0  4
  [12] __ex_table        PROGBITS        c0bee000 9ee000 001068 00   A  0   0  8
  [13] .ARM.unwind_idx   ARM_EXIDX       c0bef068 9ef068 03ff30 00  AL 18   0  4
  [14] .ARM.unwind_tab   PROGBITS        c0c2ef98 a2ef98 005088 00   A  0   0  4
  [15] .notes            NOTE            c0c34020 a34020 000024 00  AX  0   0  4
  [16] .vectors          PROGBITS        00000000 a40000 000020 00  AX  0   0  2
  [17] .stubs            PROGBITS        00001000 a41000 0002c0 00  AX  0   0 32
  [18] .init.text        PROGBITS        c0c352e0 a452e0 06b1b8 00  AX  0   0 32
  [19] .exit.text        PROGBITS        c0ca0498 ab0498 002f40 00  AX  0   0  4
  [20] .init.arch.info   PROGBITS        c0ca33d8 ab33d8 0026e8 00   A  0   0  8
  [21] .init.tagtable    PROGBITS        c0ca5ac0 ab5ac0 000048 00   A  0   0  4
  [22] .init.smpalt      PROGBITS        c0ca5b08 ab5b08 00dff8 00   A  0   0  4
  [23] .init.pv_table    PROGBITS        c0cb3b00 ac3b00 000a20 00   A  0   0  1
  [24] .init.data        PROGBITS        c0cb5000 ac5000 06de0c 00  WA
 0   0 4096
  [25] .data..percpu     PROGBITS        c0d23000 b33000 0049c0 00  WA  0   0 64
  [26] .data             PROGBITS        c0d28000 b38000 107d00 00  WA  0   0 64
  [27] .data..page_align PROGBITS        c0e30000 c40000 002000 00  WA
 0   0 4096
  [28] .bss              NOBITS          c0e32000 c42000 056ec8 00  WA  0   0 64
  [29] .comment          PROGBITS        00000000 c42000 00002d 01  MS  0   0  1
  [30] .ARM.attributes   ARM_ATTRIBUTES  00000000 c4202d 000033 00      0   0  1
  [31] .shstrtab         STRTAB          00000000 c42060 000165 00      0   0  1
  [32] .symtab           SYMTAB          00000000 c421c8 28f340 10
33 140990  4
  [33] .strtab           STRTAB          00000000 ed1508 1d5956 00      0   0  1
Key to Flags:
  W (write), A (alloc), X (execute), M (merge), S (strings)
  I (info), L (link order), G (group), T (TLS), E (exclude), x (unknown)
  O (extra OS processing required) o (OS specific), p (processor specific)

AFTER

$ readelf -S vmlinux
There are 34 section headers, starting at offset 0x10a6e50:

Section Headers:
  [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
  [ 0]                   NULL            00000000 000000 000000 00      0   0  0
  [ 1] .head.text        PROGBITS        c0208000 008000 000268 00  AX  0   0  4
  [ 2] .text             PROGBITS        c0209000 009000 5d6e7c 00  AX
 0   0 4096
  [ 3] .fixup            PROGBITS        c07dfe7c 5dfe7c 000024 00  AX  0   0  4
  [ 4] .rodata           PROGBITS        c07e0000 5e0000 3ca310 00   A  0   0 64
  [ 5] __bug_table       PROGBITS        c0baa310 9aa310 0082c8 00   A  0   0  4
  [ 6] .pci_fixup        PROGBITS        c0bb25d8 9b25d8 001910 00   A  0   0  4
  [ 7] __ksymtab         PROGBITS        c0bb3ee8 9b3ee8 008a70 00   A  0   0  4
  [ 8] __ksymtab_gpl     PROGBITS        c0bbc958 9bc958 007d60 00   A  0   0  4
  [ 9] __ksymtab_strings PROGBITS        c0bc46b8 9c46b8 027c07 00   A  0   0  1
  [10] __param           PROGBITS        c0bec2c0 9ec2c0 0015b8 00   A  0   0  4
  [11] __modver          PROGBITS        c0bed878 9ed878 000788 00   A  0   0  4
  [12] __ex_table        PROGBITS        c0bee000 9ee000 001068 00   A  0   0  8
  [13] .ARM.unwind_idx   ARM_EXIDX       c0bef068 9ef068 03ff30 00  AL 18   0  4
  [14] .ARM.unwind_tab   PROGBITS        c0c2ef98 a2ef98 005088 00   A  0   0  4
  [15] .notes            NOTE            c0c34020 a34020 000024 00  AX  0   0  4
  [16] .stubs            PROGBITS        c0c35000 a35000 0002c0 00  AX  0   0 32
  [17] .vectors          PROGBITS        c0c34000 a44000 000020 00  AX  0   0  2
  [18] .init.text        PROGBITS        c0c352e0 a452e0 06b1b8 00  AX  0   0 32
  [19] .exit.text        PROGBITS        c0ca0498 ab0498 002f40 00  AX  0   0  4
  [20] .init.arch.info   PROGBITS        c0ca33d8 ab33d8 0026e8 00   A  0   0  8
  [21] .init.tagtable    PROGBITS        c0ca5ac0 ab5ac0 000048 00   A  0   0  4
  [22] .init.smpalt      PROGBITS        c0ca5b08 ab5b08 00dff8 00   A  0   0  4
  [23] .init.pv_table    PROGBITS        c0cb3b00 ac3b00 000a20 00   A  0   0  1
  [24] .init.data        PROGBITS        c0cb5000 ac5000 06de0c 00  WA
 0   0 4096
  [25] .data..percpu     PROGBITS        c0d23000 b33000 0049c0 00  WA  0   0 64
  [26] .data             PROGBITS        c0d28000 b38000 107d00 00  WA  0   0 64
  [27] .data..page_align PROGBITS        c0e30000 c40000 002000 00  WA
 0   0 4096
  [28] .bss              NOBITS          c0e32000 c42000 056ec8 00  WA  0   0 64
  [29] .comment          PROGBITS        00000000 c42000 00002d 01  MS  0   0  1
  [30] .ARM.attributes   ARM_ATTRIBUTES  00000000 c4202d 000033 00      0   0  1
  [31] .shstrtab         STRTAB          00000000 c42060 000165 00      0   0  1
  [32] .symtab           SYMTAB          00000000 c421c8 28f330 10
33 140988  4
  [33] .strtab           STRTAB          00000000 ed14f8 1d5956 00      0   0  1
Key to Flags:
  W (write), A (alloc), X (execute), M (merge), S (strings)
  I (info), L (link order), G (group), T (TLS), E (exclude), x (unknown)
  O (extra OS processing required) o (OS specific), p (processor specific)

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 3/3] kallsyms: remove --page-offset command line option
  2016-02-03  8:16     ` Ard Biesheuvel
@ 2016-02-03  9:53       ` Maxime Coquelin
  0 siblings, 0 replies; 29+ messages in thread
From: Maxime Coquelin @ 2016-02-03  9:53 UTC (permalink / raw)
  To: linux-arm-kernel

2016-02-03 9:16 GMT+01:00 Ard Biesheuvel <ard.biesheuvel@linaro.org>:
> (+ Uwe, Maxime)
>
> Complete thread here:
> http://thread.gmane.org/gmane.linux.ports.arm.kernel/474400
>
> On 3 February 2016 at 01:05, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
>> On Tue, Feb 02, 2016 at 02:19:34PM +0100, Ard Biesheuvel wrote:
>>> The --page-offset command line option was only used for ARM, to filter
>>> symbol addresses below CONFIG_PAGE_OFFSET. This is no longer needed, so
>>> remove the functionality altogether.
>>
>> From what I remember, this option has nothing to do with MMU-ful ARM,
>> but noMMU ARM, where the .text segment can be significantly different
>> from the .data segment.
>>
>> I think you need to talk to noMMU people about this patch.
>>
>
> We have
>
> commit f6537f2f0eba4eba3354e48dbe3047db6d8b6254
> Author: Ming Lei <tom.leiming@gmail.com>
> Date:   Sat Nov 2 09:11:33 2013 +1030
>
>     scripts/kallsyms: filter symbols not in kernel address space
>
>     This patch uses CONFIG_PAGE_OFFSET to filter symbols which
>     are not in kernel address space because these symbols are
>     generally for generating code purpose and can't be run at
>     kernel mode, so we needn't keep them in /proc/kallsyms.
>
>     For example, on ARM there are some symbols which may be
>     linked in relocatable code section, then perf can't parse
>     symbols any more from /proc/kallsyms, this patch fixes the
>     problem (introduced b9b32bf70f2fb710b07c94e13afbc729afe221da)
>
> which introduces the --page-offset option to kallsyms, in response to
> your patch that moves .vectors and .stubs to VMA 0x0. Then we have
>
> commit 7122c3e9154b5d9a7422f68f02d8acf050fad2b0
> Author: Ming Lei <tom.leiming@gmail.com>
> Date:   Tue Dec 10 16:46:29 2013 +1030
>
>     scripts/link-vmlinux.sh: only filter kernel symbols for arm
>
>     Actually CONFIG_PAGE_OFFSET isn't same with PAGE_OFFSET, so
>     it isn't easy to figue out PAGE_OFFSET defined in header
>     file from scripts.
>
>     Because CONFIG_PAGE_OFFSET may not be defined in some ARCHs(
>     64bit ARCH), or defined as bogus value in !MMU case, so
>     this patch only applys the filter on ARM when CONFIG_PAGE_OFFSET
>     is defined as the original problem is only on ARM.
>
> which restricts the use of --page-offset to CONFIG_ARM, and
>
> commit cc8475305203ddfd117b81e2e732194b67d8f310
> Author: Maxime Coquelin <mcoquelin.stm32@gmail.com>
> Date:   Thu May 21 19:17:44 2015 +0800
>
>     scripts: link-vmlinux: Don't pass page offset to kallsyms if XIP Kernel
>
>     When Kernel is executed in place from ROM, the symbol addresses can be
>     lower than the page offset.
>
> which restricts the use of --page-offset to CONFIG_ARM && !CONFIG_XIP_KERNEL
>
> So it looks like the Dec 10 patch fixes some fallout related to !MMU
> that was caused by the Nov 2 patch, but the original patch has nothing
> to do with !MMU. Furthermore, 3 out of 4 !MMU configs we have in the
> tree (efm32, stm32 and vf610m4) are also XIP kernel, for which
> --page-offset isn't even used to begin with after Maxime's patch.
> Hopefully Uwe and/or Maxime can confirm, but removing the PAGE_OFFSET
> lower limit should not affect those platforms at all afaict,
> especially since, after patch 1/3, there are no symbols left to filter
> below that limit in the first place.

I agree with you, removing the PAGE_OFFSET lower limit should not
affect our platforms.

Regards,
Maxime

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 0/3] kallsyms: remove special handling for CONFIG_ARM
  2016-02-02 19:00   ` Ard Biesheuvel
  2016-02-02 19:13     ` Chris Brandt
@ 2016-02-03 13:33     ` Chris Brandt
  2016-02-03 13:41       ` Ard Biesheuvel
  1 sibling, 1 reply; 29+ messages in thread
From: Chris Brandt @ 2016-02-03 13:33 UTC (permalink / raw)
  To: linux-arm-kernel

On 2 Feb 2016, Chris Brandt wrote:
> I then applied the 3 patches and tried again and this time it booted
> up....almost.
> It looks like it makes it all the way up to when it is going to mount
> my rootfs, but then dies.


I did some debugging, and here's why I'm crashing.


If you look at my System.map file, you'll see that I've got 2 "__stubs_start" symbols.

One at 0x bf353edc, the other at 0x bf353ee0.

bf353edc T __stubs_start
bf353edc T _etext
bf353ee0 t __stubs_start
bf353ee4 t vector_rst
bf353f00 t vector_irq
bf353f80 t vector_dabt
bf354000 t vector_pabt
bf354080 t vector_und
bf354100 t vector_addrexcptn
bf354120 T vector_fiq
bf3541a0 T __stubs_end
bf3541a0 T __vectors_start
bf3541c0 t __mmap_switched
bf3541c0 T __vectors_end


So, when you get to early_trap_init(), it grabs the first __stubs_start for the memcpy:

	memcpy((void *)vectors + 0x1000, __stubs_start, __stubs_end - __stubs_start);


And when a printed out the destination, I get:

   Stubs at c09ff000 :00000000 BF006F20 EF9F0000 EA000064 etc...

Notice the first entry is 0....that's not right...and we know it's using that first __stubs_start symbol.


So, I hacked the code and put an offset of 4 like this:

	memcpy((void *)vectors + 0x1000, __stubs_start+4, __stubs_end - (__stubs_start+4));


And sure enough it booted fine.


So, why are there 2 __stubs_start symbols???
I haven't figured that one out yet.

And, I'm assuming you guys don't have 2 __stubs_start symbols either, correct?


Chris

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 0/3] kallsyms: remove special handling for CONFIG_ARM
  2016-02-03 13:33     ` Chris Brandt
@ 2016-02-03 13:41       ` Ard Biesheuvel
  2016-02-03 14:15         ` Chris Brandt
  2016-02-03 20:01         ` Russell King - ARM Linux
  0 siblings, 2 replies; 29+ messages in thread
From: Ard Biesheuvel @ 2016-02-03 13:41 UTC (permalink / raw)
  To: linux-arm-kernel

On 3 February 2016 at 14:33, Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On 2 Feb 2016, Chris Brandt wrote:
>> I then applied the 3 patches and tried again and this time it booted
>> up....almost.
>> It looks like it makes it all the way up to when it is going to mount
>> my rootfs, but then dies.
>
>
> I did some debugging, and here's why I'm crashing.
>
>
> If you look at my System.map file, you'll see that I've got 2 "__stubs_start" symbols.
>
> One at 0x bf353edc, the other at 0x bf353ee0.
>
> bf353edc T __stubs_start

This is a global symbol

> bf353edc T _etext
> bf353ee0 t __stubs_start

This is a local symbol

> bf353ee4 t vector_rst
> bf353f00 t vector_irq
> bf353f80 t vector_dabt
> bf354000 t vector_pabt
> bf354080 t vector_und
> bf354100 t vector_addrexcptn
> bf354120 T vector_fiq
> bf3541a0 T __stubs_end
> bf3541a0 T __vectors_start
> bf3541c0 t __mmap_switched
> bf3541c0 T __vectors_end
>
>
> So, when you get to early_trap_init(), it grabs the first __stubs_start for the memcpy:
>
>         memcpy((void *)vectors + 0x1000, __stubs_start, __stubs_end - __stubs_start);
>

Indeed. It grabs the first one, which is the one defined by the linker script.

>
> And when a printed out the destination, I get:
>
>    Stubs at c09ff000 :00000000 BF006F20 EF9F0000 EA000064 etc...
>
> Notice the first entry is 0....that's not right...and we know it's using that first __stubs_start symbol.
>

The first quantity after __stubs_start should be the address of
vector_swi. Can you check if the second value coincides with that?

>
> So, I hacked the code and put an offset of 4 like this:
>
>         memcpy((void *)vectors + 0x1000, __stubs_start+4, __stubs_end - (__stubs_start+4));
>
>
> And sure enough it booted fine.
>
>
> So, why are there 2 __stubs_start symbols???
> I haven't figured that one out yet.
>

One defined by entry-armv.S and one defined by the linker script

> And, I'm assuming you guys don't have 2 __stubs_start symbols either, correct?
>

Yes, we do.

This is caused by the fact that __stubs_start is not aligned correctly on XIP.

Could you try this, please?

diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index 7160658fd5d4..a5b8e7b80d17 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -164,8 +164,8 @@ SECTIONS
         * The vectors and stubs are relocatable code, and the
         * only thing that matters is their relative offsets
         */
-       __stubs_start = .;
        .stubs : {
+               __stubs_start = .;
                *(.stubs)
        }
        __stubs_end = .;

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH 0/3] kallsyms: remove special handling for CONFIG_ARM
  2016-02-03 13:41       ` Ard Biesheuvel
@ 2016-02-03 14:15         ` Chris Brandt
  2016-02-03 14:17           ` Ard Biesheuvel
  2016-02-03 20:01         ` Russell King - ARM Linux
  1 sibling, 1 reply; 29+ messages in thread
From: Chris Brandt @ 2016-02-03 14:15 UTC (permalink / raw)
  To: linux-arm-kernel

On 2 Feb 2016, Chris Brandt wrote 
> > And when a printed out the destination, I get:
> >
> >    Stubs at c09ff000 :00000000 BF006F20 EF9F0000 EA000064 etc...
> >
> > Notice the first entry is 0....that's not right...and we know it's using that
> > first __stubs_start symbol.
> >
>
> The first quantity after __stubs_start should be the address of vector_swi. Can
>  you check if the second value coincides with that?

Yes, it does.

bf006f20 T vector_swi

Which is why it booted once I modified the start address.


> This is caused by the fact that __stubs_start is not aligned correctly on XIP.
>
> Could you try this, please?
>
> diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S index 7160658fd5d4..a5b8e7b80d17 100644
> --- a/arch/arm/kernel/vmlinux.lds.S
> +++ b/arch/arm/kernel/vmlinux.lds.S
> @@ -164,8 +164,8 @@ SECTIONS
>          * The vectors and stubs are relocatable code, and the
>          * only thing that matters is their relative offsets
>          */
> -       __stubs_start = .;
>         .stubs : {
> +               __stubs_start = .;
>                 *(.stubs)
>         }
>         __stubs_end = .;


Yup, that fixed it!

bf353edc T _etext
bf353ee0 t __stubs_start
bf353ee0 T __stubs_start
bf353ee4 t vector_rst
bf353f00 t vector_irq

and my new print out:

     Stubs at c09ff000 :BF006F20 EF9F0000 EA000064 E320F000 etc...


Now that it is working on my XIP system, I will say that I'm also happy to see that extra junk in kallsyms.c and link-vmlinux.sh be removed. Those things also caused me some issues when I first started trying to get the XIP_KERNEL stuff back working.



Chris

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 0/3] kallsyms: remove special handling for CONFIG_ARM
  2016-02-03 14:15         ` Chris Brandt
@ 2016-02-03 14:17           ` Ard Biesheuvel
  0 siblings, 0 replies; 29+ messages in thread
From: Ard Biesheuvel @ 2016-02-03 14:17 UTC (permalink / raw)
  To: linux-arm-kernel

On 3 February 2016 at 15:15, Chris Brandt <Chris.Brandt@renesas.com> wrote:
> On 2 Feb 2016, Chris Brandt wrote
>> > And when a printed out the destination, I get:
>> >
>> >    Stubs at c09ff000 :00000000 BF006F20 EF9F0000 EA000064 etc...
>> >
>> > Notice the first entry is 0....that's not right...and we know it's using that
>> > first __stubs_start symbol.
>> >
>>
>> The first quantity after __stubs_start should be the address of vector_swi. Can
>>  you check if the second value coincides with that?
>
> Yes, it does.
>
> bf006f20 T vector_swi
>
> Which is why it booted once I modified the start address.
>
>
>> This is caused by the fact that __stubs_start is not aligned correctly on XIP.
>>
>> Could you try this, please?
>>
>> diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S index 7160658fd5d4..a5b8e7b80d17 100644
>> --- a/arch/arm/kernel/vmlinux.lds.S
>> +++ b/arch/arm/kernel/vmlinux.lds.S
>> @@ -164,8 +164,8 @@ SECTIONS
>>          * The vectors and stubs are relocatable code, and the
>>          * only thing that matters is their relative offsets
>>          */
>> -       __stubs_start = .;
>>         .stubs : {
>> +               __stubs_start = .;
>>                 *(.stubs)
>>         }
>>         __stubs_end = .;
>
>
> Yup, that fixed it!
>
> bf353edc T _etext
> bf353ee0 t __stubs_start
> bf353ee0 T __stubs_start
> bf353ee4 t vector_rst
> bf353f00 t vector_irq
>
> and my new print out:
>
>      Stubs at c09ff000 :BF006F20 EF9F0000 EA000064 E320F000 etc...
>
>
> Now that it is working on my XIP system, I will say that I'm also happy to see that extra junk in kallsyms.c and link-vmlinux.sh be removed. Those things also caused me some issues when I first started trying to get the XIP_KERNEL stuff back working.
>

Glad to hear that it works now. Thanks a lot for testing.

-- 
Ard.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 0/3] kallsyms: remove special handling for CONFIG_ARM
  2016-02-03 13:41       ` Ard Biesheuvel
  2016-02-03 14:15         ` Chris Brandt
@ 2016-02-03 20:01         ` Russell King - ARM Linux
  2016-02-03 20:02           ` Ard Biesheuvel
  1 sibling, 1 reply; 29+ messages in thread
From: Russell King - ARM Linux @ 2016-02-03 20:01 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Feb 03, 2016 at 02:41:29PM +0100, Ard Biesheuvel wrote:
> > bf353edc T __stubs_start
> 
> This is a global symbol
> 
> > bf353edc T _etext
> > bf353ee0 t __stubs_start
> 
> This is a local symbol

Stop this madness.  This gets you an immediate NAK on the patch set.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 0/3] kallsyms: remove special handling for CONFIG_ARM
  2016-02-03 20:01         ` Russell King - ARM Linux
@ 2016-02-03 20:02           ` Ard Biesheuvel
  2016-02-03 20:23             ` Nicolas Pitre
  0 siblings, 1 reply; 29+ messages in thread
From: Ard Biesheuvel @ 2016-02-03 20:02 UTC (permalink / raw)
  To: linux-arm-kernel

On 3 February 2016 at 21:01, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Wed, Feb 03, 2016 at 02:41:29PM +0100, Ard Biesheuvel wrote:
>> > bf353edc T __stubs_start
>>
>> This is a global symbol
>>
>> > bf353edc T _etext
>> > bf353ee0 t __stubs_start
>>
>> This is a local symbol
>
> Stop this madness.  This gets you an immediate NAK on the patch set.
>

What madness? This does not change at all with my patches, these
duplicates already exist in the current code.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 0/3] kallsyms: remove special handling for CONFIG_ARM
  2016-02-02 13:19 [PATCH 0/3] kallsyms: remove special handling for CONFIG_ARM Ard Biesheuvel
                   ` (4 preceding siblings ...)
  2016-02-02 18:59 ` Chris Brandt
@ 2016-02-03 20:14 ` Linus Walleij
  5 siblings, 0 replies; 29+ messages in thread
From: Linus Walleij @ 2016-02-03 20:14 UTC (permalink / raw)
  To: linux-arm-kernel

On Tue, Feb 2, 2016 at 2:19 PM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:

> a) CONFIG_HAVE_TCM creates a virtual region that is too far away for the
>    relative kallsyms code to reach it;

We have discussed the TCM before in context of SRAM and other
embedded on-chip RAMs.

There was some consensus that the statically allocated area for TCM is
not really scaleable, especially not for multiplatform kernels, which will
try to compile the sum total of all platforms' code tagged for TCM into this
memory which is bound to overflow if people start to take advantage
of it. It would be better to have just the machine-relevant code loaded
into TCM at boot. People have also shown interest in deploying
SRAM/TCM applications from userspace (for hard processing
kernels etc).

The best would be to advance that work and thus get rid of the
static TCM linkage I think?

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 0/3] kallsyms: remove special handling for CONFIG_ARM
  2016-02-03 20:02           ` Ard Biesheuvel
@ 2016-02-03 20:23             ` Nicolas Pitre
  2016-02-03 20:30               ` Ard Biesheuvel
  0 siblings, 1 reply; 29+ messages in thread
From: Nicolas Pitre @ 2016-02-03 20:23 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, 3 Feb 2016, Ard Biesheuvel wrote:

> On 3 February 2016 at 21:01, Russell King - ARM Linux
> <linux@arm.linux.org.uk> wrote:
> > On Wed, Feb 03, 2016 at 02:41:29PM +0100, Ard Biesheuvel wrote:
> >> > bf353edc T __stubs_start
> >>
> >> This is a global symbol
> >>
> >> > bf353edc T _etext
> >> > bf353ee0 t __stubs_start
> >>
> >> This is a local symbol
> >
> > Stop this madness.  This gets you an immediate NAK on the patch set.
> >
> 
> What madness? This does not change at all with my patches, these
> duplicates already exist in the current code.

That doesn't make it any better, and was cause for issue as Chris 
demonstrated.  Can we get rid of that duplication please?


Nicolas

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 0/3] kallsyms: remove special handling for CONFIG_ARM
  2016-02-03 20:23             ` Nicolas Pitre
@ 2016-02-03 20:30               ` Ard Biesheuvel
  2016-02-03 20:41                 ` Ard Biesheuvel
  0 siblings, 1 reply; 29+ messages in thread
From: Ard Biesheuvel @ 2016-02-03 20:30 UTC (permalink / raw)
  To: linux-arm-kernel

On 3 February 2016 at 21:23, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> On Wed, 3 Feb 2016, Ard Biesheuvel wrote:
>
>> On 3 February 2016 at 21:01, Russell King - ARM Linux
>> <linux@arm.linux.org.uk> wrote:
>> > On Wed, Feb 03, 2016 at 02:41:29PM +0100, Ard Biesheuvel wrote:
>> >> > bf353edc T __stubs_start
>> >>
>> >> This is a global symbol
>> >>
>> >> > bf353edc T _etext
>> >> > bf353ee0 t __stubs_start
>> >>
>> >> This is a local symbol
>> >
>> > Stop this madness.  This gets you an immediate NAK on the patch set.
>> >
>>
>> What madness? This does not change at all with my patches, these
>> duplicates already exist in the current code.
>
> That doesn't make it any better, and was cause for issue as Chris
> demonstrated.  Can we get rid of that duplication please?
>

4/4 in my v2 removes the redundant symbols.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 0/3] kallsyms: remove special handling for CONFIG_ARM
  2016-02-03 20:30               ` Ard Biesheuvel
@ 2016-02-03 20:41                 ` Ard Biesheuvel
  0 siblings, 0 replies; 29+ messages in thread
From: Ard Biesheuvel @ 2016-02-03 20:41 UTC (permalink / raw)
  To: linux-arm-kernel

On 3 February 2016 at 21:30, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 3 February 2016 at 21:23, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
>> On Wed, 3 Feb 2016, Ard Biesheuvel wrote:
>>
>>> On 3 February 2016 at 21:01, Russell King - ARM Linux
>>> <linux@arm.linux.org.uk> wrote:
>>> > On Wed, Feb 03, 2016 at 02:41:29PM +0100, Ard Biesheuvel wrote:
>>> >> > bf353edc T __stubs_start
>>> >>
>>> >> This is a global symbol
>>> >>
>>> >> > bf353edc T _etext
>>> >> > bf353ee0 t __stubs_start
>>> >>
>>> >> This is a local symbol
>>> >
>>> > Stop this madness.  This gets you an immediate NAK on the patch set.
>>> >
>>>
>>> What madness? This does not change at all with my patches, these
>>> duplicates already exist in the current code.
>>
>> That doesn't make it any better, and was cause for issue as Chris
>> demonstrated.  Can we get rid of that duplication please?
>>
>
> 4/4 in my v2 removes the redundant symbols.

... actually, that only removes the redundant __stub_start, and the
redundant __vectors_start is already removed in patch #1. Patch #4
(v2) may be shot down for other reasons, in that case I can simply
move the removal of __stubs_start to patch #1 instead.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 1/3] ARM: move .vectors and .stubs sections back into the kernel VMA
  2016-02-03  9:16         ` Ard Biesheuvel
@ 2016-02-08 16:39           ` Russell King - ARM Linux
  2016-02-08 17:00             ` Russell King - ARM Linux
  2016-02-08 17:01             ` Ard Biesheuvel
  0 siblings, 2 replies; 29+ messages in thread
From: Russell King - ARM Linux @ 2016-02-08 16:39 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Feb 03, 2016 at 10:16:46AM +0100, Ard Biesheuvel wrote:
> There are 34 section headers, starting at offset 0x10a6e60:
> 
> Section Headers:
>   [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
>   [15] .notes            NOTE            c0c34020 a34020 000024 00  AX  0   0  4
>   [16] .vectors          PROGBITS        00000000 a40000 000020 00  AX  0   0  2
>   [17] .stubs            PROGBITS        00001000 a41000 0002c0 00  AX  0   0 32
>   [18] .init.text        PROGBITS        c0c352e0 a452e0 06b1b8 00  AX  0   0 32
...
> Section Headers:
>   [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
>   [15] .notes            NOTE            c0c34020 a34020 000024 00  AX  0   0  4
>   [16] .stubs            PROGBITS        c0c35000 a35000 0002c0 00  AX  0   0 32
>   [17] .vectors          PROGBITS        c0c34000 a44000 000020 00  AX  0   0  2
>   [18] .init.text        PROGBITS        c0c352e0 a452e0 06b1b8 00  AX  0   0 32

Sorry, but I'm going to re-iterate my objection, and I believe that
because you're looking at readelf's output, you're making assumptions
when interpreting its output.

These figures do not truely describe how the file is laid out.  The
tool to use for this is objdump -h.  Let's compare:

$ readelf -S vmlinux
Section Headers:
  [Nr] Name        Type         Addr     Off    Size   ES Flg Lk Inf Al
  [13] .notes      NOTE         c093afd0 93afd0 000024 00  AX  0   0  4
  [14] .vectors    PROGBITS     00000000 940000 000020 00  AX  0   0  4
  [15] .stubs      PROGBITS     00001000 941000 0002c0 00  AX  0   0 32
  [16] .init.text  PROGBITS     c093b2e0 94b2e0 04862c 00  AX  0   0 32

$ objdump -h vmlinux
Sections:
Idx Name          Size      VMA       LMA       File off  Algn
 12 .notes        00000024  c093afd0  c093afd0  0093afd0  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
 13 .vectors      00000020  00000000  c093b000  00940000  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
 14 .stubs        000002c0  00001000  c093b020  00941000  2**5
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
 15 .init.text    0004862c  c093b2e0  c093b2e0  0094b2e0  2**5
                  CONTENTS, ALLOC, LOAD, READONLY, CODE

Note that objdump -h has one additional piece of information here: the
LMA - load memory address.  This tells us where the file contents
(offset into the ELF file at "File off") are to be loaded into memory,
as opposed to the VMA which describes where (at run time) the object is
to be found.

With this new information, we can see that .vectors is loaded to
0xc093b000, and occupies only 32 bytes in the binary memory image, even
though it occupies 4k in the ELF file.)  .stubs is loaded at 0xc093b020,
and occupies 0x2c0 bytes.  Furthermore, we can see .init.text is located
at 0xc093b2e0.

So, there is no memory wastage here - .vectors, .stubs and the init text
are all packed tightly together.  That's something which the readelf -S
output doesn't show us.

Please, replace the readelf -S output with objdump -h output so that we
can see what's /really/ going on here.

In any case, I still don't like your patch.  At least having these at 0
and 0x1000 means that (for some CPUs) they are located at the real VMA
address that they may appear at, though for modern CPUs, locating them
at 0xffff0000 and 0xffff1000 VMA would be more reasonable.  This has the
advantage that (jtag) debuggers are able to correctly parse the vmlinux
file and insert breakpoints into the vectors.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 1/3] ARM: move .vectors and .stubs sections back into the kernel VMA
  2016-02-08 16:39           ` Russell King - ARM Linux
@ 2016-02-08 17:00             ` Russell King - ARM Linux
  2016-02-08 17:01             ` Ard Biesheuvel
  1 sibling, 0 replies; 29+ messages in thread
From: Russell King - ARM Linux @ 2016-02-08 17:00 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Feb 08, 2016 at 04:39:35PM +0000, Russell King - ARM Linux wrote:
> On Wed, Feb 03, 2016 at 10:16:46AM +0100, Ard Biesheuvel wrote:
> > There are 34 section headers, starting at offset 0x10a6e60:
> > 
> > Section Headers:
> >   [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
> >   [15] .notes            NOTE            c0c34020 a34020 000024 00  AX  0   0  4
> >   [16] .vectors          PROGBITS        00000000 a40000 000020 00  AX  0   0  2
> >   [17] .stubs            PROGBITS        00001000 a41000 0002c0 00  AX  0   0 32
> >   [18] .init.text        PROGBITS        c0c352e0 a452e0 06b1b8 00  AX  0   0 32
> ...
> > Section Headers:
> >   [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
> >   [15] .notes            NOTE            c0c34020 a34020 000024 00  AX  0   0  4
> >   [16] .stubs            PROGBITS        c0c35000 a35000 0002c0 00  AX  0   0 32
> >   [17] .vectors          PROGBITS        c0c34000 a44000 000020 00  AX  0   0  2
> >   [18] .init.text        PROGBITS        c0c352e0 a452e0 06b1b8 00  AX  0   0 32
> 
> Sorry, but I'm going to re-iterate my objection, and I believe that
> because you're looking at readelf's output, you're making assumptions
> when interpreting its output.
> 
> These figures do not truely describe how the file is laid out.  The
> tool to use for this is objdump -h.  Let's compare:
> 
> $ readelf -S vmlinux
> Section Headers:
>   [Nr] Name        Type         Addr     Off    Size   ES Flg Lk Inf Al
>   [13] .notes      NOTE         c093afd0 93afd0 000024 00  AX  0   0  4
>   [14] .vectors    PROGBITS     00000000 940000 000020 00  AX  0   0  4
>   [15] .stubs      PROGBITS     00001000 941000 0002c0 00  AX  0   0 32
>   [16] .init.text  PROGBITS     c093b2e0 94b2e0 04862c 00  AX  0   0 32
> 
> $ objdump -h vmlinux
> Sections:
> Idx Name          Size      VMA       LMA       File off  Algn
>  12 .notes        00000024  c093afd0  c093afd0  0093afd0  2**2
>                   CONTENTS, ALLOC, LOAD, READONLY, CODE
>  13 .vectors      00000020  00000000  c093b000  00940000  2**2
>                   CONTENTS, ALLOC, LOAD, READONLY, CODE
>  14 .stubs        000002c0  00001000  c093b020  00941000  2**5
>                   CONTENTS, ALLOC, LOAD, READONLY, CODE
>  15 .init.text    0004862c  c093b2e0  c093b2e0  0094b2e0  2**5
>                   CONTENTS, ALLOC, LOAD, READONLY, CODE
> 
> Note that objdump -h has one additional piece of information here: the
> LMA - load memory address.  This tells us where the file contents
> (offset into the ELF file at "File off") are to be loaded into memory,
> as opposed to the VMA which describes where (at run time) the object is
> to be found.
> 
> With this new information, we can see that .vectors is loaded to
> 0xc093b000, and occupies only 32 bytes in the binary memory image, even
> though it occupies 4k in the ELF file.)  .stubs is loaded at 0xc093b020,
> and occupies 0x2c0 bytes.  Furthermore, we can see .init.text is located
> at 0xc093b2e0.
> 
> So, there is no memory wastage here - .vectors, .stubs and the init text
> are all packed tightly together.  That's something which the readelf -S
> output doesn't show us.
> 
> Please, replace the readelf -S output with objdump -h output so that we
> can see what's /really/ going on here.
> 
> In any case, I still don't like your patch.  At least having these at 0
> and 0x1000 means that (for some CPUs) they are located at the real VMA
> address that they may appear at, though for modern CPUs, locating them
> at 0xffff0000 and 0xffff1000 VMA would be more reasonable.  This has the
> advantage that (jtag) debuggers are able to correctly parse the vmlinux
> file and insert breakpoints into the vectors.

Another down-side to your patch is that you're placing the VMA of
.vectors 4096 bytes below .stubs - but we can have other symbols in
that VMA space.  That means you're creating the very real possibility
of confusing output we have real data at the location where you're
placing .vectors.

So no, I _really_ don't like this.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH 1/3] ARM: move .vectors and .stubs sections back into the kernel VMA
  2016-02-08 16:39           ` Russell King - ARM Linux
  2016-02-08 17:00             ` Russell King - ARM Linux
@ 2016-02-08 17:01             ` Ard Biesheuvel
  1 sibling, 0 replies; 29+ messages in thread
From: Ard Biesheuvel @ 2016-02-08 17:01 UTC (permalink / raw)
  To: linux-arm-kernel

On 8 February 2016 at 17:39, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Wed, Feb 03, 2016 at 10:16:46AM +0100, Ard Biesheuvel wrote:
>> There are 34 section headers, starting at offset 0x10a6e60:
>>
>> Section Headers:
>>   [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
>>   [15] .notes            NOTE            c0c34020 a34020 000024 00  AX  0   0  4
>>   [16] .vectors          PROGBITS        00000000 a40000 000020 00  AX  0   0  2
>>   [17] .stubs            PROGBITS        00001000 a41000 0002c0 00  AX  0   0 32
>>   [18] .init.text        PROGBITS        c0c352e0 a452e0 06b1b8 00  AX  0   0 32
> ...
>> Section Headers:
>>   [Nr] Name              Type            Addr     Off    Size   ES Flg Lk Inf Al
>>   [15] .notes            NOTE            c0c34020 a34020 000024 00  AX  0   0  4
>>   [16] .stubs            PROGBITS        c0c35000 a35000 0002c0 00  AX  0   0 32
>>   [17] .vectors          PROGBITS        c0c34000 a44000 000020 00  AX  0   0  2
>>   [18] .init.text        PROGBITS        c0c352e0 a452e0 06b1b8 00  AX  0   0 32
>
> Sorry, but I'm going to re-iterate my objection, and I believe that
> because you're looking at readelf's output, you're making assumptions
> when interpreting its output.
>
> These figures do not truely describe how the file is laid out.  The
> tool to use for this is objdump -h.  Let's compare:
>
> $ readelf -S vmlinux
> Section Headers:
>   [Nr] Name        Type         Addr     Off    Size   ES Flg Lk Inf Al
>   [13] .notes      NOTE         c093afd0 93afd0 000024 00  AX  0   0  4
>   [14] .vectors    PROGBITS     00000000 940000 000020 00  AX  0   0  4
>   [15] .stubs      PROGBITS     00001000 941000 0002c0 00  AX  0   0 32
>   [16] .init.text  PROGBITS     c093b2e0 94b2e0 04862c 00  AX  0   0 32
>
> $ objdump -h vmlinux
> Sections:
> Idx Name          Size      VMA       LMA       File off  Algn
>  12 .notes        00000024  c093afd0  c093afd0  0093afd0  2**2
>                   CONTENTS, ALLOC, LOAD, READONLY, CODE
>  13 .vectors      00000020  00000000  c093b000  00940000  2**2
>                   CONTENTS, ALLOC, LOAD, READONLY, CODE
>  14 .stubs        000002c0  00001000  c093b020  00941000  2**5
>                   CONTENTS, ALLOC, LOAD, READONLY, CODE
>  15 .init.text    0004862c  c093b2e0  c093b2e0  0094b2e0  2**5
>                   CONTENTS, ALLOC, LOAD, READONLY, CODE
>
> Note that objdump -h has one additional piece of information here: the
> LMA - load memory address.  This tells us where the file contents
> (offset into the ELF file at "File off") are to be loaded into memory,
> as opposed to the VMA which describes where (at run time) the object is
> to be found.
>
> With this new information, we can see that .vectors is loaded to
> 0xc093b000, and occupies only 32 bytes in the binary memory image, even
> though it occupies 4k in the ELF file.)  .stubs is loaded at 0xc093b020,
> and occupies 0x2c0 bytes.  Furthermore, we can see .init.text is located
> at 0xc093b2e0.
>
> So, there is no memory wastage here - .vectors, .stubs and the init text
> are all packed tightly together.  That's something which the readelf -S
> output doesn't show us.
>
> Please, replace the readelf -S output with objdump -h output so that we
> can see what's /really/ going on here.
>

OK, fair enough.

Before:

Sections:
Idx Name          Size      VMA       LMA       File off  Algn
 14 .notes        00000024  c0c90030  c0c90030  00a90030  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
 15 .vectors      00000020  00000000  c0c91000  00aa0000  2**1
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
 16 .stubs        000002c0  00001000  c0c91020  00aa1000  2**5
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
 17 .init.text    0006b250  c0c912e0  c0c912e0  00aa12e0  2**5
                  CONTENTS, ALLOC, LOAD, READONLY, CODE

and after

 14 .notes        00000024  c0c90030  c0c90030  00a90030  2**2
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
 15 .stubs        000002c0  c0c91000  c0c91000  00a91000  2**5
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
 16 .vectors      00000020  c0c90000  c0c912c0  00aa0000  2**1
                  CONTENTS, ALLOC, LOAD, READONLY, CODE
 17 .init.text    0006b250  c0c912e0  c0c912e0  00aa12e0  2**5
                  CONTENTS, ALLOC, LOAD, READONLY, CODE

So, apart from the fact that .vectors and .stubs are reordered,
nothing changes, and the LMA footprint is identical.

> In any case, I still don't like your patch.  At least having these at 0
> and 0x1000 means that (for some CPUs) they are located at the real VMA
> address that they may appear at, though for modern CPUs, locating them
> at 0xffff0000 and 0xffff1000 VMA would be more reasonable.  This has the
> advantage that (jtag) debuggers are able to correctly parse the vmlinux
> file and insert breakpoints into the vectors.
>

0xffff0000 and 0xffff1000 work for me. I am trying to get rid of this
dodgy workaround in scripts/link-vmlinux.sh(85):

if [ -n "${CONFIG_ARM}" ] && [ -z "${CONFIG_XIP_KERNEL}" ] && [ -n
"${CONFIG_PAGE_OFFSET}" ]; then
    kallsymopt="${kallsymopt} --page-offset=$CONFIG_PAGE_OFFSET"
fi

which is there because perf does not like seeing the zero based kernel
symbol addresses, and only fixes the XIP_KERNEL=n case anyway.

Alternatively, this issue can be solved by making all stubs symbols
strictly local (or absolute), i.e., something like below. Would that
be preferable to you? It also fixes the perf issue for both
XIP_KERNEL=y and =n, but keeps .vectors and .stubs where they are.

-- 
Ard.


---------8<------------
diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index 788e40c1254f..c5c50c70c080 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -1027,7 +1027,7 @@ __kuser_helper_end:
  .macro vector_stub, name, mode, correction=0
  .align 5

-vector_\name:
+.Lvector_\name:
  .if \correction
  sub lr, lr, #\correction
  .endif
@@ -1056,7 +1056,7 @@ vector_\name:
  mov r0, sp
  ARM( ldr lr, [pc, lr, lsl #2] )
  movs pc, lr @ branch to handler in SVC mode
-ENDPROC(vector_\name)
+ENDPROC(.Lvector_\name)

  .align 2
  @ handler addresses follow this label
@@ -1064,14 +1064,15 @@ ENDPROC(vector_\name)
  .endm

  .section .stubs, "ax", %progbits
+.Lstubs_start:
  @ This must be the first word
  .word vector_swi

-vector_rst:
+.Lvector_rst:
  ARM( swi SYS_ERROR0 )
  THUMB( svc #0 )
  THUMB( nop )
- b vector_und
+ b .Lvector_und

 /*
  * Interrupt dispatcher
@@ -1173,8 +1174,8 @@ vector_rst:
  * (they're not supposed to happen, and won't happen in 32-bit data mode).
  */

-vector_addrexcptn:
- b vector_addrexcptn
+.Lvector_addrexcptn:
+ b .Lvector_addrexcptn

 /*=============================================================================
  * FIQ "NMI" handler
@@ -1202,18 +1203,18 @@ vector_addrexcptn:
  .long __fiq_svc @  f

  .globl vector_fiq_offset
- .equ vector_fiq_offset, vector_fiq
+ .equ vector_fiq_offset, .Lvector_fiq - .Lstubs_start + 0x1000

  .section .vectors, "ax", %progbits
 .L__vectors_start:
- W(b) vector_rst
- W(b) vector_und
+ W(b) .Lvector_rst
+ W(b) .Lvector_und
  W(ldr) pc, .L__vectors_start + 0x1000
- W(b) vector_pabt
- W(b) vector_dabt
- W(b) vector_addrexcptn
- W(b) vector_irq
- W(b) vector_fiq
+ W(b) .Lvector_pabt
+ W(b) .Lvector_dabt
+ W(b) .Lvector_addrexcptn
+ W(b) .Lvector_irq
+ W(b) .Lvector_fiq

  .data

^ permalink raw reply related	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2016-02-08 17:01 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-02 13:19 [PATCH 0/3] kallsyms: remove special handling for CONFIG_ARM Ard Biesheuvel
2016-02-02 13:19 ` [PATCH 1/3] ARM: move .vectors and .stubs sections back into the kernel VMA Ard Biesheuvel
2016-02-03  0:03   ` Russell King - ARM Linux
2016-02-03  0:20     ` Nicolas Pitre
2016-02-03  7:56     ` Ard Biesheuvel
2016-02-03  9:13       ` Russell King - ARM Linux
2016-02-03  9:16         ` Ard Biesheuvel
2016-02-08 16:39           ` Russell King - ARM Linux
2016-02-08 17:00             ` Russell King - ARM Linux
2016-02-08 17:01             ` Ard Biesheuvel
2016-02-02 13:19 ` [PATCH 2/3] kallsyms: remove special lower address limit for CONFIG_ARM Ard Biesheuvel
2016-02-02 13:19 ` [PATCH 3/3] kallsyms: remove --page-offset command line option Ard Biesheuvel
2016-02-03  0:05   ` Russell King - ARM Linux
2016-02-03  8:16     ` Ard Biesheuvel
2016-02-03  9:53       ` Maxime Coquelin
2016-02-02 17:51 ` [PATCH 0/3] kallsyms: remove special handling for CONFIG_ARM Nicolas Pitre
2016-02-02 18:59 ` Chris Brandt
2016-02-02 19:00   ` Ard Biesheuvel
2016-02-02 19:13     ` Chris Brandt
2016-02-03 13:33     ` Chris Brandt
2016-02-03 13:41       ` Ard Biesheuvel
2016-02-03 14:15         ` Chris Brandt
2016-02-03 14:17           ` Ard Biesheuvel
2016-02-03 20:01         ` Russell King - ARM Linux
2016-02-03 20:02           ` Ard Biesheuvel
2016-02-03 20:23             ` Nicolas Pitre
2016-02-03 20:30               ` Ard Biesheuvel
2016-02-03 20:41                 ` Ard Biesheuvel
2016-02-03 20:14 ` Linus Walleij

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.