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

More RISC-V instructions in core::arch #1271

Merged
merged 6 commits into from
Jan 5, 2022

Conversation

luojia65
Copy link
Contributor

@luojia65 luojia65 commented Jan 1, 2022

This pull request includes:

  • Hypervisor extension, including memory management insturctions and virtual machine memory load and store instructions
  • Supervisor memory management, including address translation fence insturctions (important for Rust kernel development) and cache invalidation insturctions
  • Non privileged instructions FENCE.I, NOP and WFI

Module riscv64 is added to include RV64 only virtual machine memory instructions. RV64 supports all RV32 virtual machine instructions, it's implemented like:

    pub mod riscv64 {
        pub use crate::core_arch::riscv::*;
        pub use crate::core_arch::riscv64::*;
    }

(Edit: implemented like pub use crate::core_arch::riscv32::*; etc)

Most of implemented functions are marked as unsafe. That's because these functions either load or store effectively into raw pointer (hlv, hlvx and hsv instructions), or would effect how the RISC-V core handles page memory translation (memory management fences and cache invalidations).

Specifically, here's a list of implemented instruction functions from updated RISC-V core arch module:

图片

Marked as draft: more polishing on documents(done), and decide whether #[must_use] is necessary on virtual machine memory loads.(ref: ptr::read & ptr::write, does not put #[must_use] by now)

todo: wait before supervisor extension in rustc risc-v target is available
(e.g. when we're developing kernel modules or kernels themselves,
S extension is enabled)

sinval.vma instructions

FENCE.I instruction
@rust-highfive
Copy link

r? @Amanieu

(rust-highfive has picked a reviewer for you, use r? to override)

@luojia65 luojia65 marked this pull request as ready for review January 1, 2022 08:26
Copy link
Member

@Amanieu Amanieu left a comment

Choose a reason for hiding this comment

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

LGTM other than the module name issue.

crates/core_arch/src/mod.rs Outdated Show resolved Hide resolved
@luojia65
Copy link
Contributor Author

luojia65 commented Jan 4, 2022

I modified riscv to riscv32.

r? @Amanieu

@luojia65 luojia65 force-pushed the more-riscv-insns branch 2 times, most recently from 2e97688 to fb4607c Compare January 4, 2022 05:59
/// advancing the `pc` and incrementing any applicable performance counters.
#[inline]
pub fn nop() {
unsafe { asm!("nop") }
Copy link
Member

Choose a reason for hiding this comment

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

All of the asm! blocks here should use nostack since they don't push anything to the stack.

Additionally:

  • Most should use nomem, except for loads which should use readonly and stores which should use neither.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I'll check it out.
There are many privileged memory fences as well, I'd also figure it out in these intrinsic functions.

Copy link
Member

Choose a reason for hiding this comment

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

Good point about the fences, those should not use nomem/readonly because they need to act as a compiler fence.

@@ -264,7 +276,11 @@ mod arm;

#[cfg(any(target_arch = "riscv32", target_arch = "riscv64", doc))]
#[doc(cfg(any(target_arch = "riscv32", target_arch = "riscv64")))]
mod riscv;
mod riscv32;
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps calling this riscv_shared (like arm_shared) makes more sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes sense, it would be a more common module system design.

rename internal riscv32 module to riscv_shared
@luojia65
Copy link
Contributor Author

luojia65 commented Jan 5, 2022

Hello! I renamed internal shared riscv module to riscv_shared, and modified asm! options under following rules:

  • Trivial no-op instruction (NOP), wait-for-interrupt (WFI): options(nomem, nostack)
  • Unprivileged fences (FENCE.I), privileged fences (SFENCE.*, SINVAL.*, HFENCE.*, HINVAL.*): options(nostack)
  • Hypervisor loads (HLV.*, HLVX.*): options(readonly, nostack)
  • Hypervisor stores (HSV.*): options(nostack)

r? @Amanieu

@Amanieu Amanieu merged commit 2adc17a into rust-lang:master Jan 5, 2022
@luojia65 luojia65 deleted the more-riscv-insns branch January 7, 2022 06:08
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.

None yet

3 participants