openhwgroup / core-v-verif

Functional verification project for the CORE-V family of RISC-V cores.
https://docs.openhwgroup.org/projects/core-v-verif/en/latest/index.html
Other
430 stars 219 forks source link

ISS: Missing mscratchcsw register and mnxti mismatch #2147

Open silabs-hfegran opened 1 year ago

silabs-hfegran commented 1 year ago

ISS reports missing mscratchcsw register, and mismatches upon using reserved csr-instructions to access mnxti

During initialization, the rvviRefCsrSetVolatile-function reports an error when attempting to set mscratchcsw as volatile

Error (IDV) validateRefCsrSetVolatile: CSR index 0x348 not present in reference model
Error (IDV) call to RVVI-API function rvviRefCsrSetVolatile failed

When the test starts, it quickly fails with a mismatch when attempting to access mnxti with illegal csrrc instruction.

Type

Steps to Reproduce

Please provide:

  1. cv32e40x/dev (81604f4faecbeb863fdcf7b8bc6138720409e367)
  2. make test TEST=clic CFG=clic_default CV_CORE=cv32e40x USE_ISS=1 RNDSEED=0
  3. Instruction causing first mismatch:

    26ec:   345137f3            csrrc   x15,0x345,x2
    
    UVM_INFO @ 16.800 ns : uvmt_cv32e40x_imperas_dv_wrap.sv(727) reporter [ImperasDV_if] rvviRefInit() succeed
    Error (IDV) validateRefCsrSetVolatile: CSR index 0x348 not present in reference model
    Error (IDV) call to RVVI-API function rvviRefCsrSetVolatile failed
    UVM_INFO @ 16.800 ns : uvmt_cv32e40x_imperas_dv_wrap.sv(895) reporter [ImperasDV_if] ref_init() complete
    UVM_INFO @ 16.800 ns : uvmt_cv32e40x_base_test.sv(301) uvm_test_top [BASE TEST] set load_instr_mem
    UVM_INFO @ 114.300 ns : uvmt_cv32e40x_firmware_test.sv(178) uvm_test_top [TEST] Started RUN

CLIC Test start

43133.000 ns: Illegal instruction (core 0) at PC 0x000026ec: Info (IDV) Instruction executed prior to mismatch '0x26ea(rw_mnxti_without_irq_illegal+78): c398 c.sw x14,0(x15)' Error (IDV) PC mismatch (HartId:0, PC:0x000026ec rw_mnxti_without_irq_illegal+80): Info (IDV) 0> Info (IDV) . dut:0x00000000 vector_table+0 Info (IDV) . ref:0x000026ec rw_mnxti_without_irq_illegal+80 Error (IDV) Insn. bit pattern mismatch (HartId:0, PC:0x000026ec rw_mnxti_without_irq_illegal+80): Info (IDV) 1> Info (IDV) . dut:3640506f j 5364 Info (IDV) . ref:345137f3 csrrc x15,mnxti,x2 Error (IDV) CSR register value mismatch (HartId:0, PC:0x000026ec rw_mnxti_without_irq_illegal+80): Info (IDV) 2> CSR 300 (mstatus) Info (IDV) . dut:0x00001880 SD:0 XS:0(Off) FS:0(Off) MPP:3 VS:0(Off) MPIE:1 MIE:0 Info (IDV) . ref:0x00001888 SD:0 XS:0(Off) FS:0(Off) MPP:3 VS:0(Off) MPIE:1 MIE:1 Info (IDV) 3> CSR 341 (mepc) Info (IDV) . dut:0x000026ec Info (IDV) . ref:0x000022dc Info (IDV) 4> CSR 342 (mcause) Info (IDV) . dut:0x38000002 Interrupt:0 minhv:0 mpp:3 mpie:1 mpil:0 Code:2(Virtual supervisor software interrupt) Info (IDV) . ref:0xb800001f Interrupt:1 minhv:0 mpp:3 mpie:1 mpil:0 Code:31 UVM_ERROR @ 43152.300 ns : idvPkg.sv(92) reporter [] uvmt_cv32e40x_tb.imperas_dv.idv_trace2api.state_compare @ 43152.000 ns: MISMATCH

Dump Reference State GPR 0: 00000000 1: 00000612 2: 003fff30 3: 000198a0 4: 00000000 5: ffffffff 6: 000079d0 7: 00000000 8: 003fff60 9: 00019000 10: 0000000b 11: 00000000 12: 0001815c 13: 00002000 14: 00000001 15: 00019b08 16: 80808080 17: fefefeff 18: 00019000 19: 00019000 20: 00019000 21: 00019000 22: 00019000 23: 00019000 24: 00000016 25: 00000000 26: 00000000 27: 00000000 28: 00000000 29: 00000000 30: 00000000 31: 00018518

Dump Reference State CSR jvt: 00000000 mstatus: 00001888 misa: 40801104 mie: 00000000 mtvec: 00000003 mtvt: 00001000 mstatush: 00000000 mcountinhibit: 0000000d mhpmevent3: 00000000 mhpmevent4: 00000000 mhpmevent5: 00000000 mhpmevent6: 00000000 mhpmevent7: 00000000 mhpmevent8: 00000000 mhpmevent9: 00000000 mhpmevent10: 00000000 mhpmevent11: 00000000 mhpmevent12: 00000000 mhpmevent13: 00000000 mhpmevent14: 00000000 mhpmevent15: 00000000 mhpmevent16: 00000000 mhpmevent17: 00000000 mhpmevent18: 00000000 mhpmevent19: 00000000 mhpmevent20: 00000000 mhpmevent21: 00000000 mhpmevent22: 00000000 mhpmevent23: 00000000 mhpmevent24: 00000000 mhpmevent25: 00000000 mhpmevent26: 00000000 mhpmevent27: 00000000 mhpmevent28: 00000000 mhpmevent29: 00000000 mhpmevent30: 00000000 mhpmevent31: 00000000 mscratch: 00000000 mepc: 000022dc mcause: b800001f mtval: 00000000 mip: 00000000 mnxti: 00000000 mintthresh: 00000000 mscratchcswl: 00000000 tselect: 00000000 tdata1: 28001000 tdata2: 00000000 tinfo: 01008064 dcsr: 40000413 dpc: 00000000 dscratch0: 00000000 dscratch1: 00000000 mcycle: 00000000 minstret: 00000000 mhpmcounter3: 00000000 mhpmcounter4: 00000000 mhpmcounter5: 00000000 mhpmcounter6: 00000000 mhpmcounter7: 00000000 mhpmcounter8: 00000000 mhpmcounter9: 00000000 mhpmcounter10: 00000000 mhpmcounter11: 00000000 mhpmcounter12: 00000000 mhpmcounter13: 00000000 mhpmcounter14: 00000000 mhpmcounter15: 00000000 mhpmcounter16: 00000000 mhpmcounter17: 00000000 mhpmcounter18: 00000000 mhpmcounter19: 00000000 mhpmcounter20: 00000000 mhpmcounter21: 00000000 mhpmcounter22: 00000000 mhpmcounter23: 00000000 mhpmcounter24: 00000000 mhpmcounter25: 00000000 mhpmcounter26: 00000000 mhpmcounter27: 00000000 mhpmcounter28: 00000000 mhpmcounter29: 00000000 mhpmcounter30: 00000000 mhpmcounter31: 00000000 mcycleh: 00000000 minstreth: 00000000 mhpmcounterh3: 00000000 mhpmcounterh4: 00000000 mhpmcounterh5: 00000000 mhpmcounterh6: 00000000 mhpmcounterh7: 00000000 mhpmcounterh8: 00000000 mhpmcounterh9: 00000000 mhpmcounterh10: 00000000 mhpmcounterh11: 00000000 mhpmcounterh12: 00000000 mhpmcounterh13: 00000000 mhpmcounterh14: 00000000 mhpmcounterh15: 00000000 mhpmcounterh16: 00000000 mhpmcounterh17: 00000000 mhpmcounterh18: 00000000 mhpmcounterh19: 00000000 mhpmcounterh20: 00000000 mhpmcounterh21: 00000000 mhpmcounterh22: 00000000 mhpmcounterh23: 00000000 mhpmcounterh24: 00000000 mhpmcounterh25: 00000000 mhpmcounterh26: 00000000 mhpmcounterh27: 00000000 mhpmcounterh28: 00000000 mhpmcounterh29: 00000000 mhpmcounterh30: 00000000 mhpmcounterh31: 00000000 cycle: 00000000 instret: 00000000 hpmcounter3: 00000000 hpmcounter4: 00000000 hpmcounter5: 00000000 hpmcounter6: 00000000 hpmcounter7: 00000000 hpmcounter8: 00000000 hpmcounter9: 00000000 hpmcounter10: 00000000 hpmcounter11: 00000000 hpmcounter12: 00000000 hpmcounter13: 00000000 hpmcounter14: 00000000 hpmcounter15: 00000000 hpmcounter16: 00000000 hpmcounter17: 00000000 hpmcounter18: 00000000 hpmcounter19: 00000000 hpmcounter20: 00000000 hpmcounter21: 00000000 hpmcounter22: 00000000 hpmcounter23: 00000000 hpmcounter24: 00000000 hpmcounter25: 00000000 hpmcounter26: 00000000 hpmcounter27: 00000000 hpmcounter28: 00000000 hpmcounter29: 00000000 hpmcounter30: 00000000 hpmcounter31: 00000000 cycleh: 00000000 instreth: 00000000 hpmcounterh3: 00000000 hpmcounterh4: 00000000 hpmcounterh5: 00000000 hpmcounterh6: 00000000 hpmcounterh7: 00000000 hpmcounterh8: 00000000 hpmcounterh9: 00000000 hpmcounterh10: 00000000 hpmcounterh11: 00000000 hpmcounterh12: 00000000 hpmcounterh13: 00000000 hpmcounterh14: 00000000 hpmcounterh15: 00000000 hpmcounterh16: 00000000 hpmcounterh17: 00000000 hpmcounterh18: 00000000 hpmcounterh19: 00000000 hpmcounterh20: 00000000 hpmcounterh21: 00000000 hpmcounterh22: 00000000 hpmcounterh23: 00000000 hpmcounterh24: 00000000 hpmcounterh25: 00000000 hpmcounterh26: 00000000 hpmcounterh27: 00000000 hpmcounterh28: 00000000 hpmcounterh29: 00000000 hpmcounterh30: 00000000 hpmcounterh31: 00000000 mvendorid: 00000602 marchid: 00000014 mimpid: 00000000 mhartid: 00000000 mconfigptr: 00000000 mintstatus: 00000000

eroom1966 commented 1 year ago

@silabs-hfegran Can you give a little more detail on how to reproduce, previously I would do the following

export CV_CORE=cv32e40x pushd core-v-verif/${CV_CORE}/sim/uvmt make test TEST=clic CFG=clic_default CV_CORE=cv32e40x USE_ISS=1 RNDSEED=0 popd

I am thinking I need to run the clone tb or something ?

OK, think I do this cd core-v-verif ./bin/clonetb -x

eroom1966 commented 1 year ago

Hi Henrik, the manual says this core does not support user mode Can you clarify

The specification says:

To accelerate interrupt handling with multiple privilege modes, a new CSR xscratchcsw can be defined for all but the lowest privilege mode to support conditional swapping of the xscratch register when transitioning between privilege modes. The CSR instruction is used once at the entry to a handler routine and once at handler exit, so only adds two instructions to the interrupt code path.

If you don't have User mode than Machine mode is the lowest privilege mode and therefore these CSRs are not defined. --

The information contained in this electronic mail message and any attachments hereto is privileged and confidential information intended only for the use of the individual or entity named above or their designee. If the reader of this message is not the intended recipient, you are hereby notified that any dissemination, distribution or copying of this communication is strictly prohibited. If you have received this communication in error please immediately notify us by return message or by telephone and delete the original message from your mail system. Thank you.

Silabs-ArjanB commented 1 year ago

Hi @eroom1966

If you don't have User mode than Machine mode is the lowest privilege mode and therefore these CSRs are not defined.

This is not what the RISC-V CLIC specification says.

mscratchcsw is listed in the 'CLIC CSRs' section of 'smclic M-mode CLIC extension' and its behavior seems well defined also for CPUs with only machine mode.

silabs-hfegran commented 1 year ago

@silabs-hfegran Can you give a little more detail on how to reproduce, previously I would do the following

export CV_CORE=cv32e40x pushd core-v-verif/${CV_CORE}/sim/uvmt make test TEST=clic CFG=clic_default CV_CORE=cv32e40x USE_ISS=1 RNDSEED=0 popd

