core: qcom: add GPCE BAM crypto driver support

[rehman688]

This series adds support for the Qualcomm General Purpose Crypto
Engine (GPCE) using the BAM transport interface.

The series introduces:

• HWIO adaptation layer
• OP-TEE environment layer
• BAM transport support
• Crypto Engine HAL integration
• drvcrypt cipher/hash/MAC wrappers

Supported algorithms:

• AES ECB/CBC/CTR/CTS/XTS
• DES/3DES ECB/CBC
• SHA1/SHA2 family
• HMAC-SHA1/224/256/384/512
• AES-CMAC

Crypto payload transfers use BAM while register programming is
currently performed through HWIO.

[Harshal Dev (harshaldev27)]

Hi rehman688 ,

This is impossible to review in the present format unfortunately. Can you please reduce the size of the cover-letter to make it more concise? Also, please break this patch series into small bi-sectable patches each introducing one logical change. Also, at a high level I can see this isn't following the coding standards of the project. Please feed the coding standards to your AI agent and check against that. Also run check-patch script for help you with the format of patches.

[Tony J Hamilton (TonyJH1)]

Please ensure that all code has been tested, and remove all redundant/dead code. As Harshal say, split into multiple smaller patch sets that can be reviewed by the maintainers.

[Tony J Hamilton (TonyJH1)]

Split this patch set into manageable patches

[Jorge A. Ramirez-Ortiz (ldts)]

another thing we should discuss - at least internally - is the implicit decision made by placing the driver in core/drivers/ which means dropping the openssl/pkcs11 support. Are we sure we want to do this?
I havent looked at the code yet but crypto drivers (under the crypto API) sit on core/drivers/crypto/qcom/

[Sumit Garg (b49020)]

One more high level comment: since GPCE is a shared resource among Linux kernel and OP-TEE at runtime, have we run crypto tests in parallel on both sides?

I remember discussing with Bartosz Gołaszewski (@brgl) (Kernel GPCE maintainer) about BAM locking and stuff implemented in kernel driver. Are we taking care of any GPCE synchronization in OP-TEE?

[Bartosz Gołaszewski (brgl)]

One more high level comment: since GPCE is a shared resource among Linux kernel and OP-TEE at runtime, have we run crypto tests in parallel on both sides?

Thanks Sumit Garg (@b49020). I'd be interested in being able to run this kind of tests on sm8650 or lemans as it would potentially push the kernel changes forward.

[Sumit Garg (b49020)]

One more high level comment: since GPCE is a shared resource among Linux kernel and OP-TEE at runtime, have we run crypto tests in parallel on both sides?

Thanks Sumit Garg (Sumit Garg (@b49020)). I'd be interested in being able to run this kind of tests on sm8650 or lemans as it would potentially push the kernel changes forward.

Although this BAM based GPCE driver looks pretty much target agnostic, rehman688 can you confirm if it would work on Lemans as well with configs enabled?

For Lemans, steps to replicate open boot stack are here: https://ldts.github.io/qcom-buildroot/index.html

Note there was a register based GPCE driver port done earlier for Lemans here: OP-TEE/optee_os#7938, not sure if there can be any synchronization possible with direct register based approach.

[Paulo Martins (paulosmartins)]

Note there was a register based GPCE driver port done earlier for Lemans here: OP-TEE/optee_os#7938, not sure if there can be any synchronization possible with direct register based approach.

We should try to understand why this was ported like that and which use-cases it's targeting. The driver that Habeeb is porting also allows for a direct register approach, so it can also potentially replace the driver you're referring to

Split this patch set into manageable patches

Actually, one idea to reduce the size of the changes would be to first configure cryptolib using HWIO, and after that, add BAM support. What do you think rehman688 ?

[rehman688]

Note there was a register based GPCE driver port done earlier for Lemans here: OP-TEE/optee_os#7938, not sure if there can be any synchronization possible with direct register based approach.

We should try to understand why this was ported like that and which use-cases it's targeting. The driver that Habeeb is porting also allows for a direct register approach, so it can also potentially replace the driver you're referring to

Split this patch set into manageable patches

Actually, one idea to reduce the size of the changes would be to first configure cryptolib using HWIO, and after that, add BAM support. What do you think rehman688 ?

Note there was a register based GPCE driver port done earlier for Lemans here: OP-TEE/optee_os#7938, not sure if there can be any synchronization possible with direct register based approach.

We should try to understand why this was ported like that and which use-cases it's targeting. The driver that Habeeb is porting also allows for a direct register approach, so it can also potentially replace the driver you're referring to

Split this patch set into manageable patches

Actually, one idea to reduce the size of the changes would be to first configure cryptolib using HWIO, and after that, add BAM support. What do you think rehman688 ?

I think it's better to split the series algorithm-wise. I'll start with hash support over the BAM path, then follow up with cipher and MAC support in separate series.

[rehman688]

One more high level comment: since GPCE is a shared resource among Linux kernel and OP-TEE at runtime, have we run crypto tests in parallel on both sides?

Thanks Sumit Garg (Sumit Garg (Sumit Garg (@b49020))). I'd be interested in being able to run this kind of tests on sm8650 or lemans as it would potentially push the kernel changes forward.

Although this BAM based GPCE driver looks pretty much target agnostic, rehman688 can you confirm if it would work on Lemans as well with configs enabled?

For Lemans, steps to replicate open boot stack are here: https://ldts.github.io/qcom-buildroot/index.html

Note there was a register based GPCE driver port done earlier for Lemans here: OP-TEE/optee_os#7938, not sure if there can be any synchronization possible with direct register based approach.

Sumit Garg (@b49020) Yes, the driver is chipset-agnostic. For Lemans, we mainly need the Lemans-specific HWIO definitions generated from IPCAT and placed under the Lemans platform directory.

[Sumit Garg (b49020)]

rehman688 please just strip down the HWIO header to macros which are actually used. You can refer to the other Qcom driver HWIO headers reference how they were stripped off as well as adapted to match OP-TEE coding style.

[rehman688]

One more high level comment: since GPCE is a shared resource among Linux kernel and OP-TEE at runtime, have we run crypto tests in parallel on both sides?

I remember discussing with Bartosz Gołaszewski (Bartosz Gołaszewski (@brgl)) (Kernel GPCE maintainer) about BAM locking and stuff implemented in kernel driver. Are we taking care of any GPCE synchronization in OP-TEE?

In BAM mode, synchronization is handled automatically by the driver. The driver acquires the GPCE when a crypto operation starts and releases it when the operation completes, ensuring that only one operation uses the hardware at a time.

[Sumit Garg (b49020)]

Note there was a register based GPCE driver port done earlier for Lemans here: OP-TEE/optee_os#7938, not sure if there can be any synchronization possible with direct register based approach.

We should try to understand why this was ported like that and which use-cases it's targeting. The driver that Habeeb is porting also allows for a direct register approach, so it can also potentially replace the driver you're referring to

Paulo Martins (@paulosmartins) open source development is always incremental in nature. So it's natural for people to build on top of each other's work. However, what we would really like to understand is the difference among BAM and register based approach? How is the runtime synchronization maintained with the Linux kernel GPCE driver?

The use-case remains the same here for both implementations to offload crypto to GPCE in OP-TEE for Qualcomm platforms. One should use the extensive OP-TEE xtest suite to validate all the GPCE crypto offload. Have we tested that?

[Sumit Garg (b49020)]

As an example this OP-TEE/optee_os#7938 was tested on Lemans using the xtest. I would also like us to run kernel crypto tests in parallel with GPCE. Bartosz Gołaszewski (@brgl) can you provide information how to execute them?