All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Rutland <mark.rutland@arm.com>
To: Vinicius Tinti <viniciustinti@gmail.com>
Cc: "Richard W.M. Jones" <rjones@redhat.com>,
	Will Deacon <Will.Deacon@arm.com>,
	Kyle McMartin <kyle@redhat.com>,
	Catalin Marinas <Catalin.Marinas@arm.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH] arm64: annotate psci invoke functions as notrace
Date: Mon, 20 Apr 2015 10:44:01 +0100	[thread overview]
Message-ID: <20150420094400.GC15875@leverpostej> (raw)
In-Reply-To: <CALD9WKxTcfDzfK9BuWFW00c1o4CJShSrmw_Z4Nikojhw9mG-Eg@mail.gmail.com>

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

WARNING: multiple messages have this Message-ID (diff)
From: mark.rutland@arm.com (Mark Rutland)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] arm64: annotate psci invoke functions as notrace
Date: Mon, 20 Apr 2015 10:44:01 +0100	[thread overview]
Message-ID: <20150420094400.GC15875@leverpostej> (raw)
In-Reply-To: <CALD9WKxTcfDzfK9BuWFW00c1o4CJShSrmw_Z4Nikojhw9mG-Eg@mail.gmail.com>

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

  reply	other threads:[~2015-04-20  9:44 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-02-18 17:26 [PATCH] arm64: annotate psci invoke functions as notrace Kyle McMartin
2015-02-18 17:26 ` Kyle McMartin
2015-02-18 17:39 ` Mark Rutland
2015-02-18 17:39   ` Mark Rutland
2015-02-24 17:59 ` Richard W.M. Jones
2015-02-24 17:59   ` Richard W.M. Jones
2015-02-24 18:11   ` Mark Rutland
2015-02-24 18:11     ` Mark Rutland
2015-04-19 11:40     ` Vinicius Tinti
2015-04-19 11:40       ` Vinicius Tinti
2015-04-20  9:44       ` Mark Rutland [this message]
2015-04-20  9:44         ` Mark Rutland

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150420094400.GC15875@leverpostej \
    --to=mark.rutland@arm.com \
    --cc=Catalin.Marinas@arm.com \
    --cc=Will.Deacon@arm.com \
    --cc=kyle@redhat.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rjones@redhat.com \
    --cc=viniciustinti@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.