rust-embedded / riscv

Low level access to RISC-V processors
856 stars 164 forks source link

`riscv-rt`: Implement FPU initialization #157

Closed Disasm closed 11 months ago

laanwj commented 5 years ago

What kind of FPU initialization would be expected on RISC-V?

Disasm commented 5 years ago

Something like this: https://github.com/rust-embedded/riscv-rt/commit/3a76c3b26eeb995604db24afc98f1689a7514db0 (and optionally zeroing all of the FPU registers). The problem is that fcsr::clear_flags(); part doesn't work on K210, that's why I reverted that commit. IIRC instead of accessing the FPU CSRs they use single instruction to clear CSR.

laanwj commented 5 years ago

This is what lib/bsp/crt.S does:

  li t0, MSTATUS_FS
  csrs mstatus, t0

  fssr    x0
  fmv.w.x f0, x0
  fmv.w.x f1, x0
  fmv.w.x f2, x0
  fmv.w.x f3, x0
  fmv.w.x f4, x0
  fmv.w.x f5, x0
  fmv.w.x f6, x0
  fmv.w.x f7, x0
  fmv.w.x f8, x0
  fmv.w.x f9, x0
  fmv.w.x f10,x0
  fmv.w.x f11,x0
  fmv.w.x f12,x0
  fmv.w.x f13,x0
  fmv.w.x f14,x0
  fmv.w.x f15,x0
  fmv.w.x f16,x0
  fmv.w.x f17,x0
  fmv.w.x f18,x0
  fmv.w.x f19,x0
  fmv.w.x f20,x0
  fmv.w.x f21,x0
  fmv.w.x f22,x0
  fmv.w.x f23,x0
  fmv.w.x f24,x0
  fmv.w.x f25,x0
  fmv.w.x f26,x0
  fmv.w.x f27,x0
  fmv.w.x f28,x0
  fmv.w.x f29,x0
  fmv.w.x f30,x0
  fmv.w.x f31,x0

So as I understand:

So yes, they use an instruction instead of CSR access. Interesting (but annoying) that accessing the control word through csr instructions doesn't work on K210.

Looks like this is going to need a function implemented in a .s file, then, especially to clear the registers. And/or inline assembly.

I've wondered at times: why is inline assembly an optional feature?

Disasm commented 5 years ago

Looks like this is going to need a function implemented in a .s file, then, especially to clear the registers. And/or inline assembly.

Yes, something like that.

I've wondered at times: why is inline assembly an optional feature?

Inline assembly unstable AFAIK, so if you want to build this on stable, you have to use a different approach (such as pre-compiled asm functions or ThinLTO)

laanwj commented 5 years ago

Inline assembly unstable AFAIK, so if you want to build this on stable, you have to use a different approach (such as pre-compiled asm functions or ThinLTO)

Oh right (rust-lang/rust#29722), thanks, had thought it would be stable by now, but even the syntax is still under discussion.

laanwj commented 5 years ago

Okay, I didn't know this, but apparentlyfssr and friends are simply pseudo-instructions that map to CSR access: riscv/riscv-isa-manual#419

00301073          fssr    zero
00301073          fscsr         zero,zero
00301073          csrrw         zero,fcsr,zero
00301073          csrrw         zero,0x003,zero

I really wonder what causes the K210 issue now.

Disasm commented 5 years ago

I don't remember exactly, but I think I accessed only one half of the CSR (there are frm and fflag CSRs for this), maybe this was a problem.

kristate commented 4 years ago

Hello from the future, did this ever get cleared-up?

laanwj commented 4 years ago

Well you are from the future, you should know! :smile:

But no—it's still an open issue. It seems on Kendryte K210, floating point works without this kind of initialization (maybe because the bootrom takes care of it). I don't think we have Rust on any other RISC-V SoCs with floating point to compare.

luojia65 commented 4 years ago

Ref: https://github.com/riscv/opensbi/blob/master/lib/sbi/riscv_hardfp.S

romancardenas commented 1 year ago

Hi from the future! inline assembly is already stable and I opened a PR (#118). So thanks to inline assembly we can start to attend to these old issues :)

mini-ninja-64 commented 11 months ago

I am happy to have a go at implementing this, what would be the preferred approach for setting FPU as enabled?

a piece of config similar to the m-mode / s-mode switch that the crate currently uses, or something more like if in m-mode use misa to auto detect FPU presence?

I feel like I read somewhere before misa should be avoided in favour of device tree for most things so maybe it would not make sense to use it for this 🤔, especially as in s-mode we would have to call opensbi or something wierd like that for reading misa before setting the fs bit in sstatus.

romancardenas commented 11 months ago

If your target supports the f extension, you could use a configuration gate like #[cfg(riscvf)] (check this).

We do something similar here for targets supporting integer multiplication.

mini-ninja-64 commented 11 months ago

Perfect thankyou!!! ❤️