hw: move cpu kernel object into cpu local area

Fix genodelabs/genode#5310

repos/base-hw/src/core/board/pc/board.h

@@ -19,20 +19,15 @@
 /* PC virtualization */
 #include <spec/x86_64/virtualization/board.h>
 
-/*
- * As long as Board::NR_OF_CPUS is used as constant within pic.h
- * we sadly have to include Hw::Pc_board into the namespace
- * before including pic.h. This will change as soon as the number
- * O CPUs is only define per function
-*/
-namespace Board { using namespace Hw::Pc_board; };
-
 /* base-hw core includes */
 #include <spec/x86_64/pic.h>
 #include <spec/x86_64/pit.h>
 #include <spec/x86_64/cpu.h>
 
 namespace Board {
+
+	using namespace Hw::Pc_board;
+
 	class Pic : public Local_interrupt_controller
 	{ using Local_interrupt_controller::Local_interrupt_controller; };
 

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

@@ -22,6 +22,7 @@
 #include <board.h>
 #include <hw/assert.h>
 #include <hw/boot_info.h>
+#include <hw/memory_consts.h>
 
 using namespace Kernel;
 
@@ -154,7 +155,7 @@ Cpu_job & Cpu::schedule()
 		_timer.process_timeouts();
 		_scheduler.update(_timer.time());
 		time_t t = _scheduler.current_time_left();
-		_timer.set_timeout(this, t);
+		_timer.set_timeout(&_timeout, t);
 		time_t duration = _timer.schedule_timeout();
 		old_job.update_execution_time(duration);
 	}
