From: Stefan Agner <stefan@agner.ch>
To: Russell King - ARM Linux <linux@armlinux.org.uk>
Cc: raj.khem@gmail.com, ulli.kroll@googlemail.com, joel@jms.id.au,
nico@linaro.org, arnd@arndb.de, linus.walleij@linaro.org,
linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] ARM: copypage-fa: add kto and kfrom to input operands list
Date: Tue, 16 Oct 2018 00:52:58 +0200 [thread overview]
Message-ID: <2802c0e549149872aa58da5fb54471fc@agner.ch> (raw)
In-Reply-To: <20181015224614.GB30658@n2100.armlinux.org.uk>
On 16.10.2018 00:46, Russell King - ARM Linux wrote:
> On Tue, Oct 16, 2018 at 12:39:54AM +0200, Stefan Agner wrote:
>> On 16.10.2018 00:23, Russell King - ARM Linux wrote:
>> > On Tue, Oct 16, 2018 at 12:16:29AM +0200, Stefan Agner wrote:
>> >> When functions incoming parameters are not in input operands list gcc
>> >> 4.5 does not load the parameters into registers before calling this
>> >> function but the inline assembly assumes valid addresses inside this
>> >> function. This breaks the code because r0 and r1 are invalid when
>> >> execution enters v4wb_copy_user_page ()
>> >
>> > NAK. Naked functions must never be inlined. Please add a "noinline"
>> > attribute to the function rather than making things more complex.
>> >
>>
>> To be honest, I did not put much thought into this commit since it is
>> just doing to copypage-fa.c what 9a40ac86152c ("ARM: 6164/1: Add kto and
>> kfrom to input operands list.") has been done to the other copypage
>> implementations...
>>
>> [adding Khem]
>>
>> > The GCC manual states:
>> >
>> > `naked'
>> > Use this attribute on the ARM, AVR, MCORE, MSP430, NDS32, RL78, RX
>> > and SPU ports to indicate that the specified function does not
>> > need prologue/epilogue sequences generated by the compiler. It is
>> > up to the programmer to provide these sequences. The only
>> > ^^^^^^^^
>> > statements that can be safely included in naked functions are
>> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> > `asm' statements that do not have operands. All other statements,
>> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> > including declarations of local variables, `if' statements, and so
>> > forth, should be avoided. Naked functions should be used to
>> > implement the body of an assembly function, while allowing the
>> > compiler to construct the requisite function declaration for the
>> > assembler.
>> >
>> > The 'I' attribute is fine here because it is a constant that is not
>> > allowed to be in a register (and hence has no code generation side
>> > effects.)
>> >
>> > Adding operands for the input parameters, however, isn't going to
>> > work around the fact that _this_ assembly is written to be out of
>> > line and so it must never be inlined by the compiler.
>>
>> I briefly looked at a disassembled version after applying both patches,
>> it indeed leads to inlining. However, the code seems to be working
>> (thanks to asm volatile?)...
>
> Apart from v4wb_copy_user_page() and mc_copy_user_page(), how is
> Clang inlining these static functions that are only used through
> function pointers?
I only looked at copypage-xscale.c (the mc_copy_user_page() case)...
--
Stefan
next prev parent reply other threads:[~2018-10-15 22:53 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-10-15 22:16 [PATCH 1/2] ARM: copypage-fa: add kto and kfrom to input operands list Stefan Agner
2018-10-15 22:23 ` Russell King - ARM Linux
2018-10-15 22:39 ` Stefan Agner
2018-10-15 22:46 ` Russell King - ARM Linux
2018-10-15 22:52 ` Stefan Agner [this message]
2018-10-15 23:03 ` Russell King - ARM Linux
2018-10-16 8:00 ` Linus Walleij
2018-10-16 8:44 ` Russell King - ARM Linux
2018-10-16 11:35 ` Linus Walleij
2018-10-16 20:43 ` Nicolas Pitre
2018-10-16 21:59 ` Stefan Agner
2018-10-17 8:58 ` Arnd Bergmann
2018-10-17 9:04 ` [PATCH] [ALTERNATIVE] ARM: fix copypage functions for clang Arnd Bergmann
2018-10-17 9:35 ` Russell King - ARM Linux
2018-10-17 14:23 ` [PATCH 1/2] ARM: copypage-fa: add kto and kfrom to input operands list Nicolas Pitre
2018-11-05 23:00 ` Stefan Agner
2018-11-06 4:49 ` Nicolas Pitre
2018-11-06 13:16 ` Robin Murphy
2018-11-06 13:25 ` Nicolas Pitre
2018-11-07 16:27 ` Stefan Agner
2018-11-07 16:58 ` Nicolas Pitre
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=2802c0e549149872aa58da5fb54471fc@agner.ch \
--to=stefan@agner.ch \
--cc=arnd@arndb.de \
--cc=joel@jms.id.au \
--cc=linus.walleij@linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@armlinux.org.uk \
--cc=nico@linaro.org \
--cc=raj.khem@gmail.com \
--cc=ulli.kroll@googlemail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).