Skip to content
This repository has been archived by the owner on Nov 4, 2024. It is now read-only.

Support for coverage of Privilged Architecture #80

Merged
merged 24 commits into from
Jul 26, 2024

Conversation

MuhammadHammad001
Copy link
Contributor

@MuhammadHammad001 MuhammadHammad001 commented Nov 14, 2023

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
Copy link
Contributor Author

MuhammadHammad001 commented Nov 18, 2023

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
Copy link
Contributor Author

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
Copy link
Collaborator

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 MuhammadHammad001 changed the title Support for read_csr(), immediate and rs2 value error issue resolved, mstatush issue resolved Support for PMP, Virtual Memory functions Dec 1, 2023
@MuhammadHammad001
Copy link
Contributor Author

MuhammadHammad001 commented Dec 2, 2023

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
Copy link
Contributor Author

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
Copy link
Contributor Author

MuhammadHammad001 commented Dec 3, 2023

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
Copy link
Collaborator

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
Copy link
Collaborator

allenjbaum commented Dec 4, 2023

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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.

riscv_isac/coverage.py Outdated Show resolved Hide resolved
pawks
pawks previously requested changes Jan 7, 2024
Copy link
Collaborator

@pawks pawks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The documentation should also be updated to indicate the added variables for the coverpoints.

riscv_isac/InstructionObject.py Outdated Show resolved Hide resolved
riscv_isac/coverage.py Outdated Show resolved Hide resolved
riscv_isac/coverage.py Outdated Show resolved Hide resolved
riscv_isac/coverage.py Show resolved Hide resolved
riscv_isac/main.py Show resolved Hide resolved
@UmerShahidengr
Copy link
Collaborator

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

@UmerShahidengr
Copy link
Collaborator

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

@MuhammadHammad001 MuhammadHammad001 changed the title Support for PMP, Virtual Memory functions Support for coverage of Privilged Architecture Feb 29, 2024
@UmerShahidengr UmerShahidengr mentioned this pull request Mar 5, 2024
13 tasks
@MuhammadHammad001
Copy link
Contributor Author

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

@UmerShahidengr
Copy link
Collaborator

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

@MuhammadHammad001
Copy link
Contributor Author

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

UmerShahidengr
UmerShahidengr previously approved these changes Jun 9, 2024
@UmerShahidengr UmerShahidengr dismissed pawks’s stale review July 19, 2024 05:21

Dismissing this one, as this review was on the older version, and the changes have been addressed by the contributor

Copy link
Collaborator

@allenjbaum allenjbaum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great addition to the functionality of creating coverpoints

Copy link
Collaborator

@allenjbaum allenjbaum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@allenjbaum allenjbaum merged commit 6f25c30 into riscv-software-src:dev Jul 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants