Re: [PATCH v4 1/2] powerpc/uaccess: Implement unsafe_put_user() using 'asm goto'

From: Christophe Leroy
Date: Tue May 05 2020 - 11:40:27 EST


Hi,

Le 05/05/2020 Ã 16:27, Michael Ellerman a ÃcritÂ:
Christophe Leroy <christophe.leroy@xxxxxx> writes:
unsafe_put_user() is designed to take benefit of 'asm goto'.

Instead of using the standard __put_user() approach and branch
based on the returned error, use 'asm goto' and make the
exception code branch directly to the error label. There is
no code anymore in the fixup section.

This change significantly simplifies functions using
unsafe_put_user()

...

Signed-off-by: Christophe Leroy <christophe.leroy@xxxxxx>
---
arch/powerpc/include/asm/uaccess.h | 61 +++++++++++++++++++++++++-----
1 file changed, 52 insertions(+), 9 deletions(-)

diff --git a/arch/powerpc/include/asm/uaccess.h b/arch/powerpc/include/asm/uaccess.h
index 9cc9c106ae2a..9365b59495a2 100644
--- a/arch/powerpc/include/asm/uaccess.h
+++ b/arch/powerpc/include/asm/uaccess.h
@@ -196,6 +193,52 @@ do { \
})
+#define __put_user_asm_goto(x, addr, label, op) \
+ asm volatile goto( \
+ "1: " op "%U1%X1 %0,%1 # put_user\n" \
+ EX_TABLE(1b, %l2) \
+ : \
+ : "r" (x), "m<>" (*addr) \

The "m<>" here is breaking GCC 4.6.3, which we allegedly still support.

Plain "m" works, how much does the "<>" affect code gen in practice?

A quick diff here shows no difference from removing "<>".

It was recommended by Segher, there has been some discussion about it on v1 of this patch, see https://patchwork.ozlabs.org/project/linuxppc-dev/patch/4fdc2aba6f5e51887d1cd0fee94be0989eada2cd.1586942312.git.christophe.leroy@xxxxxx/

As far as I understood that's mandatory on recent gcc to get the pre-update form of the instruction. With older versions "m" was doing the same, but not anymore. Should we ifdef the "m<>" or "m" based on GCC version ?

Christophe