(ACT4) Hypervisor Interrupts Coverpoints#713
Conversation
… restricts mie/mip/hie/hideleg bins to required exact values
|
@TnnsBeast did you make changes responding to the reviews? If they are done, click the "resolve conversation" button to show they are no longer pending. |
|
What is the mode_virt field? Do these tests compile and run? Are you waiting for a review? |
There was a problem hiding this comment.
This coverage file needs pretty major refactoring to resemble the other coverage files. I gave some high level feedback about expressing cross products in terms of the inputs, not listing outputs in the cross-product, being specific about cross-product inputs so ignore_bins and binsof aren't needed, dividing into covergroups on a per-mode basis, and cross-checking the coverpoints against the testplan. Test that it compiles. It needs a heavy rewrite and then I'll need to proof from the start again.
…rof. Harris feedback from 12/11
davidharrishmc
left a comment
davidharrishmc
left a comment
There was a problem hiding this comment.
mode_virt isn't considered yet.
jordancarlin
changed the title
…nts since included in standard
SadhviNarayanan
left a comment
SadhviNarayanan
left a comment
There was a problem hiding this comment.
Left some comments, overall I think it's close!
| cp_nohint_m: cross priv_mode_m, mstatus_mie_one, hideleg_vsi_zero, mie_vsi_ones, mip_vsi_ones; | ||
| `ifdef GILEN_GT_0 | ||
| cp_mideleg_gei: cross priv_mode_m, mideleg_sgei_ro; | ||
| cp_mie_gilen: cross priv_mode_m, csrr_mie, hie_vsi_ones, hie_sgeie_one, mie_sgeie_one; |
There was a problem hiding this comment.
might be worth checking if you need all the hie_vsi_ones for this test, or if sgeie is sufficient?
| `ifdef GILEN_GT_0 | ||
| cp_hie_gilen: cross priv_mode_hs, csrr_hie, mie_vsi_ones, hie_sgeie_one; | ||
| cp_priority_sgei: cross priv_mode_hs, sstatus_sie_one, hvip_vsi_ones, hideleg_vsi_zero, hie_sgeie_one, hgeip_nonzero, hgeie_nonzero; | ||
| cp_priority_sgei_s: cross priv_mode_hs, sstatus_sie_one, hideleg_vsi_zero, hvip_vsi_ones, hie_sgeie_one, sie_ones, sip_priority, hgeip_nonzero, hgeie_nonzero; |
There was a problem hiding this comment.
is checking hgeip necessary for this?
| cp_hie_gilen: cross priv_mode_hs, csrr_hie, mie_vsi_ones, hie_sgeie_one; | ||
| cp_priority_sgei: cross priv_mode_hs, sstatus_sie_one, hvip_vsi_ones, hideleg_vsi_zero, hie_sgeie_one, hgeip_nonzero, hgeie_nonzero; | ||
| cp_priority_sgei_s: cross priv_mode_hs, sstatus_sie_one, hideleg_vsi_zero, hvip_vsi_ones, hie_sgeie_one, sie_ones, sip_priority, hgeip_nonzero, hgeie_nonzero; | ||
| cp_trigger_sgei: cross priv_mode_hs, sstatus_sie, hgeip_nonzero, hgeie_nonzero, hie_sgeie_one; |
There was a problem hiding this comment.
testplan currently has sgeip/sgeie, so if hgeip/hgeie is what to use then maybe testplan should be updated to reflect this
| cp_hideleg_hie_vs: cross priv_mode_vs, hideleg_vsi, hie_vsi, hip_vsi_ones, vsstatus_sie_one; | ||
| cp_hip_hie_vs: cross priv_mode_vs, hip_vsi, hie_vsi, hideleg_vsi_ones, vsstatus_sie_one; | ||
| cp_sie_vs: cross priv_mode_vs, hideleg_vsi_ones, hip_vsi_ones, hie_vsi_ones, vsstatus_sie; | ||
| cp_mideleg_mip_vs: cross priv_mode_vs, mstatus_mie_zero, mideleg_ones_zeros, mip_walking, mie_ones, vsstatus_sie_one; |
There was a problem hiding this comment.
for some of the cp's where you want all 1's, it might be easier to sometimes just use instr.prev to read the value of the csr
| option.per_instance = 0; | ||
| `include "general/RISCV_coverage_standard_coverpoints.svh" | ||
|
|
||
| priv_mode_u_novirt: coverpoint {ins.prev.mode_virt, ins.prev.mode} { |
There was a problem hiding this comment.
same as priv_mode_u in general cp's
…ns, GILEN sgeie check)
6 participants
Description
Adding coverpoints for hypervisor interrupts.
Related Issues
NA
Ratified/Unratified Extensions
List Extensions
Reference Model Used
Mandatory Checklist:
Optional Checklist: