[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20181015222302.GZ30658@n2100.armlinux.org.uk>
Date: Mon, 15 Oct 2018 23:23:03 +0100
From: Russell King - ARM Linux <linux@...linux.org.uk>
To: Stefan Agner <stefan@...er.ch>
Cc: ulli.kroll@...glemail.com, joel@....id.au, nico@...aro.org,
arnd@...db.de, linus.walleij@...aro.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] ARM: copypage-fa: add kto and kfrom to input
operands list
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.
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.
> Also the constant needs to be used as third input operand so account
> for that as well.
>
> This fixes copypage-fa.c what has previously done before for the other
> copypage implementations in commit 9a40ac86152c ("ARM: 6164/1: Add kto
> and kfrom to input operands list.").
>
> Signed-off-by: Stefan Agner <stefan@...er.ch>
> ---
> arch/arm/mm/copypage-fa.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mm/copypage-fa.c b/arch/arm/mm/copypage-fa.c
> index d130a5ece5d5..ec6501308c60 100644
> --- a/arch/arm/mm/copypage-fa.c
> +++ b/arch/arm/mm/copypage-fa.c
> @@ -22,7 +22,7 @@ fa_copy_user_page(void *kto, const void *kfrom)
> {
> asm("\
> stmfd sp!, {r4, lr} @ 2\n\
> - mov r2, %0 @ 1\n\
> + mov r2, %2 @ 1\n\
> 1: ldmia r1!, {r3, r4, ip, lr} @ 4\n\
> stmia r0, {r3, r4, ip, lr} @ 4\n\
> mcr p15, 0, r0, c7, c14, 1 @ 1 clean and invalidate D line\n\
> @@ -36,7 +36,7 @@ fa_copy_user_page(void *kto, const void *kfrom)
> mcr p15, 0, r2, c7, c10, 4 @ 1 drain WB\n\
> ldmfd sp!, {r4, pc} @ 3"
> :
> - : "I" (PAGE_SIZE / 32));
> + : "r" (kto), "r" (kfrom), "I" (PAGE_SIZE / 32));
> }
>
> void fa_copy_user_highpage(struct page *to, struct page *from,
> --
> 2.19.1
>
--
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
Powered by blists - more mailing lists