lowRISC / lowRISC/opentitan

UART hjson should explicitly document CTRL_REGWEN software check requirement

Open
#30,535 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
SystemVerilog
Stars
3.6k
Forks
1.1k
Avg merge
2d 22h
Merged PRs (30d)
141

Description

Description

Summary

The CTRL_REGWEN register in hw/ip/uart/data/uart.hjson describes the locking mechanism but does not explicitly state that software MUST check this register before writing to CTRL. This implicit constraint has led to both the OpenTitan DIF and the Zephyr driver independently omitting the check, resulting in potential silent failures when CTRL is locked.

Component

  • File: hw/ip/uart/data/uart.hjson
  • Register: CTRL_REGWEN
  • Affected registers: CTRL (controlled by CTRL_REGWEN)

Current hjson Description (Implicit Constraint)

{ name: "CTRL_REGWEN",
  desc: "Controls write enable for CTRL register. If 0, CTRL is locked",
  swaccess: "rw0c",
  hwaccess: "none",
  fields: [
    { bits: "0",
      name: "regwen",
      desc: "Once cleared, the CTRL register will be locked until the next reset.",
      resval: 1 }
  ]
}

Problem: The description explains what the register does ("If 0, CTRL is locked") but does not state what software must do ("Software MUST check this register before writing CTRL; if 0, the write will be silently ignored").

Cross-Ecosystem Impact

This issue was discovered through a cross-ecosystem audit comparing the OpenTitan DIF with the Zephyr uart_opentitan.c driver. Both implementations independently omitted the CTRL_REGWEN check, which strongly suggests the constraint is not explicit enough in the specification.

Implementation File Status
OpenTitan DIF sw/device/lib/dif/dif_uart.c ❌ Missing CTRL_REGWEN check before writing CTRL
Zephyr driver drivers/serial/uart_opentitan.c ❌ Missing CTRL_REGWEN check before writing CTRL

Why This Matters

Silent Failure Scenario
1. Boot ROM configures UART and locks CTRL (CTRL_REGWEN = 0)
2. Zephyr/OpenTitan DIF calls uart_init() / dif_uart_configure()
3. Code writes to CTRL register
4. Hardware silently ignores the write (CTRL is locked)
5. Driver returns success (kDifOk / 0)
6. UART remains in ROM's configuration, not the driver's intended configuration
7. **Silent failure**: Software thinks UART is configured correctly, but it is not
Security Implications

If the boot ROM sets a specific baud rate for security reasons (e.g., to match a secure peripheral) and locks CTRL, a subsequent driver that attempts to reconfigure the UART will:

  • Believe it succeeded
  • Actually leave the UART in the ROM's configuration
  • Potentially cause protocol mismatches or security boundary violations

Proposed Fix

Option A: Update hjson Description (Recommended)

Update the CTRL_REGWEN register description to explicitly state the software requirement:

{ name: "CTRL_REGWEN",
  desc: '''
    Controls write enable for the CTRL register.
    When this bit is 1, the CTRL register can be modified by software.
    When this bit is 0, the CTRL register is locked and writes are silently ignored.

    **Software Requirement**: Before writing to the CTRL register, software MUST
    read CTRL_REGWEN and verify it is 1. If CTRL_REGWEN is 0, software MUST NOT
    attempt to write CTRL and SHOULD return an error to the caller.
  ''',
  swaccess: "rw0c",
  hwaccess: "none",
  fields: [
    { bits: "0",
      name: "regwen",
      desc: "Once cleared, the CTRL register will be locked until the next reset.",
      resval: 1 }
  ]
}
Option B: Add sw_check_required Annotation (Future Enhancement)

Consider adding a machine-readable annotation to hjson that can be consumed by code generation and static analysis tools:

{ name: "CTRL_REGWEN",
  desc: "...",
  swaccess: "rw0c",
  hwaccess: "none",
  // New annotation for tooling
  sw_check_required: {
    target_register: "CTRL",
    check: "read_before_write",
    failure_action: "return_error"
  },
  ...
}

This would enable:

  • Auto-generation of lock-check code in DIFs
  • Static analysis tools to verify driver compliance
  • Cross-ecosystem audits to detect missing checks
Option C: Fix DIF (Immediate Action)

Regardless of hjson changes, the OpenTitan DIF should be updated to include the CTRL_REGWEN check:

// In dif_uart_configure() or dif_uart_init()
dif_result_t dif_uart_configure(const dif_uart_t *uart, ...) {
  // Check if CTRL is locked
  uint32_t regwen = mmio_region_read32(uart->base_addr, UART_CTRL_REGWEN_REG_OFFSET);
  if (!bitfield_bit32_read(regwen, UART_CTRL_REGWEN_REGWEN_BIT)) {
    return kDifLocked;  // or kDifError
  }
  // ... continue with configuration
}

Additional Context

This pattern may exist in other OpenTitan IP blocks. A systematic audit of all *_REGWEN registers across the hjson files would be valuable to ensure consistent documentation of software requirements.

References

  • Current hjson: hw/ip/uart/data/uart.hjson (CTRL_REGWEN register)
  • OpenTitan DIF: sw/device/lib/dif/dif_uart.c (missing check)
  • Zephyr driver: drivers/serial/uart_opentitan.c (missing check)
  • Cross-ecosystem audit methodology: "Dual-Source Ground Truth" framework

Request for Community Input

  1. Is the proposed hjson description change acceptable?
  2. Should we also update the DIF to include the check immediately?
  3. Should we audit other IP blocks for similar *_REGWEN documentation gaps?

We are happy to submit a PR for the hjson change and/or DIF fix once the community confirms the direction.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Open hw/ip/uart/data/uart.hjson and inspect CTRL_REGWEN alongside the CTRL register. Confirm whether maintainers want the documentation change alone or also the proposed DIF and annotation work, since the issue presents several options. Done means the agreed scope is explicit and the hjson remains valid.

Written by the indexing model from the issue text.

Assessment

Domain
documentation, embedded-iot
Issue type
Documentation
Difficulty
1/5
Estimated time
1-3 hours
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.