Re: [PATCH 1/1] crypto: vmx: Fix missing dependencies during boot

[Nicolai Stange <nstange@suse.de>]

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.


>
> 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.


>
> Fixes: c07f5d3da643 ("crypto: vmx - Adding support for XTS")
> Fixes: d2e3ae6f3aba ("crypto: vmx - Enabling VMX module for PPC64")
>
> Link: https://bugzilla.suse.com/show_bug.cgi?id=1195768
>
> Suggested-by: Nicolai Stange <nstange@suse.de>
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> Hi,
>
> 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

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


>
>  drivers/crypto/vmx/Kconfig | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/drivers/crypto/vmx/Kconfig b/drivers/crypto/vmx/Kconfig
> index c85fab7ef0bd..d692802fad9e 100644
> --- a/drivers/crypto/vmx/Kconfig
> +++ b/drivers/crypto/vmx/Kconfig
> @@ -2,6 +2,10 @@
>  config CRYPTO_DEV_VMX_ENCRYPT
>  	tristate "Encryption acceleration support on P8 CPU"
>  	depends on CRYPTO_DEV_VMX
> +	depends on CRYPTO_MANAGER_DISABLE_TESTS && CRYPTO_AES || CRYPTO_AES=y
> +	depends on CRYPTO_MANAGER_DISABLE_TESTS && CRYPTO_CBC || CRYPTO_CBC=y
> +	depends on CRYPTO_MANAGER_DISABLE_TESTS && CRYPTO_CTR || CRYPTO_CTR=y
> +	depends on CRYPTO_MANAGER_DISABLE_TESTS && CRYPTO_XTS || CRYPTO_XTS=y
>  	select CRYPTO_GHASH
>  	default m
>  	help

-- 
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg), GF: Ivo Totev