On Mon, 29 Jul 2019 at 10:55, Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
>
> Hi Ard,
>
> On Mon, Jul 29, 2019 at 9:44 AM Ard Biesheuvel
> <ard.biesheuvel@xxxxxxxxxx> wrote:
> > The generic aegis128 driver has been updated to support using SIMD
> > intrinsics to implement the core AES based transform, and this has
> > been wired up for ARM and arm64, which both provide a simd.h header.
> >
> > As it turns out, most architectures don't provide this header, even
> > though a version of it exists in include/asm-generic, and this is
> > not taken into account by the aegis128 driver, resulting in build
> > failures on those architectures.
> >
> > So update the aegis128 code to only import simd.h (and the related
> > header in internal/crypto) if the SIMD functionality is enabled for
> > this driver.
> >
> > Reported-by: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
>
> Thanks for your patch!
>
> > --- a/crypto/aegis128-core.c
> > +++ b/crypto/aegis128-core.c
> > @@ -8,7 +8,6 @@
> >
> > #include <crypto/algapi.h>
> > #include <crypto/internal/aead.h>
> > -#include <crypto/internal/simd.h>
> > #include <crypto/internal/skcipher.h>
> > #include <crypto/scatterwalk.h>
> > #include <linux/err.h>
> > @@ -16,7 +15,11 @@
> > #include <linux/kernel.h>
> > #include <linux/module.h>
> > #include <linux/scatterlist.h>
> > +
> > +#ifdef CONFIG_CRYPTO_AEGIS128_SIMD
> > +#include <crypto/internal/simd.h>
> > #include <asm/simd.h>
>
> Wouldn't including <crypto/internal/simd.h> unconditionally, and
> adding just
>
> #else
> static inline bool may_use_simd(void)
> {
> return false;
> }
>
> and be done with it, work too?
>
I guess, but I felt it was more appropriate to include as little of
the SIMD infrastructure as possible when building this driver without
SIMD support. Also, I think it is slightly ugly to have a alternative
local implementation of something that is provided by a header file.
> > +#endif
> >
> > #include "aegis.h"
> >
> > @@ -44,6 +47,15 @@ struct aegis128_ops {
> >
> > static bool have_simd;
> >
> > +static bool aegis128_do_simd(void)
> > +{
> > +#ifdef CONFIG_CRYPTO_AEGIS128_SIMD
> > + if (have_simd)
> > + return crypto_simd_usable();
> > +#endif
> > + return false;
> > +}
> > +
> > bool crypto_aegis128_have_simd(void);
> > void crypto_aegis128_update_simd(struct aegis_state *state, const void *msg);
> > void crypto_aegis128_encrypt_chunk_simd(struct aegis_state *state, u8 *dst,
> > @@ -66,7 +78,7 @@ static void crypto_aegis128_update(struct aegis_state *state)
> > static void crypto_aegis128_update_a(struct aegis_state *state,
> > const union aegis_block *msg)
> > {
> > - if (have_simd && crypto_simd_usable()) {
> > + if (aegis128_do_simd()) {
> > crypto_aegis128_update_simd(state, msg);
> > return;
> > }
>
> [...]
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
