Re: [PATCH 12/12] [PROBABLY WRONG] s390: void '0' constraint in inline assembly

[Martin Schwidefsky]

On Mon,  8 Apr 2019 23:26:25 +0200
Arnd Bergmann <arnd@xxxxxxxx> wrote:

> clang does not understand the contraint "0" in the CALL_ON_STACK()
> macro:
> 
> ../arch/s390/mm/maccess.c:117:10: error: invalid input constraint '0' in asm
>                 return CALL_ON_STACK(_memcpy_real, S390_lowcore.nodat_stack,
>                        ^
> ../arch/s390/include/asm/processor.h:292:20: note: expanded from macro 'CALL_ON_STACK'
>                   [_fn] "X" (fn) CALL_FMT_##nr : CALL_CLOBBER_##nr);    \
>                                  ^
> <scratch space>:207:1: note: expanded from here
> CALL_FMT_3
> ^
> ../arch/s390/include/asm/processor.h:267:20: note: expanded from macro 'CALL_FMT_3'
>  #define CALL_FMT_3 CALL_FMT_2, "d" (r4)
>                    ^
> ../arch/s390/include/asm/processor.h:266:20: note: expanded from macro 'CALL_FMT_2'
>  #define CALL_FMT_2 CALL_FMT_1, "d" (r3)
>                    ^
> ../arch/s390/include/asm/processor.h:265:32: note: expanded from macro 'CALL_FMT_1'
>  #define CALL_FMT_1 CALL_FMT_0, "0" (r2)
>                                ^
> 
> I don't know what the correct fix here would be, changing it to "d" made
> it build, since clang does understand this one.
> 
> Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
> ---
>  arch/s390/include/asm/processor.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/s390/include/asm/processor.h b/arch/s390/include/asm/processor.h
> index 700c650ffd4f..84c59c99668a 100644
> --- a/arch/s390/include/asm/processor.h
> +++ b/arch/s390/include/asm/processor.h
> @@ -262,7 +262,7 @@ static __no_kasan_or_inline unsigned short stap(void)
>  	register unsigned long r4 asm("6") = (unsigned long)(arg5)
> 
>  #define CALL_FMT_0
> -#define CALL_FMT_1 CALL_FMT_0, "0" (r2)
> +#define CALL_FMT_1 CALL_FMT_0, "d" (r2)
>  #define CALL_FMT_2 CALL_FMT_1, "d" (r3)
>  #define CALL_FMT_3 CALL_FMT_2, "d" (r4)
>  #define CALL_FMT_4 CALL_FMT_3, "d" (r5)

This is (slightly) wrong. %r2 is used as the input register for the first argument
and the result value for the call. With your patch you force the compiler to load
the first argument in two registers. One solution would be to CALL_FMT1 as

	#define CALL_FMT1 CALL_FMT_0

It still is not optimal though as for CALL_FMT_0 the "+&d" (r2) indicates an
input but CALL_ARGS_0 does not initialize r2.

I am thinking about the following patch to cover all cases:
--