linux-kernel - Re: [PATCH 1/2] ARM: drop unnecessary WASM

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <cdf451c0-3947-9be1-3bee-2366577c4c5f@arm.com>
Date:   Tue, 5 Mar 2019 23:39:13 +0000
From:   Robin Murphy <robin.murphy@....com>
To:     Stefan Agner <stefan@...er.ch>, linux@...linux.org.uk
Cc:     nicolas.pitre@...aro.org, mark.rutland@....com, arnd@...db.de,
        ard.biesheuvel@...aro.org, will.deacon@....com,
        julien.thierry@....com, ndesaulniers@...gle.com,
        linux-kernel@...r.kernel.org, thierry.reding@...il.com,
        natechancellor@...il.com, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 1/2] ARM: drop unnecessary WASM

Hi Stefan,

On 2019-03-05 10:18 pm, Stefan Agner wrote:
> The W macro for generating wide instructions when targeting Thumb-2
> is not required for the preload data instructions (pld, pldw) since
> they are only available as wide instructions. The GNU assembler seems
> to work with or without the .w appended when compiling an Thumb-2
> kernel. However, Clang's integrated assembler does not consider the
> .w variants as valid instructions:
> 
>    ./arch/arm/include/asm/processor.h:133:5: error: invalid instruction
>                            "pldw.w\t%a0 \n"
>                             ^
>    <inline asm>:2:1: note: instantiated into assembly here
>    pldw.w  [r0]
>    ^
>    1 error generated.

Have you filed a bug against Clang for that? Something like "pldwal.w" 
may be super-redundant, but it's still perfectly valid syntax. The 
"Standard assembler syntax fields" section of the Arm ARM even calls out 
that "...the .W qualifier has no effect" on ARM/A32 instructions since 
they are inherently 32-bit; that should equally apply for 32-bit only 
Thumb/T32 instructions. There are certainly a few instructions which 
don't allow a condition code (even "AL"), but off-hand I don't remember 
any not having the optional {<q>} field in their syntax.

That said, obviously the patch itself is no problem since the syntax 
*is* redundant here, but it really is just a workaround for an assembler 
bug.

Robin.

> Drop the macro to make sure non-wide variants of pld and pldw are
> emitted in all cases.
> 
> Signed-off-by: Stefan Agner <stefan@...er.ch>
> ---
>   arch/arm/include/asm/processor.h | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/include/asm/processor.h b/arch/arm/include/asm/processor.h
> index 120f4c9bbfde..042d77cf686d 100644
> --- a/arch/arm/include/asm/processor.h
> +++ b/arch/arm/include/asm/processor.h
> @@ -131,8 +131,8 @@ static inline void prefetchw(const void *ptr)
>   	__asm__ __volatile__(
>   		".arch_extension	mp\n"
>   		__ALT_SMP_ASM(
> -			WASM(pldw)		"\t%a0",
> -			WASM(pld)		"\t%a0"
> +			"pldw\t%a0",
> +			"pld\t%a0"
>   		)
>   		:: "p" (ptr));
>   }
>