From 693a2b5421a69aec3fd2c553f9ba96102ecc3001 Mon Sep 17 00:00:00 2001 From: Martin Stein Date: Fri, 9 Jul 2021 16:43:38 +0200 Subject: [PATCH] base-hw: get rid of cpu_pool() in platform.cc The function was only still used for reading the execution time of idle threads of CPUs. Certainly, it is technically fine and more performant to read these values directly from the kernel objects without doing a syscall. However, calling cpu_pool() for it provides read and write access to a lot more than only the execution time values. The interface via which Core directly reads state of the kernel should be as narrow and specific as possible. Perspectively, we want to get rid of the cpu_pool() accessor anyway. Therefore this commit introduces Kernel::read_idle_thread_execution_time(cpu_idx) as replacement. The function is implemented in kernel code and called by Core in platform.cc. Ref #4217 --- repos/base-hw/src/core/kernel/kernel.cc | 11 ++++++++ repos/base-hw/src/core/platform.cc | 37 ++++++++++++++----------- 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/repos/base-hw/src/core/kernel/kernel.cc b/repos/base-hw/src/core/kernel/kernel.cc index 43b22012f1..608fd9dddd 100644 --- a/repos/base-hw/src/core/kernel/kernel.cc +++ b/repos/base-hw/src/core/kernel/kernel.cc @@ -17,6 +17,17 @@ #include #include +namespace Kernel { + + time_t read_idle_thread_execution_time(unsigned cpu_idx); +} + + +Kernel::time_t Kernel::read_idle_thread_execution_time(unsigned cpu_idx) +{ + return cpu_pool().cpu(cpu_idx).idle_thread().execution_time(); +} + extern "C" void kernel() { diff --git a/repos/base-hw/src/core/platform.cc b/repos/base-hw/src/core/platform.cc index 663acd8835..4f42d9c8a4 100644 --- a/repos/base-hw/src/core/platform.cc +++ b/repos/base-hw/src/core/platform.cc @@ -22,13 +22,12 @@ #include #include #include -#include #include -#include /* base-internal includes */ #include #include +#include /* Genode includes */ #include @@ -36,6 +35,11 @@ using namespace Genode; +namespace Kernel { + + time_t read_idle_thread_execution_time(unsigned cpu_idx); +} + /************** ** Platform ** @@ -216,14 +220,13 @@ Platform::Platform() init_core_log(Core_log_range { core_local_addr, log_size } ); } - class Trace_source : public Trace::Source::Info_accessor, - private Trace::Control, - private Trace::Source + class Idle_thread_trace_source : public Trace::Source::Info_accessor, + private Trace::Control, + private Trace::Source { private: - Kernel::Thread &_thread; - Affinity::Location const _affinity; + Affinity::Location const _affinity; public: @@ -233,18 +236,18 @@ Platform::Platform() Info trace_source_info() const override { Trace::Execution_time execution_time { - _thread.execution_time(), 0 }; + Kernel::read_idle_thread_execution_time( + _affinity.xpos()), 0 }; - return { Session_label("kernel"), _thread.label(), + return { Session_label("kernel"), "idle", execution_time, _affinity }; } - Trace_source(Trace::Source_registry ®istry, - Kernel::Thread &thread, Affinity::Location affinity) + Idle_thread_trace_source(Trace::Source_registry ®istry, + Affinity::Location affinity) : Trace::Control { }, Trace::Source { *this, *this }, - _thread { thread }, _affinity { affinity } { registry.insert(this); @@ -252,10 +255,12 @@ Platform::Platform() }; /* create trace sources for idle threads */ - Kernel::cpu_pool().for_each_cpu([&] (Kernel::Cpu & cpu) { - new (core_mem_alloc()) Trace_source(Trace::sources(), cpu.idle_thread(), - Affinity::Location(cpu.id(), 0)); - }); + for (unsigned cpu_idx = 0; cpu_idx < _boot_info().cpus; cpu_idx++) { + + new (core_mem_alloc()) + Idle_thread_trace_source( + Trace::sources(), Affinity::Location(cpu_idx, 0)); + } log(_rom_fs); }