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

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

[Petr Vorel]

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.

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

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?

Kind regards,
Petr

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

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

[Nicolai Stange]

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

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

[Petr Vorel]

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