[PATCH] crypto: rng: Remove unused function __crypto_rng_cast()

2017-05-22 Thread Matthias Kaehlcke 
This fixes the following warning when building with clang:

crypto/rng.c:35:34: error: unused function '__crypto_rng_cast'
[-Werror,-Wunused-function]

Signed-off-by: Matthias Kaehlcke <m...@chromium.org>
---
 crypto/rng.c | 5 -
 1 file changed, 5 deletions(-)

diff --git a/crypto/rng.c b/crypto/rng.c
index f46dac5288b9..5e8469244960 100644
--- a/crypto/rng.c
+++ b/crypto/rng.c
@@ -33,11 +33,6 @@ struct crypto_rng *crypto_default_rng;
 EXPORT_SYMBOL_GPL(crypto_default_rng);
 static int crypto_default_rng_refcnt;
 
-static inline struct crypto_rng *__crypto_rng_cast(struct crypto_tfm *tfm)
-{
-   return container_of(tfm, struct crypto_rng, base);
-}
-
 int crypto_rng_reset(struct crypto_rng *tfm, const u8 *seed, unsigned int slen)
 {
u8 *buf = NULL;
-- 
2.13.0.303.g4ebf302169-goog

Re: [PATCH] crypto: arm64/sha - avoid non-standard inline asm tricks

2017-05-17 Thread Matthias Kaehlcke 
El Wed, Apr 26, 2017 at 05:11:32PM +0100 Ard Biesheuvel ha dit:

> Replace the inline asm which exports struct offsets as ELF symbols
> with proper const variables exposing the same values. This works
> around an issue with Clang which does not interpret the "i" (or "I")
> constraints in the same way as GCC.
> 
> Signed-off-by: Ard Biesheuvel 
> ---
>  arch/arm64/crypto/sha1-ce-core.S |  6 --
>  arch/arm64/crypto/sha1-ce-glue.c | 11 +++
>  arch/arm64/crypto/sha2-ce-core.S |  6 --
>  arch/arm64/crypto/sha2-ce-glue.c | 13 +
>  4 files changed, 16 insertions(+), 20 deletions(-)

Herbert, do you plan to pick this up?

Thanks

Matthias

Re: [PATCH] crypto: arm64/sha - avoid non-standard inline asm tricks

2017-04-26 Thread Matthias Kaehlcke 
El Wed, Apr 26, 2017 at 05:11:32PM +0100 Ard Biesheuvel ha dit:

> Replace the inline asm which exports struct offsets as ELF symbols
> with proper const variables exposing the same values. This works
> around an issue with Clang which does not interpret the "i" (or "I")
> constraints in the same way as GCC.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org>

Tested-by: Matthias Kaehlcke <m...@chromium.org>

Thanks!

Matthias

Re: [PATCH v2] crypto: arm64/sha: Add constant operand modifier to ASM_EXPORT

2017-04-25 Thread Matthias Kaehlcke 
El Tue, Apr 25, 2017 at 07:06:30PM +0100 Ard Biesheuvel ha dit:

> On 25 April 2017 at 18:39, Matthias Kaehlcke <m...@chromium.org> wrote:
> > Hi,
> >
> > El Tue, Apr 18, 2017 at 04:35:02PM +0100 Ard Biesheuvel ha dit:
> >
> >> On 18 April 2017 at 15:47, Paul Gortmaker <paul.gortma...@windriver.com> 
> >> wrote:
> >> > On Wed, Apr 5, 2017 at 2:34 PM, Matthias Kaehlcke <m...@chromium.org> 
> >> > wrote:
> >> >> The operand is an integer constant, make the constness explicit by
> >> >> adding the modifier. This is needed for clang to generate valid code
> >> >> and also works with gcc.
> >> >
> >> > Actually it doesn't work with all gcc.  I've got an older arm64 
> >> > toolchain that I
> >> > only use for syntax checking (and hence I don't care if it is the latest 
> >> > and
> >> > greatest) and this commit breaks it:
> >> >
> >> > arch/arm64/crypto/sha1-ce-glue.c:21:2: error: invalid 'asm': invalid
> >> > operand prefix '%c'
> >> >   asm(".globl " #sym "; .set " #sym ", %c0" :: "i"(val));
> >> >
> >> > I'm currently reverting this change locally so I can continue to use the 
> >> > old
> >> > toolchain:
> >> >
> >> > $ aarch64-linux-gnu-gcc --version
> >> > aarch64-linux-gnu-gcc (crosstool-NG linaro-1.13.1-4.8-2013.12 - Linaro
> >> > GCC 2013.11) 4.8.3 20131202 (prerelease)
> >> > Copyright (C) 2013 Free Software Foundation, Inc.
> >> >
> >> > $ aarch64-linux-gnu-as --version
> >> > GNU assembler (crosstool-NG linaro-1.13.1-4.8-2013.12 - Linaro GCC
> >> > 2013.11) 2.24.0.20131220
> >> > Copyright 2013 Free Software Foundation, Inc.
> >> >
> >> > Maybe it is finally too old and nobody cares, but I thought it worth a 
> >> > mention.
> >> >
> >>
> >> Thanks for the report. I think we care more about GCC 4.8 than about
> >> Clang, which argues for reverting this patch.
> >>
> >> I understand these issues must be frustrating if you are working on
> >> this stuff, but to me, it is not entirely obvious why we want to
> >> support Clang in the first place (i.e., what does it buy you if your
> >> distro/environment is not already using Clang for userland), and why
> >> the burden is on Linux to make modifications to support Clang,
> >> especially when it comes to GCC extensions such as inline assembly
> >> syntax.
> >>
> >> It is ultimately up to the maintainers to decide what to do with this
> >> patch, but my vote would be to revert it, especially given that the %c
> >> placeholder prefix is not documented anywhere, and appears to simply
> >> trigger some GCC internals that happen to do the right thing in this
> >> case.
> >>
> >> However, the I -> i change is arguably an improvement, and considering
> >> that the following
> >>
> >> asm("foo: .long %0" :: "i"(some value))
> >>
> >> doesn't compile with clang either, I suggest you (Matthias) file a bug
> >> against Clang to get this fixed, and we can propose another patch just
> >> for the I->i change.
> >
> > I consulted with folks with more expertise in this area than myself.
> > This is their analysis of the situation:
> >
> > "The ARM ARM specifies that the correct AArch64 instruction assembly
> > syntax is to have a hash sign (#) before an immediate.
> >
> 
> It does not specify that at all:
> 
> """
> The A64 assembly language does not require the # character to
> introduce constant immediate operands, but an assembler must allow
> immediate values introduced with or without the # character. ARM
> recommends that an A64 disassembler outputs a # before an immediate
> operand.
> """
> (ARM DDI 0487A.g page C1-121)
> 
> IOW, it only /recommends/ the # sign for *dis*assemblers. Big difference.

Indeed, thanks for the clarification.

> > Therefore, every time an inline assembly constraint is used that
> > specifies to print an immediate (like 'i' or 'I'), the immediate
> > (e.g. 42) should be printed with the hash (e.g. #42).
> >
> > Therefore, if you're using an immediate constraint where the hash sign
> > must not be printed, you have to use the "c" operand modifier. The "c"
> > operand modifier apparently got introduced to gc

Re: [PATCH v2] crypto: arm64/sha: Add constant operand modifier to ASM_EXPORT

2017-04-25 Thread Matthias Kaehlcke 
Hi,

El Tue, Apr 18, 2017 at 04:35:02PM +0100 Ard Biesheuvel ha dit:

> On 18 April 2017 at 15:47, Paul Gortmaker <paul.gortma...@windriver.com> 
> wrote:
> > On Wed, Apr 5, 2017 at 2:34 PM, Matthias Kaehlcke <m...@chromium.org> wrote:
> >> The operand is an integer constant, make the constness explicit by
> >> adding the modifier. This is needed for clang to generate valid code
> >> and also works with gcc.
> >
> > Actually it doesn't work with all gcc.  I've got an older arm64 toolchain 
> > that I
> > only use for syntax checking (and hence I don't care if it is the latest and
> > greatest) and this commit breaks it:
> >
> > arch/arm64/crypto/sha1-ce-glue.c:21:2: error: invalid 'asm': invalid
> > operand prefix '%c'
> >   asm(".globl " #sym "; .set " #sym ", %c0" :: "i"(val));
> >
> > I'm currently reverting this change locally so I can continue to use the old
> > toolchain:
> >
> > $ aarch64-linux-gnu-gcc --version
> > aarch64-linux-gnu-gcc (crosstool-NG linaro-1.13.1-4.8-2013.12 - Linaro
> > GCC 2013.11) 4.8.3 20131202 (prerelease)
> > Copyright (C) 2013 Free Software Foundation, Inc.
> >
> > $ aarch64-linux-gnu-as --version
> > GNU assembler (crosstool-NG linaro-1.13.1-4.8-2013.12 - Linaro GCC
> > 2013.11) 2.24.0.20131220
> > Copyright 2013 Free Software Foundation, Inc.
> >
> > Maybe it is finally too old and nobody cares, but I thought it worth a 
> > mention.
> >
> 
> Thanks for the report. I think we care more about GCC 4.8 than about
> Clang, which argues for reverting this patch.
> 
> I understand these issues must be frustrating if you are working on
> this stuff, but to me, it is not entirely obvious why we want to
> support Clang in the first place (i.e., what does it buy you if your
> distro/environment is not already using Clang for userland), and why
> the burden is on Linux to make modifications to support Clang,
> especially when it comes to GCC extensions such as inline assembly
> syntax.
> 
> It is ultimately up to the maintainers to decide what to do with this
> patch, but my vote would be to revert it, especially given that the %c
> placeholder prefix is not documented anywhere, and appears to simply
> trigger some GCC internals that happen to do the right thing in this
> case.
> 
> However, the I -> i change is arguably an improvement, and considering
> that the following
> 
> asm("foo: .long %0" :: "i"(some value))
> 
> doesn't compile with clang either, I suggest you (Matthias) file a bug
> against Clang to get this fixed, and we can propose another patch just
> for the I->i change.

I consulted with folks with more expertise in this area than myself.
This is their analysis of the situation:

"The ARM ARM specifies that the correct AArch64 instruction assembly
syntax is to have a hash sign (#) before an immediate.

Therefore, every time an inline assembly constraint is used that
specifies to print an immediate (like 'i' or 'I'), the immediate
(e.g. 42) should be printed with the hash (e.g. #42).

Therefore, if you're using an immediate constraint where the hash sign
must not be printed, you have to use the "c" operand modifier. The "c"
operand modifier apparently got introduced to gcc after the 4.8
release.

The binutils assembler and the clang integrated assembler accept
immediates without the hash sign as a non-official extension. Some of
the immediate constraints on gcc seem to not print out the hash sign
either; which is why the variant in the linux kernel works with gcc.

In summary, it seems to me that the inline assembly with the %c0
operand is the correct one and gcc 4.8 is simply too old to support
this."

If the above is correct it seems that the solution is not to "fix"
clang, but to use different instructions for gcc<=4.8 and newer
compilers. I am aware that this is not a popular option.

What do you think?

Thanks

Matthias

Re: [PATCH v2] crypto: arm64/sha: Add constant operand modifier to ASM_EXPORT

2017-04-18 Thread Matthias Kaehlcke 
El Tue, Apr 18, 2017 at 04:35:02PM +0100 Ard Biesheuvel ha dit:

> On 18 April 2017 at 15:47, Paul Gortmaker <paul.gortma...@windriver.com> 
> wrote:
> > On Wed, Apr 5, 2017 at 2:34 PM, Matthias Kaehlcke <m...@chromium.org> wrote:
> >> The operand is an integer constant, make the constness explicit by
> >> adding the modifier. This is needed for clang to generate valid code
> >> and also works with gcc.
> >
> > Actually it doesn't work with all gcc.  I've got an older arm64 toolchain 
> > that I
> > only use for syntax checking (and hence I don't care if it is the latest and
> > greatest) and this commit breaks it:
> >
> > arch/arm64/crypto/sha1-ce-glue.c:21:2: error: invalid 'asm': invalid
> > operand prefix '%c'
> >   asm(".globl " #sym "; .set " #sym ", %c0" :: "i"(val));

Sorry about that :(

> > I'm currently reverting this change locally so I can continue to use the old
> > toolchain:
> >
> > $ aarch64-linux-gnu-gcc --version
> > aarch64-linux-gnu-gcc (crosstool-NG linaro-1.13.1-4.8-2013.12 - Linaro
> > GCC 2013.11) 4.8.3 20131202 (prerelease)
> > Copyright (C) 2013 Free Software Foundation, Inc.
> >
> > $ aarch64-linux-gnu-as --version
> > GNU assembler (crosstool-NG linaro-1.13.1-4.8-2013.12 - Linaro GCC
> > 2013.11) 2.24.0.20131220
> > Copyright 2013 Free Software Foundation, Inc.
> >
> > Maybe it is finally too old and nobody cares, but I thought it worth a 
> > mention.
> >
> 
> Thanks for the report. I think we care more about GCC 4.8 than about
> Clang, which argues for reverting this patch.

I agree, we certainly shouldn't break GCC.

> I understand these issues must be frustrating if you are working on
> this stuff, but to me, it is not entirely obvious why we want to
> support Clang in the first place (i.e., what does it buy you if your
> distro/environment is not already using Clang for userland),

There are environments that use Clang for userland, like Chrome OS or
Android. Debian releases with GCC, but also does Clang builds of most
of its repository: http://clang.debian.net/

I would also argue that Linux directly benefits from additional error
checks provided by Clang.

> and why the burden is on Linux to make modifications to support
> Clang, especially when it comes to GCC extensions such as inline
> assembly syntax.

Personally I'm on the pragmatic side. 99.9x% of the kernel already
builds with Clang (at least for arm64 and x86), only a very tiny
fraction of the code needs adaptation, which should ideally result in
the same code for gcc and clang. And it's not like Clang is just
another proprietary or niche compiler, it's the other big open source
compiler out there, since the effort is relatively small it seems
desirable to support it without out-of-tree patches. In parallel there
is also work ongoing with upstream Clang to improve compatiblity.

> It is ultimately up to the maintainers to decide what to do with this
> patch, but my vote would be to revert it, especially given that the %c
> placeholder prefix is not documented anywhere, and appears to simply
> trigger some GCC internals that happen to do the right thing in this
> case.
> 
> However, the I -> i change is arguably an improvement, and considering
> that the following
> 
> asm("foo: .long %0" :: "i"(some value))
> 
> doesn't compile with clang either, I suggest you (Matthias) file a bug
> against Clang to get this fixed, and we can propose another patch just
> for the I->i change.

Ok, I will see if we can get this fixed in Clang.

Thanks

Matthias

Re: [PATCH 3/7] x86, LLVM: suppress clang warnings about unaligned accesses

2017-04-13 Thread Matthias Kaehlcke 
El Mon, Apr 03, 2017 at 04:01:58PM -0700 Matthias Kaehlcke ha dit:

> El Fri, Mar 17, 2017 at 04:50:19PM -0700 h...@zytor.com ha dit:
> 
> > On March 16, 2017 5:15:16 PM PDT, Michael Davidson <m...@google.com> wrote:
> > >Suppress clang warnings about potential unaliged accesses
> > >to members in packed structs. This gets rid of almost 10,000
> > >warnings about accesses to the ring 0 stack pointer in the TSS.
> > >
> > >Signed-off-by: Michael Davidson <m...@google.com>
> > >---
> > > arch/x86/Makefile | 5 +
> > > 1 file changed, 5 insertions(+)
> > >
> > >diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> > >index 894a8d18bf97..7f21703c475d 100644
> > >--- a/arch/x86/Makefile
> > >+++ b/arch/x86/Makefile
> > >@@ -128,6 +128,11 @@ endif
> > > KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args)
> > > endif
> > > 
> > >+ifeq ($(cc-name),clang)
> > >+# Suppress clang warnings about potential unaligned accesses.
> > >+KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member)
> > >+endif
> > >+
> > > ifdef CONFIG_X86_X32
> > >   x32_ld_ok := $(call try-run,\
> > >   /bin/echo -e '1: .quad 1b' | \
> > 
> > Why conditional on clang?
> 
> My understanding is that this warning is clang specific, it is not
> listed on https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html

Actually this warning affects other platforms besides x86
(e.g. arm64), I'll submit a patch that disables the warning on all
platforms.

Cheers

Matthias

Re: [PATCH 2/7] Makefile, x86, LLVM: disable unsupported optimization flags

2017-04-05 Thread Matthias Kaehlcke 
Hi Masahiro,

El Thu, Apr 06, 2017 at 03:08:26AM +0900 Masahiro Yamada ha dit:

> 2017-03-17 9:15 GMT+09:00 Michael Davidson :
> > Unfortunately, while clang generates a warning about these flags
> > being unsupported it still exits with a status of 0 so we have
> > to explicitly disable them instead of just using a cc-option check.
> >
> > Signed-off-by: Michael Davidson 
> 
> 
> Instead, does the following work for you?
> https://patchwork.kernel.org/patch/9657285/

Thanks for the pointer, I was about to give this change (or rather its
ancestor) a rework myself :)

> You need to use
> $(call cc-option, ...)
> for -falign-jumps=1 and -falign-loops=1

I can confirm that this works.

Thanks

Matthias

[PATCH v2] crypto: arm64/sha: Add constant operand modifier to ASM_EXPORT

2017-04-05 Thread Matthias Kaehlcke 
The operand is an integer constant, make the constness explicit by
adding the modifier. This is needed for clang to generate valid code
and also works with gcc.

Also change the constraint of the operand from 'I' ("Integer constant
that is valid as an immediate operand in an ADD instruction", AArch64)
to 'i' ("An immediate integer operand").

Based-on-patch-from: Greg Hackmann <ghackm...@google.com>
Signed-off-by: Greg Hackmann <ghackm...@google.com>
Signed-off-by: Matthias Kaehlcke <m...@chromium.org>
---
Changes in v2:
- Changed operand constraint from I to i
- Updated commit message
- Changed 'From' tag to 'Based-on-patch-from'

 arch/arm64/crypto/sha1-ce-glue.c | 2 +-
 arch/arm64/crypto/sha2-ce-glue.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/crypto/sha1-ce-glue.c b/arch/arm64/crypto/sha1-ce-glue.c
index aefda9868627..6b520e3f3ab1 100644
--- a/arch/arm64/crypto/sha1-ce-glue.c
+++ b/arch/arm64/crypto/sha1-ce-glue.c
@@ -18,7 +18,7 @@
 #include 
 
 #define ASM_EXPORT(sym, val) \
-   asm(".globl " #sym "; .set " #sym ", %0" :: "I"(val));
+   asm(".globl " #sym "; .set " #sym ", %c0" :: "i"(val));
 
 MODULE_DESCRIPTION("SHA1 secure hash using ARMv8 Crypto Extensions");
 MODULE_AUTHOR("Ard Biesheuvel <ard.biesheu...@linaro.org>");
diff --git a/arch/arm64/crypto/sha2-ce-glue.c b/arch/arm64/crypto/sha2-ce-glue.c
index 7cd587564a41..e3abe11de48c 100644
--- a/arch/arm64/crypto/sha2-ce-glue.c
+++ b/arch/arm64/crypto/sha2-ce-glue.c
@@ -18,7 +18,7 @@
 #include 
 
 #define ASM_EXPORT(sym, val) \
-   asm(".globl " #sym "; .set " #sym ", %0" :: "I"(val));
+   asm(".globl " #sym "; .set " #sym ", %c0" :: "i"(val));
 
 MODULE_DESCRIPTION("SHA-224/SHA-256 secure hash using ARMv8 Crypto 
Extensions");
 MODULE_AUTHOR("Ard Biesheuvel <ard.biesheu...@linaro.org>");
-- 
2.12.2.715.g7642488e1d-goog

Re: [PATCH] crypto: arm64/sha: use %c constraint code in ASM_EXPORT

2017-04-05 Thread Matthias Kaehlcke 
Hoi Ard!

El Wed, Apr 05, 2017 at 06:08:37PM +0100 Ard Biesheuvel ha dit:

> On 5 April 2017 at 17:56, Matthias Kaehlcke <m...@chromium.org> wrote:
> > From: Greg Hackmann <ghackm...@google.com>
> >
> > The current definition of ASM_EXPORT doesn't work properly with clang,
> > according to https://bugs.llvm.org//show_bug.cgi?id=27250#c3 it relies on
> > gcc specific behavior. Change the constraint from an intermediate to an
> > output expression which works with both gcc and clang.
> >
> > From: Greg Hackmann <ghackm...@google.com>
> > Commit-message-by: Matthias Kaehlcke <m...@chromium.org>
> > Signed-off-by: Greg Hackmann <ghackm...@google.com>
> > Signed-off-by: Matthias Kaehlcke <m...@chromium.org>
> > ---
> >  arch/arm64/crypto/sha1-ce-glue.c | 2 +-
> >  arch/arm64/crypto/sha2-ce-glue.c | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/arm64/crypto/sha1-ce-glue.c 
> > b/arch/arm64/crypto/sha1-ce-glue.c
> > index aefda9868627..c71e94ba0e43 100644
> > --- a/arch/arm64/crypto/sha1-ce-glue.c
> > +++ b/arch/arm64/crypto/sha1-ce-glue.c
> > @@ -18,7 +18,7 @@
> >  #include 
> >
> >  #define ASM_EXPORT(sym, val) \
> > -   asm(".globl " #sym "; .set " #sym ", %0" :: "I"(val));
> > +   asm(".globl " #sym "; .set " #sym ", %c0" :: "I"(val));
> >
> >  MODULE_DESCRIPTION("SHA1 secure hash using ARMv8 Crypto Extensions");
> >  MODULE_AUTHOR("Ard Biesheuvel <ard.biesheu...@linaro.org>");
> > diff --git a/arch/arm64/crypto/sha2-ce-glue.c 
> > b/arch/arm64/crypto/sha2-ce-glue.c
> > index 7cd587564a41..381b5fb2dcb2 100644
> > --- a/arch/arm64/crypto/sha2-ce-glue.c
> > +++ b/arch/arm64/crypto/sha2-ce-glue.c
> > @@ -18,7 +18,7 @@
> >  #include 
> >
> >  #define ASM_EXPORT(sym, val) \
> > -   asm(".globl " #sym "; .set " #sym ", %0" :: "I"(val));
> > +   asm(".globl " #sym "; .set " #sym ", %c0" :: "I"(val));
> >
> >  MODULE_DESCRIPTION("SHA-224/SHA-256 secure hash using ARMv8 Crypto 
> > Extensions");
> >  MODULE_AUTHOR("Ard Biesheuvel <ard.biesheu...@linaro.org>");
> 
> I am fine with this change, although I would really like to add a
> better reference to the commit log. It is *very* difficult to find any
> documentation regarding non-trivial uses of inline asm constraints,
> and if %c0 is the correct syntax, surely we can quote something better
> than a LLVM bugzilla entry? Also, where does the distinction between
> 'intermediate' vs 'output' expression come from?

To be honest assembly is not really my area of expertise and I relayed
the information from the LLVM bug. The link to the gcc doc on
x86(!) operand modifiers posted by Robin Murphy (thanks!) should be
helpful to provide a more accurate commit message.

Thanks

Matthias

[PATCH] crypto: arm64/sha: use %c constraint code in ASM_EXPORT

2017-04-05 Thread Matthias Kaehlcke 
From: Greg Hackmann <ghackm...@google.com>

The current definition of ASM_EXPORT doesn't work properly with clang,
according to https://bugs.llvm.org//show_bug.cgi?id=27250#c3 it relies on
gcc specific behavior. Change the constraint from an intermediate to an
output expression which works with both gcc and clang.

From: Greg Hackmann <ghackm...@google.com>
Commit-message-by: Matthias Kaehlcke <m...@chromium.org>
Signed-off-by: Greg Hackmann <ghackm...@google.com>
Signed-off-by: Matthias Kaehlcke <m...@chromium.org>
---
 arch/arm64/crypto/sha1-ce-glue.c | 2 +-
 arch/arm64/crypto/sha2-ce-glue.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/crypto/sha1-ce-glue.c b/arch/arm64/crypto/sha1-ce-glue.c
index aefda9868627..c71e94ba0e43 100644
--- a/arch/arm64/crypto/sha1-ce-glue.c
+++ b/arch/arm64/crypto/sha1-ce-glue.c
@@ -18,7 +18,7 @@
 #include 
 
 #define ASM_EXPORT(sym, val) \
-   asm(".globl " #sym "; .set " #sym ", %0" :: "I"(val));
+   asm(".globl " #sym "; .set " #sym ", %c0" :: "I"(val));
 
 MODULE_DESCRIPTION("SHA1 secure hash using ARMv8 Crypto Extensions");
 MODULE_AUTHOR("Ard Biesheuvel <ard.biesheu...@linaro.org>");
diff --git a/arch/arm64/crypto/sha2-ce-glue.c b/arch/arm64/crypto/sha2-ce-glue.c
index 7cd587564a41..381b5fb2dcb2 100644
--- a/arch/arm64/crypto/sha2-ce-glue.c
+++ b/arch/arm64/crypto/sha2-ce-glue.c
@@ -18,7 +18,7 @@
 #include 
 
 #define ASM_EXPORT(sym, val) \
-   asm(".globl " #sym "; .set " #sym ", %0" :: "I"(val));
+   asm(".globl " #sym "; .set " #sym ", %c0" :: "I"(val));
 
 MODULE_DESCRIPTION("SHA-224/SHA-256 secure hash using ARMv8 Crypto 
Extensions");
 MODULE_AUTHOR("Ard Biesheuvel <ard.biesheu...@linaro.org>");
-- 
2.12.2.715.g7642488e1d-goog

Re: [PATCH 7/7] crypto, x86, LLVM: aesni - fix token pasting

2017-04-03 Thread Matthias Kaehlcke 
El Thu, Mar 16, 2017 at 05:15:20PM -0700 Michael Davidson ha dit:

> aes_ctrby8_avx-x86_64.S uses the C preprocessor for token pasting
> of character sequences that are not valid preprocessor tokens.
> While this is allowed when preprocessing assembler files it exposes
> an incompatibilty between the clang and gcc preprocessors where
> clang does not strip leading white space from macro parameters,
> leading to the CONCAT(%xmm, i) macro expansion on line 96 resulting
> in a token with a space character embedded in it.
> 
> While this could be fixed by deleting the offending space character,
> the assembler is perfectly capable of handling the token pasting
> correctly for itself so it seems preferable to let it do so and to
> get rid or the CONCAT(), DDQ() and XMM() preprocessor macros.
> 
> Signed-off-by: Michael Davidson 
> ---
>  arch/x86/crypto/aes_ctrby8_avx-x86_64.S | 7 ++-
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/crypto/aes_ctrby8_avx-x86_64.S 
> b/arch/x86/crypto/aes_ctrby8_avx-x86_64.S
> index a916c4a61165..5f6a5af9c489 100644
> --- a/arch/x86/crypto/aes_ctrby8_avx-x86_64.S
> +++ b/arch/x86/crypto/aes_ctrby8_avx-x86_64.S
> @@ -65,7 +65,6 @@
>  #include 
>  #include 
>  
> -#define CONCAT(a,b)  a##b
>  #define VMOVDQ   vmovdqu
>  
>  #define xdata0   %xmm0
> @@ -92,8 +91,6 @@
>  #define num_bytes%r8
>  
>  #define tmp  %r10
> -#define  DDQ(i)  CONCAT(ddq_add_,i)
> -#define  XMM(i)  CONCAT(%xmm, i)
>  #define  DDQ_DATA0
>  #define  XDATA   1
>  #define KEY_128  1
> @@ -131,12 +128,12 @@ ddq_add_8:
>  /* generate a unique variable for ddq_add_x */
>  
>  .macro setddq n
> - var_ddq_add = DDQ(\n)
> + var_ddq_add = ddq_add_\n
>  .endm
>  
>  /* generate a unique variable for xmm register */
>  .macro setxdata n
> - var_xdata = XMM(\n)
> + var_xdata = %xmm\n
>  .endm
>  
>  /* club the numeric 'id' to the symbol 'name' */

Any feedback on this patch?

Thanks

Matthias

Re: [PATCH 3/7] x86, LLVM: suppress clang warnings about unaligned accesses

2017-04-03 Thread Matthias Kaehlcke 
El Fri, Mar 17, 2017 at 04:50:19PM -0700 h...@zytor.com ha dit:

> On March 16, 2017 5:15:16 PM PDT, Michael Davidson  wrote:
> >Suppress clang warnings about potential unaliged accesses
> >to members in packed structs. This gets rid of almost 10,000
> >warnings about accesses to the ring 0 stack pointer in the TSS.
> >
> >Signed-off-by: Michael Davidson 
> >---
> > arch/x86/Makefile | 5 +
> > 1 file changed, 5 insertions(+)
> >
> >diff --git a/arch/x86/Makefile b/arch/x86/Makefile
> >index 894a8d18bf97..7f21703c475d 100644
> >--- a/arch/x86/Makefile
> >+++ b/arch/x86/Makefile
> >@@ -128,6 +128,11 @@ endif
> > KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args)
> > endif
> > 
> >+ifeq ($(cc-name),clang)
> >+# Suppress clang warnings about potential unaligned accesses.
> >+KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member)
> >+endif
> >+
> > ifdef CONFIG_X86_X32
> > x32_ld_ok := $(call try-run,\
> > /bin/echo -e '1: .quad 1b' | \
> 
> Why conditional on clang?

My understanding is that this warning is clang specific, it is not
listed on https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html

Cheers

Matthias

Re: [PATCH 1/7] Makefile, LLVM: add -no-integrated-as to KBUILD_[AC]FLAGS

2017-04-03 Thread Matthias Kaehlcke 
El Thu, Mar 16, 2017 at 05:15:14PM -0700 Michael Davidson ha dit:

> Add -no-integrated-as to KBUILD_AFLAGS and KBUILD_CFLAGS
> for clang.
> 
> Signed-off-by: Michael Davidson 
> ---
>  Makefile | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index b841fb36beb2..b21fd0ca2946 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -704,6 +704,8 @@ KBUILD_CFLAGS += $(call cc-disable-warning, 
> tautological-compare)
>  # See modpost pattern 2
>  KBUILD_CFLAGS += $(call cc-option, -mno-global-merge,)
>  KBUILD_CFLAGS += $(call cc-option, -fcatch-undefined-behavior)
> +KBUILD_CFLAGS += $(call cc-option, -no-integrated-as)
> +KBUILD_AFLAGS += $(call cc-option, -no-integrated-as)
>  else
>  
>  # These warnings generated too much noise in a regular build.

Ping, any feedback on this patch?

Cheers

Matthias