[sival/aes] Add AES Interrupt Encryption tests #24572

rprakas-gsc · 2024-09-13T15:31:01Z

@moidx Checking in aes_interrupt_encryption_test.c code and modified the BUILD to include this test.

luismarques

Can you add review / commit comments explaining what overall scenario this test tests? I don't suppose there's a testplan testpoint to reference?

sw/device/tests/aes_interrupt_encryption_test.c

sw/device/tests/BUILD

sw/device/tests/aes_interrupt_encryption_test.c

engdoreis · 2024-09-23T12:21:47Z

sw/device/tests/aes_interrupt_encryption_test.c

+
+OTTF_DEFINE_TEST_CONFIG();
+
+status_t execute_test(void) {


As this function returns status_t, you can replace all instances of CHECK_DIF_OK by TRY, which would make the code cleaner in my opinion.

Ideally, I want the test code to report FAILURE & abort if the value doesn't get readout correctly , hence using CHECK_DIF_OK

The final result would be the same, TRY would return immediately and then the callee would abort by using CHECK_STATUS_OK.

sw/device/tests/aes_interrupt_encryption_test.c

rprakas-gsc · 2024-09-25T01:46:41Z

Can you add review / commit comments explaining what overall scenario this test tests? I don't suppose there's a testplan testpoint to reference?
There is. I have added the below lines in the new Commit
Added AES Interrupt Encryption tests as per test plan identified here:[…
and modified BUILD

rprakas-gsc · 2024-10-01T17:06:38Z

@luismarques @engdoreis, Gentle ping on reviewing this PR. I already incorporated the changes from your feedback and waiting for functional checks on the test code (from AES folks)

moidx · 2024-10-02T16:13:20Z

Hi @nasahlpa, can you take a look at the test implementation to ensure it matches the intended test point description? Thanks!

nasahlpa · 2024-10-04T05:35:26Z

Can you add review / commit comments explaining what overall scenario this test tests? I don't suppose there's a testplan testpoint to reference?
There is. I have added the below lines in the new Commit
Added AES Interrupt Encryption tests as per test plan identified here:[…
and modified BUILD

Could you please add the description to the git commit itself? Currently, no description is available in the git commit message. Furthermore, the contributing guideline also mentions that a subject tag should be added to the git commit message.

rprakas-gsc · 2024-10-08T18:22:29Z

Can you add review / commit comments explaining what overall scenario this test tests? I don't suppose there's a testplan testpoint to reference?
There is. I have added the below lines in the new Commit
Added AES Interrupt Encryption tests as per test plan identified here:[…
and modified BUILD

Could you please add the description to the git commit itself? Currently, no description is available in the git commit message. Furthermore, the contributing guideline also mentions that a subject tag should be added to the git commit message.

The git commit has restrictions on word limit, if I include the entire link, i will run out of the world limit and won't be able to commit successfully.

nasahlpa · 2024-10-09T06:24:36Z

Understood. But you could add something like this:

This commit implements the chip_sw_aes_interrupt_encryption test that is defined in the chip testplan. The purpose of this test is to check, whether an AES encryption can be interrupted and restored.

For the header of the git commit message you could use:

[sival/aes] Add AES interrupt encryption test

rprakas-gsc · 2024-10-09T17:30:39Z

Thanks Pascal. I will incorporate these changes in the commit message now!

…

On Tue, Oct 8, 2024 at 11:24 PM Pascal Nasahl ***@***.***> wrote: Understood. But you could add something like this: This commit implements the chip_sw_aes_interrupt_encryption test that is defined in the chip testplan. The purpose of this test is to check, whether an AES encryption can be interrupted and restored. For the header of the git commit message you could use: [sival/aes] Add AES interrupt encryption test — Reply to this email directly, view it on GitHub <#24572 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BJEK24Y6LO2262A62JNS7SLZ2TD3VAVCNFSM6AAAAABOFTOFTOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDIMBRGQYTOMJSGU> . You are receiving this because you authored the thread.Message ID: ***@***.***>

rprakas-gsc · 2024-10-09T17:36:15Z

Just updated the PR with the commit messages as suggested!

nasahlpa · 2024-10-10T07:20:11Z

Thanks! Would you mind looking into the review comments I've added? Especially the question regarding the decryption should be addressed.

moidx · 2024-10-10T16:36:35Z

Hi @nasahlpa, I wasn't able to find your comments. Can you double check if they were sent? Thanks

sw/device/tests/aes_interrupt_encryption_test.c

sw/device/tests/BUILD

sw/device/tests/aes_interrupt_encryption_test.c

nasahlpa · 2024-10-10T16:38:56Z

Hi @nasahlpa, I wasn't able to find your comments. Can you double check if they were sent? Thanks

Oh sorry, now the comments should be public.

sw/device/tests/BUILD

engdoreis · 2024-10-14T16:54:50Z

sw/device/tests/aes_interrupt_encryption_test.c

I suggest the following message for the commit 2:
[test, aes] Clean-up log message.

I would also squash the commit number 3 with the previous.

This commit implements the chip_sw_aes_interrupt_encryption test that is defined in the chip testplan. The purpose of this test is to check, whether an AES encryption can be interrupted and restored. Tue Oct 22 Incorporated the changes suggested by reviewers and added AES modes (CFB-128, OFB, CTR) called out in the test plan Tue Oct 23 Fixed lint errors Signed-off-by: Ramesh Prakash <[email protected]>

nasahlpa · 2024-10-24T06:00:50Z

sw/device/tests/aes_interrupt_encryption_test.c

+    memcpy(iv.iv, &iv_encrypt1, sizeof(iv.iv));
+    transaction.manual_operation = kDifAesManualOperationAuto;
+
+    // "Convert" key share byte arrays to `dif_aes_key_share_t`.


I think this is not necessary as key should still contain the key used for the first two blocks?

nasahlpa

Thanks @rprakas-gsc for changing the code based on the feedback. Apart from a small nit this PR looks good to me.

Maybe @engdoreis could have another look.

engdoreis

Thanks for the changes.

github-actions · 2024-10-31T22:08:16Z

Successfully created backport PR for earlgrey_1.0.0:

Cherry-pick to earlgrey_1.0.0: [sival/aes] Add AES Interrupt Encryption tests #24972

rprakas-gsc requested a review from a team as a code owner September 13, 2024 15:31

rprakas-gsc requested review from engdoreis and removed request for a team September 13, 2024 15:31

moidx self-requested a review September 13, 2024 16:13

rprakas-gsc force-pushed the a1-sival-aes-tests branch 2 times, most recently from a19f20c to ac6bdc1 Compare September 14, 2024 04:03

rprakas-gsc force-pushed the a1-sival-aes-tests branch 4 times, most recently from 79f90e7 to 4b6f2e1 Compare September 21, 2024 01:35

luismarques reviewed Sep 23, 2024

View reviewed changes

engdoreis reviewed Sep 23, 2024

View reviewed changes

sw/device/tests/aes_interrupt_encryption_test.c Outdated Show resolved Hide resolved

rprakas-gsc force-pushed the a1-sival-aes-tests branch from 9612844 to 58843a0 Compare September 25, 2024 01:31

rprakas-gsc force-pushed the a1-sival-aes-tests branch from 58843a0 to eb2cabc Compare September 25, 2024 05:06

moidx changed the title ~~Added AES Interrupt Encryption tests and modified BUILD~~ [sival] Add AES Interrupt Encryption tests Oct 2, 2024

moidx requested a review from nasahlpa October 2, 2024 16:13

rprakas-gsc changed the title ~~[sival] Add AES Interrupt Encryption tests~~ [sival/aes] Add AES Interrupt Encryption tests Oct 8, 2024

rprakas-gsc force-pushed the a1-sival-aes-tests branch from eb2cabc to 88ea80a Compare October 9, 2024 17:35

nasahlpa requested changes Oct 10, 2024

View reviewed changes

nasahlpa mentioned this pull request Oct 10, 2024

[sival/aes] Introduce aes_force_prng_reseed_test and aes_prng_reseed_test #24574

Open

engdoreis reviewed Oct 14, 2024

View reviewed changes

sw/device/tests/BUILD Show resolved Hide resolved

engdoreis reviewed Oct 14, 2024

View reviewed changes

rprakas-gsc force-pushed the a1-sival-aes-tests branch from d319b9a to 054662f Compare October 23, 2024 04:04

rprakas-gsc force-pushed the a1-sival-aes-tests branch from 054662f to 26591f0 Compare October 23, 2024 17:31

rprakas-gsc requested a review from nasahlpa October 23, 2024 17:58

nasahlpa reviewed Oct 24, 2024

View reviewed changes

nasahlpa approved these changes Oct 24, 2024

View reviewed changes

engdoreis approved these changes Oct 25, 2024

View reviewed changes

engdoreis merged commit 73ffe0d into lowRISC:master Oct 25, 2024
42 checks passed

rprakas-gsc added the CherryPick:earlgrey_1.0.0 This PR should be cherry-picked to earlgrey_1.0.0 label Oct 31, 2024

github-actions bot mentioned this pull request Oct 31, 2024

Cherry-pick to earlgrey_1.0.0: [sival/aes] Add AES Interrupt Encryption tests #24972

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[sival/aes] Add AES Interrupt Encryption tests #24572

[sival/aes] Add AES Interrupt Encryption tests #24572

rprakas-gsc commented Sep 13, 2024

luismarques left a comment

engdoreis Sep 23, 2024

rprakas-gsc Sep 24, 2024

engdoreis Oct 14, 2024

rprakas-gsc commented Sep 25, 2024

rprakas-gsc commented Oct 1, 2024

moidx commented Oct 2, 2024

nasahlpa commented Oct 4, 2024

rprakas-gsc commented Oct 8, 2024

nasahlpa commented Oct 9, 2024

rprakas-gsc commented Oct 9, 2024 via email

rprakas-gsc commented Oct 9, 2024

nasahlpa commented Oct 10, 2024

moidx commented Oct 10, 2024

nasahlpa commented Oct 10, 2024

engdoreis Oct 14, 2024 •

edited

Loading

engdoreis Oct 14, 2024 •

edited

Loading

nasahlpa Oct 24, 2024

nasahlpa left a comment

engdoreis left a comment

github-actions bot commented Oct 31, 2024


		OTTF_DEFINE_TEST_CONFIG();

		status_t execute_test(void) {

[sival/aes] Add AES Interrupt Encryption tests #24572

[sival/aes] Add AES Interrupt Encryption tests #24572

Conversation

rprakas-gsc commented Sep 13, 2024

luismarques left a comment

Choose a reason for hiding this comment

engdoreis Sep 23, 2024

Choose a reason for hiding this comment

rprakas-gsc Sep 24, 2024

Choose a reason for hiding this comment

engdoreis Oct 14, 2024

Choose a reason for hiding this comment

rprakas-gsc commented Sep 25, 2024

rprakas-gsc commented Oct 1, 2024

moidx commented Oct 2, 2024

nasahlpa commented Oct 4, 2024

rprakas-gsc commented Oct 8, 2024

nasahlpa commented Oct 9, 2024

rprakas-gsc commented Oct 9, 2024 via email

rprakas-gsc commented Oct 9, 2024

nasahlpa commented Oct 10, 2024

moidx commented Oct 10, 2024

nasahlpa commented Oct 10, 2024

engdoreis Oct 14, 2024 • edited Loading

Choose a reason for hiding this comment

engdoreis Oct 14, 2024 • edited Loading

Choose a reason for hiding this comment

nasahlpa Oct 24, 2024

Choose a reason for hiding this comment

nasahlpa left a comment

Choose a reason for hiding this comment

engdoreis left a comment

Choose a reason for hiding this comment

github-actions bot commented Oct 31, 2024

engdoreis Oct 14, 2024 •

edited

Loading

engdoreis Oct 14, 2024 •

edited

Loading