Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update lowrisc_ibex to lowRISC/ibex@123d46b4 #20889

Closed

Conversation

msfschaffner
Copy link
Contributor

@msfschaffner msfschaffner commented Jan 19, 2024

This vendors in the newest ibex version. The important fix we'd like to absorb in Darjeeling is the FI vulnerability in RF patch.

Note that this purposefully does not include newer primitive changes vendored from OT -> Ibex since these could interfere with the current state on Darjeeling (e.g. the S&P layer change in the memory scrambling primitive).


Update code from upstream repository
https://github.com/lowRISC/ibex.git to revision
123d46b4d60068502f80c117772a279db12f5af7

  • [dv] Fix paths in merge_cov.py (Sᴜᴘᴇʀ Lᴇᴇ)
  • Tweak questa timescale argument (Harry Callahan)
  • Fixup the questa build/sim command templates in rtl_simulation.yaml (Harry Callahan)
  • [rtl] Fix FI vulnerability in RF (Pascal Nasahl)
  • [doc] Update cosim version (Pascal Nasahl)
  • [util] Update check_tool_requirements.py (Gary Guo)
  • [rtl] Avoid name collision in ibex_pmp.sv (Rupert Swarbrick)
  • [dv] Fix performance counter printing in simple system (Rupert Swarbrick)
  • Fix spelling of separator (Rupert Swarbrick)
  • [dv] Add an extra key to common_project_cfg.hjson (Rupert Swarbrick)
  • [verilator] Slight refactor in ibex_tracer to avoid BLKSEQ warning (Rupert Swarbrick)
  • [verilator] Waive MULTIDRIVEN warning in ibex_tracer.sv (Rupert Swarbrick)

@msfschaffner msfschaffner requested a review from a team as a code owner January 19, 2024 00:13
@msfschaffner msfschaffner requested review from matutem and removed request for a team January 19, 2024 00:13
@msfschaffner msfschaffner self-assigned this Jan 19, 2024
@msfschaffner msfschaffner added the Priority:P3 Priority: low label Jan 19, 2024
@msfschaffner msfschaffner removed the Priority:P3 Priority: low label Jan 19, 2024
@msfschaffner
Copy link
Contributor Author

CHANGE AUTHORIZED: hw/vendor/lowrisc_ibex/rtl/ibex_pmp.sv
CHANGE AUTHORIZED: hw/vendor/lowrisc_ibex/rtl/ibex_register_file_ff.sv
CHANGE AUTHORIZED: hw/vendor/lowrisc_ibex/rtl/ibex_register_file_fpga.sv
CHANGE AUTHORIZED: hw/vendor/lowrisc_ibex/rtl/ibex_register_file_latch.sv
CHANGE AUTHORIZED: hw/vendor/lowrisc_ibex/rtl/ibex_top.sv
CHANGE AUTHORIZED: hw/vendor/lowrisc_ibex/rtl/ibex_tracer.sv

@andreaskurth
Copy link
Contributor

andreaskurth commented Jan 19, 2024

It may be worth delaying this update until lowRISC/ibex#2129 has been merged as well, so that both FI CM improvements can be included in the same vendor-update. The S&P layer change could be reverted by applying a patch during the vendoring. This would allow to keep Ibex used in Integrated aligned with upstream with the exception of the S&P layer change.

@msfschaffner
Copy link
Contributor Author

Sounds good, though we may have to be mindful of the timeline. Do you expect that change to land soon? Because if not, it may be safer to go ahead and merge this for now, and do another vendor later...

@nasahlpa
Copy link
Member

nasahlpa commented Jan 19, 2024

After a second review round for #2129, I think we should be good to merge the fix into the Ibex repo. But it is fine for me including this fix in another vendor later.

Update code from upstream repository
https://github.com/lowRISC/ibex.git to revision
123d46b4d60068502f80c117772a279db12f5af7

* [dv] Fix paths in `merge_cov.py` (Sᴜᴘᴇʀ Lᴇᴇ)
* Tweak questa timescale argument (Harry Callahan)
* Fixup the questa build/sim command templates in rtl_simulation.yaml
  (Harry Callahan)
* [rtl] Fix FI vulnerability in RF (Pascal Nasahl)
* [doc] Update cosim version (Pascal Nasahl)
* [util] Update check_tool_requirements.py (Gary Guo)
* [rtl] Avoid name collision in ibex_pmp.sv (Rupert Swarbrick)
* [dv] Fix performance counter printing in simple system (Rupert
  Swarbrick)
* Fix spelling of separator (Rupert Swarbrick)
* [dv] Add an extra key to common_project_cfg.hjson (Rupert Swarbrick)
* [verilator] Slight refactor in ibex_tracer to avoid BLKSEQ warning
  (Rupert Swarbrick)
* [verilator] Waive MULTIDRIVEN warning in ibex_tracer.sv (Rupert
  Swarbrick)

Signed-off-by: Michael Schaffner <msf@opentitan.org>
Signed-off-by: Michael Schaffner <msf@opentitan.org>
The assertion is for the same condition as in the
pinmux_strap_sampling module. However, as opposed to that module,
there are some modifications to the strap_en_i signal resulting in
a local version of it (strap_en) and the SVA needs to be
formulated for that one instead.

Signed-off-by: Michael Schaffner <msf@opentitan.org>
@nasahlpa
Copy link
Member

@msfschaffner PR#2129 is now merged into the Ibex repository.

@andreaskurth
Copy link
Contributor

The decision has been taken to delay this PR until after the Darjeeling-ES.M4 deadline

@msfschaffner
Copy link
Contributor Author

We do not plan to carry this improvement over to integrated_dev for now.

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.

3 participants