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

RVFI depends on the testbench #271

Open
MikeOpenHWGroup opened this issue May 28, 2024 · 2 comments
Open

RVFI depends on the testbench #271

MikeOpenHWGroup opened this issue May 28, 2024 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@MikeOpenHWGroup
Copy link
Member

Bug Description

Hi @mario. In #184 you indicated that compiling RVFI requires inclusion of the cv32e20-dv environment. This sounds odd. Looking at cve2_cs_registers.sv we see:

`ifdef RVFI
    logic [63:0] mstatus_extended_read;
    logic [63:0] mstatus_extended_write;

    assign  mstatus_extended_read[CSR_MSTATUS_MIE_BIT]                              = mstatus_q.mie;
    assign  mstatus_extended_read[CSR_MSTATUS_MPIE_BIT]                             = mstatus_q.mpie;
    assign  mstatus_extended_read[CSR_MSTATUS_MPP_BIT_HIGH:CSR_MSTATUS_MPP_BIT_LOW] = mstatus_q.mpp;
    assign  mstatus_extended_read[CSR_MSTATUS_MPRV_BIT]                             = mstatus_q.mprv;
    assign  mstatus_extended_read[CSR_MSTATUS_TW_BIT]                               = mstatus_q.tw;

... (code snipped)    

    assign rvfi_csr_if.rvfi_csr_addr =  rvfi_csr_addr;
    assign rvfi_csr_if.rvfi_csr_rdata = rvfi_csr_rdata;
    assign rvfi_csr_if.rvfi_csr_wdata = rvfi_csr_wdata;
    assign rvfi_csr_if.rvfi_csr_rmask = rvfi_csr_rmask;
    assign rvfi_csr_if.rvfi_csr_wmask = rvfi_csr_wmask;
    assign rvfi_csr_rmask_q =  ((~csr_wr & csr_op_en_i & ~illegal_csr_insn_o)) ? -1 : 0;
    assign rvfi_csr_wmask_q =   ((csr_wr & csr_op_en_i & ~illegal_csr_insn_o)) ? -1 : 0;
    always @(posedge clknrst_if.clk) begin
        rvfi_csr_addr  = csr_addr_i;
        rvfi_csr_rdata = csr_rdata_int;
        rvfi_csr_wdata = csr_wdata_int;
        rvfi_csr_rmask = (rvfi_csr_rmask_q);
        rvfi_csr_wmask = (rvfi_csr_wmask_q);
    end
...

The reference to clknrst_if.clk is problematic as it places a dependency on the testbench upon the RTL. This is bad practise in general. Is there a way to implement the RVFI without that dependance?

@MarioOpenHWGroup
Copy link

Yes, probably this should not be bind to RVFI variable instead of a UVM variable

@MikeOpenHWGroup
Copy link
Member Author

Indeed. AFAIK, there is no phase difference between the clk_i input to this module and clknrst_if.clk in the UVM "clk-and-reset" interface. So, maybe it is just as simple as replacing:

always @(posedge clknrst_if.clk) begin

with

always @(posedge clk_i) begin

Will that work?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants