Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[aes,sw/dv] Test AES-GCM save&restore feature #26

Draft
wants to merge 2 commits into
base: aes-gcm-review
Choose a base branch
from

Conversation

nasahlpa
Copy link
Collaborator

This PR tests the AES-GCM save & restore feature.

@nasahlpa
Copy link
Collaborator Author

@vogelpi FYI you can start the test with: ./bazelisk.sh test --test_tag_filters=verilator --test_output=streamed //sw/device/tests:aes_gcm_test

To speed-up the execution, you can comment:

//EXECUTE_TEST(result, execute_test, false, false);
//EXECUTE_TEST(result, execute_test, true, false);

@nasahlpa nasahlpa force-pushed the aes_gcm_dv_save_restore branch 2 times, most recently from e784dfe to 0e73d14 Compare February 18, 2025 13:12
Copy link
Owner

@vogelpi vogelpi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is going in the right direction. But before writing the IV, the code should check for the module to be idle. Because updates to the IV register while being busy are ignored.

Comment on lines 475 to 477
if (!aes_input_ready(aes)) {
return kDifUnavailable;
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That needs to be changed: the IV is only writable if AES is idle. So we need to check for the IDLE bit here, not the INPUT_READY bit. The input maybe writable while AES is busy...

*
* This function will load the provided IV into the AES.
*
* The peripheral must be able to accept the input (INPUT_READY set), and
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...must be idle, and...

This commit extends the aes_gcm SW test to also cover
the save and restore feature.

Signed-off-by: Pascal Nasahl <[email protected]>
@nasahlpa nasahlpa force-pushed the aes_gcm_dv_save_restore branch from 0e73d14 to 925de96 Compare February 19, 2025 14:48
Comment on lines +251 to +256
bit setup_mode = 1;
// Trigger a control update error (1) or not (0). Only applicable if setup_mode = 1.
bit control_update_error = 0;
// Index of the field which shall trigger the control update error.
int idx_error_field = 0;
`DV_CHECK_STD_RANDOMIZE_FATAL(setup_mode)
//`DV_CHECK_STD_RANDOMIZE_FATAL(setup_mode)
Copy link
Collaborator Author

@nasahlpa nasahlpa Feb 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to be removed before merging this PR.

However, in the new aes_gcm_save_restore test I am encountering the following issue when setup_mode = 0.

Then, we are configuring the AES using the following functions:

set_operation(item.operation);
set_mode(item.aes_mode);
set_key_len(item.key_len);
set_manual_operation(item.manual_op);
set_prng_reseed_rate(prs_rate_e'(item.reseed_rate));
set_sideload(item.sideload_en);

Inside these functions, e.g., the set_operation() function, we first check whether the value we want to write is actually different to the value stored in the mirrored shadow register:

virtual task set_operation(bit [1:0] operation);
if (ral.ctrl_shadowed.operation.get_mirrored_value() != operation) begin
ral.ctrl_shadowed.operation.set(operation);
csr_update(.csr(ral.ctrl_shadowed), .en_shadow_wr(1'b1), .blocking(1));
end
endtask // set_operation

When I have the following two messages:

op: AES_DEC
op: AES_ENC

In the second message the operation field gets not updated and stays at AES_DEC instead of going to AES_ENC. The reason is because the result of the check in the function shown above is that
ral.ctrl_shadowed.operation.get_mirrored_value() == AES_ENC
for the second message.

As I have now spend a lot of time debugging this issue, could you please have a look @vogelpi?

Copy link
Collaborator Author

@nasahlpa nasahlpa Feb 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this is independent of the save and restore - I have commented it out in the aes_gcm_save_restore_vseq.

To visualize the issue, I have expanded the set_operation() function as followed:

  virtual task set_operation(bit [1:0] operation);
    `uvm_info(`gfn, $sformatf("Trying to set op = %b", operation), UVM_LOW)
    if (ral.ctrl_shadowed.operation.get_mirrored_value() != operation) begin
      ral.ctrl_shadowed.operation.set(operation);
      csr_update(.csr(ral.ctrl_shadowed), .en_shadow_wr(1'b1), .blocking(1));
      `uvm_info(`gfn, $sformatf("Set op = %b", operation), UVM_LOW)
    end else begin
      `uvm_info(`gfn, $sformatf("Op is alreay %b", operation), UVM_LOW)
    end
  endtask // set_operation

When sending

op: AES_DEC - 10
op: AES_ENC - 01

I get the following output:

UVM_INFO @   7529006 ps: (aes_base_vseq.sv:133) uvm_test_top.env.virtual_sequencer [uvm_test_top.env.virtual_sequencer.aes_gcm_save_restore_vseq] Trying to set op = 10
UVM_INFO @   7689006 ps: (aes_base_vseq.sv:137) uvm_test_top.env.virtual_sequencer [uvm_test_top.env.virtual_sequencer.aes_gcm_save_restore_vseq] Set op = 10
...
UVM_INFO @  26889006 ps: (aes_base_vseq.sv:133) uvm_test_top.env.virtual_sequencer [uvm_test_top.env.virtual_sequencer.aes_gcm_save_restore_vseq] Trying to set op = 01
UVM_INFO @  26889006 ps: (aes_base_vseq.sv:139) uvm_test_top.env.virtual_sequencer [uvm_test_top.env.virtual_sequencer.aes_gcm_save_restore_vseq] Op is alreay 01

Screenshot from 2025-02-19 17-51-35

As shown in the waveform, the operation stays at AES_DEC although we want to switch to the AES_ENC mode for the second message.

@nasahlpa nasahlpa force-pushed the aes_gcm_dv_save_restore branch from 925de96 to 3da9541 Compare February 19, 2025 15:06
This commit adds a directed test that checks whether AES-GCM
operations can be stored and restored using the dedicated feature.

Signed-off-by: Pascal Nasahl <[email protected]>
@nasahlpa nasahlpa force-pushed the aes_gcm_dv_save_restore branch from 3da9541 to 0c6e617 Compare February 19, 2025 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants