Enhanced oeb checks for caravel/caravan/openframe

[d-m-bailey]

Updated to version 1.1.1

Adds a gpio check to the caravel/caravan oeb check.
Adds oeb/gpio check to openframe.
For caravel & caravan, the gpio settings are taken from the verilog/rtl/user_defines.v file.
For openframe, the gpio configuration is determined by the actual connections to the gpio configuration signals.
Bumped cvc version to 1.1.7.
Soft check is not run by default. This allows extraction with the actual PDK version of the tech file as opposed to the modified version necessary for soft check.
Only one extraction is required for both oeb check and LVS.
Separate tmp directories for oeb and LVS runs. If there are no errors in a given check, the respective tmp directory is deleted.
Add the subcut generation magic technology to a temporary tech file for each extraction.
Add display of EXTRACT_CREATE_SUBCUT with other LVS variables.
Updated README explanation of OEB checks.
Cleaned up error reporting for run_be_checks to ignore the statuses of checks that were not run.

Many changes to the CI.

• Changed pdk manager from volare to ciel.
• CI temporarily patches the caravan/caravel verilog/rtl/user_defines.v files to pass the OEB checks. The template repos retain INVALID settings for the user configurable gpios.
• openframe test data is pulled from a development branch using the recently add CF_gpio_config macros.
• the current repo is checked out and the PYTHONPATH is set to reflect the most recent files in the PR (as opposed to those in the published package).

NOTE: docker option --rm has been added. This removes the container (not image) after completion. Do we need the container for debugging of ci runs?

@marwaneltoukhy @jeffdi

[d-m-bailey]

CI was failing on docker build. Trying to add missing libraries now so temporarily changed to draft PR.

[d-m-bailey]

The problem was that the ci docker build was using the mpw_precheck/dependencies/Dockerfile from pip install and not the updated version.

Everything is passing the ci now.

[jeffdi]

Thanks for the big cleanup, Mitch — directionally this is a strong PR and the richer OEB semantics are a clear win over the old heuristics. Below is a full review. Happy to iterate on any of it.

Summary

Three themes, somewhat tangled:

1. OEB check enhancement — per-GPIO configuration validation driven by verilog/rtl/user_defines.v (caravel/caravan) or by actual tie-offs to the GPIO config signals (openframe), with per-mode ERROR/Warning tables.
2. Extraction / tech file changes — single extraction shared by OEB + LVS, separate tmp_oeb/tmp dirs, a subcutout style patched into a temp tech file, cvc-rv bump to 1.1.7, softcheck off by default, and a cleaner exit-status matrix in run_be_checks.
3. CI overhaul — volare → ciel, checkout of the PR itself with PYTHONPATH=/work/cf-precheck/src, in-place patches to user_defines.v for caravel/caravel_analog, openframe test data pulled from a fork branch, --rm on docker run, 45-min docker build timeout, missing Rocky dev libs added to Dockerfile.

---

Likely bugs

1. Stale fixed references in run_openframe_check default case

