Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220302165438.1140256-1-broonie@kernel.org>
In-Reply-To: <20220302165438.1140256-1-broonie@kernel.org>
From:   Ard Biesheuvel <ardb@kernel.org>
Date:   Thu, 3 Mar 2022 07:26:45 +0000
Message-ID: <CAMj1kXHhTNsB8V=4LNMqSXrAWXroHHNXzjS0xWERboG7a3G-Lg@mail.gmail.com>
Subject: Re: [PATCH] arm64: crypto: Don't allow v8.2 extensions to be used
 with BROKEN_GAS_INST
To:     Mark Brown <broonie@kernel.org>, Marc Zyngier <maz@kernel.org>
Cc:     Herbert Xu <herbert@gondor.apana.org.au>,
        "David S . Miller" <davem@davemloft.net>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Will Deacon <will@kernel.org>,
        Linux Crypto Mailing List <linux-crypto@vger.kernel.org>,
        Linux ARM <linux-arm-kernel@lists.infradead.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, 2 Mar 2022 at 16:54, Mark Brown <broonie@kernel.org> wrote:
>
> We support building the kernel with archaic versions of binutils which
> had some confusion regarding how instructions should be encoded for .inst
> which we work around with the __emit_inst() macro. Unfortunately we have
> not consistently used this macro, one of the places where it's missed being
> the macros that manually encode v8.2 crypto instructions. This means that
> kernels built with such toolchains have never supported use of the affected
> instructions correctly.
>
> Since these toolchains are very old (some idle research suggested 2015
> era) it seems more sensible to just refuse to build v8.2 crypto support
> with them, in the unlikely event that someone has a need to use such a
> toolchain to build a kernel which will run on a system with v8.2 crypto
> support they can always fix this properly but it seems more likely that
> we will deprecate support for these toolchains and remove __emit_inst()
> before that happens.
>
> Signed-off-by: Mark Brown <broonie@kernel.org>

IIRC this is not about .inst getting the encoding wrong, but about
confusion over the size of the generated opcode, resulting in problems
generating constants involving relative offsets between labels. (The
endian swap is there so that .long can be used on BE to emit the LE
opcodes)

This is not an issue here, so I don't think this change is necessary.

> ---
>  arch/arm64/crypto/Kconfig | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/arch/arm64/crypto/Kconfig b/arch/arm64/crypto/Kconfig
> index 2a965aa0188d..90dd62d46739 100644
> --- a/arch/arm64/crypto/Kconfig
> +++ b/arch/arm64/crypto/Kconfig
> @@ -32,12 +32,14 @@ config CRYPTO_SHA2_ARM64_CE
>  config CRYPTO_SHA512_ARM64_CE
>         tristate "SHA-384/SHA-512 digest algorithm (ARMv8 Crypto Extensions)"
>         depends on KERNEL_MODE_NEON
> +       depends on !BROKEN_GAS_INST
>         select CRYPTO_HASH
>         select CRYPTO_SHA512_ARM64
>
>  config CRYPTO_SHA3_ARM64
>         tristate "SHA3 digest algorithm (ARMv8.2 Crypto Extensions)"
>         depends on KERNEL_MODE_NEON
> +       depends on !BROKEN_GAS_INST
>         select CRYPTO_HASH
>         select CRYPTO_SHA3
>
> @@ -50,6 +52,7 @@ config CRYPTO_SM3_ARM64_CE
>  config CRYPTO_SM4_ARM64_CE
>         tristate "SM4 symmetric cipher (ARMv8.2 Crypto Extensions)"
>         depends on KERNEL_MODE_NEON
> +       depends on !BROKEN_GAS_INST
>         select CRYPTO_ALGAPI
>         select CRYPTO_LIB_SM4
>
> --
> 2.30.2
>