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

[hw,rv_core_ibex,doc] Sync Rom Patching docs to master (indegrated_dev to master) #24508

Open
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

Razer6
Copy link
Member

@Razer6 Razer6 commented Sep 4, 2024

This PR syncs the Rom Patching documentation back from integrated_dev to master. It further increases the bounds of IbexIcacheScrambleKeyRequestAfterFence_i. The PMP configuration has not yet been changed since the configuration will be done differently after lowRISC/ibex#2147 is merged and vendored.

rswarbrick and others added 15 commits September 4, 2024 14:52
This is a mostly-automated conversion from a Google document to
Markdown. I've exported images as SVGs

Signed-off-by: Rupert Swarbrick <[email protected]>
This drops the document organization section from boot rom document.
That was probably needed in the initial RFC, but isn't really needed
now that this is a manual/document.

Similarly, we drop some in-document layout text. This comes from when
the text was a proposal, rather than a document about what exists. We
probably don't need the structure notes in the document's new form.

Signed-off-by: Rupert Swarbrick <[email protected]>
This was a note saying that we might want opt-in support for PQC
signatures. It's sensible, but doesn't really belong in a document
that describes what OpenTitan integrated is at the moment.

Signed-off-by: Rupert Swarbrick <[email protected]>
This now matches the image that it's describing and I think it's
probably a bit clearer. If we go with "body", I think we need some
extra text explaining what "body" means in this context.

Signed-off-by: Rupert Swarbrick <[email protected]>
This no longer looks like the "few KB" text refers to patches that
redirect program execution, which is how I read it at first.

Signed-off-by: Rupert Swarbrick <[email protected]>
Non-trivial changes:

- Dropped the text about what OpenTitan "impacts" at a SoC level in
  the Overview section. I didn't really understand what this was
  trying to say, and I'm not sure it's really necessary here anyway.

- Lots of full stops / periods added. Especially in bulleted lists,
  where the final sentence was ended by a newline instead of a dot.

Signed-off-by: Rupert Swarbrick <[email protected]>
The "latest" patch region is a little short on details.
Clarify that latest means the patch region with the highest revision
number, amond all regions that have been fully programmed.

Signed-off-by: Samuel Ortiz <[email protected]>
When Lock Valid is set for a patch region, base ROM can skip the OTP
patch region verification. It still must compare the SRAM load code
section and match registers values against the OTP stored signature.

When Lock Valid is not set, base ROM must verify the OTP patch region
signature and if the verification passes, it should toggle the Lock
Valid bit.

Signed-off-by: Samuel Ortiz <[email protected]>
As we only have one sections for all redirections, it can only be loaded
from one single base address in SRAM. This base address should be the
first redirection register r_base value.

Signed-off-by: Samuel Ortiz <[email protected]>
The only tweaked meaning is that the text no longer looks like it's
suggesting the patch SRAM be lockable if we're using part of the main
SRAM to implement it.

This change also gets rid of some text that isn't really needed in
this document (OT's address space layout) and some text that doesn't
convey much information (size of patch RAM is TBD; address region has
a start and end).

Signed-off-by: Rupert Swarbrick <[email protected]>
The original text took me a while to properly understand. I think that
this version is equivalent, but a little easier to follow.

Signed-off-by: Rupert Swarbrick <[email protected]>
The specification defines, describes and document patch match registers
that seem to be Ibex CSRs. None of those exist, neither in Ibex itself
nor in the OT wrapper.

In order to stop relying and documenting non existing registers, this PR
modifies the ROM patch table documentation and defines simpler, CSR
agnostic descriptors for the matching and remapping configuration for
ROM patch regions. This descriptors allow for configuring the existing
Ibex wrapper address translation registers and could also be used for
configuring Ibex CSRs when and if they are implemented.

These descriptors are 2x32 bits words including the base ROM region
address, the patched ROM region size and the SRAM destination base
address. It is up to the patch loader to translate those descriptors
into Ibex wrapper register settings (for now) and potentially Ibex CSR
writes in the future.

Signed-off-by: Samuel Ortiz <[email protected]>
@Razer6 Razer6 changed the title [hw,rv_core_ibex,doc] Sync Rom Patching to master (indegrated_dev to master) [hw,rv_core_ibex,doc] Sync Rom Patching docs to master (indegrated_dev to master) Sep 4, 2024
@Razer6 Razer6 requested a review from vogelpi October 3, 2024 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants