[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

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: ***@***.***>

Signed-off-by: Ramesh Prakash <rprakas@google.com> 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.

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

nasahlpa · 2024-10-04T05:49:00Z

sw/device/tests/aes_interrupt_encryption_test.c

+  dif_aes_data_t out_data2;
+  CHECK_DIF_OK(dif_aes_read_output(&aes, &out_data2));
+  CHECK_DIF_OK(dif_aes_end(&aes));
+  // Finish the CBC decryption transaction.


Suggested change

dif_aes_data_t out_data2;

CHECK_DIF_OK(dif_aes_read_output(&aes, &out_data2));

CHECK_DIF_OK(dif_aes_end(&aes));

// Finish the CBC decryption transaction.

// Finish the CBC decryption transaction.

dif_aes_data_t out_data2;

CHECK_DIF_OK(dif_aes_read_output(&aes, &out_data2));

CHECK_DIF_OK(dif_aes_end(&aes));

nasahlpa · 2024-10-09T07:25:02Z

sw/device/tests/BUILD

@@ -6279,3 +6279,22 @@ opentitan_test(
        "//sw/device/lib/testing/test_framework:ottf_utils",
    ],
 )
+
+opentitan_test(


Could you please move this test up to the other AES tests? Alphabetically ordering the bazel entries helps to find the tests easier.

nasahlpa · 2024-10-09T07:29:55Z

sw/device/tests/BUILD

+    exec_env = {
+        "//hw/top_earlgrey:fpga_cw340_sival": None,
+    },


Suggested change

exec_env = {

"//hw/top_earlgrey:fpga_cw340_sival": None,

},

exec_env = dicts.add(

EARLGREY_TEST_ENVS,

EARLGREY_SILICON_OWNER_ROM_EXT_ENVS,

{

"//hw/top_earlgrey:fpga_cw310_sival": None,

"//hw/top_earlgrey:fpga_cw340_sival": None,

"//hw/top_earlgrey:silicon_creator": None,

},

),

By adding this, you also could run the test in Verilator. @engdoreis do you know whether there is a rule which exec environments a test needs to have?

nasahlpa · 2024-10-09T08:09:45Z

sw/device/tests/aes_interrupt_encryption_test.c

+
+  CHECK_DIF_OK(dif_aes_start(&aes, &transactiond, &key, &iv));
+
+  memcpy(cipher_text[0].data, kAesModesCipherTextCbc128,


Could you please explain what you are trying to achieve with the decryption? I assume you want to take the ciphertext generated from the interrupted AES encryption (dif_aes_t aes above) and check whether the same plaintext is generated after the decryption? If yes, why are you using kAesModesCipherTextCbc128 as an input for the decryption?

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.

Co-authored-by: Pascal Nasahl <pascal@nasahl.net> Signed-off-by: Ramesh Prakash <rprakas@google.com>

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

rprakas-gsc and others added 2 commits October 10, 2024 16:43

Update sw/device/tests/aes_interrupt_encryption_test.c

99a2e6a

Co-authored-by: Pascal Nasahl <pascal@nasahl.net> Signed-off-by: Ramesh Prakash <rprakas@google.com>

Update sw/device/tests/aes_interrupt_encryption_test.c

d319b9a

Co-authored-by: Pascal Nasahl <pascal@nasahl.net> Signed-off-by: Ramesh Prakash <rprakas@google.com>

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

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 Oct 4, 2024

nasahlpa Oct 9, 2024

nasahlpa Oct 9, 2024

nasahlpa Oct 9, 2024

nasahlpa commented Oct 10, 2024

-    exec_env = {
-        "//hw/top_earlgrey:fpga_cw340_sival": None,
-    },
+    exec_env = dicts.add(
+        EARLGREY_TEST_ENVS,
+        EARLGREY_SILICON_OWNER_ROM_EXT_ENVS,
+        {
+            "//hw/top_earlgrey:fpga_cw310_sival": None,
+            "//hw/top_earlgrey:fpga_cw340_sival": None,
+            "//hw/top_earlgrey:silicon_creator": None,
+        },
+    ),


		CHECK_DIF_OK(dif_aes_start(&aes, &transactiond, &key, &iv));

		memcpy(cipher_text[0].data, kAesModesCipherTextCbc128,


		OTTF_DEFINE_TEST_CONFIG();

		status_t execute_test(void) {

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

Are you sure you want to change the base?

[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

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 Oct 4, 2024

Choose a reason for hiding this comment

nasahlpa Oct 9, 2024

Choose a reason for hiding this comment

nasahlpa Oct 9, 2024

Choose a reason for hiding this comment

nasahlpa Oct 9, 2024

Choose a reason for hiding this comment

nasahlpa commented Oct 10, 2024