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;
Date:   Tue, 15 Feb 2022 11:40:23 +0100
From:   Petr Vorel <pvorel@suse.cz>
To:     Nicolai Stange <nstange@suse.de>
Cc:     linux-crypto@vger.kernel.org,
        Herbert Xu <herbert@gondor.apana.org.au>,
        Masahiro Yamada <masahiroy@kernel.org>,
        linux-kbuild@vger.kernel.org
Subject: Re: [PATCH 1/1] crypto: vmx: Fix missing dependencies during boot
Message-ID: <YguDF5YbyKTGsc3/@pevik>
Reply-To: Petr Vorel <pvorel@suse.cz>
References: <20220214185638.1457-1-pvorel@suse.cz>
 <87zgmsmu94.fsf@suse.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87zgmsmu94.fsf@suse.de>
Precedence: bulk

Hi Nicolai,

> Hi Petr,

> Petr Vorel <pvorel@suse.cz> writes:

> > if CRYPTO_DEV_VMX_ENCRYPT=y && !CRYPTO_MANAGER_DISABLE_TESTS
> > and either of CRYPTO_AES, CRYPTO_CBC, CRYPTO_CTR or CRYPTO_XTS is built
> > as module or disabled, alg_test() from crypto/testmgr.c complains during
> > boot about failing to allocate the generic fallback implementations
> > (2 == ENOENT):

> > [    0.540953] Failed to allocate xts(aes) fallback: -2
> > [    0.541014] alg: skcipher: failed to allocate transform for p8_aes_xts: -2
> > [    0.541120] alg: self-tests for p8_aes_xts (xts(aes)) failed (rc=-2)
> > [    0.544440] Failed to allocate ctr(aes) fallback: -2
> > [    0.544497] alg: skcipher: failed to allocate transform for p8_aes_ctr: -2
> > [    0.544603] alg: self-tests for p8_aes_ctr (ctr(aes)) failed (rc=-2)
> > [    0.547992] Failed to allocate cbc(aes) fallback: -2
> > [    0.548052] alg: skcipher: failed to allocate transform for p8_aes_cbc: -2
> > [    0.548156] alg: self-tests for p8_aes_cbc (cbc(aes)) failed (rc=-2)
> > [    0.550745] Failed to allocate transformation for 'aes': -2
> > [    0.550801] alg: cipher: Failed to load transform for p8_aes: -2
> > [    0.550892] alg: self-tests for p8_aes (aes) failed (rc=-2)


> > Check for these dependencies if crypto tests enabled.

> From my POV the problem of missing dependencies on fallback
> implementations is independent of the tests, the tests only happen to
> exhibit the issue.
I thought that after boot missing ciphers would be available from module and
loaded. But you're most likely right.


> > NOTE: this requires all these dependencies to be builtin if
> > !CRYPTO_MANAGER_DISABLE_TESTS, which is too strict on
> > CRYPTO_DEV_VMX_ENCRYPT=m.

> FWIW, I would not make the dependency conditional on
> !CRYPTO_MANAGER_DISABLE_TESTS.
You're right, I wasn't sure myself to use !CRYPTO_MANAGER_DISABLE_TESTS
as I also noticed that most of the dependencies in crypto/Kconfig and
drivers/crypto/Kconfig are described via "select".

...
> > what am I missing to allow e.g. CRYPTO_AES=m when
> > CRYPTO_DEV_VMX_ENCRYPT=m?

> If you were to leave the condition on
> !CRYPTO_MANAGER_DISABLE_TESTS away as suggested above, that is if you
> expressed the dependency like this ...

> 	config CRYPTO_DEV_VMX
> 		[...]
> 		depends on CRYPTO_AES

> ... then this would impose an upper limit (with the ordering n < m < y)
> of CRYPTO_AES on the possible values for CRYPTO_DEV_VMX. See
> Documentation/kconfig-language.rst.

> That is, if e.g. CRYPTO_AES=m, then only CRYPTO_DEV_VMX=n/m would be valid
> choices.


> I wouldn't go with "depends on", but prefer "select" in this case
> though. "select" is similar, but imposes a lower bound on the selected
> Kconfig symbol.

> That is,

> 	config CRYPTO_DEV_VMX
> 		[...]
> 		select CRYPTO_AES

Yep, I also tried select, but on CRYPTO_DEV_VMX_ENCRYPT, that's why it was not
working. Thanks for explanation and a hint => going to send v2.

Kind regards,
Petr

> would force the value of CRYPTO_AES to >= whatever the user picks for
> CRYPTO_DEV_VMX.

> (According to Documentation/kconfig-language.rst, you could even make
>  this conditional on !CRYPTO_MANAGER_DISABLE_TESTS:

>  	select CRYPTO_AES if !CRYPTO_MANAGER_DISABLE_TESTS
> )

> Note that the 'select CRYPTO_AES' approach seems consistent to what is
> done for all the other crypto drivers depending on fallbacks, c.f. e.g
> drivers/crypto/Kconfig.

> Thanks,

> Nicolai