Skip to content

WIP: CHERI Standard support #419

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

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from
Draft

WIP: CHERI Standard support #419

wants to merge 18 commits into from

Conversation

arichardson
Copy link
Member

No description provided.

@arichardson arichardson requested a review from qwattash April 22, 2025 19:08
@arichardson arichardson marked this pull request as draft April 22, 2025 19:30
@arichardson
Copy link
Member Author

Will try to fix the CI failures and then merge to main.


def setup(self):
super().setup()
self.make_args.set(FW_TEXT_START=0x80000000)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's supposed to be 0. It's built as a PIE if possible. The expectation is it self-relocates at startup.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm yes, from what I can tell this is in fact redundant. Removing it appears to be working correctly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@heshamelmatary I think you needed that, do you know what relies on it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this, I get an error like the following when using fw_jump.elf as QEMU's -bios:

qemu-system-riscv64cheri: Some ROM regions are overlapping
These ROM regions might have been loaded by direct user request or by default.
They could be BIOS/firmware images, a guest kernel, initrd or some other file loaded into guest memory.
Check whether you intended to load all this guest code, and whether it has been built to load to the correct addresses.

The following two regions overlap (in the memory address space):
/home/hesham/cheri/output/cheri-alliance-sdk/opensbi/riscv64/share/opensbi/l64pc128/generic/firmware/fw_jump.elf ELF program header segment 0 (addresses 0x0000000000000000 - 0x00000000000241c0)
mrom.reset (addresses 0x0000000000001000 - 0x0000000000001028)

QEMU's loader uses the segments' addresses when loading ELF bioses. It will work fine if we just use the binary versions rather than ELFs, but sometimes (e.g., when debugging or when we only have ELF loaders), we need to pass the bios/opensbi as an ELF.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I think we discussed the error in chat but I forgot. I think I have never noticed this because the cheribuild run target always uses the binary image version. I think we are also only installing the binary version in the sdk directory, perhaps we should install also the ELF version?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The .bin file is the same as the .elf file, I don't understand why you'd ever need to pass the .elf file to QEMU instead (though I do personally believe that's the "nicer" thing to pass). Just because QEMU is given the .bin file doesn't mean you can't give the debugger the .elf.

But yes, it seems QEMU, like many things, forgets PIEs exist in its ELF loader.

Copy link
Contributor

@qwattash qwattash Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reference. riscv-software-src/opensbi#372
Which I think has been merged here qemu/qemu@55c1365

@qwattash
Copy link
Contributor

qwattash commented Aug 7, 2025

Will try to fix the CI failures and then merge to main.

I added a WIP commit that points the cheri-alliance-llvm to a repository that currently works for us. I think this should not be merged until we have a good "official" llvm to point this to.

qwattash and others added 16 commits August 8, 2025 11:51
Otherwise it'll link to 0x0.

Co-authored-by: Hesham Almatary <[email protected]>
Imitate morello-llvm to build CHERI Alliance's LLVM and QEMU
for --riscv-cheri-isa std. This only applies to CHERI-RISC-V.
Depending on ISA switch --riscv-cheri-isa,use the appropriat SDK.
This is not currently supported by the Cheri Allliance LLVM.
Also delete unused Alliance target class
CI fails otherwise with an error:

Traceback (most recent call last):
  File "<stdin>", line 22514, in <module>
  File "<stdin>", line 22851, in BuildCheriseL4Excercises
NameError: name 'BuildAllianceOpenSBI' is not defined
Propagate the current CheriConfig depending whether the llvm_project
is queried during early dependency resolution or during Project
initialization.
…based.

Temporarily point the default branch of the cheri-alliance-llvm to the
private repository with the latest rebased branch.
This unbreaks cheribsd builds, but should not be used as the official solution.
repository = GitRepository(
"https://github.com/CHERI-Alliance/llvm-project.git", default_branch="codasip-cheri-riscv", force_branch=True
"https://github.com/veselypeta/cherillvm", default_branch="codasip-rebased", force_branch=True
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could use a temporary_url_override here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, I did not remember that was a thing

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a way to override the default branch as well?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah looks like that doesn't exist yet.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can probably add that fairly easily. It's certainly cleaner than this.

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.

5 participants