Skip to content

A-extension updates #780

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

Merged

Conversation

silabs-oysteink
Copy link
Contributor

@silabs-oysteink silabs-oysteink commented Feb 13, 2023

  • Fixed issue in a_decoder where transaction size was set to byte instead of word
  • Added tracking for type of atomic instruction, LSU now has EX and WB outputs for atomic type
  • Connected the 'atomic_trans_i' to the LSU PMA (tied to zero in IF)
    -- PMA will flag misaligned atomics as an error using the same exception codes as for regular loads and stores
  • SC.W did not take exokay into account, it will now write 1 on failure an 0 on success.
  • Restricting outstanding transactions while handling atomics:
    -- No other memory transaction (including atomics) may asserts its address phase while there are outstanding atomics
    -- No atomic may assert its address phase while there are outstanding memory transaction (including atomics)
  • Watchpoint triggers will treat AMOs as both read and write

SEC clean when A_EXT=0.

- Tracking type of atomic instruction in the LSU to know if is there is LR/SC or AMO in the WB part of the LSU.
- Added code to allow SC to write success or failure to the RF depending on th exokay signal.

Signed-off-by: Oystein Knauserud <[email protected]>
…be flagged with an exception.

Fixed issue in the a_decoder which set lsu_size=2'b00 (Byte) instead of 2'b10 (Word). This seems to be old code from the cv32e40p which uses a different encoding for lsu_size compared to the e40x which uses encoding directly from funct3.

Signed-off-by: Oystein Knauserud <[email protected]>
…ng transactions.

- Not allowing any LSU transactions onto the bus if there are outstanding atomic transaction.

Implemented by halting the EX stage when the above conditions occur.

Signed-off-by: Oystein Knauserud <[email protected]>
… as they always perform both a read and write.

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 Feb 13, 2023
@@ -81,6 +85,19 @@ module cv32e40x_debug_triggers_sva
$stable(tdata2_q)) // Checking stability of ALL tdata2, not just the one selected
else `uvm_error("debug_triggers", "tdata2_q changed after set/clear with rs1==0")

generate
for (genvar idx=0; idx<DBG_NUM_TRIGGERS; idx++) begin
a_amo_hit_on_load:
Copy link
Contributor

Choose a reason for hiding this comment

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

Name of assertion and error message do not match the intent

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated and added comment.

@@ -643,7 +643,7 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
// Also halting EX if an offloaded instruction in WB may cause an exception, such that a following offloaded
// instruction can correctly receive commit_kill.
// Halting EX when an instruction in WB may cause an interrupt to become pending.
ctrl_fsm_o.halt_ex = ctrl_byp_i.minstret_stall || ctrl_byp_i.xif_exception_stall || ctrl_byp_i.irq_enable_stall || ctrl_byp_i.mnxti_ex_stall;
ctrl_fsm_o.halt_ex = ctrl_byp_i.minstret_stall || ctrl_byp_i.xif_exception_stall || ctrl_byp_i.irq_enable_stall || ctrl_byp_i.mnxti_ex_stall || ctrl_byp_i.atomic_stall;
Copy link
Contributor

Choose a reason for hiding this comment

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

Update above comments with reason why atomic stall is needed

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 comment

@@ -118,6 +118,14 @@ module cv32e40x_pma import cv32e40x_pkg::*;
pma_err_o = 1'b1;
end

// Check that atomic accesses are not misaligned
// Not strictly at part of the PMA, but reusing the PMA logic for flagging errors
Copy link
Contributor

Choose a reason for hiding this comment

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

at -> a

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@@ -60,6 +61,7 @@ module cv32e40x_load_store_unit import cv32e40x_pkg::*;
output logic lsu_split_0_o, // Misaligned access is split in two transactions (to controller)
output logic lsu_first_op_0_o, // First operation is active in EX
output logic lsu_last_op_0_o, // Last operation is active in EX
output lsu_atomic_e lsu_atomic_0_o, // IS there an atomic in EX, and of which type
Copy link
Contributor

Choose a reason for hiding this comment

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

IS -> Is

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@@ -717,6 +756,7 @@ module cv32e40x_load_store_unit import cv32e40x_pkg::*;
//////////////////////////////////////////////////////////////////////////////
// MPU
//////////////////////////////////////////////////////////////////////////////
assign mpu_trans_atomic = |(mpu_trans.atop);
Copy link
Contributor

Choose a reason for hiding this comment

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

Only use bit 5

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@@ -1087,6 +1087,15 @@ typedef struct packed
logic accepted; // Was the offloaded instruction accepted or not?
} xif_meta_t;

// Struct for signaling if there is an atomic LSU instruction in WB, and of which type
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove the 'in WB' part everywhere; this type is also used in EX

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

@@ -232,6 +238,24 @@ module cv32e40x_controller_bypass import cv32e40x_pkg::*;
end
end

generate
Copy link
Contributor

Choose a reason for hiding this comment

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

Indent everything 2 positions to the right

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indented

// 1: There is an atomic instruction in EX while we have outstanding transactions on the bus
// 2: There is any LSU instruction in EX while there is an outstanding atomic transfer in progress
if ((id_ex_pipe_i.lsu_en && (lsu_atomic_ex_i != AT_NONE) && id_ex_pipe_i.instr_valid) ||
(id_ex_pipe_i.lsu_en && ex_wb_pipe_i.lsu_en && (lsu_atomic_wb_i != AT_NONE) && ex_wb_pipe_i.instr_valid)) begin
Copy link
Contributor

Choose a reason for hiding this comment

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

(id_ex_pipe_i.lsu_en && ex_wb_pipe_i.lsu_en && (lsu_atomic_wb_i != AT_NONE) && ex_wb_pipe_i.instr_valid) ->
(id_ex_pipe_i.lsu_en && id_ex_pipe_i.instr_valid && ex_wb_pipe_i.lsu_en && (lsu_atomic_wb_i != AT_NONE) && ex_wb_pipe_i.instr_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.

Updated

Signed-off-by: Oystein Knauserud <[email protected]>
Signed-off-by: Oystein Knauserud <[email protected]>
@Silabs-ArjanB Silabs-ArjanB merged commit 5091d8e into openhwgroup:master Feb 13, 2023
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.

2 participants