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

Ability to change bench app options for RiscV #17

Open
malus-brandywine opened this issue Feb 8, 2022 · 3 comments
Open

Ability to change bench app options for RiscV #17

malus-brandywine opened this issue Feb 8, 2022 · 3 comments

Comments

@malus-brandywine
Copy link
Contributor

Hi,

Does anyone remember why sel4bench configuration for RiscV was not allowed to change bench app options?

If it was skipped because it was not necessary then I'd like to fix it.

I would add a line to
https://github.com/seL4/sel4bench/blob/master/apps/sel4bench/CMakeLists.txt

--- a/apps/sel4bench/CMakeLists.txt
+++ b/apps/sel4bench/CMakeLists.txt
@@ -35,14 +35,15 @@ config_string(
 
 # Default dependencies on kernel benchmarking features. Declared here so that
 # all the benchmark applications can use it
 if(
     (KernelArchX86 AND KernelExportPMCUser AND KernelX86DangerousMSR)
     OR (KernelArchARM AND KernelArmExportPMUUser)
     OR (KernelArchArmCortexA8 AND KernelDangerousCodeInjection)
+    OR (KernelArchRiscV)
 )
     set(DefaultBenchDeps TRUE)
 else()
     set(DefaultBenchDeps FALSE)
 endif()
 
 find_package(musllibc REQUIRED)

(copying it to devel mailing list)

Thanks!

@axel-h
Copy link
Member

axel-h commented Feb 11, 2022

see PR #20

@kent-mcleod
Copy link
Member

DefaultBenchDeps is used to convey when a certain set of benchmark dependencies are met that benchmark apps can then assume to be present. What it means is not very well defined, and it would likely be an improvement if it was broken down into more fine-grained feature flags. It's probably fine to enable for RISC-V because it now supports reading the cycle counter. Although support for reading other event counters only seems implemented for hifive, which could some benchmark apps to not work on non-hifive platforms.

@kent-mcleod
Copy link
Member

So essentially, this setting should only be enabled for platforms that can properly run all of the benchmarks that it enables.

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

No branches or pull requests

3 participants