core-v-verif icon indicating copy to clipboard operation
core-v-verif copied to clipboard

Integrity attribute for CV32E40S and width of mimpid_patch for CV32E40X/S

Open Silabs-ArjanB opened this issue 3 years ago • 1 comments

I issued the following pull requests:

https://github.com/openhwgroup/core-v-verif/pull/1220 https://github.com/openhwgroup/core-v-verif/pull/1221

They are to make core-v-verif compile after changing mimpid_i[31:0] to mimpid_patch_i[3:0] in both CV32E40X and CV32E40S and after adding the integrity attribute (only) in the CV32E40S.

I only made the code compile. One of the things that is for sure missing is that I assigned integrity to 0 for all regions defined in cv32e40s/tb/uvmt/uvmt_cv32e40s_constants.sv. Also I did not update any of lib/uvm_agents/uvma_rvvi_ovpsim/uvma_rvvi_ovpsim_agent.sv lib/uvm_agents/uvma_pma/src/comps/uvma_pma_region_cov_model.sv lib/uvm_agents/uvma_pma/src/comps/uvma_pma_sb.sv (it was not clear to me whether how/if these files are shared between 40X and 40S; note that the integrity bit will only exist in the 40S).

Furthermore I am unsure about the following line in lib/uvm_agents/uvma_core_cntrl/uvma_core_cntrl_cfg.sv:

rand bit [MAX_XLEN-1:0] mimpid_patch;

The actual mimpid_patch pin is 4 bits, but the above seems to need to keep its original width.

Silabs-ArjanB avatar Mar 03 '22 13:03 Silabs-ArjanB

Adding signals to the pma-configuration bits will in general not cause issues - removing bits on the other hand breaks the riscv-dv based tests.

  • lib/uvm_agents/uvma_rvvi_ovpsim/uvma_rvvi_ovpsim_agent.sv This is where our ovpsim-configuration (ovpsim.ic) is created based on the common configuration object, this is used by all the cv32e40*-cores. Should not contain any core-specific code, as this is shared between cores.
  • lib/uvm_agents/uvma_pma/src/comps/uvma_pma_region_cov_model.sv Common coverage model for PMA - this will probably need some adaptations after the pma configuration bit changes, as the the existence of fields depends on which core being used.
  • lib/uvm_agents/uvma_pma/src/comps/uvma_pma_sb.sv Scoreboard for PMA, checks for legality of accesses based on configured PMA attributes

The randomized mimpid_patch is potentially overriden by a command line function that takes an XLEN-sized variable, thus the function will fail if used with a smaller-sized variable. This should not make a difference in practice, as long as we only use the lower 4 bits of the array. That said, the ISS depends on having a full 32-bit mimpid available, otherwise we will run into comparison mismatches when checking accesses to this register, and this needs to be supplied from the configuration object.

silabs-hfegran avatar Mar 03 '22 14:03 silabs-hfegran

I have to dissect this ticket.


I issued the following pull requests:

#1220 #1221

They are to make core-v-verif compile after changing mimpid_i[31:0] to mimpid_patch_i[3:0] in both CV32E40X and CV32E40S and after adding the integrity attribute (only) in the CV32E40S.

Ok. Good. They were merged a long time ago.


I only made the code compile. One of the things that is for sure missing is that I assigned integrity to 0 for all regions defined in cv32e40s/tb/uvmt/uvmt_cv32e40s_constants.sv.

So that's why they were all 0. I changed that a long time ago too.


Also I did not update any of lib/uvm_agents/uvma_rvvi_ovpsim/uvma_rvvi_ovpsim_agent.sv lib/uvm_agents/uvma_pma/src/comps/uvma_pma_region_cov_model.sv lib/uvm_agents/uvma_pma/src/comps/uvma_pma_sb.sv (it was not clear to me whether how/if these files are shared between 40X and 40S; note that the integrity bit will only exist in the 40S).

TODO what do?

Henrik commented on what those files are for. So it is still an open question what we must do about the above.


Furthermore I am unsure about the following line in lib/uvm_agents/uvma_core_cntrl/uvma_core_cntrl_cfg.sv:

rand bit [MAX_XLEN-1:0] mimpid_patch;

The actual mimpid_patch pin is 4 bits, but the above seems to need to keep its original width.

Henrik replied to that, so that is solved.

silabs-robin avatar Dec 12 '23 14:12 silabs-robin

Closing based on: The only thing remaining is "Also I did not update any of lib/uvm_agents/uvma_rvvi_ovpsim/uvma_rvvi_ovpsim_agent.sv lib/uvm_agents/uvma_pma/src/comps/uvma_pma_region_cov_model.sv lib/uvm_agents/uvma_pma/src/comps/uvma_pma_sb.sv". In the time since this issue was created, both PMA and Integrity has seen lots of focused verification effort. And since the remaining part of this issue is of a general notion (and not a specific thing missing), then I deem it to be no reasonable grounds for suspecting that the focused verification efforts have not sufficiently handled this case.

silabs-robin avatar Dec 12 '23 15:12 silabs-robin