AESKEYS: Organized and Scrubbed AESKEYS SVD and register files. #329
Conversation
|
Don't mind the failed "Verify Register and SVD Files" check. The errors are due to 1) the ME13A/ME55 register and SVD files weren't updated in this PR and the runner is unable to find those files (this will be done in separate PRs), and 2) setting up the deprecated register files (e.g. aes_key_regs.h). The workflow complained that newly deprecated files were edited which caused an error. However, this is a special case because the purpose of this PR is to deprecate unsupported files, so the runner will expectedly throw an error. All the important checks relating to this PR did pass though. |
sihyung-maxim
requested review from
lorne-maxim,
kevin-gillespie,
Jacob-Scheiffler and
Jake-Carter
| typedef struct { | ||
| __IO uint128_t key0; /**< <tt>\b 0x000:</tt> AESKEYS KEY0 Register */ | ||
| __IO uint128_t key1; /**< <tt>\b 0x010:</tt> AESKEYS KEY1 Register */ | ||
| __IO uint256_t puf_key1; /**< <tt>\b 0x020:</tt> AESKEYS PUF_KEY1 Register */ |
There was a problem hiding this comment.
In the resource maps it says the PUF keys are at 0x40007800?
There was a problem hiding this comment.
Yes, the PUF keys and AES keys share the same location, but the PUF for the ME18 doesn't work. I have a separate PR that deletes references to the PUF, and I plan on deleting these keys once this PR is approved and merged.
| @@ -351,7 +351,11 @@ typedef enum { | |||
|
|
|||
| /******************************************************************************/ | |||
| /* AES Keys */ | |||
| #define MXC_BASE_AESKEY ((uint32_t)0x40005000UL) | |||
| #define MXC_BASE_AESKEYS ((uint32_t)0x40005000UL) | |||
There was a problem hiding this comment.
Possibly need PUF keys base address as well if resource map correct.
There was a problem hiding this comment.
Ditto ""
"Yes, the PUF keys and AES keys share the same location, but the PUF for the ME18 doesn't work. I have a separate PR that deletes references to the PUF, and I plan on deleting these keys once this PR is approved and merged."
| @@ -85,7 +87,7 @@ extern "C" { | |||
| * @ingroup aes_registers | |||
| * Structure type to access the AES Registers. | |||
| */ | |||
| typedef struct { | |||
| typedef struct __attribute__((deprecated("mxc_aes_regs_t struct and aes_regs.h no longer supported. Use aeskeys_regs.h and MXC_AESKEYS (mxc_aeskeys_regs_t) for AES Key Access. 1-10-2023"))) { | |||
There was a problem hiding this comment.
Consider wrapping this in an #if defined(__GNUC__) to avoid compatibility issues with IAR/Keil.
@ftariqAnalogTx Can you confirm whether IAR/Keil support the deprecated attribute?
There was a problem hiding this comment.
I added the guards for IAR/Keil.
There was a problem hiding this comment.
Would something like this work instead? Only thing that bothers me is that on IAR/Keil you have different behavior by routing to the new struct, and the new struct has different member names.
#if defined(__GNUC__)
__attribute__((deprecated("mxc_aes_key_regs_t struct and aes_key_regs.h no longer supported. Use aeskeys_regs.h and MXC_AESKEYS (mxc_aeskeys_regs_t) for AES Key Access. 1-10-2023")))
#else
#warning "mxc_aes_key_regs_t struct and aes_key_regs.h no longer supported. Use aeskeys_regs.h and MXC_AESKEYS (mxc_aeskeys_regs_t) for AES Key Access. 1-10-2023"
#endif
typedef struct {
__IO uint32_t aes_key0[4]; /**< <tt>\b 0x00:</tt> AES_KEY AES_KEY0 Register */
__R uint32_t rsv_0x10_0x1f[4];
__IO uint32_t aes_key1[4]; /**< <tt>\b 0x20:</tt> AES_KEY AES_KEY1 Register */
__R uint32_t rsv_0x30_0xff[52];
__IO uint32_t aes_key2[4]; /**< <tt>\b 0x100:</tt> AES_KEY AES_KEY2 Register */
__R uint32_t rsv_0x110_0x17f[28];
__IO uint32_t aes_key3[4]; /**< <tt>\b 0x180:</tt> AES_KEY AES_KEY3 Register */
} mxc_aes_key_regs_t;There was a problem hiding this comment.
You're right. That didn't come to mind when I was making the changes.
| __R uint32_t rsv_0x0_0x7[2]; | ||
| __IO uint32_t en; /**< <tt>\b 0x08:</tt> ECC EN Register */ | ||
| } mxc_ecc_regs_t; | ||
| #else | ||
| #warning "Defining deprecated type "mxc_ecc_regs_t" to "mxc_trimsir_regs_t" (trimsir_regs.h)." | ||
| typedef mxc_ecc_regs_t mxc_trimsir_regs_t; |
There was a problem hiding this comment.
Thanks, looks great. Just need to do this one too then we're ready to merge this beast in :)
There was a problem hiding this comment.
Thanks, missed this one.
|
Besides the ME13B and ME55 (CMSIS still WIP), the SVD action should work for all other parts after this PR is merged. The CMSIS register files should reach a stable point. |
The AES Key SVD files were scattered in the PeriphDrivers which made bringing up a new part tedious to handle. I've organized the AESKEYS SVD file into a single location within Libraries/PeriphDrivers/AES/AESKEYS. I've also scrubbed the peripheral and register-field names to remove redundancy in the macro definitions and to follow the set naming convention (message me for more info). The new standardized name for the block will be AESKEYS which was decided in late November (11-30-2022). Previously, a combination of AES_KEY, AES_KEYS, and AESKEY was used throughout the MSDK.
The original names and AES Key register files still exist for compatibility-sake, but the old names have been deprecated and will be scheduled for removal later in a couple of releases from now.
The ME13A and ME55 won't have these changes pushed into main yet.