| Message ID | 20220214185638.1457-1-pvorel@suse.cz (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/1] crypto: vmx: Fix missing dependencies during boot | expand |
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
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
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
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(+)