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
444 stars 221 forks source link

Overview of open issues related to CV32E40X/CV32E40S Imperas ISS #1285

Open Silabs-ArjanB opened 2 years ago

Silabs-ArjanB commented 2 years ago

This issue will be used to provide an overview of open issues related to CV32E40X/CV32E40S Imperas ISS. The list will be continuously updated. Issues that exist on both CV32E40X and CV32E40S will not necessarily be mentioned twice.

  1. Resolved - No schedule supplied for ISS 0.3.0.
  2. Resolved - mret in debug mode increments minstret (https://github.com/openhwgroup/cv32e40x/issues/558).
  3. Resolved - ISS mismatch if debug entry occurs on first instruction of NMI ISR handler.
  4. Resolved - (0.2.0) Default ecode_mask=255 incorrect (should be 2047)
  5. Resolved - (0.2.0) Default mtvec_mask=0xffff_ff01 incorrect (should be 0xffff_ff81)
  6. Resolved - (0.2.0) Default tcontrol_undefined incorrect (Default NUM_TRIGGERS parameter = 1, tcontrol should be present by default, and should be conditionally enabled/disabled based on number of triggers implemented)
  7. Resolved - (0.2.0) misa.B sometimes set, 'add_Extensions' issue (see text below in this issue)
  8. Resolved - (0.2.0) Debug cause dcsr[8:6] mismatch when external debug is taken over single step (see text below in this issue)
  9. Resolved - (0.2.0) https://github.com/openhwgroup/core-v-verif/issues/1327
  1. Resolved - No schedule supplied for ISS 0.3.0.
  2. Open - (0.2.0) Default value instret_undefined=T removes minstret (which should be present). Too strict coupling between instret and minstret.
  3. Open - (0.2.0) New model Documentation lacks provisions for bitmanip-extensions
  4. Awaiting merge - (0.2.0) Bitmanip enable-overrides appears to not function correctly (e.g. ctz triggers illegal instruction when enabled)
  5. Awaiting merge - (0.2.0) Default ecode_mask=255 incorrect (should be 2047)
  6. Awaiting merge - (0.2.0) Default mtvec_mask=0xffff_ff01 incorrect (should be 0xffff_ff81)
  7. Open - (0.2.0) Default tcontrol_undefined incorrect (Default NUM_TRIGGERS parameter = 1, tcontrol should be present by default, and should be conditionally enabled/disabled based on number of triggers implemented)
  8. Resolved - (0.2.0) misa.X should be 1
  9. Resolved - (0.2.0) cycle_undefined=T removes mcycle csr (https://github.com/openhwgroup/core-v-verif/issues/1326)
  10. Open - dcsr.prv seems not implemented - https://github.com/openhwgroup/core-v-verif/issues/1366
eroom1966 commented 2 years ago

Having now cleared the backlog of work with Marton (non specified behavior for NMI) in the legacy testbench/model - I am now starting to look at these other issues.

As we agreed in a previous meeting, we now have 2 model Variants which are called (in the case of CV32E40X)

Our assumption is that the CV32E40X is the 0.2.0, and the CV32E40X_DEV will be 0.4.0 as this is now the dev spec do you agree ?

We are currently working on CV32E40X, with the issues listed above. We will then be working on CV32E40S, with the issues listed above. As we go through each of these individual line items, we will file specific issues related to each.

Silabs-ArjanB commented 2 years ago

Hi @eroom1966

Our assumption is that the CV32E40X is the 0.2.0, and the CV32E40X_DEV will be 0.4.0 as this is now the dev spec do you agree ?

It is okay that you skip 0.3.0 and move to 0.4.0 directly. However, please provide us with a schedule for when we can expect the 0.4.0 ISS.

eroom1966 commented 2 years ago

@Silabs-ArjanB

for the issue mentioned here

mret in debug mode increments minstret (https://github.com/openhwgroup/cv32e40x/issues/558).

Which cores does this apply to E40P, E40X, E40S

and also which version - is this a clarification to the Current release (0.2.0) or a change to the DEV release (0.4.0 ?) of the RM ?

eroom1966 commented 2 years ago

(0.2.0) Bitmanip enable-overrides appears to not function correctly (e.g. ctz triggers illegal instruction when enabled)

can you please tell me how to reproduce this issue

silabs-hfegran commented 2 years ago

@Silabs-ArjanB

for the issue mentioned here

mret in debug mode increments minstret (openhwgroup/cv32e40x#558).

Which cores does this apply to E40P, E40X, E40S

and also which version - is this a clarification to the Current release (0.2.0) or a change to the DEV release (0.4.0 ?) of the RM ?

Hi @eroom1966, all the issues listed here are related to the latest version of the ISS that you submitted a while ago (prior to the fixes that you and Marton worked on recently). The ISS with the interrupt-related fixes is currently in PR, and will be merged to X shortly, while work to get these changes onto the S-variant will probably happen sometime next week. If certain issues mentioned here are already resolved in this variant, you may disregard the issues in question.

Issues mentioned above with "default value" basically means that the default is not correct, and that this can be configured correctly by using an override. Consequently, this just means that we would like these to be set as defaults, and not require an override.

as for the mret-issue, I am not entirely certain, but I would expect the behavior to be identical between S and X (please correct me if I am wrong @Silabs-ArjanB)

silabs-hfegran commented 2 years ago

(0.2.0) Bitmanip enable-overrides appears to not function correctly (e.g. ctz triggers illegal instruction when enabled)

can you please tell me how to reproduce this issue

Thanks for looking into this @eroom1966, I just did a quick test now to reproduce this issue on cv32e40s/dev latest change the default.yaml in core-v-verif/cv32e40s/tests/cfg/default.yaml to match the following (Basically enable Zb* extensions in RTL and in ISS):

name: default description: Default configuration for CV32E40S simulations compile_flags: +define+ZBA_ZBB_ZBC_ZBS ovpsim: > --showoverrides cflags: > plusargs: > +enable_zba_extension=1 +enable_zbb_extension=1 +enable_zbc_extension=1 +enable_zbs_extension=1 cv_sw_march: rv32imc_zba1p00_zbb1p00_zbc1p00_zbs1p00

Then make test TEST=b_ext_test should reproduce the issue consistently

This is the output of my --showoverrides with this configuration

# root/cpu/Bit_Manipulation_Extension --override root/cpu/Zba=1 (Boolean) (default=T) (override) Specify that Zba is implemented --override root/cpu/Zbb=1 (Boolean) (default=T) (override) Specify that Zbb is implemented --override root/cpu/Zbc=1 (Boolean) (default=T) (override) Specify that Zbc is implemented --override root/cpu/Zbe=0 (Boolean) (default=T) (override) Specify that Zbe is implemented (ignored if version 1.0.0) --override root/cpu/Zbf=0 (Boolean) (default=T) (override) Specify that Zbf is implemented (ignored if version 1.0.0) --override root/cpu/Zbm=0 (Boolean) (default=T) (override) Specify that Zbm is implemented (ignored if version 1.0.0) --override root/cpu/Zbp=0 (Boolean) (default=T) (override) Specify that Zbp is implemented (ignored if version 1.0.0) --override root/cpu/Zbr=0 (Boolean) (default=T) (override) Specify that Zbr is implemented (ignored if version 1.0.0) --override root/cpu/Zbs=1 (Boolean) (default=T) (override) Specify that Zbs is implemented --override root/cpu/Zbt=0 (Boolean) (default=T) (override) Specify that Zbt is implemented (ignored if version 1.0.0) --override root/cpu/bitmanip_version=1.0.0 (Enumeration) (default=0.90) (override) Specify required Bit Manipulation Architecture version

eroom1966 commented 2 years ago

Hi @silabs-hfegran I now see this test passing with a minor adjustment to the configuration, we were simply missing the inclusion of .bitmanip_version = RVBV_1_0_0, // Bit Manipulation version 1.0.0 having added this the test now passes

UVM_INFO @ 16.800 ns : uvmt_cv32e40s_base_test.sv(294) uvm_test_top [BASE TEST] set load_instr_mem UVM_INFO @ 181.800 ns : uvmt_cv32e40s_firmware_test.sv(137) uvm_test_top [TEST] Started RUN Testing SHnADD INFO, SH1ADD result as expected test: 0x13 INFO, SH2ADD result as expected test: 0x21 INFO, SH3ADD result as expected test: 0x3d

as soon as Marton gets the core-v-verif changes implemented, I can put in a PR to address these issues raised above

silabs-hfegran commented 2 years ago

Hi @silabs-hfegran I now see this test passing with a minor adjustment to the configuration, we were simply missing the inclusion of .bitmanip_version = RVBV_1_0_0, // Bit Manipulation version 1.0.0 having added this the test now passes

UVM_INFO @ 16.800 ns : uvmt_cv32e40s_base_test.sv(294) uvm_test_top [BASE TEST] set load_instr_mem UVM_INFO @ 181.800 ns : uvmt_cv32e40s_firmware_test.sv(137) uvm_test_top [TEST] Started RUN Testing SHnADD INFO, SH1ADD result as expected test: 0x13 INFO, SH2ADD result as expected test: 0x21 INFO, SH3ADD result as expected test: 0x3d

as soon as Marton gets the core-v-verif changes implemented, I can put in a PR to address these issues raised above

Great, thanks!

Silabs-ArjanB commented 2 years ago

for the issue mentioned here mret in debug mode increments minstret (https://github.com/openhwgroup/cv32e40x/issues/558). Which cores does this apply to E40P, E40X, E40S

Both cores.

and also which version - is this a clarification to the Current release (0.2.0) or a change to the DEV release (0.4.0 ?) of the RM ?

I believe we only discovered this after 0.2.0 and documented it in 0.4.0. It is however fine if you already fix this in 0.2.0 of the ISS (I saw your message that there is already an option in the ISS to get the desired behavior and that is of course fine with us as well).

eroom1966 commented 2 years ago

@Silabs-ArjanB

Both cores.

do you mean all three cores ? I listed three, not two

Silabs-ArjanB commented 2 years ago

@eroom1966 I meant the CV32E40X and the CV32E40S only, not the CV32E40P.

silabs-mateilga commented 2 years ago

Item 3 is solved with PR #1307

Silabs-ArjanB commented 2 years ago

Item 3 is solved with PR #1307

@silabs-mateilga Which item 3? Can you indicate which of the CV32E40X and which of the CV32E40S items are still open?

silabs-mateilga commented 2 years ago

That is item 3 for 40x. I'll try to get an overview of the other issues today.

eroom1966 commented 2 years ago

@silabs-hfegran @silabs-mateilga I have a pull request ready for E40X, to address the issues listed

I cannot work on E40S (dev/cv32e40s), it appears that the testbench is still referring to the old NMI signals, so my verilog platform fails to elaborate, what should I do ?

silabs-mateilga commented 2 years ago

@eroom1966 thanks, the PR is merged now. Does this cover all the issues listed under 40x?

We are going to merge (cvverif) 40x to 40s as soon as possible, to migrate these fixes, in addition to the other items that are lagging on the 40s. I'll let you know when it is done.

eroom1966 commented 2 years ago

@eroom1966 thanks, the PR is merged now. Does this cover all the issues listed under 40x?

yes, as far as my tests show - you will also notice I re-enabled the debug_test as part of the ci_check, as this was fixed by changing the selection regarding the debug_eret_mode

So now for E40P debug_eret_mode is RVDRM_JUMP E40X, E40S debug_eret_mode is RVDRM_TRAP

silabs-mateilga commented 2 years ago

@eroom1966 I've been running a few tests on your latest commits, and have found two issues.

  1. misa bit 1, "B", mismatch for some configurations reproduceable by running "make test TEST=hello_world CFG=no_bitmanip" This bit is defined as "reserved" in the latest privileged spec, and should always read 0. For some configurations, the ISS reports this bit as high.

  2. Debug cause dcsr[8:6] mismatch when external debug is taken over single step. the following scenario results in a dcsr mismatch:

    • core (dret) leaves debug in single step mode
    • core retires lw instruction in machine mode
    • external debug request is pending for next instruction
    • core takes external debug request (higher priority than single step)
    • ISS takes single step debug, mismatch is detected

    The haltreq signal to the ISS is set for the retirement of the last instruction in this sequence, but the ISS does not seem to take this in to consideration.

    reproduce (xcelium 21.07.a001) generate: make comp_corev-dv gen_corev-dv TEST=corev_rand_instr_obi_err_debug CFG=pma_test_cfg_1 SIMULATOR=xrun USE_ISS=YES COV=YES RUN_INDEX=287 GEN_START_INDEX=287 RNDSEED=-102679561 simulate: make test TEST=corev_rand_instr_obi_err_debug CFG=pma_test_cfg_1 SIMULATOR=xrun USE_ISS=YES COV=YES RUN_INDEX=287 GEN_START_INDEX=287 RNDSEED=-102679561

    I'd be happy to do a shared screen session on any of these issues, if it is required.

eroom1966 commented 2 years ago

@silabs-mateilga investigating issue 1, I think this is something we discovered yesterday with the behavior of the add_Extensions parameter.

unfortunately something in git seems broken, I am getting this error due to bitbucket.org ?

git clone https://github.com/openhwgroup/cv32e40x /home/moore/git/eroom1966/core-v-verif/core-v-cores/cv32e40x; cd /home/moore/git/eroom1966/core-v-verif/core-v-cores/cv32e40x; git checkout 32c31a07f84beba70b2afd0afa7b6738771bed9f Cloning into '/home/moore/git/eroom1966/core-v-verif/core-v-cores/cv32e40x'... remote: Enumerating objects: 17289, done. remote: Counting objects: 100% (535/535), done. remote: Compressing objects: 100% (226/226), done. remote: Total 17289 (delta 344), reused 442 (delta 294), pack-reused 16754 Receiving objects: 100% (17289/17289), 8.14 MiB | 11.25 MiB/s, done. Resolving deltas: 100% (12759/12759), done. Checking out files: 100% (175/175), done. Note: checking out '32c31a07f84beba70b2afd0afa7b6738771bed9f'.

You are in 'detached HEAD' state. You can look around, make experimental changes and commit them, and you can discard any commits you make in this state without impacting any branches by performing another checkout.

If you want to create a new branch to retain commits you create, you may do so (now or later) by using -b with the checkout command again. Example:

git checkout -b

HEAD is now at 32c31a07 Merge pull request #587 from Silabs-ArjanB/ArjanB_dbgx git clone https://bitbucket.org/verilab/svlib/src/master/svlib --recurse /home/moore/git/eroom1966/core-v-verif/cv32e40x/vendor_lib/verilab/svlib; cd /home/moore/git/eroom1966/core-v-verif/cv32e40x/vendor_lib/verilab/svlib; git checkout c25509a7e54a880fe8f58f3daa2f891d6ecf6428 Cloning into '/home/moore/git/eroom1966/core-v-verif/cv32e40x/vendor_lib/verilab/svlib'... fatal: unable to access 'https://bitbucket.org/verilab/svlib/src/master/svlib/': Could not resolve host: bitbucket.org /bin/bash: line 0: cd: /home/moore/git/eroom1966/core-v-verif/cv32e40x/vendor_lib/verilab/svlib: No such file or directory fatal: reference is not a tree: c25509a7e54a880fe8f58f3daa2f891d6ecf6428 make[1]: [/home/moore/git/eroom1966/core-v-verif/mk/uvmt/uvmt.mk:231: /home/moore/git/eroom1966/core-v-verif/cv32e40x/vendor_lib/verilab/svlib] Error 128 make[1]: Leaving directory '/home/moore/git/eroom1966/core-v-verif/cv32e40x/sim/uvmt' make: [Makefile.uvmt:43: test] Error 2

UPDATE bitbucket is back ...

eroom1966 commented 2 years ago

@silabs-mateilga I can confirm that the add_Extensions issue was the same problem you are seeing in (1) above now investigating (2)

eroom1966 commented 2 years ago

@silabs-mateilga github definitely having issues, when trying to run the command

make comp_corev-dv gen_corev-dv TEST=corev_rand_instr_obi_err_debug CFG=pma_test_cfg_1 SIMULATOR=xrun USE_ISS=YES COV=YES RUN_INDEX=287 GEN_START_INDEX=287 RNDSEED=-102679561

failing with

Cloning into '/home/moore/git/eroom1966/core-v-verif/cv32e40x/vendor_lib/google/riscv-dv'... fatal: unable to access 'https://github.com/silabs-hfegran/riscv-dv.git/': Failed to connect to github.com port 443: Connection timed out /bin/bash: line 0: cd: /home/moore/git/eroom1966/core-v-verif/cv32e40x/vendor_lib/google/riscv-dv: No such file or directory fatal: reference is not a tree: 87d9ae2d60d928e3c6afcd6ff1aacb5298f2904b

eroom1966 commented 2 years ago

https://github.com/openhwgroup/cv32e40x/issues/340

Here was our previous discussion on this - why did this test not fail after the April/4th change, and has only been seen now ? copying in Mike @MikeOpenHWGroup

eroom1966 commented 2 years ago

@silabs-mateilga @MikeOpenHWGroup Hi Marton, Mike, do you have a schedule for the update of the core-v-verif infrastructure to include the necessary changes for the NMI updates.

I am currently blocked on working on the CV32E40S until I have a testbench which supports the model interfaces (as per the E40X)

silabs-mateilga commented 2 years ago

Hi Lee, sorry for the delayed response.

When PR 1314 is approved, you should be able to continue work on 40s. This is by no means a finished or steady state of the 40s/dev branch, but it has the nmi updates and should allow you to pursue the other issues.

-Marton

From: Lee Moore @.> Sent: fredag 24. juni 2022 10:59 To: openhwgroup/core-v-verif @.> Cc: Marton Teilgard @.>; Mention @.> Subject: Re: [openhwgroup/core-v-verif] Overview of open issues related to CV32E40X/CV32E40S Imperas ISS (Issue #1285)

CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.

@silabs-mateilgahttps://urldefense.com/v3/__https:/github.com/silabs-mateilga__;!!N30Cs7Jr!WY22H0VOpHQOcZKZKLsO6qD3ygP0TyvNdU7I3H4-420J3awE2Kv0BLPBxsiZy_ryV2DHTPS950LGtt7zfOtCN-o$ @MikeOpenHWGrouphttps://urldefense.com/v3/__https:/github.com/MikeOpenHWGroup__;!!N30Cs7Jr!WY22H0VOpHQOcZKZKLsO6qD3ygP0TyvNdU7I3H4-420J3awE2Kv0BLPBxsiZy_ryV2DHTPS950LGtt7zOW89bIM$ Hi Marton, Mike, do you have a schedule for the update of the core-v-verif infrastructure to include the necessary changes for the NMI updates.

I am currently blocked on working on the CV32E40S until I have a testbench which supports the model interfaces (as per the E40X)

— Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https:/github.com/openhwgroup/core-v-verif/issues/1285*issuecomment-1165359925__;Iw!!N30Cs7Jr!WY22H0VOpHQOcZKZKLsO6qD3ygP0TyvNdU7I3H4-420J3awE2Kv0BLPBxsiZy_ryV2DHTPS950LGtt7z-N6kzzA$, or unsubscribehttps://urldefense.com/v3/__https:/github.com/notifications/unsubscribe-auth/AOE32Y4BUD3DQYGSX42NAJTVQV2D7ANCNFSM5XQM7VCA__;!!N30Cs7Jr!WY22H0VOpHQOcZKZKLsO6qD3ygP0TyvNdU7I3H4-420J3awE2Kv0BLPBxsiZy_ryV2DHTPS950LGtt7zS5YfW3k$. You are receiving this because you were mentioned.Message ID: @.***>

eroom1966 commented 2 years ago

great looks like this has been merged now - some quick comments, looks like an override has been applied multiple times I am seeing this warning when running the hello-world smoke test Info (CF_PMM_R) Accepted: 3 --override root/cpu/mtvec_mask=0xffffff81 Info (CF_PMM_R) Ignored : 13 --override root/cpu/mtvec_mask=0xffffff81

silabs-mateilga commented 2 years ago

@Silabs-ArjanB please add this issue to the 40s list:

https://github.com/openhwgroup/core-v-verif/issues/1326

silabs-oysteink commented 2 years ago

@Silabs-ArjanB I added issue #1327 to core-v-verif, related to ISS marking push/pop instructions as illegal. Should be added to this list.

silabs-mateilga commented 2 years ago

@Silabs-ArjanB 40s item 9 is resolved

silabs-anvesten commented 2 years ago

@Silabs-ArjanB I added issue #1366 to core-v-verif, ISS mismatch bug. I believe it should be added to the list

silabs-mateilga commented 2 years ago

This could be as simple as the test not creating this scenario, or the tests failing on other issues before this.

-Marton

From: Lee Moore @.> Sent: torsdag 23. juni 2022 12:27 To: openhwgroup/core-v-verif @.> Cc: Marton Teilgard @.>; Mention @.> Subject: Re: [openhwgroup/core-v-verif] Overview of open issues related to CV32E40X/CV32E40S Imperas ISS (Issue #1285)

CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.

openhwgroup/cv32e40x#340https://urldefense.com/v3/__https:/github.com/openhwgroup/cv32e40x/issues/340__;!!N30Cs7Jr!Tnjigm9OTb54g3yB3f9WFE0pDhHU0dUOTBOt9AUkRxWz3vEHx7Fh6bPlkk0XnwMPzzi6VcV0l7DmiKThJctQRhE$

Here was our previous discussion on this - why did this test not fail after the April/4th change, and has only been seen now ? copying in Mike @MikeOpenHWGrouphttps://urldefense.com/v3/__https:/github.com/MikeOpenHWGroup__;!!N30Cs7Jr!Tnjigm9OTb54g3yB3f9WFE0pDhHU0dUOTBOt9AUkRxWz3vEHx7Fh6bPlkk0XnwMPzzi6VcV0l7DmiKThZpYG8qU$

— Reply to this email directly, view it on GitHubhttps://urldefense.com/v3/__https:/github.com/openhwgroup/core-v-verif/issues/1285*issuecomment-1164240518__;Iw!!N30Cs7Jr!Tnjigm9OTb54g3yB3f9WFE0pDhHU0dUOTBOt9AUkRxWz3vEHx7Fh6bPlkk0XnwMPzzi6VcV0l7DmiKThPtLmnXI$, or unsubscribehttps://urldefense.com/v3/__https:/github.com/notifications/unsubscribe-auth/AOE32Y2M3N6NFDOQJM4WWYLVQQ3WTANCNFSM5XQM7VCA__;!!N30Cs7Jr!Tnjigm9OTb54g3yB3f9WFE0pDhHU0dUOTBOt9AUkRxWz3vEHx7Fh6bPlkk0XnwMPzzi6VcV0l7DmiKThj6eScsQ$. You are receiving this because you were mentioned.Message ID: @.***>