plat-versal: add support for the Versal Net variant #6738

jcorbier · 2024-03-05T17:30:10Z

This series upgrades the AMD/Xilinx port with the following:

Add support for a new SoC in the Versal family
Add a hardware crypto accelerator driver for this specific SoC
Fixes for more recent versions of the Xilinx software environment (PLM APIs have changed in 2023)

Versal Net is a new SoC flavor based on the Versal architecture. This commit introduces it in versal platform code. Signed-off-by: Jeremie Corbier <[email protected]>

Make it more generic and still provide a default IPI channel to the PMC for the other drivers. Signed-off-by: Jeremie Corbier <[email protected]>

PLM HWRNG driver cannot provide more than 32 bytes of entropy at a time. Split bigger requests into 32 bytes chunks. Signed-off-by: Jeremie Corbier <[email protected]>

The Versal Net variant comes with a dedicated PKI engine. This driver makes use of the engine for ECDSA P-256, P-384, and P-521 sign, verify and key generation operations. Signed-off-by: Jeremie Corbier <[email protected]>

The original HUK driver generated the HUK using SHA-256. This commit replaces this mechanism with the more robust HKDF-SHA256. Signed-off-by: Jeremie Corbier <[email protected]>

Add simple PTA allowing to dynamically load data in the Versal PL. Signed-off-by: Jeremie Corbier <[email protected]>

- update crypto API IDs - update calls to the KAT subsystem Signed-off-by: Jeremie Corbier <[email protected]>

The XilNvm API has heavily changed between Versal and Versal Net. This commit adds support for the Net variant. Signed-off-by: Jeremie Corbier <[email protected]>

XilSecure has been updated to pack the public exponent right after the modulus rather than at a fixed 512 bytes (RSA 4096 key size) offset. See commit below for more details: Xilinx/embeddedsw@c2dd2eb Signed-off-by: Jeremie Corbier <[email protected]>

ldts · 2024-03-06T10:25:47Z

thanks @jcorbier

I need to ask that the changes to support the more recent AMD/Xilinx tools maintain backwards compatibility. We should be able to query the ABI at runtime - maybe even propose whatever is needed to AMD/Xilinx https://github.com/Xilinx/embeddedsw .

I'd like to understand as well the level of testing that has been done with this software (just the output of xtest, to check if you encountered any regressions (ie this is the changelog for 4.1.0 #6574 (comment) ).

Thirdly is there anything that you also plan on posting to https://github.com/OP-TEE/optee_docs ?

jcorbier · 2024-03-06T10:38:16Z

Thanks @ldts for your feedback.

I need to ask that the changes to support the more recent AMD/Xilinx tools maintain backwards compatibility. We should be able to query the ABI at runtime - maybe even propose whatever is needed to AMD/Xilinx https://github.com/Xilinx/embeddedsw .

Noted. Let me see how best we can implement that.

I'd like to understand as well the level of testing that has been done with this software (just the output of xtest, to check if you encountered any regressions (ie this is the changelog for 4.1.0 #6574 (comment) ).

I don't have access to the logs right now but the current state is the same as for Versal in 4.1.0.

Thirdly is there anything that you also plan on posting to https://github.com/OP-TEE/optee_docs ?

Yes, a working version is available here https://github.com/ProvenRun/optee_docs/tree/versal_net_port
It needs some additional work before I create a pull request for it though.

Same thing for build and manifest repositories.

ldts · 2024-03-08T11:06:39Z

we should split the drivers (rng/nvm) into a different files (versal_net_rng, versal_net_nvm?)

jcorbier · 2024-03-08T13:39:45Z

we should split the drivers (rng/nvm) into a different files (versal_net_rng, versal_net_nvm?)

Agreed, the initial thinking for the current implementation was to avoid as much code duplication as possible between versal and versal_net but in the end it makes things much more complicated than needed.

nathan-menhorn · 2024-03-12T15:08:46Z

Hi @jcorbier any updates on this PR?

jcorbier · 2024-03-12T15:47:37Z

Hi @jcorbier any updates on this PR?

Hi @nathan-menhorn, still working out the details of what needs to be done to properly split versal/versal-net code, including the TRNG update. I'll try and push an update to this PR by end of this week.

core/arch/arm/plat-versal/conf.mk

core/pta/versal/fpga_pta.c

