fix: Update default DV_TARGET to cv32a60x in CSR access test script

[DymShanks]

Resolves #3223 by aligning the default target with the existing testlist yaml files.

[cainria]

Note: ongoing discussion about this change in the related issue: #3223 (comment)

chasl59 says:

Hi @DymShanks , Please check the GitHub history of the file: cva6/verif/regress/dv-riscv-csr-access-test.sh You will find that the previous change to the file made the change to use this new value as the default: DV_TARGET=cv32a6_imac_sv32

Probably, somebody made that change on purpose. The change you propose might revert that change. That might not be the right way forward. I am not sure whom to ask about that. Maybe Mike Thompson, but I am not sure.

(Bold annotation added by me to highlight the fact that the issue comment is actually related to this PR)

[DymShanks]

@chasl59 and @calnria, great catch! I just ran a git log and found commit 4391fc4. You are completely right—@JeanRochCoulon explicitly changed the default target from cv32a60x to cv32a6_imac_sv32 back in June 2024 to support generating the FPGA bitstream (PR #2229).

Since reverting this script might break the FPGA flow, it seems the actual bug is simply that the corresponding yaml file (testlist_riscv-csr-access-test-cv32a6_imac_sv32.yaml) was either never committed or was accidentally deleted from the verif/tests/ directory.

@JeanRochCoulon or @MikeOpenHWGroup, would you prefer I generate and add the missing YAML file to this PR so the FPGA target works again, or should we transition the default target back to cv32a60x? I'm standing by to pivot and update this PR with whichever architectural approach you prefer!

[cainria]

I confirm testlist_riscv-csr-access-test-cv32a6_imac_sv32.yaml has never been committed.

Looking for **/*sv32*.yaml in the history, I only get verif/tests/testlist_riscv-mmu-sv32-arch-test-cv32a60x.yaml and verif/tests/testlist_riscv-mmu-sv32-arch-test-cv32a6_imac_sv32.yaml.

[JeanRochCoulon]

Hello @DymShanks
Yes, I recommand to add a new testlist file rather than modifying something that works. RTL modifications are sometimes easier than flow modification !

[DymShanks]

Understood, @JeanRochCoulon! That makes perfect sense regarding the flow modification. I have just pushed a new commit that reverts the bash script back to defaulting to cv32a6_imac_sv32, and I have added the missing testlist_riscv-csr-access-test-cv32a6_imac_sv32.yaml file to the repository. Ready for your review

[chasl59]

Hi @DymShanks,
git displays a comment about your commit e1bd44c. Here is a screenshot:

I am not sure exactly why it is complaining, but it might be good to understand.
Best regards,
Charles

[DymShanks]

Hi @chasl59, thanks for pointing that out! That commit (e1bd44c) is simply the local merge commit I generated to sync my local feature branch with the remote branch after pushing the reverted bash script and the new YAML file. >
GitHub displays that warning because the commit currently only exists in my fork (DymShanks/cva6) and hasn't been integrated into the upstream openhwgroup repository yet. It is totally harmless and will resolve itself automatically the moment this PR gets merged.

[chasl59]

Hi @DymShanks
Thanks. That's good to know. Yesterday, I saw the same notice here:
https://github.com/pulp-platform/apb_uart/tree/6c7dde3d749ac8274377745c105da8c8b8cd27c6

That commit has been part of cva6 for 3 years. Perhaps that lingering notice was caused by something else.
Thanks and regards,
Charles

[cainria]

Hi @DymShanks and @chasl59

This notification is the normal GitHub behavior since e1bd44c is part of this PR branch, which is in the DymShanks repository and not in the openhwgroup's one as the PR/branch has not been merged.

For the commit pointed by Charles, I guess this is the same issue: this commit is not in the pulp-platform repository.
It might be a commit from a pull request, which has been rebased or squashed but not merged as-is (i.e. not with the same commit hash).

[JeanRochCoulon]

After more reflections, considering the fact that the three testlists are the same (except maybe the gcc_opts, but it could be aligned, I hope). I think it makes sense moving testlist_riscv-csr-access-test-cv32a6_imac_sv32.yaml into testlist_riscv-csr-access-test.yaml and modifying the script to always refer this file. Sorry for this late input.

[DymShanks]

Thanks for the feedback, Consolidating the testlists is a great call for long-term maintainability. I will update the PR to merge the tests into the generic testlist_riscv-csr-access-test.yaml and align the gcc_opts. Alongside that, I will remove the specific cv32a6_imac_sv32.yaml file and modify the test script so it universally points to the unified yaml. I’ll push these changes shortly.

[github-actions]

👋 Hi there!

This pull request seems inactive. Need more help or have updates? Feel free to let us know. If there are no updates within the next few days, we'll go ahead and close this PR. 😊