Formal cleanup #2245
Draft
Formal cleanup #2245
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Here's a high-level overview of what this commit does: - Compiles Sail into SystemVerilog including patchin compiler bugs - Create a TCL file that tells JasperGold what to prove and assume - Check memory operations modelling the LSU Most of these properties now prove without time-bound on the response from memory due to alternative LSUs - Check memory even with Smepmp errors: Continues on top of riscv/sail-riscv#196 - CSR verification - Checks for instruction types such as B-Type, I-Type, R-Type - Check illegal instructions and WFI instructions - Using psgen language for proof generation - Documentation on how to use the setup - Wrap around proof that proves instructions executed in a row still match the specification. - Liveness proof to guarantee instructions will retire within a upper bound of cycles. All of these proofs make heavy use of the concept of k-induction. All the different properties and steps are necessary to help the tool get the useful properties it needs to prove the next step. The instruction correctness, wrap-around and liveness all give us increased confidence that Ibex is trace-equivalent to Sail. Throughout this process an issue was found in Ibex where the pipeline was not flushing properly on changing PMP registers using clear: lowRISC#2193 Alternative LSUs: This makes all top level memory properties prove quickly and at a low proof effort (1 or 2-induction). Three 'alternative LSUs' representing three stages of memory instructions: 1. Before the first response is received, in the EX stage 2. After the first response is received, but not the second grant, also in the EX stage 3. Before the last response is received in the WB stage. In each case we ask 'if the response came now, would the result be correct?'. Similar is applied for CSRs/PC though less directly. This is particularly interesting (read: ugly) in the case of a PMP error wbexc_exists makes Wrap properties fast to prove. The bottleneck becomes SpecPastNoWbexcPC, which fails only due to a bug. See the comment in riscv.proof. Co-authored-by: Marno van der Maas <[email protected]>
This lets fusesoc do the heavy lifting in identify the correct files for us. Fusesoc is already extensively used for this purpose for synthesis and simulation. This also allows us to automatically patch the fusesoc-provided src files to work around some current restrictions in the formal flow, without modifying the main repository tree. Signed-off-by: Harry Callahan <[email protected]>
This includes Flake setup prove_no_liveness isn't great by any means, but it will prove everything orders of magnitude faster than prove -bg -all. The dev shell (nix develop .#formal-dev) is identical to the normal shell, but prints some information on how to swap out components. This is also documented in the README. Jasper Gold options: - allow_unsupported_OS is required on both the machines I use. - acquire_proj means that if JG is killed (which happens somewhat often) the next it runs it will still be able to take ownership of the project. Co-authored-by: Louis-Emile Ploix <[email protected]> Co-authored-by: Marno van der Maas <[email protected]> Signed-off-by: Harry Callahan <[email protected]>
|
Results from running |
Mostly line lengths and indexes. This commits also adds a Verible waiver file.
marnovandermaas
force-pushed
the
formal_cleanup
branch
from
98d948d to
d1ddec1
Compare
rswarbrick
reviewed
Jan 13, 2025
| @@ -5,6 +5,15 @@ Prerequisities (in your PATH): | |||
| - [psgen](https://github.com/mndstrmr/psgen) | |||
|
|
|||
| Build instructions: | |||
| - Clone this repository | |||
| - `cd dv/formal` | |||
| - `nix develop .#formal` | |||
There was a problem hiding this comment.
Hmm. I don't think I know where this flake gets defined. (Maybe just pull this line to the following commit?)
| # Licensed under the Apache License, Version 2.0, see LICENSE for details. | ||
| # SPDX-License-Identifier: Apache-2.0 | ||
| name: "lowrisc:ibex:ibex_formal:0.1" | ||
| description: "Ibex, a small RV32 CPU core" |
There was a problem hiding this comment.
Nit: I'd consider dropping the "core" suffix. It contains ibex_core and there are just too many cores involved! 🍎
Comment on lines
+7
to
+8
| - if (csr_op_en_o == 1'b1 && (csr_op_o == CSR_OP_WRITE || csr_op_o == CSR_OP_SET)) begin | ||
| + if (csr_op_en_o == 1'b1 && (csr_op_o == CSR_OP_WRITE || csr_op_o == CSR_OP_SET || csr_op_o == CSR_OP_CLEAR)) begin |
There was a problem hiding this comment.
I think this needs a documentation comment to explain what it does (and why the patch makes sense to include). Maybe a README in the patches directory?
Similarly for the patch to ibex_top.sv
| - Add to path with `export PATH=<sail-dir>/bin:$PATH` | ||
| - Update `LOWRISC_SAIL_SRC` to point to the source root with `export LOWRISC_SAIL_SRC=<sail-dir>`. | ||
| - sail-riscv: (`https://github.com/lowrisc/sail-riscv`) Can be swapped out by changing `LOWRISC_SAIL_RISCV_SRC`. | ||
| - The current version is at `05bcf4cb5301c34a612abe893df25c0242a716b2` |
There was a problem hiding this comment.
I'd possibly rephrase this to something like "This has been tested with version X"? ("current" leaves a bit of a hostage to fortune!)
dv/formal/rtl-changes.patch
Outdated
| @@ -1,46 +0,0 @@ | |||
| diff --git a/rtl/ibex_id_stage.sv b/rtl/ibex_id_stage.sv | |||
| index 3a358af7..3c19d763 100644 | |||
| --- a/rtl/ibex_id_stage.sv | |||
There was a problem hiding this comment.
Hmm. Did we need this patch in the previous commit? If not, can we forget about it entirely?
|
|
||
| if {$argc > 0} {puts "\$argv: $argv"} | ||
| if {$argc >= 6} { | ||
| set argv5 [lindex $argv 5] |
There was a problem hiding this comment.
Oof, this is a little sad. Would it make sense to define some names for the arguments to make the code a bit easier to follow?
| - `jg verify.tcl` invokes JasperGold interactively, sourcing the configuration & run script. | ||
| Requires the above two steps to be executed first. | ||
|
|
||
| After this command, Jasper Gold should start up and should execute the commands in the TCL file. |
There was a problem hiding this comment.
Nit: Stray space before Gold. Also, I think Cadence have stopped using that name: isn't it just "Jasper" now?
| @@ -124,7 +124,10 @@ ConstantBoot: assume property (boot_addr_i == $past(boot_addr_i)); | |||
| // 3. Always fetch enable | |||
| FetchEnable: assume property (fetch_enable_i == IbexMuBiOn); | |||
| // 4. Never try to sleep if we couldn't ever wake up | |||
| WFIStart: assume property (`IDC.ctrl_fsm_cs == SLEEP |-> `CSR.mie_q.irq_software | `CSR.mie_q.irq_timer | `CSR.mie_q.irq_external); | |||
| WFIStart: assume property (`IDC.ctrl_fsm_cs == SLEEP |-> | |||
| `CSR.mie_q.irq_software | | |||
There was a problem hiding this comment.
This is nice. I'd probably be inclined to move it slightly further left and also surround by parens to make it easy for humans to parse.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds an initial formal flow to the Ibex repository. It proves equivalence of the Ibex RTL and the Sail specification. For details and limitations please see the documentation.
A big thank you to @mndstrmr and @hcallahan-lowrisc for putting this work together.