core/drivers/versal_mbox.c

core/drivers/crypto/versal/ecc.c

core/drivers/crypto/versal/ecc_mbox.c

core/drivers/versal_mbox.c

ldts · 2024-03-31T19:17:50Z

@etienne-lms could you hold your comments until the patchset is updated please?

There are a couple of functional changes that need addressing first

ABI runtime detection (not only to support the older toolchain but also to be covered for changes in the future)
use different files for Versal net instead of conditional macros (since some drivers are radically different).

So I suggest we wait for that before we go into details (ie default configs, coding standards and so on) as some files will change quite a bit

Split NVM code into two seperate drivers, one for Versal, one for Versal Net, since both variants have very different NVM PLM code. Signed-off-by: Jeremie Corbier <[email protected]>

jcorbier · 2024-04-02T14:12:21Z

@etienne-lms could you hold your comments until the patchset is updated please?

Indeed, I'll be pusing fixup commits in the coming hours/days.

- Do not enable RPMB configs on Versal - Do not modify Versal IPI ID - Do not enable FPGA PTA on Versal Signed-off-by: Jeremie Corbier <[email protected]>

- s!invokeCommandEntryPoint!invoke_command! - Return TEE_ERROR_NOT_SUPPORTED rather than BAD_PARAMS in case an invalid command ID is used - Re-order includes as requested Signed-off-by: Jeremie Corbier <[email protected]>

- Add timeout in case the remote processor is unresponsive - versal_mbox_free explicitely sets the pointer to NULL Signed-off-by: Jeremie Corbier <[email protected]>

core/drivers/crypto/versal/ecc.c

core/drivers/crypto/versal/ecc_mbox.c

core/pta/versal/fpga_pta.c

core/drivers/versal_net_nvm.c

etienne-lms · 2024-04-03T08:01:03Z

core/drivers/versal_pm.c

-#define VERSAL_PM_MAJOR		0
-#define VERSAL_PM_MINOR		1
+#define VERSAL_PM_MAJOR		1
+#define VERSAL_PM_MINOR		0


Deserves a specific commit IMHO.

core/drivers/crypto/versal/authenc.c

No need for all the different error labels. Signed-off-by: Jeremie Corbier <[email protected]>

nathan-menhorn · 2024-05-07T21:54:57Z

Hi @jcorbier what's the current status of this PR? Thanks.

nathan-menhorn · 2024-05-23T20:49:32Z

Hi @jcorbier any updates on this PR? Are patches to address all the comments in the PR still estimated to come by the end of the month? Thanks.

Signed-off-by: Jeremie Corbier <[email protected]>

nathan-menhorn · 2024-07-29T13:54:56Z

Hi @jcorbier what's the status of this PR? Last we discussed updates were supposed to be pushed a few weeks ago? Thanks.

Signed-off-by: Jeremie Corbier <[email protected]>

… PLM Signed-off-by: Jeremie Corbier <[email protected]>

- Make the check stricter ; major and minor versions should match - Fix supported Versal PM version (1.0 rather than 0.1) Signed-off-by: Jeremie Corbier <[email protected]>

There were cases whe the ephemeral key needed to perform ECDSA signatures was not properly free'd in case an error happened during the process. Signed-off-by: Jeremie Corbier <[email protected]>

core/arch/arm/plat-versal/conf.mk

core/drivers/crypto/versal/authenc.c

core/drivers/crypto/versal/ecc.c

core/drivers/crypto/versal/ecc_mbox.c

core/drivers/versal_puf.c

core/include/drivers/versal_mbox.h

core/include/drivers/versal_trng.h

core/pta/versal/fpga_pta.c

ldts · 2024-08-22T13:00:14Z

@jcorbier do you plan on folding the commits as per the initial patch-set for further review? I can then have a a better look - last time I checked I found a simple regression (easy to fix).

Also I was testing the Xen hypervisor with the tip of OP-TEE on the vck190 evaluation kit and I found it to be broken. I was wondering if this is a configuration (optee+xen on Versal) that you have tested? I believe probably nobody has yet (@nathan-menhorn ?)

nathan-menhorn · 2024-08-26T03:00:24Z

Hi @ldts no testing has been performed on Xen+optee yet as there haven't been any customers requests.

nathan-menhorn · 2024-09-11T14:30:30Z

