crypto - qce: make algos optional and remove the BROKEN label
Subsystems (5)
v11 cover crypto - qce: make algos optional and remove the BROKEN label
This is the follow-up to the three smaller series fixing crypto self-tests in the QCE driver. It makes the crypto algorithms opt-in via a module parameter and removes the BROKEN label so that the driver is built by default.
The dependencies are:
- https://lore.kernel.org/all/[email protected]/
- https://lore.kernel.org/all/[email protected]/
- https://lore.kernel.org/all/[email protected]/
---
Changes in v11:
- Split out the patches making algorithms optional and removing the
BROKEN Kconfig label into their own series
- Link to v10: https://patch.msgid.link/[email protected]
Changes in v10:
- don't pass a NULL buffer to sg_init_one()
- fix byte order of the imported hash state on empty finalization
- don't recompute the CTR counter in software for every request
- reject skcipher requests with a too short scatterlist
- check fragmentation for empty CCM decryption requests too
- don't use a static loop iterator (duh...)
- initialize the return value in qce_handle_queue()
- Link to v9: https://patch.msgid.link/[email protected]
Changes in v9:
- Rebase on top of cryptodev/master
- Rephrase Kconfig help text changes
- Link to v8: https://patch.msgid.link/[email protected]
Changes in v8:
- Add a patch making the crypto driver *not* register algos by default
but only if the user explicitly requests it with the provided module
parameter
- Use the "offloader" name for the crypto IP instead of "accelerator"
- Fix device module table issues
- Make the core driver use a list (despite it only having a single
member now) of child auxiliary devices to register to make it clear we
have the intention of extending this once we start the work on secure
media playback
- Link to v7: https://patch.msgid.link/[email protected]
Changes in v7:
- Add follow-up changes converting the QCE crypto driver to auxiliary
bus, adding the core QCE driver under drivers/soc/ registering the
auxiliary device and removing the BROKEN Kconfig label
- Link to v6: https://patch.msgid.link/[email protected]
Changes in v6:
- Handle all zero-length HMAC finalizations (like imported state with
data already hashed), not only the genuinely empty message case
- Add an additional patch addressing the fragmented skcipher payload
issue
- Link to v5: https://patch.msgid.link/[email protected]
Changes in v5:
- Dropped patch 1/8 that's already queued
- Use the pre-allocated fallback ahash for HMAC transforms (Herbert)
- Link to v4: https://patch.msgid.link/[email protected]
Changes in v4:
- Remove remaining ECB and DES3 bits
- Pick up tags
- Link to v3: https://patch.msgid.link/[email protected]
Changes in v3:
- Remove even more algorithms and dead code in patch 1/8
- Link to v2: https://patch.msgid.link/[email protected]
Changes in v2:
- Add fixes for the full suite of crypto self-tests
- Add Fixes and Cc tags
- Link to v1: https://patch.msgid.link/[email protected]/
---
Bartosz Golaszewski (2):
crypto: qce - Only register algos if the user really wants it Revert "crypto: qce - Mark QCE as BROKEN"
arch/arm/configs/multi_v7_defconfig | 1 + arch/arm64/configs/defconfig | 1 + drivers/crypto/Kconfig | 22 +++++++++++++--------- drivers/crypto/qce/core.c | 12 +++++++++--- 4 files changed, 24 insertions(+), 12 deletions(-)
---
base-commit: 178aff4cf425b3fc51ff09c571fa1bfbc1eb8ea4
change-id: 20261022-qce-fixes-broken-label-e6aaf9624fe9
prerequisite-message-id: <[email protected]>
prerequisite-patch-id: 7c93b8489dbf6d4edf932389c4c931d897f38ec2
prerequisite-message-id: <[email protected]>
prerequisite-patch-id: d3f97f2fcbc99093e51f4b749eda4c2d0cff4e92
prerequisite-patch-id: 7c4695a73eaad5db182edb09a608c6bc51883c82
prerequisite-patch-id: f3743392de54aa4ce3da76d11637dfeedb922b3a
prerequisite-patch-id: 02e9caf718743f7860aa19031e6809bdd718878e
prerequisite-message-id: <[email protected]>
prerequisite-patch-id: 4764101927c437b38d71379f95b98c82a20a48ca
prerequisite-patch-id: e918a7b007c3a1b88b526fdf7865b811155deceb
prerequisite-patch-id: 8aba9f114cc43e24c8568caf5ca4e7bb82e54b67
Best regards,
Signature · 1 line
Bartosz Golaszewski <[email protected]>
PATCH v11 1/2 crypto: qce - Only register algos if the user really wants it
This driver's crypto algorithms still have a few issues that need addressing which don't currently trigger any self-test failures. For that reason: don't register the algorithms unless the user really wants it.
Add a module parameter to the QCE driver that by default causes the driver to *not* register the crypto algorithms. It will only be one if the user explicitly passes the do_register_algos argument as true.
The power-management part of the driver is kept as is to allow scaling down of interconnects gating of clocks by runtime PM.
--- drivers/crypto/qce/core.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
drivers/crypto/qce/core.c +9 -3
@@ -10,6 +10,7 @@#include <linux/interconnect.h> #include <linux/interrupt.h> #include <linux/module.h> +#include <linux/moduleparam.h> #include <linux/platform_device.h> #include <linux/pm.h> #include <linux/pm_runtime.h>
@@ -22,6 +23,9 @@#include "sha.h" #include "aead.h" +static bool do_register_algos; +module_param (do_register_algos, bool,0444 ); +#define QCE_QUEUE_LENGTH 1#define QCE_DEFAULT_MEM_BANDWIDTH 393600
@@ -259,9 +263,11 @@ static int qce_crypto_probe(struct platform_device *pdev) qce->async_req_enqueue = qce_async_request_enqueue; qce->async_req_done = qce_async_request_done; - ret =devm_qce_register_algs (qce); -if (ret) -return ret; +if (do_register_algos) { + ret =devm_qce_register_algs (qce); +if (ret) +return ret; + }/* Configure autosuspend after successful init */ pm_runtime_set_autosuspend_delay (dev,100 );
Signature · 1 line
2.47.3
Re: [PATCH v11 1/2] crypto: qce - Only register algos if the user really wants it
Quoted 12 lines · Bartosz Golaszewski
This driver's crypto algorithms still have a few issues that need addressing which don't currently trigger any self-test failures. For that reason: don't register the algorithms unless the user really wants it.
Add a module parameter to the QCE driver that by default causes the driver to *not* register the crypto algorithms. It will only be one if the user explicitly passes the
do_register_algosargument as true.The power-management part of the driver is kept as is to allow scaling down of interconnects gating of clocks by runtime PM.
Quoted 6 lines
Tested-by Kuldeep Singh <[email protected]>Tested-by Pankaj Patil <[email protected]>Signed-off-by Bartosz Golaszewski <[email protected]>---
Konrad
[email protected]
AI reviewRe: [PATCH v11 1/2] crypto: qce - Only register algos if the user really wants it
In reply to Bartosz GolaszewskiThank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] crypto: qce: missing kernel-parameters.txt doc for do_register_algos --
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] crypto: qce: missing kernel-parameters.txt doc for do_register_algos --
commit 47675c12b3c6637f5e62afe90e1fb4f153328b57
Author: Bartosz Golaszewski <[email protected]>
crypto: qce - Only register algos if the user really wants it
This commit adds a do_register_algos module parameter to the QCE crypto driver so that crypto algorithms are not registered by default during probe unless explicitly enabled by the user.
Quoted 4 lines
drivers/crypto/qce/core.c +0
-0
[ ... ]
drivers/crypto/qce/core.c line 27
Quoted 6 lines
drivers/crypto/qce/core.c +2
-0@@ -22,6 +23,9 @@#include "sha.h" #include "aead.h" +static bool do_register_algos; +module_param (do_register_algos, bool,0444 );
[Severity: Low]
In drivers/crypto/qce/core.c, module_param(do_register_algos, bool, 0444) exposes qcrypto.do_register_algos as a kernel command-line and module parameter to gate devm_qce_register_algs(qce) in qce_crypto_probe().
Should Documentation/admin-guide/kernel-parameters.txt also be updated to document qcrypto.do_register_algos, along with a parameter description in drivers/crypto/qce/core.c?
Quoted 19 lines
drivers/crypto/qce/core.c +6
-3+#define QCE_QUEUE_LENGTH 1#define QCE_DEFAULT_MEM_BANDWIDTH 393600@@ -259,9 +263,11 @@ static int qce_crypto_probe(struct platform_device *pdev) qce->async_req_enqueue = qce_async_request_enqueue; qce->async_req_done = qce_async_request_done; - ret =devm_qce_register_algs (qce); -if (ret) -return ret; +if (do_register_algos) { + ret =devm_qce_register_algs (qce); +if (ret) +return ret; + }/* Configure autosuspend after successful init */ pm_runtime_set_autosuspend_delay (dev,100 );
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/20261022-qce-fixes-broken-label-v11-0-0d33c34fd0c3@oss.qualcomm.com?part=1
PATCH v11 2/2 Revert "crypto: qce - Mark QCE as BROKEN"
In reply to Bartosz GolaszewskiThe self-tests for this driver have been fixed. The crypto algorithms are only registered if the user explicitly requests it with the provided module parameter defaulting to false. There's no reason to further keep it unbuildable with the BROKEN label so revert commit df373d39c6f0 ("crypto: qce - Mark QCE as BROKEN").
While at it: update the Kconfig description by saying this is an "offloader" and not an "accelerator".
--- arch/arm/configs/multi_v7_defconfig | 1 + arch/arm64/configs/defconfig | 1 + drivers/crypto/Kconfig | 22 +++++++++++++--------- 3 files changed, 15 insertions(+), 9 deletions(-)
arch/arm/configs/multi_v7_defconfig +1 -0
@@ -1323,6 +1323,7 @@ CONFIG_CRYPTO_DEV_ATMEL_AES=m CONFIG_CRYPTO_DEV_ATMEL_TDES=m CONFIG_CRYPTO_DEV_ATMEL_SHA=m CONFIG_CRYPTO_DEV_MARVELL_CESA=m +CONFIG_CRYPTO_DEV_QCE=m CONFIG_CRYPTO_DEV_ROCKCHIP=m CONFIG_CRYPTO_DEV_STM32_HASH=m CONFIG_CRYPTO_DEV_STM32_CRYP=m
arch/arm64/configs/defconfig +1 -0
@@ -1949,6 +1949,7 @@ CONFIG_CRYPTO_AES_ARM64_CE_CCM=y CONFIG_CRYPTO_DEV_SUN8I_CE=m CONFIG_CRYPTO_DEV_FSL_CAAM=m CONFIG_CRYPTO_DEV_FSL_DPAA2_CAAM=m +CONFIG_CRYPTO_DEV_QCE=m CONFIG_CRYPTO_DEV_TEGRA=m CONFIG_CRYPTO_DEV_ZYNQMP_AES=m CONFIG_CRYPTO_DEV_ZYNQMP_SHA3=m
drivers/crypto/Kconfig +13 -9
@@ -527,17 +527,21 @@ source "drivers/crypto/marvell/Kconfig" source "drivers/crypto/intel/Kconfig" config CRYPTO_DEV_QCE - tristate "Qualcomm crypto engine accelerator" - depends on (BROKEN && ARCH_QCOM) || COMPILE_TEST + tristate "Qualcomm crypto engine offloader" + depends on ARCH_QCOM || COMPILE_TEST depends on HAS_IOMEM help - This driver supports Qualcomm crypto engine accelerator - hardware. To compile this driver as a module, choose M here. The - module will be called qcrypto. - - This driver does not have exclusive access to the - hardware, causing races with the secure world. It - is also slower than the CPU. + This driver supports Qualcomm crypto engine offloader hardware. To + compile this driver as a module, choose M here. The module will be + called qcrypto. + + NOTE: With qcrypto.do_register_algos module parameter set to true, + this driver does not have exclusive access to the hardware, and may + cause races with the secure world. It is also *slower* than the + CPU for the same algorithms. By default it will not register crypto + algorithms and only scale down the interconnect bandwidth and disable + the relevant clocks. Use qcrypto.do_register_algos=1 at your own + risk! config CRYPTO_DEV_QCE_SKCIPHER bool
Signature · 1 line
2.47.3
Re: [PATCH v11 2/2] Revert "crypto: qce - Mark QCE as BROKEN"
Quoted 9 lines · Bartosz Golaszewski
The self-tests for this driver have been fixed. The crypto algorithms are only registered if the user explicitly requests it with the provided module parameter defaulting to false. There's no reason to further keep it unbuildable with the BROKEN label so revert commit
df373d39c6f0("crypto: qce - Mark QCE as BROKEN").While at it: update the
Kconfigdescription by saying this is an "offloader" and not an "accelerator".
Quoted 7 lines
Fixes df373d39c6f0 ("crypto: qce - Mark QCE as BROKEN")Tested-by Kuldeep Singh <[email protected]>Tested-by Pankaj Patil <[email protected]>Signed-off-by Bartosz Golaszewski <[email protected]>---
Konrad
[email protected]
AI reviewRe: [PATCH v11 2/2] Revert "crypto: qce - Mark QCE as BROKEN"
In reply to Bartosz GolaszewskiThank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] crypto: qce: missing documentation for qcrypto.do_register_algos parameter --
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] crypto: qce: missing documentation for qcrypto.do_register_algos parameter --
commit 7402540901cd549c757f6e4846f133d63cd4febc
Author: Bartosz Golaszewski <[email protected]>
Revert "crypto: qce - Mark QCE as BROKEN"
This commit reverts marking the Qualcomm crypto engine driver as broken now that self-tests are fixed and algorithm registration is gated behind the do_register_algos module parameter. It also updates the Kconfig help text to describe the parameter and re-enables the driver in arm and arm64 defconfigs.
Quoted 22 lines
drivers/crypto/Kconfig +3
-9@@ -527,17 +527,21 @@ source "drivers/crypto/marvell/Kconfig" source "drivers/crypto/intel/Kconfig" config CRYPTO_DEV_QCE - tristate "Qualcomm crypto engine accelerator" - depends on (BROKEN && ARCH_QCOM) || COMPILE_TEST + tristate "Qualcomm crypto engine offloader" + depends on ARCH_QCOM || COMPILE_TEST depends on HAS_IOMEM help - This driver supports Qualcomm crypto engine accelerator - hardware. To compile this driver as a module, choose M here. The - module will be called qcrypto. - - This driver does not have exclusive access to the - hardware, causing races with the secure world. It - is also slower than the CPU. + This driver supports Qualcomm crypto engine offloader hardware. To
drivers/crypto/Kconfig source "drivers/crypto/marvell/Kconfig" · line 544
Quoted 10 lines
drivers/crypto/Kconfig +10
-0+ compile this driver as a module, choose M here. The module will be + called qcrypto. + + NOTE: With qcrypto.do_register_algos module parameter set to true, + this driver does not have exclusive access to the hardware, and may + cause races with the secure world. It is also *slower* than the + CPU for the same algorithms. By default it will not register crypto + algorithms and only scale down the interconnect bandwidth and disable + the relevant clocks. Use qcrypto.do_register_algos=1 at your own + risk!
[Severity: Low]
The help text for CRYPTO_DEV_QCE in drivers/crypto/Kconfig instructs users to pass the qcrypto.do_register_algos=1 parameter, which was introduced in commit 47675c12b3c6 ("crypto: qce - Only register algos if the user really wants it"):
drivers/crypto/qce/core.c:
static bool do_register_algos;
module_param(do_register_algos, bool, 0444);However, neither commit adds an entry for qcrypto.do_register_algos to Documentation/admin-guide/kernel-parameters.txt or provides a MODULE_PARM_DESC() in drivers/crypto/qce/core.c (unlike aes_sw_max_len in drivers/crypto/qce/skcipher.c).
Should qcrypto.do_register_algos be documented in Documentation/admin-guide/kernel-parameters.txt and given a MODULE_PARM_DESC() entry?
Quoted 3 lines
drivers/crypto/Kconfig +0
-0config CRYPTO_DEV_QCE_SKCIPHER bool
[ ... ]
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/20261022-qce-fixes-broken-label-v11-0-0d33c34fd0c3@oss.qualcomm.com?part=2
Bartosz Golaszewski
maintainerQuoted 5 lines · Bartosz Golaszewski
This is the follow-up to the three smaller series fixing crypto self-tests in the QCE driver. It makes the crypto algorithms opt-in via a module parameter and removes the BROKEN label so that the driver is built by default.
Quoted 7 lines
The dependencies are:
- https://lore.kernel.org/all/[email protected]/
- https://lore.kernel.org/all/[email protected]/
- https://lore.kernel.org/all/[email protected]/Signed-off-by Bartosz Golaszewski <[email protected]>---
Hi Herbert!
Gentle ping, can these three series + this one now get queued for v7.4? The three dependencies have no more sashiko comments. The ones for this one are minor about missing module params docs so can probably be ignored or I can send a follow up.
Thanks,
Bartosz
Bartosz Golaszewski
maintainerQuoted 14 lines · Bartosz Golaszewski
Quoted 5 lines · Bartosz Golaszewski
This is the follow-up to the three smaller series fixing crypto self-tests in the QCE driver. It makes the crypto algorithms opt-in via a module parameter and removes the BROKEN label so that the driver is built by default.
Quoted 7 lines
The dependencies are:
- https://lore.kernel.org/all/[email protected]/
- https://lore.kernel.org/all/[email protected]/
- https://lore.kernel.org/all/[email protected]/Signed-off-by Bartosz Golaszewski <[email protected]>---
Quoted 7 lines
Hi Herbert!
Gentle ping, can these three series + this one now get queued for v7.4? The three dependencies have no more sashiko comments. The ones for this one are minor about missing module params docs so can probably be ignored or I can send a follow up.
Hi, another gentle ping since v7.4-rc1 is tagged. Is there anything I can do to move this forward?
Thanks,
Bartosz
Herbert Xu
maintainerQuoted 14 lines · Bartosz Golaszewski
Quoted 5 lines · Bartosz Golaszewski
This is the follow-up to the three smaller series fixing crypto self-tests in the QCE driver. It makes the crypto algorithms opt-in via a module parameter and removes the BROKEN label so that the driver is built by default.
Quoted 7 lines
The dependencies are:
- https://lore.kernel.org/all/[email protected]/
- https://lore.kernel.org/all/[email protected]/
- https://lore.kernel.org/all/[email protected]/Signed-off-by Bartosz Golaszewski <[email protected]>---
Quoted 6 lines
Hi Herbert!
Gentle ping, can these three series + this one now get queued for v7.4? The three dependencies have no more sashiko comments. The ones for this one are minor about missing module params docs so can probably be ignored or I can send a follow up.
Did you miss the Sashiko comments about missing documentation? Please address them first.
Thanks,
Signature · 3 lines
Email: Herbert Xu <[email protected]> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt