-
Notifications
You must be signed in to change notification settings - Fork 423
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
Perf counters #388
Perf counters #388
Changes from 16 commits
7a62d60
248ad24
20f638e
d9d841d
5dd6ff2
cff3bc3
6f57843
d50a1bc
6d3c518
c06d7f5
659a8b1
ca82922
aef4b4a
9a8cdde
ff2f9ef
495b6d9
7882b0f
02ada2a
7b520aa
5148bba
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -150,7 +150,7 @@ class VexRiscvSmpClusterWithPeripherals(p : VexRiscvSmpClusterParameter) extends | |
plic.addTarget(core.cpu.externalSupervisorInterrupt) | ||
List(clint.logic, core.cpu.logic).produce { | ||
for (plugin <- core.cpu.config.plugins) plugin match { | ||
case plugin: CsrPlugin if plugin.utime != null => plugin.utime := clint.logic.io.time | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here, would need to preserve that feature in the CsrPlugin. (even if that is a duplication) to not break people code There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The feature is still in CSRPlugin, even if disabled when CounterService is present (which in this demo it is), so it should break no existing code which uses the Counters in CsrPlugin. |
||
case plugin: CounterPlugin if plugin.time != null => plugin.time := clint.logic.io.time | ||
case _ => | ||
} | ||
} | ||
|
@@ -204,15 +204,11 @@ object VexRiscvSmpClusterGen { | |
|
||
val misa = Riscv.misaToInt(s"ima${if(withFloat) "f" else ""}${if(withDouble) "d" else ""}${if(rvc) "c" else ""}${if(withSupervisor) "s" else ""}") | ||
val csrConfig = if(withSupervisor){ | ||
var c = CsrPluginConfig.openSbi(mhartid = hartId, misa = misa).copy(utimeAccess = CsrAccess.READ_ONLY, withPrivilegedDebug = privilegedDebug) | ||
var c = CsrPluginConfig.openSbi(mhartid = hartId, misa = misa).copy(withPrivilegedDebug = privilegedDebug) | ||
if(csrFull){ | ||
c = c.copy( | ||
mcauseAccess = CsrAccess.READ_WRITE, | ||
mbadaddrAccess = CsrAccess.READ_WRITE, | ||
ucycleAccess = CsrAccess.READ_ONLY, | ||
uinstretAccess = CsrAccess.READ_ONLY, | ||
mcycleAccess = CsrAccess.READ_WRITE, | ||
minstretAccess = CsrAccess.READ_WRITE | ||
mbadaddrAccess = CsrAccess.READ_WRITE | ||
) | ||
} | ||
c | ||
|
@@ -232,13 +228,10 @@ object VexRiscvSmpClusterGen { | |
mscratchGen = forceMscratch, | ||
mcauseAccess = CsrAccess.READ_ONLY, | ||
mbadaddrAccess = CsrAccess.READ_ONLY, | ||
mcycleAccess = CsrAccess.NONE, | ||
minstretAccess = CsrAccess.NONE, | ||
ecallGen = true, | ||
ebreakGen = true, | ||
wfiGenAsWait = false, | ||
wfiGenAsNop = true, | ||
ucycleAccess = CsrAccess.NONE, | ||
withPrivilegedDebug = privilegedDebug | ||
) | ||
} | ||
|
@@ -358,6 +351,19 @@ object VexRiscvSmpClusterGen { | |
catchAddressMisaligned = true, | ||
fenceiGenAsAJump = false | ||
), | ||
new CounterPlugin(if(csrFull) CounterPluginConfig() else CounterPluginConfig( | ||
NumOfCounters = 0, | ||
mcycleAccess = CsrAccess.NONE, | ||
ucycleAccess = CsrAccess.NONE, | ||
minstretAccess = CsrAccess.NONE, | ||
uinstretAccess = CsrAccess.NONE, | ||
mcounterenAccess = CsrAccess.NONE, | ||
scounterenAccess = CsrAccess.NONE, | ||
mcounterAccess = CsrAccess.NONE, | ||
ucounterAccess = CsrAccess.NONE, | ||
meventAccess = CsrAccess.NONE, | ||
mcountinhibitAccess = CsrAccess.NONE | ||
)), | ||
new YamlPlugin(s"cpu$hartId.yaml") | ||
) | ||
) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to still keep those option ?
I know that would be redundant to the CounterPlugin implementation, but that would preserve backward compatibility without any disruption. (not break users setups)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(this is a general comment, so would also apply for the other demo)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The options are still available in the CSRPlugin. I removed them from here and other places, since CsrAccess.NONE was and is the default value for these.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be possible to revert all the removals of "mcycleAccess = CsrAccess.NONE" and similar things ?
They are there to make it clear to the user that it isn't included, and that if they want they can just turn it on there.