hw: sanitize scheduler, cpu, and timer interplay

* Remove afiinity artefact from Cpu class
* Scheduler::need_to_schedule => Scheduler::ned_to_update (consistency)
* Don't re-use last scheduler context after new scheduling
  decision, might have been deleted (potential use-after-free)
* Move Timer::process_update into timer's interrupt handling routine
* Move execution time of Cpu_context into Scheduler::Context
* Re-order scheduler and idle context initialization (ref before initialized)

Ref #5509

repos/base-hw/src/core/kernel/cpu.cc

@@ -68,14 +68,6 @@ void Cpu_context::_interrupt(Irq::Pool &user_irq_pool)
 }
 
 
-void Cpu_context::affinity(Cpu &cpu)
-{
-	_cpu().scheduler().remove(*this);
-	_cpu_ptr = &cpu;
-	_cpu().scheduler().insert(*this);
-}
-
-
 void Cpu_context::quota(unsigned const q)
 {
 	_cpu().scheduler().quota(*this, q);
@@ -122,7 +114,7 @@ Cpu::Idle_thread::Idle_thread(Board::Address_space_id_allocator &addr_space_id_a
 void Cpu::assign(Context &context)
 {
 	_scheduler.ready(static_cast<Scheduler::Context&>(context));
-	if (_id != executing_id() && _scheduler.need_to_schedule())
+	if (_id != executing_id() && _scheduler.need_to_update())
 		trigger_ip_interrupt();
 }
 
@@ -139,22 +131,12 @@ bool Cpu::handle_if_cpu_local_interrupt(unsigned const irq_id)
 }
 
 
-Cpu::Context & Cpu::schedule_next_context(Context &last)
+Cpu::Context & Cpu::schedule_next_context()
 {
 	if (_state == SUSPEND || _state == HALT)
 		return _halt_job;
 
-	/* update schedule if necessary */
-	if (_scheduler.need_to_schedule()) {
-		_timer.process_timeouts();
-		_scheduler.update(_timer.time());
-		time_t t = _scheduler.current_time_left();
-		_timer.set_timeout(&_timeout, t);
-		time_t duration = _timer.schedule_timeout();
-		last.update_execution_time(duration);
-	}
-
-	/* return current context */
+	_scheduler.update();
 	return current_context();
 }
 
@@ -182,9 +164,9 @@ Cpu::Cpu(unsigned                     const  id,
 	_id               { id },
 	_pic              { global_irq_ctrl },
 	_timer            { *this },
-	_scheduler        { _idle, _quota(), _fill() },
 	_idle             { addr_space_id_alloc, user_irq_pool, cpu_pool, *this,
 	                    core_pd },
+	_scheduler        { _timer, _idle, _quota(), _fill() },
 	_ipi_irq          { *this },
 	_global_work_list { cpu_pool.work_list() }
 {

repos/base-hw/src/core/kernel/cpu.h

@@ -99,10 +99,9 @@ class Kernel::Cpu : public Core::Cpu, private Irq::Pool,
 		State          _state { RUN };
 		unsigned const _id;
 		Board::Pic     _pic;
-		Timeout        _timeout {};
 		Timer          _timer;
-		Scheduler      _scheduler;
 		Idle_thread    _idle;
+		Scheduler      _scheduler;
 		Ipi            _ipi_irq;
 
 		Inter_processor_work_list &_global_work_list;
@@ -150,7 +149,7 @@ class Kernel::Cpu : public Core::Cpu, private Irq::Pool,
 		/**
 		 * Return the context that should be executed next
 		 */
-		Context& schedule_next_context(Context &last);
+		Context& schedule_next_context();
 
 		Board::Pic & pic()   { return _pic; }
 		Timer      & timer() { return _timer; }

repos/base-hw/src/core/kernel/cpu_context.h

@@ -35,8 +35,7 @@ class Kernel::Cpu_context : private Scheduler::Context
 
 		friend class Cpu;
 
-		time_t _execution_time { 0 };
-		Cpu   *_cpu_ptr;
+		Cpu *_cpu_ptr;
 
 		/*
 		 * Noncopyable
@@ -83,25 +82,12 @@ class Kernel::Cpu_context : private Scheduler::Context
 
 		virtual ~Cpu_context();
 
-		/**
-		 * Link context to CPU 'cpu'
-		 */
-		void affinity(Cpu &cpu);
-
 		/**
 		 * Set CPU quota of the context to 'q'
 		 */
 		void quota(unsigned const q);
 
-		/**
-		 * Update total execution time
-		 */
-		void update_execution_time(time_t duration) { _execution_time += duration; }
-
-		/**
-		 * Return total execution time
-		 */
-		time_t execution_time() const { return _execution_time; }
+		using Scheduler::Context::execution_time;
 
 		/**
 		 * Handle exception that occured during execution of this context

repos/base-hw/src/core/kernel/main.cc

@@ -70,7 +70,7 @@ void Kernel::Main::_handle_kernel_entry(Genode::Cpu_state *state)
 			Cpu &cpu = _cpu_pool.cpu(Cpu::executing_id());
 			Cpu::Context &recent = cpu.current_context();
 			if (state) recent.exception(*state);
-			context = &cpu.schedule_next_context(recent);
+			context = &cpu.schedule_next_context();
 		},
 		[&] () { _cpu_pool.cpu(Cpu::executing_id()).panic(*state); });
 

repos/base-hw/src/core/kernel/scheduler.cc

@@ -51,6 +51,13 @@ Scheduler::Context::~Context()
 }
 
 
+void Scheduler::Timeout::timeout_triggered()
+{
+	if (_scheduler._state == UP_TO_DATE)
+		_scheduler._state = OUT_OF_DATE;
+}
+
+
 void Scheduler::_consumed(unsigned const time)
 {
 	if (_super_period_left > time) {
@@ -149,15 +156,21 @@ bool Scheduler::_schedule_slack()
 }
 
 
-void Scheduler::update(time_t time)
+void Scheduler::update()
 {
 	using namespace Genode;
 
+	if (!need_to_update())
+		return;
+
+	time_t time = _timer.time();
 	unsigned const duration =
 		min(min((unsigned)(time-_last_time), _current_quantum),
 		    _super_period_left);
 	_last_time = time;
 
+	if (_current) _current->_execution_time += duration;
+
 	/* do not detract the quota of idle or removed context */
 	if (_current && _current != &_idle) {
 		unsigned const r = (_state != YIELD) ? _current_quantum - duration : 0;
@@ -169,13 +182,14 @@ void Scheduler::update(time_t time)
 
 	_state = UP_TO_DATE;
 
-	if (_schedule_priotized())
-		return;
+	if (!_schedule_priotized())
+		if (!_schedule_slack())
+			_set_current(_idle, _slack_quota);
 
-	if (_schedule_slack())
-		return;
+	_timer.set_timeout(&_timeout,
+	                   Genode::min(_current_quantum, _super_period_left));
+	_timer.schedule_timeout();
 
-	_set_current(_idle, _slack_quota);
 }
 
 
@@ -298,18 +312,20 @@ Scheduler::Context& Scheduler::current()
 {
 	if (!_current) {
 		Genode::error("attempt to access invalid scheduler's current context");
-		update(_last_time);
+		update();
 	}
 	return *_current;
 }
 
 
-Scheduler::Scheduler(Context       &idle,
+Scheduler::Scheduler(Timer         &timer,
+                     Context       &idle,
                      unsigned const super_period_length,
                      unsigned const slack_quota)
 :
 	_slack_quota(slack_quota),
 	_super_period_length(super_period_length),
+	_timer(timer),
 	_idle(idle)
 {
 	_set_current(idle, slack_quota);

repos/base-hw/src/core/kernel/scheduler.h

@@ -20,6 +20,7 @@
 #include <util/list.h>
 #include <util/misc_math.h>
 #include <kernel/configuration.h>
+#include <kernel/timer.h>
 
 namespace Kernel { class Scheduler; }
 
@@ -79,6 +80,8 @@ class Kernel::Scheduler
 				List         _helper_list {};
 				Context     *_destination { nullptr };
 
+				time_t _execution_time { 0 };
+
 				bool _ready { false };
 
 				void _reset() { _priotized_time_left = _quota; }
@@ -104,6 +107,9 @@ class Kernel::Scheduler
 				void help(Context &c);
 				void helping_finished();
 				Context& helping_destination();
+
+				time_t execution_time() const {
+					return _execution_time; }
 		};
 
 	private:
@@ -174,6 +180,15 @@ class Kernel::Scheduler
 				}
 		};
 
+		struct Timeout : Kernel::Timeout
+		{
+			Scheduler &_scheduler;
+
+			Timeout(Scheduler &scheduler) : _scheduler(scheduler) {}
+
+			virtual void timeout_triggered() override;
+		};
+
 		enum State { UP_TO_DATE, OUT_OF_DATE, YIELD };
 
 		unsigned const _slack_quota;
@@ -182,8 +197,11 @@ class Kernel::Scheduler
 		unsigned _super_period_left { _super_period_length };
 		unsigned _current_quantum { 0 };
 
-		time_t _last_time { 0 };
-		State  _state { UP_TO_DATE };
+		Timer  &_timer;
+		Timeout _timeout { *this };
+		time_t  _last_time { 0 };
+
+		State _state { UP_TO_DATE };
 
 		Context_list _rpl[cpu_priorities]; /* ready lists by priority   */
 		Context_list _upl[cpu_priorities]; /* unready lists by priority */
@@ -206,21 +224,25 @@ class Kernel::Scheduler
 		bool _schedule_priotized();
 		bool _schedule_slack();
 
+		/**
+		 * Noncopyable
+		 */
+		Scheduler(const Scheduler&) = delete;
+		Scheduler& operator=(const Scheduler&) = delete;
+
 	public:
 
-		Scheduler(Context       &idle,
+		Scheduler(Timer         &timer,
+		          Context       &idle,
 		          unsigned const super_period_length,
 		          unsigned const slack_quota);
 
-		bool need_to_schedule() const { return _state != UP_TO_DATE; }
-
-		void timeout() {
-			if (_state == UP_TO_DATE) _state = OUT_OF_DATE; }
+		bool need_to_update() const { return _state != UP_TO_DATE; }
 
 		/**
-		 * Update state according to the current (absolute) time
+		 * Update state
 		 */
-		void update(time_t time);
+		void update();
 
 		/**
 		 * Set 'context' ready
@@ -253,9 +275,6 @@ class Kernel::Scheduler
 		void quota(Context &context, unsigned const quota);
 
 		Context& current();
-
-		unsigned current_time_left() const {
-			return Genode::min(_current_quantum, _super_period_left); }
 };
 
 #endif /* _CORE__KERNEL__SCHEDULER_H_ */

repos/base-hw/src/core/kernel/timer.cc

@@ -20,13 +20,13 @@
 using namespace Kernel;
 
 
-void Timer::Irq::occurred() { _cpu.scheduler().timeout(); }
+void Timer::Irq::occurred() { _timer._process_timeouts(); }
 
 
 Timer::Irq::Irq(unsigned id, Cpu &cpu)
 :
 	Kernel::Irq { id, cpu.irq_pool(), cpu.pic() },
-	_cpu        { cpu }
+	_timer { cpu.timer() }
 { }
 
 
@@ -79,7 +79,7 @@ time_t Timer::schedule_timeout()
 }
 
 
-void Timer::process_timeouts()
+void Timer::_process_timeouts()
 {
 	/*
 	 * Walk through timeouts until the first whose end time is in the future.
@@ -100,7 +100,7 @@ void Timer::process_timeouts()
 }
 
 
-Timer::Timer(Cpu & cpu)
+Timer::Timer(Cpu &cpu)
 :
 	_device(cpu.id()), _irq(interrupt_id(), cpu),
 	_last_timeout_duration(_max_value())

repos/base-hw/src/core/kernel/timer.h

@@ -65,11 +65,11 @@ class Kernel::Timer
 		{
 			private:
 
-				Cpu & _cpu;
+				Timer &_timer;
 
 			public:
 
-				Irq(unsigned id, Cpu & cpu);
+				Irq(unsigned id, Cpu &cpu);
 
 				void occurred() override;
 		};
@@ -88,6 +88,8 @@ class Kernel::Timer
 
 		time_t _duration() const;
 
+		void _process_timeouts();
+
 	public:
 
 		Timer(Cpu & cpu);
@@ -97,8 +99,6 @@ class Kernel::Timer
 		 */
 		time_t schedule_timeout();
 
-		void process_timeouts();
-
 		void set_timeout(Timeout * const timeout, time_t const duration);
 
 		time_t us_to_ticks(time_t const us) const;