I am thinking I need to run the clone tb or something ?

OK, think I do this cd core-v-verif ./bin/clonetb -x

Correct, from a clean state you would need to do the clonetb -x step - note that if you are reusing an existing env. you should add the -clone flag to the clonetb command, otherwise you might not be on the correct version of cv32e40x-dv.

eroom1966 commented 1 year ago

mscratchcsw is listed in the 'CLIC CSRs' section of 'smclic M-mode CLIC extension' and its behavior seems well defined also for CPUs with only machine mode.

Hi @Silabs-ArjanB The spec says this regarding this register

(NEW) 0x348 mscratchcsw Conditional scratch swap on priv mode change

But it is not possible to have a priv mode change, your specification supports only M mode ?

Can I request you ask for a definitive clarification on the specification

Silabs-ArjanB commented 1 year ago

@eroom1966 That doesn't mean the CSR is not there. The CV32E40X is machine mode only and contains the mscratchcsw CSR as mandated by the RISC-V CLIC specification.

eroom1966 commented 1 year ago

@Silabs-ArjanB

We interpret this differently, can I refer you to the following

https://github.com/riscv/riscv-fast-interrupt/blob/master/clic.adoc#4710-scratch-swap-csr-xscratchcsw-for-multiple-privilege-modes

  1. The title of the section for the csr xscratchcsw says

    4.7.10. Scratch Swap CSR (xscratchcsw) for Multiple Privilege Modes

The CV32E40X does not have multiple privilege modes, it has a single privilege mode - Machine Mode

  1. The description says

    To accelerate interrupt handling with multiple privilege modes, a new CSR xscratchcsw can be defined for all but the lowest privilege mode

The lowest level privilege mode in the CV32E40X, is Machine Mode. This clearly says that this CSR can be defined "for all BUT the lowest privilege mode"

Do you disagree with this text ? if so, I think you should raise an issue with the clic specification for further clarification

Silabs-ArjanB commented 1 year ago

I agree with the RISC-V CLIC text, but my interpretation is different from yours (and maybe wrong).

The lowest level privilege mode in the CV32E40X, is Machine Mode. This clearly says that this CSR can be defined "for all BUT the lowest privilege mode"

When the CLIC spec talks about 'lowest privilege mode' I interpret that to mean user mode as opposed to the 'lowest privilege mode in a given CPU'. Of course mscratchcsw is useless in the 40X, I fully agree with that. I will file a question in the CLIC TG.

Silabs-ArjanB commented 1 year ago

Hi @eroom1966

You were right about the CLIC spec interpretation with respect to mscratchcsw. We will remove mscratchcsw from the CV32E40X.

Best regards, Arjan

eroom1966 commented 1 year ago

@Silabs-ArjanB Thanks for the update

@silabs-hfegran I wonder is there still an issue here, even without the issue regarding the mscratchcsw register - I will take a quick look

eroom1966 commented 1 year ago

@silabs-hfegran you were right, I had stupidly not updated the E40X with the requirement of the legal ways to write mnxti Having fixed this, I now see that the following instruction is successfully retired in the RTL

134235.000 ns: RET,0,12765,00003648,"348116f3 csrrw x13,mscratchcsw,x2",x13=003fff20,,,,

This same instruction causes the REF model to take an exception because (as discussed earlier in the thread) this register is absent when the implementation does not support U mode. In which case, the REF model generates the following error

Warning (RISCV_ILL) CPU 'refRoot/cpu' 0x00003648 348116f3 csrrw x13,mscratchcsw,x2: Illegal instruction - extension U (User mode) is absent

I will push the additions for the CSR write accesses for CV32E40X and send you a notification, this will allow you to see this same error

silabs-hfegran commented 1 year ago

Great, thanks @eroom1966, I will update the test env. as soon as we have the register removed from the RTL as well.

eroom1966 commented 1 year ago

@silabs-hfegran

mnxti change added to CV32E40X

commit cc98e6459b8da5672bc1f961249ce16ec58d1223 (HEAD -> main, origin/main, origin/HEAD) Author: eroom1966 moore@imperas.com Date: Tue Aug 29 11:23:46 2023 +0100

Add fix for mnxti access instructions on E40X