[PATCH] ARM: VDSO: depend on CPU_V7

[PATCH] ARM: VDSO: depend on CPU_V7

[Nathan Lynch]

(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
-- 
1.9.3

[PATCH] ARM: VDSO: depend on CPU_V7

[Nathan Lynch]

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?

[PATCH] ARM: VDSO: depend on CPU_V7

[Arnd Bergmann]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: randconfig-vds-bug.gz
Type: application/gzip
Size: 16868 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20150416/29cd4fb7/attachment-0001.gz>

[PATCH] ARM: VDSO: depend on CPU_V7

[Russell King - ARM Linux]

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?

-- 
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

[PATCH] ARM: VDSO: depend on CPU_V7

[Nathan Lynch]

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.

[PATCH] ARM: VDSO: depend on CPU_V7

[Arnd Bergmann]

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

[PATCH] ARM: VDSO: depend on CPU_V7

[Nathan Lynch]

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.