| Message ID | 1428529265-26772-1-git-send-email-nathan_lynch@mentor.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 04/08/2015 04:41 PM, Nathan Lynch wrote: > (Arnd reported a build break with the VDSO code when targeting v4 (but > not v4t). I haven't been able to recreate it locally because all the > toolchains I have at hand convert 'bx lr' to 'mov pc,lr' when > targeting v4.) > > The __get_datapage routine in the VDSO uses 'bx lr' to return to the > caller. This is inappropriate when targeting v4 CPUs, and the VDSO is > unlikely to be useful for pre-v7 CPUs anyway due to its reliance on > the generic timers extension, so the easy thing to do here is just > make CONFIG_VDSO depend on CONFIG_CPU_V7. > > An alternative considered was to use 'ldr pc,lr' when armv4 (or lower) > is enabled, but Arnd pointed out that this would be broken when > running with a kernel that supports both v4 arnd v4t, and you have a > thumb user space. > > Reported-by: Arnd Bergmann <arnd@arndb.de> > Signed-off-by: Nathan Lynch <nathan_lynch@mentor.com> > --- > arch/arm/mm/Kconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig > index b7644310236b..b4f92b9a13ac 100644 > --- a/arch/arm/mm/Kconfig > +++ b/arch/arm/mm/Kconfig > @@ -827,7 +827,7 @@ config KUSER_HELPERS > > config VDSO > bool "Enable VDSO for acceleration of some system calls" > - depends on AEABI && MMU > + depends on AEABI && MMU && CPU_V7 > default y if ARM_ARCH_TIMER > select GENERIC_TIME_VSYSCALL > help Before I put this in RMK's patch queue I'd prefer to get an ack or more details (kernel config, toolchain) on the build failure, since I've been unable to recreate it. Arnd?
On Thursday 16 April 2015 10:25:50 Nathan Lynch wrote: > > diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig > > index b7644310236b..b4f92b9a13ac 100644 > > --- a/arch/arm/mm/Kconfig > > +++ b/arch/arm/mm/Kconfig > > @@ -827,7 +827,7 @@ config KUSER_HELPERS > > > > config VDSO > > bool "Enable VDSO for acceleration of some system calls" > > - depends on AEABI && MMU > > + depends on AEABI && MMU && CPU_V7 > > default y if ARM_ARCH_TIMER > > select GENERIC_TIME_VSYSCALL > > help > > Before I put this in RMK's patch queue I'd prefer to get an ack or more > details (kernel config, toolchain) on the build failure, since I've been > unable to recreate it. Arnd? > Good idea. I checked the patch against the failed randconfig, and must unfortunately report that it does not fix the problem. I'm attaching the .config to this email. Two things to note: - this is building for mach-rpc, so CONFIG_CPU_32v3 is enabled, and presumably this is required for reproducing the problem, while building for ARMv4 as I previously said in error does not cause the problem. - I have verified that CONFIG_VDSO is disabled here, but the file is still getting compiled. Arnd
On Thu, Apr 16, 2015 at 05:42:18PM +0200, Arnd Bergmann wrote: > On Thursday 16 April 2015 10:25:50 Nathan Lynch wrote: > > > diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig > > > index b7644310236b..b4f92b9a13ac 100644 > > > --- a/arch/arm/mm/Kconfig > > > +++ b/arch/arm/mm/Kconfig > > > @@ -827,7 +827,7 @@ config KUSER_HELPERS > > > > > > config VDSO > > > bool "Enable VDSO for acceleration of some system calls" > > > - depends on AEABI && MMU > > > + depends on AEABI && MMU && CPU_V7 > > > default y if ARM_ARCH_TIMER > > > select GENERIC_TIME_VSYSCALL > > > help > > > > Before I put this in RMK's patch queue I'd prefer to get an ack or more > > details (kernel config, toolchain) on the build failure, since I've been > > unable to recreate it. Arnd? > > > > Good idea. I checked the patch against the failed randconfig, and must > unfortunately report that it does not fix the problem. > > I'm attaching the .config to this email. Two things to note: > > - this is building for mach-rpc, so CONFIG_CPU_32v3 is enabled, and > presumably this is required for reproducing the problem, while > building for ARMv4 as I previously said in error does not cause > the problem. Why doesn't it solve the problem? In your config file, CPU_V7 is not set, so VDSO won't be set either. Are you absolutely sure you tested with the above patch applied?
On 04/16/2015 11:10 AM, Russell King - ARM Linux wrote: > On Thu, Apr 16, 2015 at 05:42:18PM +0200, Arnd Bergmann wrote: >> On Thursday 16 April 2015 10:25:50 Nathan Lynch wrote: >>>> diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig >>>> index b7644310236b..b4f92b9a13ac 100644 >>>> --- a/arch/arm/mm/Kconfig >>>> +++ b/arch/arm/mm/Kconfig >>>> @@ -827,7 +827,7 @@ config KUSER_HELPERS >>>> >>>> config VDSO >>>> bool "Enable VDSO for acceleration of some system calls" >>>> - depends on AEABI && MMU >>>> + depends on AEABI && MMU && CPU_V7 >>>> default y if ARM_ARCH_TIMER >>>> select GENERIC_TIME_VSYSCALL >>>> help >>> >>> Before I put this in RMK's patch queue I'd prefer to get an ack or more >>> details (kernel config, toolchain) on the build failure, since I've been >>> unable to recreate it. Arnd? >>> >> >> Good idea. I checked the patch against the failed randconfig, and must >> unfortunately report that it does not fix the problem. >> >> I'm attaching the .config to this email. Two things to note: >> >> - this is building for mach-rpc, so CONFIG_CPU_32v3 is enabled, and >> presumably this is required for reproducing the problem, while >> building for ARMv4 as I previously said in error does not cause >> the problem. > > Why doesn't it solve the problem? > > In your config file, CPU_V7 is not set, so VDSO won't be set either. > Are you absolutely sure you tested with the above patch applied? I'm puzzled as well. Using this config, I can force the assembler error: arch/arm/vdso/datapage.S: Assembler messages: arch/arm/vdso/datapage.S:13: Error: selected processor does not support ARM mode `bx lr' if I do 'make arch/arm/vdso/' but otherwise the build system doesn't seem to enter that directory here. Still looking into it though.
On Thursday 16 April 2015 11:33:11 Nathan Lynch wrote: > > if I do 'make arch/arm/vdso/' but otherwise the build system doesn't > seem to enter that directory here. > > Still looking into it though. > Ah, my mistake. I typed 'make arch/arm/vdso' and expected the files to all be left out, but instead the way the Makefile is written, it does not enter the directory at all when VDSO is turned off. I tried it again now, building the kernel normally and that works, so Acked-by: Arnd Bergmann <arnd@arndb.de> Arn
On 04/16/2015 12:14 PM, Arnd Bergmann wrote: > On Thursday 16 April 2015 11:33:11 Nathan Lynch wrote: >> >> if I do 'make arch/arm/vdso/' but otherwise the build system doesn't >> seem to enter that directory here. >> >> Still looking into it though. >> > > Ah, my mistake. I typed 'make arch/arm/vdso' and expected the files > to all be left out, but instead the way the Makefile is written, it > does not enter the directory at all when VDSO is turned off. > > I tried it again now, building the kernel normally and that works, so > > Acked-by: Arnd Bergmann <arnd@arndb.de> Thanks, in light of our discussion (esp. the v4 vs v3 confusion) I've amended the commit message but added your ack. Submitted as 8342/1.
diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig index b7644310236b..b4f92b9a13ac 100644 --- a/arch/arm/mm/Kconfig +++ b/arch/arm/mm/Kconfig @@ -827,7 +827,7 @@ config KUSER_HELPERS config VDSO bool "Enable VDSO for acceleration of some system calls" - depends on AEABI && MMU + depends on AEABI && MMU && CPU_V7 default y if ARM_ARCH_TIMER select GENERIC_TIME_VSYSCALL help
(Arnd reported a build break with the VDSO code when targeting v4 (but not v4t). I haven't been able to recreate it locally because all the toolchains I have at hand convert 'bx lr' to 'mov pc,lr' when targeting v4.) The __get_datapage routine in the VDSO uses 'bx lr' to return to the caller. This is inappropriate when targeting v4 CPUs, and the VDSO is unlikely to be useful for pre-v7 CPUs anyway due to its reliance on the generic timers extension, so the easy thing to do here is just make CONFIG_VDSO depend on CONFIG_CPU_V7. An alternative considered was to use 'ldr pc,lr' when armv4 (or lower) is enabled, but Arnd pointed out that this would be broken when running with a kernel that supports both v4 arnd v4t, and you have a thumb user space. Reported-by: Arnd Bergmann <arnd@arndb.de> Signed-off-by: Nathan Lynch <nathan_lynch@mentor.com> --- arch/arm/mm/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)