@jcorbier @ldts @etienne-lms just keeping this PR alive. We should be expecting some input from @jcorbier soon.

Signed-off-by: Jeremie Corbier <[email protected]>

jcorbier · 2024-09-18T15:35:54Z

@jcorbier do you plan on folding the commits as per the initial patch-set for further review? I can then have a a better look - last time I checked I found a simple regression (easy to fix).

Yes, there a couple more things I want to fix then I'll force push a clean patchset to clean up the current fixup commits mess.

Signed-off-by: Jeremie Corbier <[email protected]>

nathan-menhorn · 2024-09-18T19:01:09Z

core/drivers/versal_net_nvm.c

@@ -994,8 +1010,16 @@ TEE_Result versal_efuse_write_revoke_ppk(enum versal_nvm_ppk_type type)
 	return versal_efuse_write_misc(&misc_ctrl);
 }

+/*
+ * versal_efuse_write_revoke_id expects an efuse identifier between
+ * 1 and 256. 


@jcorbier 0 to 255

nathan-menhorn · 2024-09-18T19:03:27Z

core/drivers/versal_net_nvm.c

 TEE_Result versal_efuse_write_revoke_id(uint32_t id)
 {
+	if ((id < VERSAL_NET_REVOKE_EFUSE_MIN) ||


@jcorbier check should be between 0 and 255.

I'm not sure why the AMD software was implemented this way as this is very confusing and it doesn't match the OFFCHIP_REVOKE function, which expects values from 1 - 256, but this function expects values from 0 to 255

See the error handling of
https://github.com/Xilinx/embeddedsw/blob/master/lib/sw_services/xilnvm/src/versal_net/server/xnvm_efuse.c#L615C21-L617
compared to
https://github.com/Xilinx/embeddedsw/blob/master/lib/sw_services/xilnvm/src/versal_net/server/xnvm_efuse.c#L701-L703

nathan-menhorn · 2024-09-19T14:39:58Z

core/drivers/versal_net_nvm.c

@@ -1012,12 +1014,12 @@ TEE_Result versal_efuse_write_revoke_ppk(enum versal_nvm_ppk_type type)

 /*
 * versal_efuse_write_revoke_id expects an efuse identifier between
- * 1 and 256. 
+ * 1 and 256.


nathan-menhorn · 2024-09-19T14:40:36Z

core/drivers/versal_net_nvm.c

+	if (id < VERSAL_NET_REVOKE_EFUSE_MIN ||
+	    id > VERSAL_NET_REVOKE_EFUSE_MAX)


@jcorbier checks needs to be between 0 and 255 for this function.

jcorbier added 9 commits March 5, 2024 18:04

plat-versal: add support for Versal Net variant

9f1cd1b

Versal Net is a new SoC flavor based on the Versal architecture. This commit introduces it in versal platform code. Signed-off-by: Jeremie Corbier <[email protected]>

drivers: versal: rework mbox driver

0b8feb3

Make it more generic and still provide a default IPI channel to the PMC for the other drivers. Signed-off-by: Jeremie Corbier <[email protected]>

drivers: versal: use PLM for generic RNG operations

e0f5515

PLM HWRNG driver cannot provide more than 32 bytes of entropy at a time. Split bigger requests into 32 bytes chunks. Signed-off-by: Jeremie Corbier <[email protected]>

drivers: versal: PKI engine driver for Versal Net

5f23b2b

The Versal Net variant comes with a dedicated PKI engine. This driver makes use of the engine for ECDSA P-256, P-384, and P-521 sign, verify and key generation operations. Signed-off-by: Jeremie Corbier <[email protected]>

drivers: versal: use HKDF to derive the HUK

5e0d4a9

The original HUK driver generated the HUK using SHA-256. This commit replaces this mechanism with the more robust HKDF-SHA256. Signed-off-by: Jeremie Corbier <[email protected]>

core: pta: add Versal FPGA PTA

caf9ea9

Add simple PTA allowing to dynamically load data in the Versal PL. Signed-off-by: Jeremie Corbier <[email protected]>

drivers: versal: update to support more recent versions of the PLM

9267cb6

- update crypto API IDs - update calls to the KAT subsystem Signed-off-by: Jeremie Corbier <[email protected]>

drivers: versal: NVM driver for Versal Net

10d334a

The XilNvm API has heavily changed between Versal and Versal Net. This commit adds support for the Net variant. Signed-off-by: Jeremie Corbier <[email protected]>

drivers: versal: fix RSA driver

c668e58

XilSecure has been updated to pack the public exponent right after the modulus rather than at a fixed 512 bytes (RSA 4096 key size) offset. See commit below for more details: Xilinx/embeddedsw@c2dd2eb Signed-off-by: Jeremie Corbier <[email protected]>

etienne-lms reviewed Mar 20, 2024

View reviewed changes

fixup! drivers: versal: NVM driver for Versal Net

d477ff3

Split NVM code into two seperate drivers, one for Versal, one for Versal Net, since both variants have very different NVM PLM code. Signed-off-by: Jeremie Corbier <[email protected]>

jcorbier added 3 commits April 2, 2024 17:06

fixup! plat-versal: add support for Versal Net variant

9dd3ef9

- Do not enable RPMB configs on Versal - Do not modify Versal IPI ID - Do not enable FPGA PTA on Versal Signed-off-by: Jeremie Corbier <[email protected]>

fixup! core: pta: add Versal FPGA PTA

1dd1efa

- s!invokeCommandEntryPoint!invoke_command! - Return TEE_ERROR_NOT_SUPPORTED rather than BAD_PARAMS in case an invalid command ID is used - Re-order includes as requested Signed-off-by: Jeremie Corbier <[email protected]>

fixup! drivers: versal: rework mbox driver

6d01a34

- Add timeout in case the remote processor is unresponsive - versal_mbox_free explicitely sets the pointer to NULL Signed-off-by: Jeremie Corbier <[email protected]>

etienne-lms reviewed Apr 3, 2024

View reviewed changes

fixup! drivers: versal: rework mbox driver

446148d

No need for all the different error labels. Signed-off-by: Jeremie Corbier <[email protected]>

jcorbier added 4 commits June 20, 2024 16:02

fixup! drivers: versal: PKI engine driver for Versal Net

cc6883d

Signed-off-by: Jeremie Corbier <[email protected]>

fixup! drivers: versal: NVM driver for Versal Net

8f59535

Signed-off-by: Jeremie Corbier <[email protected]>

fixup! drivers: versal: PKI engine driver for Versal Net

a9df558

Signed-off-by: Jeremie Corbier <[email protected]>

fixup! drivers: versal: rework mbox driver

5678e3c

Signed-off-by: Jeremie Corbier <[email protected]>

github-actions bot added the Stale label Jul 29, 2024

github-actions bot removed the Stale label Jul 30, 2024

jcorbier added 9 commits August 14, 2024 11:57

fixup! drivers: versal: PKI engine driver for Versal Net

02325fa

Signed-off-by: Jeremie Corbier <[email protected]>

fixup! drivers: versal: rework mbox driver

62d5500

Signed-off-by: Jeremie Corbier <[email protected]>

fixup! drivers: versal: NVM driver for Versal Net

a0f7495

Signed-off-by: Jeremie Corbier <[email protected]>

fixup! drivers: versal: use PLM for generic RNG operations

76e0faa

Signed-off-by: Jeremie Corbier <[email protected]>

fixup! core: pta: add Versal FPGA PTA

c3c5971

Signed-off-by: Jeremie Corbier <[email protected]>

fixup! drivers: versal: PKI engine driver for Versal Net

3c91efc

Signed-off-by: Jeremie Corbier <[email protected]>

fixup! drivers: versal: update to support more recent versions of the…

5899f1b

… PLM Signed-off-by: Jeremie Corbier <[email protected]>

drivers: versal: fix PM ABI check

d699e7f

- Make the check stricter ; major and minor versions should match - Fix supported Versal PM version (1.0 rather than 0.1) Signed-off-by: Jeremie Corbier <[email protected]>

drivers: versal: free ECC ephemeral key in case of error

6e74be9

There were cases whe the ephemeral key needed to perform ECDSA signatures was not properly free'd in case an error happened during the process. Signed-off-by: Jeremie Corbier <[email protected]>

etienne-lms reviewed Aug 21, 2024

View reviewed changes

jcorbier added 10 commits September 18, 2024 15:37

fixup! plat-versal: add support for Versal Net variant

41deb61

Signed-off-by: Jeremie Corbier <[email protected]>

fixup! drivers: versal: PKI engine driver for Versal Net

aa36c59

Signed-off-by: Jeremie Corbier <[email protected]>

fixup! drivers: versal: rework mbox driver

8471ab7

Signed-off-by: Jeremie Corbier <[email protected]>

fixup! drivers: versal: NVM driver for Versal Net

d68d7d4

Signed-off-by: Jeremie Corbier <[email protected]>

fixup! drivers: versal: rework mbox driver

083edc4

Signed-off-by: Jeremie Corbier <[email protected]>

fixup! drivers: versal: PKI engine driver for Versal Net

3b506e9

Signed-off-by: Jeremie Corbier <[email protected]>

fixup! core: pta: add Versal FPGA PTA

8058dd7

Signed-off-by: Jeremie Corbier <[email protected]>

fixup! drivers: versal: NVM driver for Versal Net

b47ed4f

Signed-off-by: Jeremie Corbier <[email protected]>

fixup! drivers: versal: PKI engine driver for Versal Net

b60b529

Signed-off-by: Jeremie Corbier <[email protected]>

fixup! drivers: versal: NVM driver for Versal Net

e67fe83

Signed-off-by: Jeremie Corbier <[email protected]>

jcorbier added 2 commits September 19, 2024 14:13

fixup! drivers: versal: PKI engine driver for Versal Net

4fe7b58

Signed-off-by: Jeremie Corbier <[email protected]>

fixup! drivers: versal: NVM driver for Versal Net

456e017

Signed-off-by: Jeremie Corbier <[email protected]>

nathan-menhorn reviewed Sep 19, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

plat-versal: add support for the Versal Net variant #6738

plat-versal: add support for the Versal Net variant #6738

jcorbier commented Mar 5, 2024

ldts commented Mar 6, 2024 •

edited

Loading

jcorbier commented Mar 6, 2024

ldts commented Mar 8, 2024

jcorbier commented Mar 8, 2024

nathan-menhorn commented Mar 12, 2024

jcorbier commented Mar 12, 2024

ldts commented Mar 31, 2024

jcorbier commented Apr 2, 2024

etienne-lms Apr 3, 2024

nathan-menhorn commented May 7, 2024

nathan-menhorn commented May 23, 2024

nathan-menhorn commented Jul 29, 2024

ldts commented Aug 22, 2024 •

edited

Loading

nathan-menhorn commented Aug 26, 2024

nathan-menhorn commented Sep 11, 2024

jcorbier commented Sep 18, 2024

nathan-menhorn Sep 18, 2024

nathan-menhorn Sep 18, 2024

nathan-menhorn Sep 19, 2024

nathan-menhorn Sep 19, 2024

		if (id < VERSAL_NET_REVOKE_EFUSE_MIN \|\|
		id > VERSAL_NET_REVOKE_EFUSE_MAX)

plat-versal: add support for the Versal Net variant #6738

Are you sure you want to change the base?

plat-versal: add support for the Versal Net variant #6738

Conversation

jcorbier commented Mar 5, 2024

ldts commented Mar 6, 2024 • edited Loading

jcorbier commented Mar 6, 2024

ldts commented Mar 8, 2024

jcorbier commented Mar 8, 2024

nathan-menhorn commented Mar 12, 2024

jcorbier commented Mar 12, 2024

ldts commented Mar 31, 2024

jcorbier commented Apr 2, 2024

etienne-lms Apr 3, 2024

Choose a reason for hiding this comment

nathan-menhorn commented May 7, 2024

nathan-menhorn commented May 23, 2024

nathan-menhorn commented Jul 29, 2024

ldts commented Aug 22, 2024 • edited Loading

nathan-menhorn commented Aug 26, 2024

nathan-menhorn commented Sep 11, 2024

jcorbier commented Sep 18, 2024

nathan-menhorn Sep 18, 2024

Choose a reason for hiding this comment

nathan-menhorn Sep 18, 2024

Choose a reason for hiding this comment

nathan-menhorn Sep 19, 2024

Choose a reason for hiding this comment

nathan-menhorn Sep 19, 2024

Choose a reason for hiding this comment

ldts commented Mar 6, 2024 •

edited

Loading

ldts commented Aug 22, 2024 •

edited

Loading