Skip to content

[dv] Mask read-only fields in CSR test generator (Fixes #1337)#2442

Open
RKNAGA18 wants to merge 1 commit into
lowRISC:masterfrom
RKNAGA18:fix-csr-readonly-mask
Open

[dv] Mask read-only fields in CSR test generator (Fixes #1337)#2442
RKNAGA18 wants to merge 1 commit into
lowRISC:masterfrom
RKNAGA18:fix-csr-readonly-mask

Conversation

@RKNAGA18
Copy link
Copy Markdown

This PR pivots to fixing the root cause of the timeouts (Bullet Point 1 of #1337): the Python script generates csrrw/csrrs/csrrc instructions that attempt to overwrite RO (Read-Only) fields, which triggers an Illegal Instruction exception in the hardware and hangs the simulation.

Changes

Modified the data generation loop in gen_csr_test.py.

Before writing the raw rand_rs1_val to the assembly payload, it is now passed through the internal csr_write() function to explicitly mask out read-only bits based on the YAML definitions.

Generated a .patch file in vendor/patches/google_riscv-dv/ to persist this fix across upstream vendor updates.

Impact
The generated assembly now strictly respects read-only boundaries. It prevents the core from triggering hardware exceptions during standard register checks, eliminating the timeouts without breaking reset value predictions.

@elliotb-lowrisc
Copy link
Copy Markdown
Contributor

Hi @RKNAGA18, thanks again for your interest in this. Did you see any improvement when running tests from this change? Only, I'm a little worried that the current spec ("The RISC-V Instruction Set Manual, Volume II - Privileged Architecture - Version 20260120") seems to claim that writing to read-only fields would not generate an illegal instruction:

Instructions that access a non-existent CSR are reserved. Attempts to access a CSR without appropriate privilege level raise illegal-instruction exceptions or, as described in Section 22.6.1, virtual-instruction exceptions. Attempts to write a read-only register raise illegal-instruction exceptions. A read/write register might also contain some bits that are read-only, in which case writes to the read-only bits are ignored.

@RKNAGA18
Copy link
Copy Markdown
Author

Hi @elliotb-lowrisc, I actually misstated the failure mechanism in my PR description, Thank you for pulling the exact ISA spec.
You are absolutely right that Ibex correctly handles writes to RO fields by silently ignoring them, so no Illegal Instruction exception is actually being triggered by the core in this scenario.
The timeouts were actually being caused by a prediction mismatch leading to an infinite failure loop. Because the Python script was injecting unmasked random data, there was a divergence between what the Python script predicted the register would hold and what the compliant hardware actually held (since the hardware rightly ignored the RO bits).
When the generated assembly executed the bne instruction to check the read-back value, it detected this mismatch and branched to the csr_fail label. Since this is a bare-metal test, hitting csr_fail puts the core into a terminal loop, which manifested as the simulation timeout I was seeing.
By applying the csr_write mask before the payload is formatted into the assembly, this patch ensures the Python script's generated test data perfectly aligns with the hardware's correct, spec-compliant behavior. The bne checks now pass, and the regression completes successfully.
I'd be happy to update the PR description to accurately reflect the bne mismatch rather than a hardware exception

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants