Page MenuHomeFreeBSD

qat: Add Intel® 4xxx Series platform support
ClosedPublic

Authored by MichalX.Gulbicki_intel.com on Aug 18 2022, 1:45 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Jun 8, 5:57 PM
Unknown Object (File)
Sat, Jun 8, 5:57 PM
Unknown Object (File)
Sat, Jun 8, 5:57 PM
Unknown Object (File)
Sat, Jun 8, 5:57 PM
Unknown Object (File)
Sat, Jun 8, 5:57 PM
Unknown Object (File)
Sat, Jun 8, 5:57 PM
Unknown Object (File)
Fri, Jun 7, 2:54 PM
Unknown Object (File)
Sat, Jun 1, 10:32 PM

Details

Summary

The driver exposes complete cryptography and data compression
API in the kernel and integrates with Open Crypto Framework.
Details of supported operations, devices and usage can be found
in man and on 01.org.

OCF backend changes:

  • changed GCM/CCM MAC validation policy to generate MAC by HW and validate by SW due to the QAT HW limitations.

Patch co-authored by: Krzysztof Zdziarski <krzysztofx.zdziarski@intel.com>
Patch co-authored by: Michal Jaraczewski <michalx.jaraczewski@intel.com>
Patch co-authored by: Michal Gulbicki <michalx.gulbicki@intel.com>
Patch co-authored by: Julian Grajkowski <julianx.grajkowski@intel.com>
Patch co-authored by: Piotr Kasierski <piotrx.kasierski@intel.com>
Patch co-authored by: Adam Czupryna <adamx.czupryna@intel.com>
Patch co-authored by: Konrad Zelazny <konradx.zelazny@intel.com>
Patch co-authored by: Katarzyna Rucinska <katarzynax.kargol@intel.com>
Patch co-authored by: Lukasz Kolodzinski <lukaszx.kolodzinski@intel.com>
Patch co-authored by: Zbigniew Jedlinski <zbigniewx.jedlinski@intel.com>

Sponsored by: Intel Corporation

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 49131
Build 46020: arc lint + arc unit

Event Timeline

I'm pretty sure you can mark this a a draft review.

This revision is now accepted and ready to land.Aug 24 2022, 12:09 AM
This revision now requires review to proceed.Dec 19 2022, 7:57 AM

No manual page changes, so approval still holds. Remember to bump .Dd on commit.

This revision is now accepted and ready to land.Dec 19 2022, 2:22 PM

I did not look at any of the common bits of the driver, just at the FreeBSD-specific OCF bits.

sys/dev/qat/qat/qat_ocf.c
1074

I'm curious why you need this? OCF in 13.0 and later mandates HW MAC verification for both EtA and AEAD modes. Or rather, clients must request MAC verification (CRYPTO_OP_VERIFY_DIGEST) in any decryption requests. Trying to set CRYPTO_OP_DECRYPT with CRYPTO_OP_COMPUTE_DIGEST is rejected in crypto.c before it even gets to the driver by crp_sanity() if INVARIANTS is enabled (it causes a panic even) and this requirement is documented in crypto_request(9).

sys/dev/qat/qat/qat_ocf.c
1074

This change has been introduced as result of following discussion:

due the HW limitations when it comes to MAC validation (such like mlen and crp_digest_start)
we have decided to this partially in SW (the same approach as initial QAT OCF implementation).
HW is responsible for generating MAC and SW checks if it is consistent with provided one - taking into
account mlen as well (see symDpCallback). It has been tested and accepted by Mark.

In case of GCM and CCM, QAT driver forces HW MAC validation regardles of what the caller sets in
sessionSetupData.verifyDigest. To overcome this limitation and not break backward compatibility
we have introduced:

  • icp_sal_setForceAEADMACVerify(CpaInstanceHandle instanceHandle, CpaBoolean forceAEADMacVerify)

By default force flag is set to true - OCF disables this to generate MAC by HW and perform additional checks. Please let me know if this answers your questions.

sys/dev/qat/qat/qat_ocf.c
1074

Ok. This is fine, but some other options you might consider:

  1. You could reject GCM requests whose csp_auth_mlen specifies a value that QAT doesn't support. I think there are some NIST KAT tests that use tag lengths that aren't 16, but I think all of the other use cases in the kernel (IPsec, TLS, disk encryption) all use 16 byte tags.
  1. You could work around the issue of needing the digest adjacent to the payload by just rewriting your scatter/gather lists you submit to the firmware to make the fields adjacent. This is the approach I used in ccr(4) which had some similar constraints. I get the "main" scatter/gather list from the buffer, but I then use the offset in struct cryptop to construct the scatter/gather list I send to the hardware, and am able to construct a scatter/gather list that always has AAD || IV || plaintext/ciphertext || MAC.
sys/dev/qat/qat/qat_ocf.c
1074

Yes, it makes sense. We can change implementation and upstream together with one of the next drops (we plan a few next year).

Some concerns:

  1. Further performance measurements are required to ensure that adding another flat buffer to SGL performs at least as well as writing calculated digest back to DRAM.
  2. Another concern is that it breaks the backward compatibility with the original QAT OCF driver from FBSD 13 - not sure if we can do this.

Let us know if it is acceptable to you to land this patch and address this comment in the next drops?

Oh, I was just making some suggestions on alternate options you might want to evaluate, I think your existing approach is fine. I think it is also fine for you to decide in the future if you want to adopt a different approach.

QAT Gen4 firmware placeholders replaced by official firmware binaries

This revision now requires review to proceed.Jan 13 2023, 3:15 PM

Please include some comment about the GCM digest verification in the commit log message. Ideally it would be committed as a separate patch, but I'm not sure if that's a lot of work to split out.

This revision is now accepted and ready to land.Jan 13 2023, 3:56 PM
This revision now requires review to proceed.Jan 16 2023, 10:08 AM
This revision is now accepted and ready to land.Jan 19 2023, 12:55 AM