Support for coverage of Privilged Architecture

[MuhammadHammad001]

mnemonic flag support:

The mnemonic can be used to write a coverpoint that should target a instruction of interest only. For instance, lr or sc. Coverpoint can be written in the following pattern:

csr_comb:
    mnemonic == 'lr' : 0

[MuhammadHammad001]

Update to check the registers only of interest for read_csr() function:

Previously, if we used any coverpoint in the following pattern to compare the old_value of the register with the current value, it used to compare the register values with every instruction which was not required and we had no way to detect that whether the coverpoint of interest hit because of the required instruction or not. Now, if the user uses the coverpoints of the following pattern, then the isac will only check the instructions of interest for the coverpoint.

    (pmpcfg0    & 0x80 == 0x80) and (old("pmpcfg0") & 0xFF) ^ (pmpcfg0 & 0xFF) == 0x00 and old("pmpcfg0") != 0: 0

Explanation of the coverpoint pattern

The above coverpoint will check whether the value of the register pmpcfg0 is changed or not while trying to change in the assembly. So, consider the following:

mem[X,0x800003B0] -> 0x9073
mem[X,0x800003B2] -> 0x3A03
[176] [M]: 0x800003B0 (0x3A039073) csrrw zero, pmpcfg0, t2
CSR pmpcfg0 -> 0x00000000
CSR pmpcfg0 <- 0x9F9F9F9F (input: 0xFFFFFFFF)


mem[X,0x800003B4] -> 0xB073
mem[X,0x800003B6] -> 0x3A03
[177] [M]: 0x800003B4 (0x3A03B073) csrrc zero, pmpcfg0, t2
CSR pmpcfg0 -> 0x9F9F9F9F
CSR pmpcfg0 <- 0x9F9F9F9F (input: 0x00000000)

As, we try to change the value of the pmpcfg0 but it is not changed because of the lock bit, therefore, the value in the old_csr_regfile will not change which we can check using the coverpoint:

old("pmpcfg0") & 0xFF) ^ (pmpcfg0 & 0xFF) == 0x00 

[MuhammadHammad001]

Mode Check Support:

In this commit, I have added the support for checking the Mode in which the hart is currently executed. Consider the following part of the log:

mem[X,0x80000A84] -> 0x2403
mem[X,0x80000A86] -> 0x1E41
[205] [M]: 0x80000A84 (0x1E412403) lw fp, 484(sp)
mem[R,0x800031E4] -> 0x5BFDDB7D
x8 <- 0x5BFDDB7D

Previously, there was no mechanism to determine the execution mode of the instruction.

Solution

Now, users can easily check the mode in which the hart executed the instruction using the following coverpoint:

    "mode == 'M' and ((pmpaddr2) >= 0x00000000) and ((pmpaddr2) <= 0xFFFFFFFF)": 0

This coverpoint ensures that the current instruction of interest is executed in the required mode.

Check address of Label defined in the test:

Earlier, there was no support for checking the address of labels defined in the test. Now, users can employ the following coverpoint pattern to find the address of any labeled point in the test:

    "get_addr('rvtest_data') == 0x80000388": 0

Usage

The get_addr() function can be valuable for users writing coverpoints, ensuring that any exceptions occurring during the test are specifically within the designated area of interest.

[allenjbaum]

Without going into implementation details, this looks really good, and are necessary for many tests. Have tests been written, along with a cgf file, so we can be sure they actually work properly?

[MuhammadHammad001]

Virtual Memory Implementation

In this commit, I have implemented the requested variables for the Physical address, virtual address, page table walk addresses for the instructions and data accesses in the issue # 57 by Mr. Pawan in the RISC-V ISAC.

Implementation

Consider the following log which will be used as a reference to explain the implementation:

mem[X,0x800007D4] -> 0x0073
mem[X,0x800007D6] -> 0x3020
[444] [M]: 0x800007D4 (0x30200073) mret
CSR mstatus <- 0x00000080
ret-ing from M to S


mem[R,0x82002900] -> 0x20800401
mem[R,0x82001000] -> 0x200000CF
mem[X,0x800007D8] -> 0xE737
mem[X,0x800007DA] -> 0x0000
[445] [S]: 0x900007D8 (0x0000E737) lui a4, 14
x14 <- 0x0000E000


mem[X,0x800007DC] -> 0x0713
mem[X,0x800007DE] -> 0xEAD7
[446] [S]: 0x900007DC (0xEAD70713) addi a4, a4, 3757
x14 <- 0x0000DEAD


mem[X,0x800007E0] -> 0xA423
mem[X,0x800007E2] -> 0x00ED
[447] [S]: 0x900007E0 (0x00EDA423) sw a4, 8(s11)
mem[R,0x82002930] -> 0x20800401
mem[R,0x8200100C] -> 0x20800CCF
mem[0x8200311C] <- 0x0000DEAD

• As you may see in the log that the mode is switched to the S-mode using mret instruction and address translation is enabled using the satp register. For this test, the SV-32 is implemented and level 0 is used.

Previously, there was no way to access any of the page table walks, virtual address and physical address in case of virtual memory implementation.

Solution and Explanation

Now, consider the following coverpoint for understanding:

check_virtual_memory:
  config:
    - check ISA:=regex(.*32.*); check ISA:=regex(.*I.*Zicsr.*);  def rvtest_mtrap_routine=True; option VM = SV32;'
  mnemonics:
    sw: 0
  val_comb:
    'len_dptw == 2 and dptw1a == 0x82002930 and dptw0a == 0x8200100C and ieva == 0x900007E0 and iepa == 0x800007E0 and len_iptw == 2 and iptw1a == 0x82002900 and iptw0a == 0x82001000': 0

whose purpose is to target the following instruction in the above log file:

mem[X,0x800007E0] -> 0xA423
mem[X,0x800007E2] -> 0x00ED
[447] [S]: 0x900007E0 (0x00EDA423) sw a4, 8(s11)
mem[R,0x82002930] -> 0x20800401
mem[R,0x8200100C] -> 0x20800CCF
mem[0x8200311C] <- 0x0000DEAD

First of all, if you want to enable the following variables you need to add a flag option VM = SV(addressing scheme) where addressing scheme in this case was SV-32. You need to add this flag under the config node.

  config:
    - check ISA:=regex(.*32.*); check ISA:=regex(.*I.*Zicsr.*);  def rvtest_mtrap_routine=True; option VM = SV32;'

As, you may see in the coverpoint, there are multiple variables being used, I will write the purpose of every variable one by one:

1. ieva Virtual address of the instruction access
2. iepa Physical address of the instruction access
3. depa Physical address of the data access
4. ieva_align Alignment check for the ieva
5. iepa_align Alignment check for the iepa
6. depa_align Alignment check for the depa

Instruction access page table walks

Now, for the page table walks for the address access of the instructions: iptw0a,iptw1a,iptw2a,iptw3a,iptw4a

These variables are used for the page table walks for the instruction. To check how many were done in your case, you may use the following variable: len_iptw

Data access page table walks

Now, for the page table walks for the address access of the data: dptw0a,dptw1a,dptw2a,dptw3a,dptw4a

State for data access variables is not stored as these walks will be tracked on every data access. Their purpose is same as the instruction ones expect they are used for the data access page table walks.

Important

State of the variables for the page table walks of instruction access has also been stored which you may verify from the above sw a4, 8(s11) coverpoint. When the first page table walk is done, the state of these instruction page table walk address variables is not changed untill the mode is switched back to M - mode or the page table is accessed again making it easy for the user to track.

[MuhammadHammad001]

Value at Memory Location

In this commit, I have added the support for reading the value stored at a specific address. Consider the following coverpoint:

    'mem_val(get_addr("rvtest_data"),4) == 0xBEEDCAFE': 0

As, you may see that get_addr() that is added in previous commit was used to extract the address using the label is used in this coverpoint to get the address of rvtest_data and then the 4 written after the comma represents the size of memory we want to extract from the memory location. In this case, I wanted to get 4 Bytes, therefore, 4 is written. This is a function defined in the csr_comb.

User may also write a specific address of his/her interest to check the value stored at that address.

'(mem_val(0x80000388, 2) == 0xBEEDCAFE)': 0

[MuhammadHammad001]

PTE Permission Access Function

In this commit, I have added a function named as get_pte_per() which will make sure that permissions are set for the required physical address and also make sure that the PPN of the address at which the pte is stored is same as the PPN of the page table so we are sure that the pte is stored at the required location.

The python function is given as:

            def get_pte_per(pa, pte_addr, pgtb_addr):
                for match in matches_for_options:
                    if match[0] == 'VM' and match[1] in ['SV39', 'SV48', 'SV57']:
                        pte_size = 8
                    elif match[0] == 'VM' and match[1] in ['SV32']:
                        pte_size = 4
                if (pgtb_addr >> 12) == (pte_addr >> 12):
                    if ((pa >> 12) == (get_mem_val(pte_addr, pte_size) >> 10)):
                        return get_mem_val(pte_addr, pte_size) & 0x3FF
                    else:
                        return None
                else:
                    return None

The coverpoint for this functionality is given as:

check_for_the_isac:
  config:
    - check ISA:=regex(.*32.*); check ISA:=regex(.*I.*Zicsr.*);  def rvtest_mtrap_routine=True; option VM = SV32;
  mnemonics:
    sw: 0
    csrrw: 0
    csrrs: 0
    csrrc: 0
    lw: 0
  val_comb:
    'get_pte_per(get_addr("rvtest_data"),rs1_val + imm_val,get_addr("rvtest_slvl1_pg_tbl")) == 0xCF': 0

So, using this coverpoint we are sure that the pte is set for the rvtest_data

[allenjbaum]

This happened quite a bit faster than I expected. I'm not sure this has what I think is needed from a coverage viewpoint,

• You've defined how many levels there are, but we really need to know the level of any particular PTE access (from 5..1 in that order. SV57 will always start with 5; SV32 will always start with 2; all can end at any value start..1)
• I don't think it is important to know page table walk addresses; we shouldn't be dependent on the absolute values of addresses because they will often be offset (depending on where code is loaded). I'd go further than that that - it don't know why we would ever need that in a cover point.
• we definitely do need to know what the contents that are read/written from/to a page table entry at each level, and I'm not seeing it here (or I'm not understanding your definitions, which is equally likely).
• I don't think I care just about PTE permissions because that's covered by the PTE contents (above) and a mask, which is pretty much what your get_pte_per for the leaf page table entry, but we need masks for many more bits than that, and many more PTE levels than that, so it needs to be more general, e.g. get_pte_prop(lvl, prop_name) where prop_name is v, rwx, u, g, a, d, rsvd, ppn. I think we can get away without specifying RV64 vs RV32.
• we do need to know whether a PTE access is a read or a write at each level, e.g. get_pte_type(lvl) returning rd or wt

[allenjbaum]

Also note that the level variable start value is dependent on the SATP mode (and, eventually the VSATP as well; when Sail implements H-extension, we will need to be able to distinguish them) but can end anywhere between start and 1.
That means that in order to correctly label the level of an access

• code must now read SATP.mode for the first PTE access (only) ,
• = each following read decrements the level one,
• each writes use the value of the preceding access level and don't decrement it.

Also note that tests for VM should make sure that TLBs are turned off in whichever model is the reference model (for these tests only) so they don't skip any access.

[MuhammadHammad001]

VIrtual Memory Implementation:

Now, the coverpoints don't need a special lablel for the virtual memory else the track of all the virtual memory registers is kept by the isac itself using the satp register.

1. Now, the contents of the page table walk access have also been added which user may access using the label iptw0cont or dptw0cont e.t.c

    get_mem_val(2181042244,4) == 545259727: 0 #dptw0
    get_mem_val(2181048576,4) == 545260545: 0 #dptw1

PTE Permissions function updated to GET_PTE

The function to get the pte permissions has been updated to get_pte which you may use like the following coverpoint:

  val_comb:
    'get_pte(get_addr("rvtest_data"),rs1_val + imm_val,get_addr("rvtest_slvl1_pg_tbl")) == 0x800000CF': 0

Function to check the specific permission added

Now, the function to check the specific permission of the pte using the prop_name has also been added which you may use like the following:

    get_pte_prop('U',get_addr("begin_signature"), rs1_val + imm_val, get_addr("rvtest_slvl1_pg_tbl")) == 1: 0

[MuhammadHammad001]

Macros file Support Added

In this commit, I have added support for using a macro in the coverpoints. Please consider the following macros/header file for the coverpoint:

common:
 shift_bit: 0x001
self:
 final_value: 0x012

The header file is written in the yaml format. Anything under the common label is available for all the cgf files that may include the header file. User needs to add the following flags while running the risc-v isac command:

     -h -> header file flag
    -cm -> cgf macro flag 

cm This flag allows to add the macros written specifically for the covergroup file. Complete command may look something like the following:

 riscv_isac --verbose info coverage -d                         -t /home/hammad/riscv-arch-test/riscof_work/torr_tests.S/torr_tests.log --parser-name c_sail -o coverage.rpt                          --sig-label begin_signature  end_signature  -e /home/hammad/riscv-arch-test/riscof_work/torr_tests.S/ref.elf -c /home/hammad/riscv-ctg/sample_cgfs/dataset.cgf -c /home/hammad/PMP_Coverpoints/checkcover_1.cgf -x32   -l virtual_memory_functions_test -h /home/hammad/header_file.yaml -cm self

Consider the following coverpoint:

  val_comb:
    ${shift_bit} == 1: 0
    ${final_value} == 0x012: 0

So, when user may run the above command, the coverpoints will be replaced by the values assigned in the headerfile written in yaml file.

[allenjbaum]

I would have named it "Exc_type" rather than call type, (since it will be confused with the Ecall instruction ABI parameter), but that's not important. Aren't call_type and exc_num redundant, that is, each call type is just an ascii string that defines the exc_num? It may not be the case (there may be different causes that have the same exc_num, for example). In any case - it doesn't hurt sof don't remove anything.

But, if you're exposing tval and exc_num (which I'm assuming is the cause CSR) ... which tval, cause? Mtval/Mcause or Stval/Scause ? (and eventually the hypervisor versions) Not that if you're in Mmode, it code can still read&write stval.

We will need to add xtval2 & xtinst eventually for hypervisor traps , though there is no hurry for that.

On Thu, Jan 18, 2024 at 11:09 AM HammadBashir @.***> wrote:

Trap Register Variables

I have added few variables that keep track of the current instruction exceptions/traps. The variables are

1. mode_change = From which mode we switched and to which mode. For instance, S to M
2. exc_num = Exception number
3. tval = tval
4. call_type = Type of exception. For instance load-page-fault or e-call

Coverpoint example

User may write the coverpoints for these variables in the following pattern:

val_comb: call_type == "load-page-fault": 0 exc_num == 15: 0

So, these are simple variables, user may change the above coverpoints depending upon the use-case.

— Reply to this email directly, view it on GitHub https://github.com/riscv-software-src/riscv-isac/pull/80#issuecomment-1899056737, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHPXVJTL3CKPRZK4EOWKTQ3YPFXP5AVCNFSM6AAAAAA7LAAW6OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOJZGA2TMNZTG4 . You are receiving this because you commented.Message ID: @.***>

[MuhammadHammad001]

Trap Register Variables

This is the updated comment as I have added few variables that keep track of the current instruction exceptions/traps. The variables are

1. mode_change = From which mode we switched and to which mode. For instance, S to M
2. mcause/scause = Exception number
3. mtval/stval = tval
4. call_type = Type of exception. For instance load-page-fault or e-call

Coverpoint example

User may write the coverpoints for these variables in the following pattern:

val_comb:
    call_type == "load-page-fault": 0
    mcause == 15: 0

Explanation

The code has been updated such that it will check from the mode_change whether the mode change was to supervisor or machine mode, then it will update the respective register i.e., in supervisor or machine mode register respectively. Please refer to the commit: https://github.com/riscv-software-src/riscv-isac/pull/80/commits/ac4f34251089db834c1aaf7274db8385b7f6f281

Now, the arch-state for these mcause/scause, mtval/stval is also maintained which means that their values will only be updated when they are changed in the log by the instruction or by a trap.

[MuhammadHammad001]

@pawks , @allenjbaum , @neelgala
I would be really thankful if anyone please start the review process for this PR.

[UmerShahidengr]

@allenjbaum, @MuhammadHammad001 has resolved all conflicts in this PR, can we please merge it before any other conflict occurs?

[UmerShahidengr]

@pawks, your attention is also required here as this PR is stalled for a few weeks.

[MuhammadHammad001]

@pawks All the required changes have been made, can you please review it again for any further changes

[UmerShahidengr]

@allenjbaum I am mentioning you again, can you merge it now? It has been stalled for quite a few weeks

[MuhammadHammad001]

Introducing Translator Support