The variable fixed was globally renamed to fixed_out (good — it disambiguates from the new fixed_oeb). But the default: branch of the switch(gpio_config) in run_openframe_check still has two stale references:

} else if ( out_connections[i] > 0 && ! (i in fixed) ) {        # should be fixed_out
    if ( oeb_min[i] !~ /vss/ ) {
        messages[...] = sprintf("GPIO %2d: ERROR: oeb must have possible low for output", i);
    } else if ( in_connections[i] > 0 && oeb_sim[i] ~ /vss/ ) {
        ...
    }
}
...
if ( in_connections[i] > 0 || (out_connections[i] > 0 && ! (i in fixed)) ) { # should be fixed_out

Because fixed is now never populated, (i in fixed) is always false, so openframe GPIOs with a tied-off output can spuriously trip "oeb must have possible low for output" or "both analog and digital connections" whenever the dm[2:0] configuration is unrecognized. Narrow regression (default branch only) but worth fixing before merge.

2. subcutout insertion point in run_extract is fragile

awk '
/^cifoutput/ {searching=1}
searching && /^end$/ {
    print BLOCK
    searching=0
}
{print}
' BLOCK="$(cat .../subcutout.txt)" .../$PDK.tech >$WORK_ROOT/$PDK.tech

This inserts BLOCK before the first ^end$ after cifoutput. Magic tech files in sky130 typically have several style … end blocks inside cifoutput, so the first end closes the first internal style — not cifoutput itself. subcutout.txt also begins with style subcutout but has no trailing end, so the net effect is:

 style <first existing style>
  ...rules...
 style subcutout     <-- implicitly closes the first style
  ...rules...
  calma 81 53
 end                 <-- closes subcutout, not the first style
 style <next>

If sky130A.tech happens to have cifoutput … end with no intermediate end on your setup, this works by coincidence. A more robust implementation would either:

• track depth and insert before the last end of the cifoutput block,
• give subcutout.txt its own end and insert it right after a well-known existing style, or
• append the block outside cifoutput if magic tolerates that.

3. Missing end in subcutout.txt

Related to #2: the two new tech/sky130{A,B}/subcutout.txt files do not terminate the style with end. They rely on the awk trick above hijacking the next end it finds. If you fix #2, add the end here.

4. Typo in OEB report message

case //: {
    printf "missing mode ";
    messages[...] = sprintf("GPIO %2d: ERROR: missing gpio configuraton", gpio);

configuraton → configuration. This shows up in the user-visible cvc.oeb.report.

5. MGMT_STD_* modes never emit ERRORs — intentional?

MGMT_STD_INPUT_*, MGMT_STD_BIDIRECTIONAL, and MGMT_STD_ANALOG emit only Warnings, while the symmetric MGMT_STD_OUTPUT branch does raise an ERROR for in_connections[i] > 0 ("user input connection to management output gpio - undriven"). Is the asymmetry intentional? If a user wires io_in[i] to logic on a MGMT_STD_BIDIRECTIONAL pin where mgmt is always driving, the user's input is likewise undriven in user mode — same root cause. Worth confirming against the error table now linked from the README.

---

CI concerns

6. PR depends on an unmerged branch in a personal fork

- name: Checkout ${{ matrix.repo }} using development branch to pass OEB check
  if: matrix.repo == 'openframe_user_project'
  uses: actions/checkout@v4
  with:
    repository: d-m-bailey/openframe_user_project-cf
    path: ${{ matrix.repo }}
    ref: CF_gpio_config

• Job is green only while d-m-bailey/openframe_user_project-cf@CF_gpio_config exists and isn't rebased. If that branch moves or gets deleted, CI breaks silently.
• This PR effectively can't be merged until the CF_gpio_config changes are in chipfoundry/openframe_user_project. Please block merge on that prerequisite, or call it out as a hard blocker in the PR description.

7. In-CI rewriting of user_defines.v masks the check being tested

gawk -i inplace '\
  /USER_CONFIG_GPIO_[567]_/ {sub(/INVALID/, "USER_STD_OUTPUT"); print; next} \
  /USER_CONFIG_GPIO_3._/ {sub(/INVALID/, "USER_STD_OUTPUT"); print; next} \
  {sub(/INVALID/, "MGMT_STD_ANALOG"); print}' caravel_user_project/verilog/rtl/user_defines.v

• /USER_CONFIG_GPIO_3._/ uses . as a regex wildcard (matches _). Use /USER_CONFIG_GPIO_3[0-9]_/ for safety.
• The patches make CI a test of the extraction pipeline, not of the OEB check itself — every golden path has INVALID rewritten away. Consider adding at least one matrix entry that retains INVALID and asserts the OEB check fails with the expected error messages. Otherwise a regression that causes the check to silently pass everything won't be caught here.
• Please upstream the golden user_defines.v to the template repos (as the PR body mentions for openframe) so CI doesn't need in-place rewrites.

8. docker run --rm

You asked: do we need the container for CI debug? With GitHub Actions, almost never — the container is discarded when the runner is recycled anyway, and logs/artifacts are what you actually need. Keep --rm. You already bind-mount $OUTPUT_DIR and upload precheck_results/<tag> via actions/upload-artifact, which covers debug.

9. PYTHONPATH=/work/cf-precheck/src overrides the installed package

Fine approach for "run the PR's code without rebuilding the image", but:

• The python3 -c "import cf_precheck; print(cf_precheck.__file__)" debug line is still in the workflow — was that meant to be removed with the other debug statements?
• The shell form uses bash -lc "… \"$VAR\" …" with heavy escaping. A heredoc or a small shim script would be easier to maintain and less prone to quoting bugs.

10. PDK cache key

key: ${{ runner.os }}-pdk-${{ hashFiles('**/ciel.toml') }}

If no ciel.toml exists in the repo (install is literally ciel enable 3e0e31dc… --pdk sky130A), hashFiles returns an empty string and every run shares the same key Linux-pdk-. Works today, but makes PDK upgrades non-atomic. Suggest pinning the PDK hash directly:

key: ${{ runner.os }}-pdk-3e0e31dcce8519a7dbb82590346db16d91b7244f-sky130A

11. Hardcoded --pdk sky130A

ciel enable … --pdk sky130A. The old volare enable didn't restrict PDKs. Given Oeb.__supported_pdks__ = ["gf180mcuC", "gf180mcuD", "sky130A", "sky130B"], any future matrix entry that exercises sky130B or gf180 will need this parameterized.

---

Smaller nits

• run_extract: typo "Adding subcutout sytle to tech file." → "style".
• run_oeb_check: trailing whitespace on exec $LVS_ROOT/run_openframe_check $extract_flag .
• run_oeb_check: several new warning strings read "user oeb to connection management …" — should be "connection to management …" (MGMT_STD_INPUT, MGMT_STD_OUTPUT, MGMT_STD_BIDIRECTIONAL, MGMT_STD_ANALOG).
• run_oeb_check: PADRING_TYPE is set only inside two of the three $DESIGN_NAME == *user_…* branches and defaulted with ${PADRING_TYPE:-} later. Consider defaulting it explicitly near the top for readability.
• run_openframe_check: wc -l < $SIGNOFF_ROOT/cvc.oeb.report < 10 is a clever abnormal-exit heuristic; please make the threshold a named variable with a comment ("header printed but no per-GPIO rows"). Future edits to the header row could silently change the gate.
• run_be_checks: the new final branch prints SOFT ${scheck_status:--} LVS $lvs_status CVC ${cvc_status:--} OEB ${oeb_status:--} — great improvement. Consider also distinguishing "skipped" from "not run due to earlier failure" (both currently render as -).
• magicrc: you now exit if PDK is unknown or if EXTRACT_STYLE is neither "" nor "nowell". Good. Worth also logging which WORK_ROOT/LVS_ROOT is in use alongside the tech-file print.
• Oeb.__supported_type__: adding "openframe" is correct, but please sanity-check that the type-selection logic elsewhere in cf_precheck routes openframe through run_oeb_check → run_openframe_check (the exec dispatch inside run_oeb_check does this, but a direct call site would bypass).
• README.md: the link to a Google Doc is handy short-term, but having the error table as versioned markdown in the repo would keep it in lock-step with the code that implements it.
• Commit history: 51 commits, many pure CI flailing ("Guess you need to export.", "Print the changes to the config file. duh.", "Forgot backslash at end of line.", "more debugging", "Add more debugging displays.", etc.). A squash-on-merge — or interactive cleanup into ~5 logical commits (OEB semantics / extraction+tech / run_be_checks cleanup / CI / version bump) — would make git log and git blame much more useful downstream.

---

Questions

1. Is making CVC-RV errors (including fatal power/model definition failures) "warnings only" intentional and final? Commit 3c66c96 mentions it, but downstream tools reading exit codes may rely on the old behavior.
2. Why default RUN_SOFTCHECK=0? The commit message says it's so extraction can use the true PDK tech file, but this silently downgrades a previously-run check. Could CI include a matrix entry with RUN_SOFTCHECK=1 so we don't lose coverage?
3. The openframe dm config is recovered from actual connectivity. What happens if a design drives dm[2:0] through logic (and/or tree, mux, etc.) rather than a direct tie? GetBit only recognizes vccd*/vssd*/gpio_loopback_*; worth calling out as a known limitation in the README.
4. On EXTRACT_CREATE_SUBCUT: now that config.py prints it alongside the other LVS vars, should it also be documented in the project LVS config docs?

---

Verdict

Good direction. Must-fix before merge from my side:

• the fixed → fixed_out default-branch regression in run_openframe_check (#1),
• confirm the subcutout tech-file insertion is correct against the actual sky130 magic tech structure (#2/#3),
• resolve the CI dependency on d-m-bailey/openframe_user_project-cf@CF_gpio_config (#6).

The rest are polish and can land in follow-ups. Nice work on the new configuration-driven OEB semantics.

[d-m-bailey]

@jeffdi @marwaneltoukhy I've incorporated the AI suggestions that I felt were appropriate.

This is ready to merge and announce.