seL4 / sel4bench

sel4 benchmarking applications and support library.
Other
18 stars 30 forks source link

Ability to change bench app options for RiscV #17

Open malus-brandywine opened 2 years ago

malus-brandywine commented 2 years ago

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 commented 2 years ago

see PR https://github.com/seL4/sel4bench/pull/20

kent-mcleod commented 2 years ago

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 commented 2 years ago

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