Add [m/s/h]envcfg CSRs to satisfy spec requirements #3373

jerryz123 · 2023-05-19T01:42:24Z

I believe wiring them to 0 is ok.

Related issue:

Type of change: bug report | feature request | other enhancement

Impact: no functional change | API addition (no impact on existing code) | API modification

Development Phase: proposal | implementation

Release Notes

ZenithalHourlyRate · 2023-05-19T06:14:30Z

I believe wiring them to 0 is ok.

For other bits it is ok as we do not have these extensions now (I'm finding people to implement them).

For FIOM, I believe it is not the case for us? Especially when user enabled H extension they might want to enable FIOM

If S-mode is not supported, or if satp.MODE is read-only zero (always Bare), the implementation may make FIOM read-only zero

Or the software could set FIOM but maybe we can make it a no-op. Though I am not sure if it is OK and am still checking it.

jerryz123 · 2023-05-19T06:42:52Z

The spec seems to indicate that FIOM is writable, while in spike, FIOM is hardwired to 0.

ZenithalHourlyRate · 2023-05-19T06:51:00Z

Though I am not sure if it is OK and am still checking it.

Seems like rocket does not distinguish between PI and PR (similarly for other bits) so the implication requirement by FIOM (e.g. PI implies PR) is automatically there. We can safely do no-op when FIOM is set to 1.

rocket-chip/src/main/scala/rocket/RocketCore.scala

Lines 349 to 350 in 284b360

    
           val id_fence_pred = id_inst(0)(27,24) 
        
           val id_fence_succ = id_inst(0)(23,20)

jerryz123 · 2023-05-19T07:07:52Z

Ok. I'm happy to change this such that only FIOM is writable in these registers.
I will also expose a fiom output from the CSR file since other core implementations may need to check FIOM for ordering fences.

src/main/scala/rocket/CSR.scala

ZenithalHourlyRate · 2023-05-22T18:35:43Z

src/main/scala/rocket/CSR.scala

@@ -1381,6 +1414,13 @@ class CSRFile(
    }
    if (usingUser) {
      when (decoded_addr(CSRs.mcounteren)) { reg_mcounteren := wdata }
+      when (decoded_addr(CSRs.menvcfg))    { reg_menvcfg.write(wdata) }


I spend some time finding the path for xxxh CSRs, like mstatush and cycleh. Turned out there was some special utility

rocket-chip/src/main/scala/rocket/CSR.scala

Lines 1573 to 1581 in bd3276d

def writeCounter(lo: Int, ctr: WideCounter, wdata: UInt) = {

if (xLen == 32) {

val hi = lo + CSRs.mcycleh - CSRs.mcycle

when (decoded_addr(lo)) { ctr := Cat(ctr(ctr.getWidth-1, 32), wdata) }

when (decoded_addr(hi)) { ctr := Cat(wdata(ctr.getWidth-33, 0), ctr(31, 0)) }

} else {

when (decoded_addr(lo)) { ctr := wdata(ctr.getWidth-1, 0) }

}

}

I believe the current implementation does not handle csrw menvcfgh

I also do not find the path for writing mstatush, do you have any clue?

I also do not find the path for writing mstatush, do you have any clue?

Seems like this is part of privileged spec 1.12 (envcfg is also added in 1.12). Is rocket compatible with privileged spec 1.11 only and we need to spend some time upgrading?

That snippet is for counters only. We ought to work towards making it compliant with 1.12.

Other parts are fine with me now

That snippet is for counters only. We ought to work towards making it compliant with 1.12.

I think menvcfgh and henvcfgh are the only problem left.

Writes do menvcfgh and menvcfgh should cause no effects at the moment, since FIOM is the low bit.

Understood. I assumed that we must have a decoded_addr(menvcfgh) somewhere to make it writable. Turned out it is not the case.

jerryz123 · 2023-05-23T06:15:59Z

@Mergifyio backport master

mergify · 2023-05-23T06:16:02Z

backport master

✅ Backports have been created

#3376 Add [m/s/h]envcfg CSRs to satisfy spec requirements (backport #3373) has been created for branch master

(cherry picked from commit 9967142)

Add [m/s/h]envcfg CSRs to satisfy spec requirements (backport #3373)

* bump to Chisel 3.5.6 (#3222) * Remove deprecated code for BarrelShifter Cherry-picked chipsalliance/chisel@7372c9e Should use BarrelShifter from chisel3.std, but it is not published, see chipsalliance/chisel#2997 * Fix scala reflect error scala.reflect.internal.Types: constructor RecordMap in class RecordMap cannot be accessed in package util from package util in package rocketchip * explicit add legacy connect operators * replace all cde dependencies. * change api-config to cde in build.sbt * bump cde submodule * fix Makefile * IDecode: Fix aes64ks1i imm decode It is not rs2, it is imm Related to #3255 * CryptoNIST: refactor rnum This removes a redundant port As rnum is encoded in rs2 as imm now, we can get it just from rs2 * Remove redundant TLBExceptions V bit This was introduced by "(185cac8) Add hypervisor extension (#2841)" This is a dead code, as no circuit is producing and consuming this bit. This was discovered when migrating Core.scala to chisel3, where strict checking was applied for IO and firrtl found this is not connected. In the context of hypervisor extension, V bit, or _virtualization mode_, indicates whether the hart is currently executing in a guest. For TLBReq, the V bit is needed as it affects the PTW thus TLB behavior on whether to do _Two-Stage Address Translation_. However, the bit is not needed for TLBException. The exceptions (pf, gf, ae, ma) have no V=1/V=0 variants. Also, there is no point to add V bit for gf (guest page fault). The io.resp.gf added below in the original patch also does not connect io.resp.gf.v. If this V bit should be added for this specific exception, it should be connected. I assume this was added accidentally, as the original "extends CoreBundle()(p)" modification seems irrelevant. * update build system for cde bump * Fix unconnected io.start for TLRAMXbarTest and TLMulticlientXbarTest (cherry picked from commit 34d7309) * Fold HasPeripheryDebugModuleImp into HasPeripheryDebug (cherry picked from commit a2682ca) * Move HasDebugModule out of TileContextType (cherry picked from commit 0e4af6d) * Pinning nix to 2.13.3 in github workflows Nix 2.14 released with incompatibilities with cachix/install-nix-action. This PR pins nix to 2.13.3 to avoid CI fails See: cachix/install-nix-action#161 (cherry picked from commit 84533ae) * L1TLB: VS-mode SFENCE misses tera/giga-page entries fragmented superpages (#3297) (cherry picked from commit 8b52a6f) * feat: port Chisel2 to Chisel3 devices/ (cherry picked from commit 1ff0db3) * fix: add `chiselTypeOf` when inst Wire(in.d) (cherry picked from commit e645d94) * feat: port Chisel2 to Chisel3 rocket/ (cherry picked from commit d6a982b) * feat: port Chisel2 to Chisel3 amba/ (cherry picked from commit b2fd991) * Fix CharCount RoCC example bug Reset recv_beat (cherry picked from commit f19a90a) * Change CharCountRoCC Example to use dcacheParams RoCC accesses D$, not I$ (cherry picked from commit 61ea81c) * mill: fix empty cross arg for riscv-tests.Suite riscv-tests.suite[] wont compile (cherry picked from commit d86c011) * Hypervisor: encodeVirtualAddress extra MSB to canonicalize VS-disabled (#3314) (cherry picked from commit 0504a9b) * Support devOverride for diplomatic SRAMs (cherry picked from commit 12e21a6) * Support overriding the DTS node for diplomatic SRAMs (cherry picked from commit c8edec3) * Fix no-debug-node designs (cherry picked from commit 3b8d3c1) * all isaDTS strings to lowercase (#3333) (#3334) (cherry picked from commit 58c8249) Co-authored-by: Yangyu Chen <cyy@cyyself.name> * Deprecate old BusWrapper methods (#3337) (#3340) (cherry picked from commit 2570db7) Co-authored-by: Jerry Zhao <jerryz123@berkeley.edu> * Fix AXI4 RegisterRouter on Wire Clone (#3341) In legacy code it is ok to clone a Wire using Wire() now the requirement is more strict and WireDefault is needed Related to #3059 The Output is added for code style consistency Closes #3324 (cherry picked from commit fb5d7d0) Co-authored-by: Zenithal <i@zenithal.me> * Support TLFilter.mSubtract with multiple AddressSets (#3339) (#3343) Enables filtering out multiple ranges at once. (cherry picked from commit 4110563) Co-authored-by: Jerry Zhao <jerryz123@berkeley.edu> * Improve boundaryBuffers API | move boundaryBuffers to within-Tile (#3342) (#3344) * Add API to force TLBuffers into RocketTile as boundaryBuffers * Generate boundaryBuffers within the Tile, not the TilePRCIDomain (cherry picked from commit 3f74d79) Co-authored-by: Jerry Zhao <jerryz123@berkeley.edu> * Implement Zicond extension (#3329) * Instructions: sync from riscv-opcodes (add zicond) * Zicond: implement czero.eqz, czero.nez * Zicond: fix implementation * Zicond: reduce code duplication * Add ISA extension when using Zicond * Fix ISA extensions ordering (cherry picked from commit 62162c5) * feat: port Chisel2 to Chisel3 tilelink/ (cherry picked from commit 6c23100) * fix: not fully initialized wires in Edge.scala + using `DontCare` (cherry picked from commit 3902497) * fix: add chiselTypeOf when inst Wire + in.d and out.a; (cherry picked from commit e52773e) * fix: not fully initialized wires in Edge.scala + using `DontCare` (cherry picked from commit 5f7278b) * Revert to old := connects instead of :#= * Support removing the nonstandard CEASE from rocket (cherry picked from commit 956a1ff) * fix: Chisel3 #2944 Move SourceInfo to package experimental * Update debug_rom.S (cherry picked from commit a9ae0b9) * Update debug_rom_nonzero.S (cherry picked from commit 6ba9437) * Remove cloneType Ref to #2889 * Bump hardfloat * Bump to chisel3.6 * Switch to json4s-native * Update HeterogeneousBag to chisel3.6 * Bump mill flow to chisel 3.6.0 * Switch HeterogeneousBag to VectorMap * Update src/main/scala/util/HeterogeneousBag.scala Co-authored-by: Jack Koenig <koenig@sifive.com> * support circt and bump to 3.6 * Generalize Vec[TracedInstruction] to a TraceBundle (cherry picked from commit 85aca71) * Add time to TraceBundle (cherry picked from commit 57af718) * Supporting adding custom stuff to TraceBundle (cherry picked from commit f6f59c1) * Fix BlockableTraceBundle (cherry picked from commit efa8337) * Bump nix toolchain version * Fix TLSourceShrinker (cherry picked from commit d503368) * Support RoCC accels which define CSRs (#3358) (cherry picked from commit 7ddf02a) * Add support for an unsynthesizable ROB to produce a TracedInstruction stream from Rocket with wdata (cherry picked from commit d6c09c9) # Conflicts: # src/main/scala/rocket/RocketCore.scala # src/main/scala/subsystem/Configs.scala * Fix memory leak in debug_rob (cherry picked from commit b3f391c) * Fix unittests (cherry picked from commit a806851) * Fix TL unittests (cherry picked from commit ecf08f5) * Support blockable credited interfaces (cherry picked from commit c8cf935) * Make AsyncQueue use Rawmodule (cherry picked from commit 8db7364) * Support dynamic credit count in senders for CreditedIO (cherry picked from commit 5a5c127) * Fix TLJbarTest (cherry picked from commit e76a4ea) * Add [m/s/h]envcfg CSRs to satisfy spec requirements (#3373) (cherry picked from commit 9967142) * fix: Werror match may not be exhaustive (#3268) (#3384) + add `case _` to make match be exhaustive (cherry picked from commit 05d9db7) Co-authored-by: SingularityKChen <chency_singularity@163.com> * fix: empty argument list (#3262) (#3386) + add `()` to those empty argument list function call; Auto-application to `()` is deprecated. Supply the empty argument list `()` explicitly to invoke method (cherry picked from commit 9b383c5) Co-authored-by: SingularityKChen <chency_singularity@163.com> * fix: bit extraction use .U.extract(i) (#3263) (#3385) + replace `.U(i)` with `.U.extract(i)` (cherry picked from commit 9a1dc2d) Co-authored-by: SingularityKChen <chency_singularity@163.com> * TLB: must_alloc swapped AMO Logical/Arithmetic (#3390) (cherry picked from commit dc275c4) Co-authored-by: John Ingalls <43973001+ingallsj@users.noreply.github.com> * TLB: vsstatus.SUM check should not apply to Stage-2 fault before Stage-1 response (#3393) (#3399) (cherry picked from commit 43e0af1) Co-authored-by: John Ingalls <43973001+ingallsj@users.noreply.github.com> * CSR: optionally set delegable hypervisor exceptions (#3401) Given that usingHypervisor is used to distinguish whether the hardware supports hypervisor extensions, we should use it for the delegable exceptions as well. (cherry picked from commit 026f4c9) * Enable WARL custom CSRs, long-latency CSR accesses (#3388) * Support setting custom CSRs from datapath * Support CSR stalls (cherry picked from commit 005c6db) * PTW: set ae_ptw for out-of-range non-leaf PTEs (#3407) (#3410) For PTEs whose physical address is out-of-range, we need to set `ae_ptw` instead of `ae_final` to raise access-fault. Because non-leaf PTEs will not have R or X bits set, `ae_final` will be overrided by page-fault exceptions. (cherry picked from commit b8dad7f) Co-authored-by: Yinan Xu <xuyinan@ict.ac.cn> --------- Co-authored-by: Jiuyang Liu <liu@jiuyang.me> Co-authored-by: Zenithal <i@zenithal.me> Co-authored-by: Hansung Kim <hansung_kim@berkeley.edu> Co-authored-by: Liu Xiaoyi <circuitcoder0@gmail.com> Co-authored-by: John Ingalls <43973001+ingallsj@users.noreply.github.com> Co-authored-by: singularity <chency_singularity@163.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> Co-authored-by: Yangyu Chen <cyy@cyyself.name> Co-authored-by: rewired <39949564+rewired-gh@users.noreply.github.com> Co-authored-by: Jack Koenig <koenig@sifive.com> Co-authored-by: Yinan Xu <xuyinan@ict.ac.cn>

jerryz123 force-pushed the envcfg branch 3 times, most recently from d4b37d5 to c261dd8 Compare May 19, 2023 16:52

jerryz123 requested review from ZenithalHourlyRate and sequencer May 21, 2023 03:34

ZenithalHourlyRate reviewed May 22, 2023

View reviewed changes

src/main/scala/rocket/CSR.scala Outdated Show resolved Hide resolved

src/main/scala/rocket/CSR.scala Outdated Show resolved Hide resolved

jerryz123 force-pushed the envcfg branch from c261dd8 to b2dd756 Compare May 22, 2023 18:09

ZenithalHourlyRate reviewed May 22, 2023

View reviewed changes

src/main/scala/rocket/CSR.scala Outdated Show resolved Hide resolved

src/main/scala/rocket/CSR.scala Outdated Show resolved Hide resolved

src/main/scala/rocket/CSR.scala Outdated Show resolved Hide resolved

ZenithalHourlyRate reviewed May 22, 2023

View reviewed changes

jerryz123 force-pushed the envcfg branch from b2dd756 to 6b73e00 Compare May 22, 2023 20:12

Add [m/s/h]envcfg CSRs to satisfy spec requirements

0a5881a

jerryz123 force-pushed the envcfg branch from 6b73e00 to 0a5881a Compare May 22, 2023 20:19

jerryz123 requested a review from ZenithalHourlyRate May 22, 2023 21:07

ZenithalHourlyRate approved these changes May 23, 2023

View reviewed changes

ZenithalHourlyRate merged commit 9967142 into dev May 23, 2023

ZenithalHourlyRate deleted the envcfg branch May 23, 2023 01:26

mergify bot mentioned this pull request May 23, 2023

Add [m/s/h]envcfg CSRs to satisfy spec requirements (backport #3373) #3376

Merged

mergify bot pushed a commit that referenced this pull request May 23, 2023

Add [m/s/h]envcfg CSRs to satisfy spec requirements (#3373)

b08a4b1

(cherry picked from commit 9967142)

jerryz123 added a commit that referenced this pull request May 23, 2023

Merge pull request #3376 from chipsalliance/mergify/bp/master/pr-3373

0586532

Add [m/s/h]envcfg CSRs to satisfy spec requirements (backport #3373)

sequencer mentioned this pull request May 30, 2023

Remove SFC #3379

Merged

sequencer mentioned this pull request Jun 20, 2023

Clean up all deprecations from chisel3p6 to embrace chisel5 #3392

Closed

ZenithalHourlyRate mentioned this pull request Jul 25, 2023

CSR: add mconfigptr and default to zero #3446

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add [m/s/h]envcfg CSRs to satisfy spec requirements #3373

Add [m/s/h]envcfg CSRs to satisfy spec requirements #3373

Uh oh!

jerryz123 commented May 19, 2023

Uh oh!

ZenithalHourlyRate commented May 19, 2023 •

edited

Loading

Uh oh!

jerryz123 commented May 19, 2023

Uh oh!

ZenithalHourlyRate commented May 19, 2023

Uh oh!

jerryz123 commented May 19, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ZenithalHourlyRate May 22, 2023 •

edited

Loading

Uh oh!

ZenithalHourlyRate May 22, 2023

Uh oh!

jerryz123 May 22, 2023

Uh oh!

ZenithalHourlyRate May 23, 2023

Uh oh!

jerryz123 May 23, 2023

Uh oh!

ZenithalHourlyRate May 23, 2023

Uh oh!

jerryz123 commented May 23, 2023

Uh oh!

mergify bot commented May 23, 2023 •

edited

Loading

Uh oh!

Uh oh!

	def writeCounter(lo: Int, ctr: WideCounter, wdata: UInt) = {
	if (xLen == 32) {
	val hi = lo + CSRs.mcycleh - CSRs.mcycle
	when (decoded_addr(lo)) { ctr := Cat(ctr(ctr.getWidth-1, 32), wdata) }
	when (decoded_addr(hi)) { ctr := Cat(wdata(ctr.getWidth-33, 0), ctr(31, 0)) }
	} else {
	when (decoded_addr(lo)) { ctr := wdata(ctr.getWidth-1, 0) }
	}
	}

Add [m/s/h]envcfg CSRs to satisfy spec requirements #3373

Add [m/s/h]envcfg CSRs to satisfy spec requirements #3373

Uh oh!

Conversation

jerryz123 commented May 19, 2023

Uh oh!

ZenithalHourlyRate commented May 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jerryz123 commented May 19, 2023

Uh oh!

ZenithalHourlyRate commented May 19, 2023

Uh oh!

jerryz123 commented May 19, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ZenithalHourlyRate May 22, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ZenithalHourlyRate May 22, 2023

Choose a reason for hiding this comment

Uh oh!

jerryz123 May 22, 2023

Choose a reason for hiding this comment

Uh oh!

ZenithalHourlyRate May 23, 2023

Choose a reason for hiding this comment

Uh oh!

jerryz123 May 23, 2023

Choose a reason for hiding this comment

Uh oh!

ZenithalHourlyRate May 23, 2023

Choose a reason for hiding this comment

Uh oh!

jerryz123 commented May 23, 2023

Uh oh!

mergify bot commented May 23, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Backports have been created

Uh oh!

Uh oh!

ZenithalHourlyRate commented May 19, 2023 •

edited

Loading

ZenithalHourlyRate May 22, 2023 •

edited

Loading

mergify bot commented May 23, 2023 •

edited

Loading