Conversation
|
@sebasthechill Please make your pull request to "core" instead of "main". I will handle all pull requests from our subsystem to main. |
Meowcaroni
left a comment
Meowcaroni
left a comment
There was a problem hiding this comment.
@sebasthechill Sorry for the super late review on this. Looks pretty good (I really appreciate the distinction between combinational and sequential signals via "d/o" and "q"), just need some edits to combinational logic implementation and maybe a change to the data width.
| // main parameters for widths and timeouts | ||
| parameter int TIMEOUT_CYCLES = 256, | ||
| parameter int ADDR_WIDTH = 32, | ||
| parameter int DATA_WIDTH = 64, |
There was a problem hiding this comment.
Unsure why the page table entry width is 64-bit instead of 32-bit for the CPU's expected data word size (I might be missing something). Please provide justification in a reply or switch to 32 (edit one-pager as well if so)
| function automatic logic pte_invalid(input logic [DATA_WIDTH-1:0] pte); | ||
| logic v, r, w; | ||
| begin | ||
| v = pte[0]; | ||
| r = pte[1]; | ||
| w = pte[2]; | ||
|
|
||
| // invalid if v = 0, w = 1 while r = 0, or upper bits are set | ||
| pte_invalid = (!v) || | ||
| (!r && w) || | ||
| ((DATA_WIDTH > 32) && (|pte[DATA_WIDTH-1:32])); | ||
| end | ||
| endfunction |
There was a problem hiding this comment.
While this function works, it can be simplified to an assign statement (avoiding synthesis issues) as follows:
| function automatic logic pte_invalid(input logic [DATA_WIDTH-1:0] pte); | |
| logic v, r, w; | |
| begin | |
| v = pte[0]; | |
| r = pte[1]; | |
| w = pte[2]; | |
| // invalid if v = 0, w = 1 while r = 0, or upper bits are set | |
| pte_invalid = (!v) || | |
| (!r && w) || | |
| ((DATA_WIDTH > 32) && (|pte[DATA_WIDTH-1:32])); | |
| end | |
| endfunction | |
| logic v, r, w; // Page table entry flags | |
| logic pte_invalid; // Internal flag for invalid entry | |
| assign v = axi_r_data_i[0]; | |
| assign r = axi_r_data_i[1]; | |
| assign w = axi_r_data_i[2]; | |
| // invalid if v = 0, w = 1 while r = 0, or upper bits are set | |
| assign pte_invalid = (!v) || | |
| (!r && w) || | |
| ((DATA_WIDTH > 32) && (|pte[DATA_WIDTH-1:32])); |
| // pte points to next level | ||
| function automatic logic pte_is_pointer(input logic [DATA_WIDTH-1:0] pte); | ||
| logic v, r, x; | ||
| begin | ||
| v = pte[0]; | ||
| r = pte[1]; | ||
| x = pte[3]; | ||
| pte_is_pointer = v && !r && !x; | ||
| end | ||
| endfunction | ||
|
|
||
| // pte is a valid leaf mapping | ||
| function automatic logic pte_is_leaf(input logic [DATA_WIDTH-1:0] pte); | ||
| logic v, r, x; | ||
| begin | ||
| v = pte[0]; | ||
| r = pte[1]; | ||
| x = pte[3]; | ||
| pte_is_leaf = v && (r || x); | ||
| end | ||
| endfunction | ||
|
|
||
| // check the A bit (ptw doesn't set A/D, so treat A=0 as fault) | ||
| function automatic logic pte_has_ad_fault(input logic [DATA_WIDTH-1:0] pte); | ||
| begin | ||
| pte_has_ad_fault = !pte[6]; | ||
| end | ||
| endfunction | ||
|
|
||
| // superpage alignment check for l1 leaf | ||
| function automatic logic pte_superpage_misaligned(input logic [DATA_WIDTH-1:0] pte); | ||
| logic [PPN_WIDTH-1:0] ppn; | ||
| begin | ||
| ppn = pte[PPN_WIDTH-1:10]; | ||
| pte_superpage_misaligned = |ppn[9:0]; | ||
| end | ||
| endfunction | ||
|
|
||
| // detect axi read access faults | ||
| function automatic logic axi_access_fault(input logic [1:0] resp); | ||
| begin | ||
| axi_access_fault = (resp != 2'b00); | ||
| end | ||
| endfunction |
There was a problem hiding this comment.
Can also convert functions directly into combinational logic as suggested above. While these can be good for code reusability when multiple inputs need to undergo the same logical output, they are only used for one input in this file and can therefore be written more efficiently/safely.
3 participants
Description
.md Documentation files for all 3 files under MMU (One pagers).
Related Issue(s)
MMU Subsystem
Type