[PATCH] arm64: annotate psci invoke functions as notrace

[PATCH] arm64: annotate psci invoke functions as notrace

[Kyle McMartin]

Using GCC 5 to build the kernel with ftrace enabled, we encounter the
following error as a result of the mcount prologue changing the expected
register use of the function parameters,

/tmp/cc8Kpn7A.s: Assembler messages:
/tmp/cc8Kpn7A.s:41: Error: .err encountered
/tmp/cc8Kpn7A.s:42: Error: .err encountered
/tmp/cc8Kpn7A.s:43: Error: .err encountered
/tmp/cc8Kpn7A.s:101: Error: .err encountered
/tmp/cc8Kpn7A.s:102: Error: .err encountered
/tmp/cc8Kpn7A.s:103: Error: .err encountered
scripts/Makefile.build:257: recipe for target 'arch/arm64/kernel/psci.o' failed

Fix this by annotating the function as notrace, to suppress the
generation of profiling prologues and epilogues on the function.

Signed-off-by: Kyle McMartin <kyle@redhat.com>

--- a/arch/arm64/kernel/psci.c
+++ b/arch/arm64/kernel/psci.c
@@ -113,7 +113,7 @@ static void psci_power_state_unpack(u32 power_state,
  * The following two functions are invoked via the invoke_psci_fn pointer
  * and will not be inlined, allowing us to piggyback on the AAPCS.
  */
-static noinline int __invoke_psci_fn_hvc(u64 function_id, u64 arg0, u64 arg1,
+static noinline notrace int __invoke_psci_fn_hvc(u64 function_id, u64 arg0, u64 arg1,
 					 u64 arg2)
 {
 	asm volatile(
@@ -128,7 +128,7 @@ static noinline int __invoke_psci_fn_hvc(u64 function_id, u64 arg0, u64 arg1,
 	return function_id;
 }
 
-static noinline int __invoke_psci_fn_smc(u64 function_id, u64 arg0, u64 arg1,
+static noinline notrace int __invoke_psci_fn_smc(u64 function_id, u64 arg0, u64 arg1,
 					 u64 arg2)
 {
 	asm volatile(

Re: [PATCH] arm64: annotate psci invoke functions as notrace

[Mark Rutland]

Hi Kyle,

On Wed, Feb 18, 2015 at 05:26:38PM +0000, Kyle McMartin wrote:
> Using GCC 5 to build the kernel with ftrace enabled, we encounter the
> following error as a result of the mcount prologue changing the expected
> register use of the function parameters,
> 
> /tmp/cc8Kpn7A.s: Assembler messages:
> /tmp/cc8Kpn7A.s:41: Error: .err encountered
> /tmp/cc8Kpn7A.s:42: Error: .err encountered
> /tmp/cc8Kpn7A.s:43: Error: .err encountered
> /tmp/cc8Kpn7A.s:101: Error: .err encountered
> /tmp/cc8Kpn7A.s:102: Error: .err encountered
> /tmp/cc8Kpn7A.s:103: Error: .err encountered
> scripts/Makefile.build:257: recipe for target 'arch/arm64/kernel/psci.o' failed
> 
> Fix this by annotating the function as notrace, to suppress the
> generation of profiling prologues and epilogues on the function.
> 
> Signed-off-by: Kyle McMartin <kyle@redhat.com>

Will and I have patches for arm64 and arm, moving the invocations out to
separate assembly files as discussed when Andy Whitcroft reported the
issue previously [1].

Given that GCC 5 is still in development we were planning to post those
when -rc1 arrives.

Thanks,
Mark.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-January/319344.html

> 
> --- a/arch/arm64/kernel/psci.c
> +++ b/arch/arm64/kernel/psci.c
> @@ -113,7 +113,7 @@ static void psci_power_state_unpack(u32 power_state,
>   * The following two functions are invoked via the invoke_psci_fn pointer
>   * and will not be inlined, allowing us to piggyback on the AAPCS.
>   */
> -static noinline int __invoke_psci_fn_hvc(u64 function_id, u64 arg0, u64 arg1,
> +static noinline notrace int __invoke_psci_fn_hvc(u64 function_id, u64 arg0, u64 arg1,
>  					 u64 arg2)
>  {
>  	asm volatile(
> @@ -128,7 +128,7 @@ static noinline int __invoke_psci_fn_hvc(u64 function_id, u64 arg0, u64 arg1,
>  	return function_id;
>  }
>  
> -static noinline int __invoke_psci_fn_smc(u64 function_id, u64 arg0, u64 arg1,
> +static noinline notrace int __invoke_psci_fn_smc(u64 function_id, u64 arg0, u64 arg1,
>  					 u64 arg2)
>  {
>  	asm volatile(
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

Re: [PATCH] arm64: annotate psci invoke functions as notrace

[Richard W.M. Jones]

On Wed, Feb 18, 2015 at 12:26:38PM -0500, Kyle McMartin wrote:
> Using GCC 5 to build the kernel with ftrace enabled, we encounter the
> following error as a result of the mcount prologue changing the expected
> register use of the function parameters,
> 
> /tmp/cc8Kpn7A.s: Assembler messages:
> /tmp/cc8Kpn7A.s:41: Error: .err encountered
> /tmp/cc8Kpn7A.s:42: Error: .err encountered
> /tmp/cc8Kpn7A.s:43: Error: .err encountered
> /tmp/cc8Kpn7A.s:101: Error: .err encountered
> /tmp/cc8Kpn7A.s:102: Error: .err encountered
> /tmp/cc8Kpn7A.s:103: Error: .err encountered
> scripts/Makefile.build:257: recipe for target 'arch/arm64/kernel/psci.o' failed
> 
> Fix this by annotating the function as notrace, to suppress the
> generation of profiling prologues and epilogues on the function.
> 
> Signed-off-by: Kyle McMartin <kyle@redhat.com>
> 
> --- a/arch/arm64/kernel/psci.c
> +++ b/arch/arm64/kernel/psci.c
> @@ -113,7 +113,7 @@ static void psci_power_state_unpack(u32 power_state,
>   * The following two functions are invoked via the invoke_psci_fn pointer
>   * and will not be inlined, allowing us to piggyback on the AAPCS.
>   */
> -static noinline int __invoke_psci_fn_hvc(u64 function_id, u64 arg0, u64 arg1,
> +static noinline notrace int __invoke_psci_fn_hvc(u64 function_id, u64 arg0, u64 arg1,
>  					 u64 arg2)
>  {
>  	asm volatile(
> @@ -128,7 +128,7 @@ static noinline int __invoke_psci_fn_hvc(u64 function_id, u64 arg0, u64 arg1,
>  	return function_id;
>  }
>  
> -static noinline int __invoke_psci_fn_smc(u64 function_id, u64 arg0, u64 arg1,
> +static noinline notrace int __invoke_psci_fn_smc(u64 function_id, u64 arg0, u64 arg1,
>  					 u64 arg2)
>  {
>  	asm volatile(

I need this patch in order to compile the upstream kernel on aarch64
using gcc 5.  Can it not be added temporarily while the longer term
fix, whatever that is, is worked out?

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW

Re: [PATCH] arm64: annotate psci invoke functions as notrace

[Mark Rutland]

On Tue, Feb 24, 2015 at 05:59:50PM +0000, Richard W.M. Jones wrote:
> On Wed, Feb 18, 2015 at 12:26:38PM -0500, Kyle McMartin wrote:
> > Using GCC 5 to build the kernel with ftrace enabled, we encounter the
> > following error as a result of the mcount prologue changing the expected
> > register use of the function parameters,
> > 
> > /tmp/cc8Kpn7A.s: Assembler messages:
> > /tmp/cc8Kpn7A.s:41: Error: .err encountered
> > /tmp/cc8Kpn7A.s:42: Error: .err encountered
> > /tmp/cc8Kpn7A.s:43: Error: .err encountered
> > /tmp/cc8Kpn7A.s:101: Error: .err encountered
> > /tmp/cc8Kpn7A.s:102: Error: .err encountered
> > /tmp/cc8Kpn7A.s:103: Error: .err encountered
> > scripts/Makefile.build:257: recipe for target 'arch/arm64/kernel/psci.o' failed
> > 
> > Fix this by annotating the function as notrace, to suppress the
> > generation of profiling prologues and epilogues on the function.
> > 
> > Signed-off-by: Kyle McMartin <kyle@redhat.com>
> > 
> > --- a/arch/arm64/kernel/psci.c
> > +++ b/arch/arm64/kernel/psci.c
> > @@ -113,7 +113,7 @@ static void psci_power_state_unpack(u32 power_state,
> >   * The following two functions are invoked via the invoke_psci_fn pointer
> >   * and will not be inlined, allowing us to piggyback on the AAPCS.
> >   */
> > -static noinline int __invoke_psci_fn_hvc(u64 function_id, u64 arg0, u64 arg1,
> > +static noinline notrace int __invoke_psci_fn_hvc(u64 function_id, u64 arg0, u64 arg1,
> >  					 u64 arg2)
> >  {
> >  	asm volatile(
> > @@ -128,7 +128,7 @@ static noinline int __invoke_psci_fn_hvc(u64 function_id, u64 arg0, u64 arg1,
> >  	return function_id;
> >  }
> >  
> > -static noinline int __invoke_psci_fn_smc(u64 function_id, u64 arg0, u64 arg1,
> > +static noinline notrace int __invoke_psci_fn_smc(u64 function_id, u64 arg0, u64 arg1,
> >  					 u64 arg2)
> >  {
> >  	asm volatile(
> 
> I need this patch in order to compile the upstream kernel on aarch64
> using gcc 5.  Can it not be added temporarily while the longer term
> fix, whatever that is, is worked out?

As I mentioned in my reply, Will was waiting for -rc1 to post our
patches (which move this out to asm for arm and arm64). He's out of the
office today, but I expect they will be posted tomorrow (and hopefully
queued shortly thereafter).

Mark.

Re: [PATCH] arm64: annotate psci invoke functions as notrace

[Vinicius Tinti]

On Tue, Feb 24, 2015 at 3:11 PM, Mark Rutland <mark.rutland@arm.com> wrote:
> On Tue, Feb 24, 2015 at 05:59:50PM +0000, Richard W.M. Jones wrote:
>> On Wed, Feb 18, 2015 at 12:26:38PM -0500, Kyle McMartin wrote:
>> > Using GCC 5 to build the kernel with ftrace enabled, we encounter the
>> > following error as a result of the mcount prologue changing the expected
>> > register use of the function parameters,
>> >
>> > /tmp/cc8Kpn7A.s: Assembler messages:
>> > /tmp/cc8Kpn7A.s:41: Error: .err encountered
>> > /tmp/cc8Kpn7A.s:42: Error: .err encountered
>> > /tmp/cc8Kpn7A.s:43: Error: .err encountered
>> > /tmp/cc8Kpn7A.s:101: Error: .err encountered
>> > /tmp/cc8Kpn7A.s:102: Error: .err encountered
>> > /tmp/cc8Kpn7A.s:103: Error: .err encountered
>> > scripts/Makefile.build:257: recipe for target 'arch/arm64/kernel/psci.o' failed
>> >
>> > Fix this by annotating the function as notrace, to suppress the
>> > generation of profiling prologues and epilogues on the function.
>> >
>> > Signed-off-by: Kyle McMartin <kyle@redhat.com>
>> >
>> > --- a/arch/arm64/kernel/psci.c
>> > +++ b/arch/arm64/kernel/psci.c
>> > @@ -113,7 +113,7 @@ static void psci_power_state_unpack(u32 power_state,
>> >   * The following two functions are invoked via the invoke_psci_fn pointer
>> >   * and will not be inlined, allowing us to piggyback on the AAPCS.
>> >   */
>> > -static noinline int __invoke_psci_fn_hvc(u64 function_id, u64 arg0, u64 arg1,
>> > +static noinline notrace int __invoke_psci_fn_hvc(u64 function_id, u64 arg0, u64 arg1,
>> >                                      u64 arg2)
>> >  {
>> >     asm volatile(
>> > @@ -128,7 +128,7 @@ static noinline int __invoke_psci_fn_hvc(u64 function_id, u64 arg0, u64 arg1,
>> >     return function_id;
>> >  }
>> >
>> > -static noinline int __invoke_psci_fn_smc(u64 function_id, u64 arg0, u64 arg1,
>> > +static noinline notrace int __invoke_psci_fn_smc(u64 function_id, u64 arg0, u64 arg1,
>> >                                      u64 arg2)
>> >  {
>> >     asm volatile(
>>
>> I need this patch in order to compile the upstream kernel on aarch64
>> using gcc 5.  Can it not be added temporarily while the longer term
>> fix, whatever that is, is worked out?
>
> As I mentioned in my reply, Will was waiting for -rc1 to post our
> patches (which move this out to asm for arm and arm64). He's out of the
> office today, but I expect they will be posted tomorrow (and hopefully
> queued shortly thereafter).
>
> Mark.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Hi,

I notice that the mainline kernel moved these psci calls to a separate
file but I
was wondering how can it guarantee that the function register
placement will hold?

If you build the kernel with -O0 some function register allocation changes as
opposed to -O2 or if you use another compiler such as Clang. In LLVMLinux we
solved this by using one of Andy's solution which is to use register placement:

  register u32 function_id_r0 asm ("r0") = function_id;
  register u32 arg0_r1 asm ("r1") = arg0;
  register u32 arg1_r2 asm ("r2") = arg1;
  register u32 arg2_r3 asm ("r3") = arg2;

Sorry by the late reply.

Regards,
Tinti

-- 
Simplicity is the ultimate sophistication

Re: [PATCH] arm64: annotate psci invoke functions as notrace

[Mark Rutland]

> >> > -static noinline int __invoke_psci_fn_hvc(u64 function_id, u64 arg0, u64 arg1,
> >> > +static noinline notrace int __invoke_psci_fn_hvc(u64 function_id, u64 arg0, u64 arg1,
> >> >                                      u64 arg2)

[...]

> >> > -static noinline int __invoke_psci_fn_smc(u64 function_id, u64 arg0, u64 arg1,
> >> > +static noinline notrace int __invoke_psci_fn_smc(u64 function_id, u64 arg0, u64 arg1,
> >> >                                      u64 arg2)

[...]

> > As I mentioned in my reply, Will was waiting for -rc1 to post our
> > patches (which move this out to asm for arm and arm64). He's out of the
> > office today, but I expect they will be posted tomorrow (and hopefully
> > queued shortly thereafter).
> >
> > Mark.

[...]

> Hi,
> 
> I notice that the mainline kernel moved these psci calls to a separate
> file but I
> was wondering how can it guarantee that the function register
> placement will hold?

The commit in question [1] moves the issuing of the HVC and SMC into an
assembly file, and the C code calls these as opaque functions. 

Due to this the compiler *must* respect the AAPCS and place the values
into the expected registers (x0 to x3).

> If you build the kernel with -O0 some function register allocation changes as
> opposed to -O2 or if you use another compiler such as Clang.

Surely this is only true if all the functions live in the same
compilation unit? Or perhaps with LTO (surely this cannot modify
assembly which was not generated by the compiler)?

If the compiler is rearranging registers for a function call it knows
nothing about, in violation of the AAPCS, then that compiler sounds
broken.

> In LLVMLinux we solved this by using one of Andy's solution which is
> to use register placement:
> 
>   register u32 function_id_r0 asm ("r0") = function_id;
>   register u32 arg0_r1 asm ("r1") = arg0;
>   register u32 arg1_r2 asm ("r2") = arg1;
>   register u32 arg2_r3 asm ("r3") = arg2;

Surely this is only necessary when the call issuing function is
implemented in C?

Mark.

[1] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=f5e0a12ca2d939e47995f73428d9bf1ad372b289