Update OpenTitan to Earlgrey-M2.5.2-RC0
#3586
Conversation
There was a problem hiding this comment.
Essentially none of the getting started / set up instructions previously contained in the opentitan/earlgrey-cw310 crate's README work on the tape-out revision of the upstream OpenTitan repository (or on any recent development revision).
This PR is a net-positive, as it gets us closer to having upstream Tock work on the "frozen" OpenTitan chip. From talking to @cfrantz, it is the first in a series of PRs to align with this fixed hardware revision, and it makes sense to me to finalize the usage instructions once Tock is confirmed to work on that fixed target.
We should eventually resurrect & update some of the instructions around app loading & OTBN support, once those are confirmed to be working as well.
There was a problem hiding this comment.
On a quick glance, it doesn't look like the linked OT guide has any directions for running Tock (yet?), but in the long term, if it helps avoid duplication and makes sure things don't fall out of date here, it's perfectly fine for this readme to just link to specific sections of OT resources as-appropriate.
There was a problem hiding this comment.
It looks like you are just deleting all of the documentation to help users get started. We should leave these in as they explain to people how to run Tock on OpenTitan. If any changes are required let's just make the changes directly instead of deleting everything
boards/opentitan/README.md
Outdated
| ### Setup OpenTitan | ||
|
|
||
| ```shell | ||
| git clone https://github.com/lowRISC/opentitan.git | ||
| cd opentitan | ||
|
|
||
| # Use the OpenTitan_SHA currently supported by Tock | ||
| git checkout <OpenTitan_SHA> | ||
| pip3 install --user -r python-requirements.txt | ||
| ``` | ||
| Make sure to follow [OpenTitan getting started instructions](https://docs.opentitan.org/doc/getting_started/) to setup required dependencies/toolchains. | ||
|
|
||
| ### **Fedora dependencies quick install** | ||
|
|
||
| Note: the OpenTitan documentation provides an easy installation for packages for Ubuntu based distributions. This is an equivalent command to install the (mostly) same packages for Fedora users. | ||
|
|
||
| ```shell | ||
| sudo dnf install autoconf bison make automake gcc gcc-c++ kernel-devel \ | ||
| clang-tools-extra clang cmake curl \ | ||
| doxygen flex g++ git golang lcov elfutils-libelf \ | ||
| libftdi libftdi-devel ncurses-compat-libs openssl-devel \ | ||
| systemd-devel libusb redhat-lsb-core \ | ||
| make ninja-build perl pkgconf python3 python3-pip python3-setuptools \ | ||
| python3-urllib3 python3-wheel srecord tree xsltproc zlib-devel xz clang-tools-extra \ | ||
| clang11-libs clang-devel elfutils-libelf-devel | ||
| ``` |
There was a problem hiding this comment.
We should leave this in, it's currently tested and works
There was a problem hiding this comment.
This is covered by OpenTitan's getting started guide.
With the recent adoption of PEP-668 by Debian distributions, the pip3 install line is out-of-date (to be fair, it's also out-of-date in the OT docs, but we should really be referring to a single source-of-truth concerning system setup).
I'd prefer to see the Fedora blurb get added to OT's documentation.
There was a problem hiding this comment.
Fair, we can drop the Setup OpenTitan section and point to the OT docs.
I agree the Fedora steps being in OT official docs would be nice. Let's leave them here until that happens
There was a problem hiding this comment.
boards/opentitan/README.md
Outdated
| ChipWhisper CW310 | ||
| ----------------- | ||
|
|
||
| To use `make flash` you first need to clone the OpenTitan repo and ensure that | ||
| the Python dependencies are installed. | ||
|
|
||
| Then you need to build the OpenTitan tools: | ||
|
|
||
| ```shell | ||
| ./bazelisk.sh build //sw/host/opentitantool | ||
| ``` | ||
|
|
||
| You might need to run these commands to get it to work | ||
|
|
||
| ```shell | ||
| ln -s /usr/bin/ld.lld /usr/sbin/ld.lld | ||
| ln -s /usr/bin/gcc /usr/sbin/gcc | ||
| ``` | ||
|
|
||
| Next connect to the board's serial with a second terminal: | ||
|
|
||
| ```shell | ||
| screen /dev/ttyACM1 115200,cs8,-ixon,-ixoff | ||
| ``` | ||
|
|
||
| Then you need to flash the bitstream with: | ||
|
|
||
|
|
||
| ```shell | ||
| ./bazel-bin/sw/host/opentitantool/opentitantool.runfiles/lowrisc_opentitan/sw/host/opentitantool/opentitantool --interface=cw310 fpga load-bitstream lowrisc_systems_chip_earlgrey_cw310_0.1.bit |
There was a problem hiding this comment.
Again, we should leave these steps in, why would we want to remove them?
There was a problem hiding this comment.
Since the documentation changes seem too controversial, I've reverted them. We can create a new PR or issue to discuss changing the docs.
Considering the only change to get Tock running on the latest hardware is a linker script change we should just make the linker script change and SHA change and start from there. Also, can you run the unit tests. Tock has an extensive range of tests for OpenTitan, we don't want to regress those. |
74e3967
f7ca545 to
74e3967
Compare
|
|
It seems to be outside a PMP region, we probably need to change this region to start a little earlier. The best bet is to have a look at the elf symbols and see what we should use instead |
The store fault location is beyond the end of the stack (ie: stack overflow). I added a bit more stack, after which all tests passed except the AES test (different failure) and the flash_ctrl test. |
Earlgrey-M2.5.2-RC0 is the version of OpenTitan that will be in the first Earlgrey chip. Signed-off-by: Chris Frantz <cfrantz@google.com>
75684eb to
5e273bb
Compare
Your Makefile changes look good to me and I've pulled them into this PR. The changes to the RAM layout to preserve the I'd suggest we defer dealing with the |
Ah did not know that, sounds good then. |
Running the newer changes on Verilator (with the stack size increase), I'm still seeing the overflow. Were your tests on HW? |
Yeah, in main.rs just experimenting with this change The storefault occurs just outside of the newly allocated stack region at |
I'm sorry: I forgot to push the stack size change. Could you try giving one more kilobyte of stack space in main.rs (change from 0x1000 bytes to 0x1400 bytes) and see if that helps? |
Ah right! cool! that seems to have gotten around the overflow. |
|
#3590 is a fix for the flash failures (unrelated to the OT bump). These tests previously used to run in the order listed (rust update?). Which is required, since we lock some regions for testing access permissions. |
- Update the manifest layout in the linker scripts to match the expectations of the OpenTitan `test_rom`. - Fix missing _manifest symbol board initialization code. - Add the manifest extension table. - Add the `manifest_version` field. - Update the RAM size: OpenTitan has 128K of SRAM. - Eliminate the referense to the non-existent `.riscv.open_titan_trap_vectored` section. Co-authored-by: Michał Mazurek <maz@semihalf.com> Signed-off-by: Chris Frantz <cfrantz@google.com> Signed-off-by: Michał Mazurek <maz@semihalf.com>
Adds required changes to build against opentitan Earlgrey-M2.5.2-RC0. Signed-off-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
The tests overflow the stack; give some more space so they can run. Signed-off-by: Chris Frantz <cfrantz@google.com>
These tests need to run in a particular order, because we are locking certain flash regions (sticky) for negative testing. The test-runner seems to invoke them alphabetically by fn name. Signed-off-by: Wilfred Mallawa <wilfred.mallawa@wdc.com>
The AES tests are order-sensitive. The CCM and GCM tests depend on the VirtualAES128 peripheral configured during kernel initialization in `main.rs`. In order to operate correctly, the virtual peripheral depends on the `set_client` calls to the low-level AES driver to configure the virtual peripheral correctly. The ECB, CBC and CTR tess, on the other hand, call `set_client` on the AES driver, thus breaking the VirtualAES128 configuration. Run the CCM and GCM tests first, then run the ECB, CBC and CTR tests. Signed-off-by: Chris Frantz <cfrantz@google.com>
ec92e08 to
5a1668d
Compare
I've pulled the commit from #3590 into this PR. It appears the AES test failures are also related to test ordering and I've added a commit that sets their ordering so the work. See my commit message to understand the ordering. I can successfully run all of the tests on a CW310 FPGA board with the Earlgrey-M2.5.2-RC0 bitstream. |
Nice! I haven't tested loading an OTBN binary (needs Vivado 2020.2) installed to generate a loadable elf. Did your tests include otbn/rsa testing aswell? If the binary was not loaded, the test suite will skip these tests with "Unable to find otbn-rsa, disabling RSA support". |
The instructions in the the README for creating the
I'm no expert in the design and execution of this particular test. I don't even know if the interface between the I would prefer to mark this test as broken for now and get Tock updated to the Earlgrey_ES hardware revision and fix this test later (or more precisely: have someone familiar with the proper operation of this test investigate the failure and fix it). |
IMHO, I think this PR is ready to go.
Getting this PR submitted will clear the way for #3587 and #3597 as well as some additional auto-generated hardware constants work I'd like to submit. @alistair23 @twilfredo WDYT? |
|
I really don't like merging something that breaks the unit tests. They are really useful for testing changes and catching regressions in Tock. I think in this case it's probably worth going ahead and merging this though. We can update the OTBN/RSA unit tests next. Catching up to the tapeout release is pretty cool! |
Ah right didn't know that, the tests probably just invoked this then. Cheers.
|
Pull Request Overview
This pull request updates OpenTitan to refer to
Earlgrey-M2.5.2-RC0. This tag represents the version of the OpenTitan codebase that will go into the first OpenTitan chip.Testing Strategy
This pull request was tested by booting the tock kernel on a CW310 FPGA board with the
Earlgrey-M2.5.2-RC0bitstream.TODO or Help Wanted
Documentation Updated
boards/opentitan/README.mdand leaves TODOs to write new documentation.Formatting
make prepush.