@@ -188,6 +189,17 @@ Cpu::Cpu(unsigned                     const  id,
 	_global_work_list { cpu_pool.work_list() }
 {
 	_arch_init();
+
+	/*
+	 * We insert the cpu objects in order into the cpu_pool's list
+	 * to ensure that the cpu with the lowest given id is the first
+	 * one.
+	 */
+	Cpu * cpu = cpu_pool._cpus.first();
+	while (cpu && cpu->next() && (cpu->next()->id() < _id))
+		cpu = cpu->next();
+	cpu = (cpu && cpu->id() < _id) ? cpu : nullptr;
+	cpu_pool._cpus.insert(this, cpu);
 }
 
 
@@ -195,6 +207,15 @@ Cpu::Cpu(unsigned                     const  id,
  ** Cpu_pool **
  **************/
 
+template <typename T>
+static inline T* cpu_object_by_id(unsigned const id)
+{
+	using namespace Hw::Mm;
+	addr_t base = CPU_LOCAL_MEMORY_AREA_START + id*CPU_LOCAL_MEMORY_SLOT_SIZE;
+	return (T*)(base + CPU_LOCAL_MEMORY_SLOT_OBJECT_OFFSET);
+}
+
+
 void
 Cpu_pool::
 initialize_executing_cpu(Board::Address_space_id_allocator  &addr_space_id_alloc,
@@ -203,22 +224,13 @@ initialize_executing_cpu(Board::Address_space_id_allocator  &addr_space_id_alloc
                          Board::Global_interrupt_controller &global_irq_ctrl)
 {
 	unsigned id = Cpu::executing_id();
-	_cpus[id].construct(
+	Genode::construct_at<Cpu>(cpu_object_by_id<void>(id), id,
-		id, addr_space_id_alloc, user_irq_pool, *this, core_pd, global_irq_ctrl);
+	                          addr_space_id_alloc, user_irq_pool,
+	                          *this, core_pd, global_irq_ctrl);
 }
 
 
 Cpu & Cpu_pool::cpu(unsigned const id)
 {
-	assert(id < _nr_of_cpus && _cpus[id].constructed());
+	return *cpu_object_by_id<Cpu>(id);
-	return *_cpus[id];
 }
-
-
-using Boot_info = Hw::Boot_info<Board::Boot_info>;
-
-
-Cpu_pool::Cpu_pool(unsigned nr_of_cpus)
-:
-	_nr_of_cpus(nr_of_cpus)
-{ }

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

@@ -36,35 +36,8 @@ namespace Kernel {
 }
 
 
-/*
+class Kernel::Cpu : public Core::Cpu, private Irq::Pool,
- * The 'Cpu' class violates the "Effective C++" practices because it publicly
+                    public Genode::List<Cpu>::Element
- * inherits the 'Genode::Cpu' base class, which does not have a virtual
- * destructor. Since 'Cpu' implements the 'Timeout' interface, however, it has
- * a vtable.
- *
- * Adding a virtual destructor in the base class would be unnatural as the base
- * class hierarchy does not represent an abstract interface.
- *
- * Inheriting the 'Genode::Cpu' class privately is not an option because the
- * user of 'Cpu' class expects architecture-specific register definitions to be
- * provided by 'Cpu'. Hence, all those architecture- specific definitions would
- * end up as 'using' clauses in the generic class.
- *
- * XXX Remove the disabled warning, e.g., by one of the following approaches:
- *
- * * Prevent 'Cpu' to have virtual methods by making 'Timeout' a member instead
- *   of a base class.
- *
- * * Change the class hierarchy behind 'Genode::Cpu' such that
- *   architecture-specific bits do no longer need to implicitly become part
- *   of the public interface of 'Cpu'. For example, register-definition types
- *   could all be embedded in an 'Arch_regs' type, which the 'Cpu' class could
- *   publicly provide via a 'typedef Genode::Cpu::Arch_regs Arch_regs'.
- *   Then, the 'Genode::Cpu' could be inherited privately.
- */
-#pragma GCC diagnostic ignored "-Wnon-virtual-dtor"
-
-class Kernel::Cpu : public Core::Cpu, private Irq::Pool, private Timeout
 {
 	private:
 
@@ -126,6 +99,7 @@ class Kernel::Cpu : public Core::Cpu, private Irq::Pool, private Timeout
 		State          _state { RUN };
 		unsigned const _id;
 		Board::Pic     _pic;
+		Timeout        _timeout {};
 		Timer          _timer;
 		Scheduler      _scheduler;
 		Idle_thread    _idle;
@@ -155,8 +129,6 @@ class Kernel::Cpu : public Core::Cpu, private Irq::Pool, private Timeout
 		    Pd                                 &core_pd,
 		    Board::Global_interrupt_controller &global_irq_ctrl);
 
-		static inline unsigned primary_id() { return 0; }
-
 		/**
 		 * Raise the IPI of the CPU
 		 */
@@ -212,60 +184,41 @@ class Kernel::Cpu : public Core::Cpu, private Irq::Pool, private Timeout
 };
 
 
-/*
- * See the comment above the 'Cpu' class definition.
- */
-#pragma GCC diagnostic pop
-
-
 class Kernel::Cpu_pool
 {
 	private:
 
 		Inter_processor_work_list  _global_work_list {};
-		unsigned                   _nr_of_cpus;
+		Genode::List<Cpu>          _cpus {};
-		Genode::Constructible<Cpu> _cpus[Board::NR_OF_CPUS];
+
+		friend class Cpu;
 
 	public:
 
-		Cpu_pool(unsigned nr_of_cpus);
-
 		void
 		initialize_executing_cpu(Board::Address_space_id_allocator  &addr_space_id_alloc,
 		                         Irq::Pool                          &user_irq_pool,
 		                         Pd                                 &core_pd,
 		                         Board::Global_interrupt_controller &global_irq_ctrl);
 
-		/**
-		 * Return whether CPU object is valid and is constructed.
-		 */
-		bool cpu_valid(unsigned const id) const {
-			return id < _nr_of_cpus && _cpus[id].constructed(); }
-
-		/**
-		 * Return object of CPU 'id'
-		 */
 		Cpu & cpu(unsigned const id);
 
 		/**
 		 * Return object of primary CPU
 		 */
-		Cpu & primary_cpu() { return cpu(Cpu::primary_id()); }
+		Cpu & primary_cpu() { return *_cpus.first(); }
-
-		/**
-		 * Return object of current CPU
-		 */
-		Cpu & executing_cpu() { return cpu(Cpu::executing_id()); }
 
 		void for_each_cpu(auto const &fn)
 		{
-			for (unsigned i = 0; i < _nr_of_cpus; i++) fn(cpu(i));
+			Cpu * c = _cpus.first();
+			while (c) {
+				fn(*c);
+				c = c->next();
+			}
 		}
 
 		Inter_processor_work_list & work_list() {
 			return _global_work_list; }
-
-		unsigned nr_of_cpus() { return _nr_of_cpus; }
 };
 
 #endif /* _CORE__KERNEL__CPU_H_ */

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

@@ -40,7 +40,7 @@ class Kernel::Main
 		static Main *_instance;
 
 		Lock                                    _data_lock           { };
-		Cpu_pool                                _cpu_pool;
+		Cpu_pool                                _cpu_pool            { };
 		Irq::Pool                               _user_irq_pool       { };
 		Board::Address_space_id_allocator       _addr_space_id_alloc { };
 		Core::Core_platform_pd                  _core_platform_pd    { _addr_space_id_alloc };
@@ -52,8 +52,6 @@ class Kernel::Main
 
 		void _handle_kernel_entry();
 
-		Main(unsigned nr_of_cpus);
-
 	public:
 
 		static Core::Platform_pd &core_platform_pd();
@@ -63,12 +61,6 @@ class Kernel::Main
 Kernel::Main *Kernel::Main::_instance;
 
 
-Kernel::Main::Main(unsigned nr_of_cpus)
-:
-	_cpu_pool { nr_of_cpus }
-{ }
-
-
 void Kernel::Main::_handle_kernel_entry()
 {
 	Cpu &cpu = _cpu_pool.cpu(Cpu::executing_id());
@@ -94,7 +86,7 @@ void Kernel::main_initialize_and_handle_kernel_entry()
 {
 	using Boot_info = Hw::Boot_info<Board::Boot_info>;
 
-	static volatile bool     instance_initialized   { false };
+	static Lock              init_lock;
 	static volatile unsigned nr_of_initialized_cpus { 0 };
 	static volatile bool     kernel_initialized     { false };
 
@@ -102,34 +94,27 @@ void Kernel::main_initialize_and_handle_kernel_entry()
 		*reinterpret_cast<Boot_info*>(Hw::Mm::boot_info().base) };
 
 	unsigned const nr_of_cpus  { boot_info.cpus };
-	bool     const primary_cpu { Cpu::executing_id() == Cpu::primary_id() };
 
-	if (primary_cpu) {
+	/**
+	 * Let the first CPU create a Main object and initialize the static
+	 * reference to it.
+	 */
+	{
+		Lock::Guard guard(init_lock);
 
-		/**
+		static Main instance;
-		 * Let the primary CPU create a Main object and initialize the static
-		 * reference to it.
-		 */
-		static Main instance { nr_of_cpus };
 		Main::_instance = &instance;
 
-	} else {
-
-		/**
-		 * Let secondary CPUs block until the primary CPU has managed to set
-		 * up the Main instance.
-		 */
-		while (!instance_initialized) { }
 	}
 
-	if (Main::_instance->_cpu_pool.cpu_valid(Cpu::executing_id())) {
+	/* the CPU resumed if the kernel is already initialized */
-		/* the CPU resumed since the cpu object is already valid */
+	if (kernel_initialized) {
+
 		{
 			Lock::Guard guard(Main::_instance->_data_lock);
 
-			if (kernel_initialized) {
+			if (nr_of_initialized_cpus == nr_of_cpus) {
 				nr_of_initialized_cpus = 0;
-				kernel_initialized     = false;
 
 				Main::_instance->_serial.init();
 				Main::_instance->_global_irq_ctrl.init();
@@ -140,12 +125,11 @@ void Kernel::main_initialize_and_handle_kernel_entry()
 			Main::_instance->_cpu_pool.cpu(Cpu::executing_id()).reinit_cpu();
 
 			if (nr_of_initialized_cpus == nr_of_cpus) {
-				kernel_initialized = true;
 				Genode::raw("kernel resumed");
 			}
 		}
 
-		while (!kernel_initialized) { }
+		while (nr_of_initialized_cpus < nr_of_cpus) { }
 
 		Main::_instance->_handle_kernel_entry();
 		/* never reached */
@@ -158,7 +142,6 @@ void Kernel::main_initialize_and_handle_kernel_entry()
 		 * CPU pool.
 		 */
 		Lock::Guard guard(Main::_instance->_data_lock);
-		instance_initialized = true;
 		Main::_instance->_cpu_pool.initialize_executing_cpu(
 			Main::_instance->_addr_space_id_alloc,
 			Main::_instance->_user_irq_pool,
@@ -174,42 +157,40 @@ void Kernel::main_initialize_and_handle_kernel_entry()
 	 */
 	while (nr_of_initialized_cpus < nr_of_cpus) { }
 
-	if (primary_cpu) {
+	/**
-
+	 * Let the primary CPU initialize the core main thread and finish
-		/**
+	 * initialization of the boot info.
-		 * Let the primary CPU initialize the core main thread and finish
+	 */
-		 * initialization of the boot info.
+	{
-		 */
-
 		Lock::Guard guard(Main::_instance->_data_lock);
 
-		Main::_instance->_cpu_pool.for_each_cpu([&] (Kernel::Cpu &cpu) {
+		if (Cpu::executing_id() == Main::_instance->_cpu_pool.primary_cpu().id()) {
-			boot_info.kernel_irqs.add(cpu.timer().interrupt_id());
+			Main::_instance->_cpu_pool.for_each_cpu([&] (Kernel::Cpu &cpu) {
-		});
+				boot_info.kernel_irqs.add(cpu.timer().interrupt_id());
-		boot_info.kernel_irqs.add((unsigned)Board::Pic::IPI);
+			});
+			boot_info.kernel_irqs.add((unsigned)Board::Pic::IPI);
 
-		Main::_instance->_core_main_thread.construct(
+			Main::_instance->_core_main_thread.construct(
-			Main::_instance->_addr_space_id_alloc,
+				Main::_instance->_addr_space_id_alloc,
-			Main::_instance->_user_irq_pool,
+				Main::_instance->_user_irq_pool,
-			Main::_instance->_cpu_pool,
+				Main::_instance->_cpu_pool,
-			Main::_instance->_core_platform_pd.kernel_pd());
+				Main::_instance->_core_platform_pd.kernel_pd());
 
-		boot_info.core_main_thread_utcb =
+			boot_info.core_main_thread_utcb =
-			(addr_t)Main::_instance->_core_main_thread->utcb();
+				(addr_t)Main::_instance->_core_main_thread->utcb();
 
-		Genode::log("");
+			Genode::log("");
-		Genode::log("kernel initialized");
+			Genode::log("kernel initialized");
-		kernel_initialized = true;
+			kernel_initialized = true;
-
+		}
-	} else {
-
-		/**
-		 * Let secondary CPUs block until the primary CPU has initialized the
-		 * core main thread and finished initialization of the boot info.
-		 */
-		while (!kernel_initialized) {;}
 	}
 
+	/**
+	 * Let secondary CPUs block until the primary CPU has initialized the
+	 * core main thread and finished initialization of the boot info.
+	 */
+	while (!kernel_initialized) {;}
+
 	Main::_instance->_handle_kernel_entry();
 }
 

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

@@ -718,9 +718,8 @@ void Thread::_call_new_irq()
 	Genode::Irq_session::Polarity polarity =
 		(Genode::Irq_session::Polarity) (user_arg_3() & 0b11);
 
-	_call_new<User_irq>(
+	_call_new<User_irq>((unsigned)user_arg_2(), trigger, polarity, *c,
-		(unsigned)user_arg_2(), trigger, polarity, *c,
+	                    _cpu->pic(), _user_irq_pool);
-		_cpu_pool.executing_cpu().pic(), _user_irq_pool);
 }
 
 

repos/base-hw/src/core/spec/x86_64/kernel/thread.cc

@@ -142,9 +142,8 @@ void Kernel::Thread::_call_suspend()
 
 	/* single core CPU case */
 	if (cpu_count == 1) {
-		auto &cpu = _cpu_pool.executing_cpu();
+		/* current CPU triggers final ACPI suspend outside kernel lock */
-		/* this CPU triggers final ACPI suspend outside kernel lock */
+		_cpu->next_state_suspend();
-		cpu.next_state_suspend();
 		return;
 	}
 

repos/base-hw/src/include/hw/spec/x86_64/pc_board.h

@@ -24,7 +24,11 @@ namespace Hw::Pc_board {
 	struct Serial;
 	enum Dummies { UART_BASE, UART_CLOCK };
 
-	static constexpr Genode::size_t NR_OF_CPUS = 32;
+	/**
+	 * The constant 'NR_OF_CPUS' defines the _maximum_ of cpus currently
+	 * supported on x86. The actual number is detected at booting.
+	 */
+	static constexpr Genode::size_t NR_OF_CPUS = 256;
 }