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

Misaligned pointer update. #812

Merged

Conversation

silabs-oysteink
Copy link
Contributor

Detecting misaligned pointers in the IF stage. Propagating alignment status down the pipeline and flagging exception code 0x0 if not ok.

Added assertions for checking that only mret pointers can cause misaligned instruction address exceptions. Added assertions to check that the alignment buffer only signal one valid for each pointer (currently failing), and that the alignment buffer must signal instr_valid_o if is not currently prefetching and the controller wants us to prefetch (currently failing).

The new and failing assertions will be addressed in a coming PR

…status down the pipeline and flagging exception code 0x0 if not ok.

Added assertions for checking that only mret pointers can cause misaligned instruction address exceptions.
Addded assertions to check that the alignment buffer only signal one valid for each pointer (currently failing), and that the alignment buffer must signal instr_valid_o if is not currently prefetching and the controller wants us to prefetch (currently failing).

Signed-off-by: Oystein Knauserud <[email protected]>
@silabs-oysteink silabs-oysteink added the Component:RTL For issues in the RTL (e.g. for files in the rtl directory) label Mar 22, 2023
@@ -195,12 +199,18 @@ module cv32e40x_alignment_buffer import cv32e40x_pkg::*;
mpu_status_unaligned = MPU_RE_FAULT;
end

if((resp_q[rptr2].align_status != MPU_OK) || (resp_q[rptr].align_status != MPU_OK)) begin
Copy link
Contributor

Choose a reason for hiding this comment

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

MPU_OK -> ALIGN_OK

@@ -214,19 +224,24 @@ module cv32e40x_alignment_buffer import cv32e40x_pkg::*;
mpu_status_unaligned = MPU_RE_FAULT;
end

if((resp_q[rptr].align_status != MPU_OK) || (resp_i.align_status != MPU_OK)) begin
Copy link
Contributor

Choose a reason for hiding this comment

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

MPU_OK -> ALIGN_OK

Copy link
Contributor

Choose a reason for hiding this comment

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

align_status should be checked wherever mpu_status is checked. Seems to be missing on line 1028:
if (!(if_id_pipe_i.instr.bus_resp.err || (if_id_pipe_i.instr.mpu_status != MPU_OK))) begin // todo: add check for alignment check in IF (mret pointer)

Copy link
Contributor

@silabs-oivind silabs-oivind left a comment

Choose a reason for hiding this comment

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

Please see comments in files.

Added assertion to check that pointer flags in IF are cleared after a pointer has been sent to ID.

Signed-off-by: Oystein Knauserud <[email protected]>
…or deciding on clic/mret pointer jumps in ID.

Signed-off-by: Oystein Knauserud <[email protected]>
Added some comments for assertions.

Signed-off-by: Oystein Knauserud <[email protected]>
…r misaligned poitners even though the bus response had arrived.

Signed-off-by: Oystein Knauserud <[email protected]>
@silabs-oysteink
Copy link
Contributor Author

Added four new commits:

1: Clearing of 'instr_is_*_ptr_o' flags in the alignment buffer. This ensure the IF stage does not see any pointer flags after a pointer has been sent from IF to ID. Not a real issue on E40X, but on E40S this could cause dummy instructions to be marked as pointers. SEC state unknown as it runs for a really long time.
2: Fixes according to review comments so far - not SEC clean as a check of align_status within the controller was added.
3: Fixed the issue where pointers could issue two instr_valid_o for the cases where the pointer itself looked like two compressed instructions. - Not SEC clean (bugfix)
4: Fixed an issue where the alignment buffer would deadlock for a misaligned pointer (it expected two responses). (Not SEC clean, bugfix)

The assertion 'a_no_outstanding_instr_valid attempts to check for deadlocks within the alignment buffer. If there are no outstanding requests or new requests being initiated, the alignment buffer must signal a valid instruction on instr_valid_o.

!(instr_is_clic_ptr_o || instr_is_mret_ptr_o || instr_is_tbljmp_ptr_o);

assign aligned_is_compressed = (instr[1:0] != 2'b11) &&
!(instr_is_clic_ptr_o || instr_is_mret_ptr_o || instr_is_tbljmp_ptr_o);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not setting these signals for pointers ensure we never issue two instr_valid_o for pointers.

end else if (instr_is_clic_ptr_o || instr_is_mret_ptr_o || instr_is_tbljmp_ptr_o) begin
// currently outputting a pointer, valid whenever the response arrives or we have
// the pointer within index 0 of the buffer.
instr_valid_o = valid;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added to ensure we always signal valid for pointers, including misaligned ones.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component:RTL For issues in the RTL (e.g. for files in the rtl directory)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants