[PATCH 1/2] ARM: copypage-fa: add kto and kfrom to input operands list

On Tue, Oct 16, 2018 at 12:52:58AM +0200, Stefan Agner wrote:

On 16.10.2018 00:46, Russell King - ARM Linux wrote:

quoted

On Tue, Oct 16, 2018 at 12:39:54AM +0200, Stefan Agner wrote:

quoted

On 16.10.2018 00:23, Russell King - ARM Linux wrote:

quoted

On Tue, Oct 16, 2018 at 12:16:29AM +0200, Stefan Agner wrote:

quoted

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]

quoted

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

The two I mention are different from the rest, because they are used
from other functions within the same file.  The rest are all used
through function pointers and should, therefore, never be inlined.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up