We've incorporated a script for translating CGF files to reduce redundancy at the user-end while writing the coverpoints. This script was developed in collaboration with Mr. @allenjbaum. To illustrate its functionality, let's examine a specific coverpoint:

PMP_TOR_priority_r:
  config:
    - check ISA:=regex(.*32.*); check ISA:=regex(.*I.*Zicsr.*);  def rvtest_mtrap_routine=True;
  mnemonics:
    "{csrrs, csrrw, lw, sw}" : 0
  csr_comb:
    ((pmpcfg3 >> 24) & {0x8C, 0x8A, 0x89}) == (0x8F & $1): 0                                  #Read, Write, execute Permission given to low priority region
    ((pmpcfg0 >> 24) & {0x8C, 0x8A, 0x89}) == (0x89 & $1): 0                                  #Read Permission given to high priority region
    pmpcfg{0, 3} != 0 and ((old("pmpcfg$1")) ^ (pmpcfg$1) != 0x00): 0                         #pmpcfg0(H.P0) and pmpcfg3(L.P) have been updated
    (old("pmpaddr{2, 3, 14, 15}")) ^ (pmpaddr$1) != 0x00: 0                                   #pmpaddr has been used and updated from the previous value i.e., 0x000
  val_comb:
    #Test for exceptions
    mode == {'M','S','U'} and (mcause != ${Load_access_fault}): 0                                             #No read fault
    mode == {'M','S','U'} and (mcause ==${Store_access_fault}): 0                                             #Write fault
    mode == 'M' and mode_change == {'M to M', 'U to M', 'S to M'} and (mcause == ${fetch_access_fault}): 0     #execute fault
    #check for the accesses
    ? (mnemonic == "lw" or mnemonic == "sw") and ((pmpcfg0 >> 24) & 0x9F == 0x89) and (rs1_val + imm_val >= (pmpaddr2 << 2)) and (rs1_val + imm_val < (pmpaddr3 << 2))
    : 0

The above will be processed by the translator_cgf.py and will result in the following coverpoint after the macros are resolved as well:

PMP_TOR_priority_r:
  config:
  - check ISA:=regex(.*32.*); check ISA:=regex(.*I.*Zicsr.*);  def rvtest_mtrap_routine=True;
  csr_comb:
    ((pmpcfg0 >> 24) & 0x89) == (0x89 & 0x89): 0
    ((pmpcfg0 >> 24) & 0x8A) == (0x89 & 0x8A): 0
    ((pmpcfg0 >> 24) & 0x8C) == (0x89 & 0x8C): 0
    ((pmpcfg3 >> 24) & 0x89) == (0x8F & 0x89): 0
    ((pmpcfg3 >> 24) & 0x8A) == (0x8F & 0x8A): 0
    ((pmpcfg3 >> 24) & 0x8C) == (0x8F & 0x8C): 0
    (old("pmpaddr14")) ^ (pmpaddr14) != 0x00: 0
    (old("pmpaddr15")) ^ (pmpaddr15) != 0x00: 0
    (old("pmpaddr2")) ^ (pmpaddr2) != 0x00: 0
    (old("pmpaddr3")) ^ (pmpaddr3) != 0x00: 0
    pmpcfg0 != 0 and ((old("pmpcfg0")) ^ (pmpcfg0) != 0x00): 0
    pmpcfg3 != 0 and ((old("pmpcfg3")) ^ (pmpcfg3) != 0x00): 0
  mnemonics:
    csrrs: 0
    csrrw: 0
    lw: 0
    sw: 0
  val_comb:
    ? (mnemonic == "lw" or mnemonic == "sw") and ((pmpcfg0 >> 24) & 0x9F == 0x89)
      and (rs1_val + imm_val >= (pmpaddr2 << 2)) and (rs1_val + imm_val < (pmpaddr3
      << 2))
    : 0
    mode == 'M' and (mcause != 0x05): 0
    mode == 'M' and (mcause == 0x07): 0
    mode == 'M' and mode_change == 'M to M' and (mcause == 0x01): 0
    mode == 'M' and mode_change == 'S to M' and (mcause == 0x01): 0
    mode == 'M' and mode_change == 'U to M' and (mcause == 0x01): 0
    mode == 'S' and (mcause != 0x05): 0
    mode == 'S' and (mcause == 0x07): 0
    mode == 'U' and (mcause != 0x05): 0
    mode == 'U' and (mcause == 0x07): 0