Use of illegal_bins in the CV32E40P coverage model

[MikeOpenHWGroup]

In pull-request #2394 I made a comment about using illegal_bins. An illegal bin is excluded from coverage, but if the goal is to exclude bins it is best to use ignore_bins. When an illegal bin is "hit" the simulator will exclude the coverage and also emit a run-time error. Unfortunately the run-time error is simulator dependent and is not visible to the UVM messaging service, so it is not possible for a hit on an illegal bin to invoke uvm_error().

So if the illegal bins in the CV32E40P functional coverage models are real errors that should cause the simulation to fail, we need another method to catch these events in a simulator independent way.

Do we have a way to catch these in the CV32E40P environment?

[dd-baoshan]

In current cv32e40p tb enviroment, the pass/fail test status is determined by using uvm report server to check on any fail/fatal count.
But this lead to a hole whereby any runtime error is not able to be captured in simulation. runtime error can be triggered by coverge illegal bins or assertions (immediate or property).
In the past, we missed out these errors due to tb not able to generate correct status if test is failing with only runtime error. As such, we implement a simple post processing check in vsim.mk (refer image below) to check any runtime error printed in vsim.log file (by right a proper log parser should be created to capture such runtime error and also report first error message in log parser, unfortunately this idea was put aside due to reason that i could not recall).

This simple post sim check is only apply to vsim only. We can have similar post check in xrun and vcs log provided we know the pattern of runtime error reporting in these simulators log file.

[MikeOpenHWGroup]

There are four issues here:

1. As @dd-baoshan says, the run-time errors caused by hitting illegal bins does not automatically cause a simulation failure. I would strongly recommend adding a rule to our coding style guidelines to prohibit illegal coverage bins.
2. If the check for run-time errors fails, there is no message written to stdout.
3. The POST_TEST is only included in vsim.mk and appears to tuned to catch only Questasim run-time errors. It should be moved to Common.mk and made generic.
4. The logic in POST_TEST excludes COV_TEST if there are run-time errors, but no UVM errors.

[MikeOpenHWGroup]

I'd like to copy @dd-baoshan's excellent summary of the situation from pull-request #2394 to here because it is directly applicable to this discussion (its been edited slightly):

There are some points we need to think ahead how to address these runtime errors. These are the sources of SystemVerilog simulator run time errors in CORE-V-VERIF:

1. assertion (immediate or property)
   • any assertion will print "Error" message in log file and Erorr count is increase in vsim log file.
   • we can implement the use of uvm_error by adding "else `uvm_error()" in the assertions clause.
2. illegal bins in cover points
   • any hit will print "Error" message in log file and Erorr count is increase in vsim log file.

There are few things we need to take notes. illegal bins are allow according to SystemVerilog LRM. Even though we can set rules to prohibit the use of illegal bins in this tb, this will not become a full proof resolution because:-

1. we cannot prevent third party packages from using it. e.g Imperas coverage model is using illegal bins for illegal instruction checks in its ISACOV.
2. sometimes designers has tendency to code assertions in rtl files (note: here is referring to assertions that are not bind into rtl) and there is no obligation to prevent them from not using uvm_error because
   • using uvm_error macro requires import of uvm_package and macro included in rtl. (not all testbench is implement with uvm_methodology)
   • designers sometimes does not aware of UVM methodology and and these macro. they usually prefer using $error, $warning, or $fatal in assertions if necessary.

[MikeOpenHWGroup]

@dd-baoshan said:

There are some points we need to think ahead how to address these runtime errors...

Great points. My response:

• Just because a code construct is legal SystemVerilog according to the LRM, it does not mean we can or should use it.
• All projects should have coding style guidelines that define the specific coding styles used by the project. The guildlines for CORE-V-VERIF are here.
• In OpenHW, it is the responsibility of the Committers to ensure that the coding style guidelines are followed.

Having said that, our guidelines are not explicit about the specific conditions @dd-baoshan mentions, so I propose we update the CORE-V-VERIF coding style guidelines to:

1. Forbid the use of illegal_bins in coverage models.
2. Require the use of uvm_error for all SVAssertions.
3. Forbid the use of $error, $warning, or $fatal anywhere in the testbench.

Having said all that, it is probably a good idea to retain the POST_TEST rule in the Makefiles. In theory, it will not be necessary, but there is always an exception to every rule.

[pascalgouedo]

Hi @MikeOpenHWGroup
What about

Forbid the use of $error, $warning, or $fatal anywhere in the testbench.

used in assertions included in RTL design files?
Especially if we don't want to add uvm packages in those RTL files...

[MikeOpenHWGroup]

What about

Forbid the use of $error, $warning, or $fatal anywhere in the testbench.

used in assertions included in RTL design files?
Especially if we don't want to add uvm packages in those RTL files...

Yes, I know that the RTL often does this. It is a bad practise.

We already have a good example of how this can be done in cv32e40p_prefetch_controller_sva.sv.

This is a clean approach as it keeps the assertions in its own file (which can use SV bind to connect to the RTL). I do not understand why we would avoid using uvm_packages since they are always available.

Another approach is to use assertion-macros similar to what the Ibex uses. These macros can optionally use UVM messaging (link).

[pascalgouedo]

Yes I wanted to extract all assertions from different CV32E40P RTL files to put them in specific files like prefetch_controller.
But it was really at the very bottom of the todo list and never came close to the top of the list 😀.

Maybe intermediate solution could be to use Ibex way?
But I am not sure we have all macros definitions to map all CV32E40P assertions created by ETH people...
And it is maybe too dangerous to do it now while I just created a RTL Freeze tentative tag earlier today.