[PATCH] scripts/link-vmlinux.sh: correct base address for some ARM special cases

[PATCH] scripts/link-vmlinux.sh: correct base address for some ARM special cases

[Uwe Kleine-König]

The toolchain problem described in commit f6537f2f0eba
(scripts/kallsyms: filter symbols not in kernel address space) makes it
necessary to filter out some kernel symbols from /proc/kallsyms to make
perf happy.

For that to work correctly the address used to filter should be just
below the kernel text segment. Depending on kernel configuration
CONFIG_PAGE_OFFSET is not the right address to use. See the comments
introduced by this commit for the glory details.

Fixes: b9b32bf70f2f (ARM: use linker magic for vectors and vector stubs)
Cc: stable at vger.kernel.org
Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
---
Hello,

note I'm not entirely sure if the diagnose "toolchain problem" is right.
That's what Arnd suggested in #armlinux. So maybe the commit log wants
some fixing.

This is probably stable material as 7122c3e9154b and f6537f2f0eba were
considered for stable, too. As b9b32bf70f2f has a stable annotation,
too, maybe older stable kernel also need fixing.

Other than that I wonder if "--page-offset" is a misnomer. Better use
"--kernel-start"?  I didn't do this change here because patches for
stable should be minimal. But maybe it's worth a followup patch?

Best regards
Uwe

 scripts/link-vmlinux.sh | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 2dcb37736d84..493b4ccdf5fc 100644
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -82,8 +82,24 @@ kallsyms()
 		kallsymopt="${kallsymopt} --all-symbols"
 	fi
 
-	if [ -n "${CONFIG_ARM}" ] && [ -n "${CONFIG_PAGE_OFFSET}" ]; then
-		kallsymopt="${kallsymopt} --page-offset=$CONFIG_PAGE_OFFSET"
+	if [ -n "${CONFIG_ARM}" ]; then
+		# There are some ARM toolchains that generate some internal
+		# symbols that then appear in /proc/kallsyms and these disturbe
+		# perf. So filter out all symbols below PAGE_OFFSET.
+		# ARM is a bit complicated here. The page-offset isn't
+		# completely determined in Kconfig as of 3.14-rc. Without MMU
+		# it's not CONFIG_PAGE_OFFSET that is used but (indirectly)
+		# CONFIG_DRAM_BASE. But for XIP we want the XIP PHYS_ADDR
+		# though.
+		if [ -n "${CONFIG_XIP_PHYS_ADDR}" ]; then
+			page_offset="${CONFIG_XIP_PHYS_ADDR}"
+		elif [ -z "${CONFIG_MMU}" ]; then
+			page_offset="${CONFIG_DRAM_BASE}"
+		else
+			page_offset="$CONFIG_PAGE_OFFSET"
+		fi
+
+		kallsymopt="${kallsymopt} --page-offset=$page_offset"
 	fi
 
 	local aflags="${KBUILD_AFLAGS} ${KBUILD_AFLAGS_KERNEL}               \
-- 
1.8.5.3

[PATCH] scripts/link-vmlinux.sh: correct base address for some ARM special cases

[Russell King - ARM Linux]

On Wed, Feb 26, 2014 at 12:11:20PM +0100, Uwe Kleine-K?nig wrote:
> The toolchain problem described in commit f6537f2f0eba
> (scripts/kallsyms: filter symbols not in kernel address space) makes it
> necessary to filter out some kernel symbols from /proc/kallsyms to make
> perf happy.
> 
> For that to work correctly the address used to filter should be just
> below the kernel text segment. Depending on kernel configuration
> CONFIG_PAGE_OFFSET is not the right address to use. See the comments
> introduced by this commit for the glory details.

Is there any reason why PAGE_OFFSET should not be DRAM_BASE for nommu?

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH] scripts/link-vmlinux.sh: correct base address for some ARM special cases

[Uwe Kleine-König]

On Wed, Feb 26, 2014 at 11:15:07AM +0000, Russell King - ARM Linux wrote:
> On Wed, Feb 26, 2014 at 12:11:20PM +0100, Uwe Kleine-K?nig wrote:
> > The toolchain problem described in commit f6537f2f0eba
> > (scripts/kallsyms: filter symbols not in kernel address space) makes it
> > necessary to filter out some kernel symbols from /proc/kallsyms to make
> > perf happy.
> > 
> > For that to work correctly the address used to filter should be just
> > below the kernel text segment. Depending on kernel configuration
> > CONFIG_PAGE_OFFSET is not the right address to use. See the comments
> > introduced by this commit for the glory details.
> 
> Is there any reason why PAGE_OFFSET should not be DRAM_BASE for nommu?
Nothing I'm aware of[1]. Talking to Arnd earlier today on irc:

1393409816 < ukleinek> arnd: PAGE_OFFSET doesn't make much sense on
		!MMU, does it?
1393409828 < arnd> no
1393409844 < arnd> but I think we just set it to PHYS_OFFSET normally
1393409864 < arnd> which does the right thing, except with XIP_KERNEL,
		where the kernel doesn't actually run from RAM

PAGE_OFFSET is only set to PLAT_PHYS_OFFSET (for !MMU) in
arch/arm/include/asm/memory.h which in turn is set to CONFIG_PHYS_OFFSET
which is:

	hex "Physical address of main memory" if MMU
	...
	default DRAM_BASE if !MMU

So yes, I think we can do:

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1595,6 +1595,7 @@ endchoice
 
 config PAGE_OFFSET
 	hex
+	default DRAM_BASE if !MMU
 	default 0x40000000 if VMSPLIT_1G
 	default 0x80000000 if VMSPLIT_2G
 	default 0xC0000000

and then drop the MMU case in the patch that is discussed here and also
simplify a few other locations. I'll test a bit with this and then send
it out with a few other fixes I have in the queue.

For stable I'd still suggest not to do this though but stick to the
original patch that started this thread.

Best regards
Uwe

[1] maybe apart from being only one of serveral things that could be
improved for !MMU.
-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH] scripts/link-vmlinux.sh: correct base address for some ARM special cases

[Russell King - ARM Linux]

On Wed, Feb 26, 2014 at 02:46:05PM +0100, Uwe Kleine-K?nig wrote:
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1595,6 +1595,7 @@ endchoice
>  
>  config PAGE_OFFSET
>  	hex
> +	default DRAM_BASE if !MMU
>  	default 0x40000000 if VMSPLIT_1G
>  	default 0x80000000 if VMSPLIT_2G
>  	default 0xC0000000

I'd prefer this actually:

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 24d65aae0491..09289d7b7f68 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1593,6 +1593,7 @@ config BL_SWITCHER_DUMMY_IF
 
 choice
 	prompt "Memory split"
+	depends on MMU
 	default VMSPLIT_3G
 	help
 	  Select the desired split between kernel and user memory.
@@ -1610,6 +1611,7 @@ endchoice
 
 config PAGE_OFFSET
 	hex
+	default PHYS_OFFSET if !MMU
 	default 0x40000000 if VMSPLIT_1G
 	default 0x80000000 if VMSPLIT_2G
 	default 0xC0000000
diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
index 8756e4bcdba0..5ccc4a627192 100644
--- a/arch/arm/include/asm/memory.h
+++ b/arch/arm/include/asm/memory.h
@@ -104,10 +104,6 @@
 #define END_MEM     		(UL(CONFIG_DRAM_BASE) + CONFIG_DRAM_SIZE)
 #endif
 
-#ifndef PAGE_OFFSET
-#define PAGE_OFFSET		PLAT_PHYS_OFFSET
-#endif
-
 /*
  * The module can be at any place in ram in nommu mode.
  */

It's more to the point.  The first hunk hides the "Memory split" option
which is irrelevent on !MMU.  We already know that PAGE_OFFSET should
be the same as PHYS_OFFSET for noMMU, so let's make that explicit.
Note that it already is by way of the bit in the last hunk - which as
a result of this change can now be removed... especially so as we have
nothing defining PAGE_OFFSET in arch/arm/*/include...

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH] scripts/link-vmlinux.sh: correct base address for some ARM special cases

[Arnd Bergmann]

On Wednesday 26 February 2014 13:53:52 Russell King - ARM Linux wrote:
> It's more to the point.  The first hunk hides the "Memory split" option
> which is irrelevent on !MMU.  We already know that PAGE_OFFSET should
> be the same as PHYS_OFFSET for noMMU, so let's make that explicit.
> Note that it already is by way of the bit in the last hunk - which as
> a result of this change can now be removed... especially so as we have
> nothing defining PAGE_OFFSET in arch/arm/*/include...

Looks good to me as well. I first wasn't sure about what this would do
for platforms that define their own PLAT_PHYS_OFFSET, but we they are
broken anyway if you set a DRAM_BASE that is not the same as
PLAT_PHYS_OFFSET.

	Arnd

[PATCH] scripts/link-vmlinux.sh: correct base address for some ARM special cases

[Uwe Kleine-König]

On Wed, Feb 26, 2014 at 01:53:52PM +0000, Russell King - ARM Linux wrote:
> On Wed, Feb 26, 2014 at 02:46:05PM +0100, Uwe Kleine-K?nig wrote:
> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > --- a/arch/arm/Kconfig
> > +++ b/arch/arm/Kconfig
> > @@ -1595,6 +1595,7 @@ endchoice
> >  
> >  config PAGE_OFFSET
> >  	hex
> > +	default DRAM_BASE if !MMU
> >  	default 0x40000000 if VMSPLIT_1G
> >  	default 0x80000000 if VMSPLIT_2G
> >  	default 0xC0000000
> 
> I'd prefer this actually:
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 24d65aae0491..09289d7b7f68 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1593,6 +1593,7 @@ config BL_SWITCHER_DUMMY_IF
>  
>  choice
>  	prompt "Memory split"
> +	depends on MMU
>  	default VMSPLIT_3G
>  	help
>  	  Select the desired split between kernel and user memory.
This hunk is already in my working copy, too :-)

> @@ -1610,6 +1611,7 @@ endchoice
>  
>  config PAGE_OFFSET
>  	hex
> +	default PHYS_OFFSET if !MMU
>  	default 0x40000000 if VMSPLIT_1G
>  	default 0x80000000 if VMSPLIT_2G
>  	default 0xC0000000
> diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
> index 8756e4bcdba0..5ccc4a627192 100644
> --- a/arch/arm/include/asm/memory.h
> +++ b/arch/arm/include/asm/memory.h
> @@ -104,10 +104,6 @@
>  #define END_MEM     		(UL(CONFIG_DRAM_BASE) + CONFIG_DRAM_SIZE)
>  #endif
>  
> -#ifndef PAGE_OFFSET
> -#define PAGE_OFFSET		PLAT_PHYS_OFFSET
> -#endif
> -
>  /*
>   * The module can be at any place in ram in nommu mode.
>   */
> 
> It's more to the point.  The first hunk hides the "Memory split" option
> which is irrelevent on !MMU.  We already know that PAGE_OFFSET should
> be the same as PHYS_OFFSET for noMMU, so let's make that explicit.
> Note that it already is by way of the bit in the last hunk - which as
> a result of this change can now be removed... especially so as we have
> nothing defining PAGE_OFFSET in arch/arm/*/include...
Looks reasonable. Maybe we can also get rid of PLAT_PHYS_OFFSET then.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH] scripts/link-vmlinux.sh: correct base address for some ARM special cases

[Arnd Bergmann]

On Wednesday 26 February 2014 15:06:32 Uwe Kleine-K?nig wrote:
> > It's more to the point.  The first hunk hides the "Memory split" option
> > which is irrelevent on !MMU.  We already know that PAGE_OFFSET should
> > be the same as PHYS_OFFSET for noMMU, so let's make that explicit.
> > Note that it already is by way of the bit in the last hunk - which as
> > a result of this change can now be removed... especially so as we have
> > nothing defining PAGE_OFFSET in arch/arm/*/include...
>
> Looks reasonable. Maybe we can also get rid of PLAT_PHYS_OFFSET then.

I think we still need PLAT_PHYS_OFFSET for the configurations that cannot
use ARM_PATCH_PHYS_VIRT, which are:

* Anything using XIP_KERNEL with MMU=y
* mach-realview with the custom __phys_to_virt hack
* ZBOOT_ROM

	Arnd

[PATCH] scripts/link-vmlinux.sh: correct base address for some ARM special cases

[Arnd Bergmann]

On Wednesday 26 February 2014 15:16:40 Arnd Bergmann wrote:
> On Wednesday 26 February 2014 15:06:32 Uwe Kleine-K?nig wrote:
> > > It's more to the point.  The first hunk hides the "Memory split" option
> > > which is irrelevent on !MMU.  We already know that PAGE_OFFSET should
> > > be the same as PHYS_OFFSET for noMMU, so let's make that explicit.
> > > Note that it already is by way of the bit in the last hunk - which as
> > > a result of this change can now be removed... especially so as we have
> > > nothing defining PAGE_OFFSET in arch/arm/*/include...
> >
> > Looks reasonable. Maybe we can also get rid of PLAT_PHYS_OFFSET then.
> 
> I think we still need PLAT_PHYS_OFFSET for the configurations that cannot
> use ARM_PATCH_PHYS_VIRT, which are:
> 
> * Anything using XIP_KERNEL with MMU=y
> * mach-realview with the custom __phys_to_virt hack
> * ZBOOT_ROM

Ah, nevermind. We need either PLAT_PHYS_OFFSET /or/ CONFIG_PHYS_OFFSET
for these case, but we could in theory drop the former.

	Arnd

[PATCH] scripts/link-vmlinux.sh: correct base address for some ARM special cases

[Russell King - ARM Linux]

On Wed, Feb 26, 2014 at 03:06:32PM +0100, Uwe Kleine-K?nig wrote:
> Looks reasonable. Maybe we can also get rid of PLAT_PHYS_OFFSET then.

I don't think so - try grep.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.