Page MenuHomeFreeBSD

caam: Introduce new driver for NXP CAAM controller
Needs ReviewPublic

Authored by pkochanowski_sii.pl on Jun 14 2024, 7:34 AM.
Tags
None
Referenced Files
F102321333: D45589.diff
Sun, Nov 10, 3:53 PM
Unknown Object (File)
Fri, Oct 18, 7:42 AM
Unknown Object (File)
Tue, Oct 15, 10:21 PM
Unknown Object (File)
Oct 8 2024, 10:49 AM
Unknown Object (File)
Sep 27 2024, 3:44 AM
Unknown Object (File)
Sep 18 2024, 10:42 AM
Unknown Object (File)
Sep 13 2024, 3:18 AM
Unknown Object (File)
Sep 12 2024, 11:38 AM

Details

Reviewers
andrew
manu
Group Reviewers
arm64
Summary

The NXP CAAM (Cryptographic Acceleration and Assurance Module) is a
controller present on QoriQ platform (for example LS1046A SoC).
This controller allows to execute job descriptors on Job Rings that
allows to perform cryptographic operations.

The driver consist of driver caam that acts as a simplebus master
for the caam_jr drivers responsible for handling the CAAM Job Rings.

This driver is based on ARM TF-A firmware implementation ported to
the FreeBSD system.

Authored-by: Pawel Kochanowski <pkochanowski@sii.pl>
Co-Authored-by: Dawid Górecki <dgorecki@sii.pl>
Co-Authored-by: Maciej Szczepaniak <mszczepaniak2@sii.pl>
Obtained from: Sii Poland
Sponsored by: Alstom Group

Test Plan

Change tested on device based on LS1046A.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

There are a few style issues in this. I've commented on some of the issues, but there are more of the same type that should also be fixed.

sys/arm64/qoriq/caam/caam.c
12–15

__FBSDID use has been removed

206
250
469

The kernel malloc with M_WAITOK can't fail so doesn't need a NULL test.

519

Why is this needed? I though simplebus already handled bus resources.

581–582

Why are you not using the existing simplebus implementation?

sys/arm64/qoriq/caam/caam.h
39–42

Are both of these true for all hardware?

sys/arm64/qoriq/caam/jr/caam_jr_pool.c
84

There is no need to check both. If ret is non-zero then vmem_alloc won't have allocated anything.

val_packett.cool added inline comments.
sys/arm64/qoriq/caam/caam.c
156

This should not be statically determined.. If we have hardware differences, we need to have different FDT compatible strings.

Added fixes for some of the comments, tested on LS1046 device, fixed one bug in resources release.

pkochanowski_sii.pl added inline comments.
sys/arm64/qoriq/caam/caam.c
156

This was in inherited code from ARM TF-A, there of course it is done during the boot and the bootloader is device specific.
I was not able to find any node in device tree that could show if the device have the proper downstream caches. I have removed the setup dedicated for LS208xA but kept the common write cache as it seems to be common.

519

The FDT defines the sec_jr as subdevices, but the address ranges for registers used in job rings are defined as offsets in global CAAM register space:

		crypto: crypto@1700000 {
			compatible = "fsl,sec-v5.4", "fsl,sec-v5.0",
				     "fsl,sec-v4.0";
			fsl,sec-era = <8>;
			#address-cells = <1>;
			#size-cells = <1>;
			ranges = <0x0 0x00 0x1700000 0x100000>;
			reg = <0x00 0x1700000 0x0 0x100000>;
			interrupts = <GIC_SPI 75 IRQ_TYPE_LEVEL_HIGH>;

			sec_jr0: jr@10000 {
				compatible = "fsl,sec-v5.4-job-ring",
					     "fsl,sec-v5.0-job-ring",
					     "fsl,sec-v4.0-job-ring";
				reg = <0x10000 0x10000>;
				interrupts = <GIC_SPI 71 IRQ_TYPE_LEVEL_HIGH>;
			};

We were not able to use the device tree without creating our own resource manager. If there is any option to manage it without custom code please let us know.

534

Small fix after retesting on device. After this devctl detach caam0 works correclty.

sys/arm64/qoriq/caam/caam.h
39–42

The endianess was updated to be detected from device tree and then used in runtime. By default big endian order is used.
The 64bit version is used by default as we have fixed the driver to aarch64 that need to use 64-bit addressing IMHO.

There are a few style issues in this. I've commented on some of the issues, but there are more of the same type that should also be fixed.

Hi,

First of all thank you for taking your time to look into this patch. I have updated the style related comments and we updated all patches in the stack.
I also answered to other comments inline.

BR,
Pawel.

Added a DEINIT on the job_arg in the caam_run_descriptor_jr_blocking that was found in internal code reviews.