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

Move to CHERIoT-platform submodule and catch up with upstream changes #35

Merged
merged 15 commits into from
Feb 15, 2024
Merged

Move to CHERIoT-platform submodule and catch up with upstream changes #35

merged 15 commits into from
Feb 15, 2024

Conversation

nwf-msr
Copy link
Contributor

@nwf-msr nwf-msr commented Feb 12, 2024

When reviewing, you'll probably want to also look at the changes in the submodule relative to upstream, CHERIoT-Platform/sail-riscv@main...CHERIoT-Platform:sail-riscv:cheriot-submodule, and/or the upstream changes included herein, CHERIoT-Platform/sail-riscv@6d0cc0f...CHERIoT-Platform:sail-riscv:main .

At least locally, with sail mainline, this commit builds both the csim and rvfi targets.

See sail-riscv's 182dc52ef468884f31c0e002118a8c75df0a69fb
See sail-riscv's 3e6a55f69ee74ba5dcbd63506f992151870c24a6
See sail-riscv's d5e89a71e3a84495c1b88a7749c25fd6b9da684b
See sail-riscv's 563446c477f5e905df905e0d30371a2c4d51d7a5
Copy link
Collaborator

@davidchisnall davidchisnall left a comment

Choose a reason for hiding this comment

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

Looks good to me, one place where even more things can be deleted (yay for deleting things!).

.github/workflows/compile.yml Show resolved Hide resolved
.github/workflows/compile.yml Outdated Show resolved Hide resolved
@rmn30
Copy link
Collaborator

rmn30 commented Feb 13, 2024

This looks good but I was expecting to see some changes to adjust to CHERIoT-Platform/sail-riscv@9547a30 . Upstream made these changes: CTSRD-CHERI/sail-cheri-riscv@dab72aa . I must be missing something.

@rmn30
Copy link
Collaborator

rmn30 commented Feb 13, 2024

Indeed, I've tested and the simulator builds but doesn't use CHERI instructions correctly. Will investigate.

@rmn30
Copy link
Collaborator

rmn30 commented Feb 13, 2024

Replacing cheri_decode_ext.sail with the following and removing riscv_decode_ext.sail in the Makefile is sufficient to get a working simulator but is not optimal as we still get incorrect disassembly for compressed instructions. I will think about a solution more similar to upstream.

val ext_decode_compressed : bits(16) -> ast
function ext_decode_compressed(bv) = {
  let ast = encdec_compressed(bv);
  match ast {
    C_JAL(imm) => CJAL(sign_extend(imm @ 0b0), ra),
    C_JALR(rs1) => CJALR(zero_extend(0b0), rs1, ra),
    C_J(imm) => CJAL(sign_extend(imm @ 0b0), zreg),
    C_JR(rs1) => CJALR(zero_extend(0b0), rs1, zreg),
    C_MV(rd, 0b1 @ rs2 : bits(4)) => CMove(rd, 0b0 @ rs2),
    C_ADDI4SPN(rdc, nzimm) => CIncAddrImmediate(creg2reg_idx(rdc), sp, zero_extend(nzimm @ 0b00)),
    C_ADDI16SP(imm) => CIncAddrImmediate(sp, sp, sign_extend(imm @ 0x0)),
    OTHER => OTHER
  }
}

val ext_decode : bits(32) -> ast
function ext_decode(bv) = {
  let ast = encdec(bv);
  match ast {
    UTYPE(imm, 0b0 @ cd : bits(4), RISCV_AUIPC) => AUIPCC(imm, 0b0 @ cd),
    RISCV_JAL(imm, cd) => CJAL(imm, cd),
    RISCV_JALR(imm, cs1, cd) => CJALR(imm, cs1, cd),
    OTHER => OTHER
  }
}

@rmn30
Copy link
Collaborator

rmn30 commented Feb 13, 2024

I have just pushed a branch that extends this PR to fix decode. @nwf would you like to incorporate that commit into this PR?

azure-pipelines.yml Outdated Show resolved Hide resolved
src/cheri_regs.sail Outdated Show resolved Hide resolved
nwf-msr and others added 7 commits February 14, 2024 23:33
This is a strict subset of the sail-riscv model's riscv_step_rvfi.sail to omit
fregs absent in CHERIoT.
Remove riscv_patches now pushed to that repo
Now that we're using a patched submodule rather than applying patches here, we
don't need to fib to git about identity.

Thanks to David Chisnall for spotting the oversight.
…hook.

riscv/sail-riscv#205 changed the way decode hooks used to override existing RISC-V instructions work.
This commit imports changes from upstream to adapt to the change. Relevant commit:
CTSRD-CHERI/sail-cheri-riscv@d760e53
@nwf-msr
Copy link
Contributor Author

nwf-msr commented Feb 15, 2024

Thanks @rmn30; I'd completely missed that. With your change added, the CHERIoT RTOS test suite passes for both release and test builds.

@rmn30 rmn30 merged commit a0d5ae5 into CHERIoT-Platform:main Feb 15, 2024
2 checks passed
davidchisnall pushed a commit to CHERIoT-Platform/devcontainer that referenced this pull request Feb 15, 2024
@nwf-msr nwf-msr deleted the 202402-new-submodule branch September 18, 2024 17:28
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.

4 participants