| Message ID | 1393413080-26942-1-git-send-email-u.kleine-koenig@pengutronix.de |
|---|---|
| State | New |
| Headers | show |
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?
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
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
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
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
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.
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} \
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@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(-)