hw: distinct pagefault and IPC message type

ref #874

base-hw/include/base/ipc_pager.h

@@ -79,30 +79,28 @@ namespace Genode
 	{
 		private:
 
-			Pagefault _pagefault; /* data of lastly received pagefault */
+			Pagefault_msg _pagefault_msg;
-			Mapping   _mapping; /* mapping to resolve last pagefault */
+			Mapping       _mapping;
 
 			/**
 			 * Backend for wait_for_fault and wait_for_first_fault
 			 */
-			void _wait_for_fault(size_t s);
+			void _wait_for_fault();
+
+			/**
+			 * Get global name of pager thread
+			 */
+			static unsigned _thread_id()
+			{
+				return Genode::thread_get_my_native_id();
+			}
 
 		public:
 
 			/**
 			 * Constructor
 			 */
-			Ipc_pager() :
+			Ipc_pager() : Native_capability(_thread_id(), 0) { }
-				Native_capability(Genode::thread_get_my_native_id(), 0)
-			{
-				/* check if we can distinguish all message types */
-				if (sizeof(Pagefault) == sizeof(Pagefault_resolved))
-				{
-					kernel_log() << __PRETTY_FUNCTION__
-					             << ": Message types indiscernible\n";
-					while (1) ;
-				}
-			}
 
 			/**
 			 * Wait for the first pagefault request
@@ -125,12 +123,12 @@ namespace Genode
 			/**
 			 * Request instruction pointer of current page fault
 			 */
-			addr_t fault_ip() { return _pagefault.virt_ip; }
+			addr_t fault_ip() { return _pagefault_msg.virt_ip; }
 
 			/**
 			 * Request fault address of current page fault
 			 */
-			addr_t fault_addr() { return _pagefault.virt_address; }
+			addr_t fault_addr() { return _pagefault_msg.virt_address; }
 
 			/**
 			 * Set parameters for next reply
@@ -159,17 +157,17 @@ namespace Genode
 			/**
 			 * Return thread ID of last faulter
 			 */
-			Native_thread_id last() const { return _pagefault.thread_id; }
+			Native_thread_id last() const { return _pagefault_msg.thread_id; }
 
 			/**
 			 * Return badge for faulting thread
 			 */
-			unsigned badge() const { return _pagefault.thread_id; }
+			unsigned badge() const { return _pagefault_msg.thread_id; }
 
 			/**
 			 * Return true if last fault was a write fault
 			 */
-			bool is_write_fault() const { return _pagefault.write; }
+			bool is_write_fault() const { return _pagefault_msg.write; }
 
 			/**
 			 * Return true if last fault was an exception

base-hw/include/base/native_types.h

@@ -48,61 +48,52 @@ namespace Genode
 	 */
 	inline Native_thread_id thread_invalid_id() { return 0; }
 
-	/**
-	 * Describes a pagefault
-	 */
-	struct Pagefault
-	{
-		unsigned thread_id;    /* thread ID of the faulter */
-		Tlb *    tlb;          /* TLB to wich the faulter is assigned */
-		addr_t   virt_ip;      /* the faulters virtual instruction pointer */
-		addr_t   virt_address; /* virtual fault address */
-		bool     write;        /* write access attempted at fault? */
-
-		/**
-		 * Placement new operator
-		 */
-		void * operator new (size_t, void * p) { return p; }
-
-		/**
-		 * Construct invalid pagefault
-		 */
-		Pagefault() : thread_id(0) { }
-
-		/**
-		 * Construct valid pagefault
-		 */
-		Pagefault(unsigned const tid, Tlb * const tlb,
-		          addr_t const vip, addr_t const va, bool const w)
-		:
-			thread_id(tid), tlb(tlb), virt_ip(vip),
-			virt_address(va), write(w)
-		{ }
-
-		/**
-		 * Validation
-		 */
-		bool valid() const { return thread_id != 0; }
-	};
-
-	/**
-	 * Types of synchronously communicated messages
-	 */
-	struct Msg_type
-	{
-		enum Id {
-			INVALID = 0,
-			IPC     = 1,
-		};
-	};
-
 	/**
 	 * Message that is communicated synchronously
 	 */
 	struct Msg
 	{
-		Msg_type::Id type;
+		/**
-		uint8_t      data[];
+		 * Types of synchronously communicated messages
+		 */
+		struct Type
+		{
+			enum Id {
+				INVALID   = 0,
+				IPC       = 1,
+				PAGEFAULT = 2,
+			};
+		};
+
+		Type::Id type;
+		uint8_t  data[];
+	};
+
+	/**
+	 * Message that reports a pagefault
+	 */
+	struct Pagefault_msg : Msg
+	{
+		unsigned thread_id;
+		Tlb *    tlb;
+		addr_t   virt_ip;
+		addr_t   virt_address;
+		bool     write;
+
+		static void init(void * const p, unsigned const tid, Tlb * const tlb,
+		                 addr_t const vip, addr_t const va, bool const w)
+		{
+			Pagefault_msg * msg = (Pagefault_msg *)p;
+			msg->Msg::type      = Msg::Type::PAGEFAULT;
+			msg->thread_id      = tid;
+			msg->tlb            = tlb;
+			msg->virt_ip        = vip;
+			msg->virt_address   = va;
+			msg->write          = w;
+		}
+
+		void * base() { return this; }
+		size_t size() { return sizeof(Pagefault_msg); }
 	};
 
 	/**
@@ -120,30 +111,43 @@ namespace Genode
 	struct Native_utcb
 	{
 		union {
-			uint8_t data[1 << MIN_MAPPING_SIZE_LOG2];
+			uint8_t       data[1 << MIN_MAPPING_SIZE_LOG2];
-			Msg     msg;
+			Msg           msg;
-			Ipc_msg ipc_msg;
+			Ipc_msg       ipc_msg;
+			Pagefault_msg pagefault_msg;
 		};
 
-		/**
+		void syscall_wait_for_request(void * & buf_base, size_t & buf_size)
-		 * Get the base of the UTCB region
+		{
-		 */
+			msg.type = Msg::Type::INVALID;
-		void * base() { return data; }
+			buf_base = base();
+			buf_size = size();
+		}
+
+		void syscall_request_and_wait(void * & msg_base, size_t & msg_size,
+		                              void * & buf_base, size_t & buf_size)
+		{
+			msg.type = Msg::Type::IPC;
+			msg_base = ipc_msg_base();
+			msg_size = ipc_msg_size();
+			buf_base = base();
+			buf_size = size();
+		}
+
+		void syscall_reply(void * & msg_base, size_t & msg_size)
+		{
+			msg.type = Msg::Type::IPC;
+			msg_base = ipc_msg_base();
+			msg_size = ipc_msg_size();
+		}
 
-		/**
-		 * Get the size of the UTCB region
-		 */
 		size_t size() { return sizeof(data) / sizeof(data[0]); }
-
+		void * base() { return &data; }
-		/**
+		addr_t top() { return (addr_t)base() + size(); }
-		 * Get the top of the UTCB region
+		void * ipc_msg_base() { return &ipc_msg; }
-		 */
+		size_t ipc_msg_size() { return ipc_msg_header_size() + ipc_msg.size; }
-		addr_t top()  { return (addr_t)data + size(); }
+		size_t ipc_msg_max_size() { return top() - (addr_t)&ipc_msg; }
-
+		size_t ipc_msg_header_size() { return (addr_t)ipc_msg.data - (addr_t)&ipc_msg; }
-		/**
-		 * Maximum size of an IPC message that can be held by the UTCB
-		 */
-		size_t ipc_msg_max_size() { return top() - (addr_t)ipc_msg.data; }
 	};
 
 	struct Cap_dst_policy

base-hw/src/base/ipc.cc

@@ -123,7 +123,7 @@ void Ipc_client::_call()
 
 	/* receive reply */
 	Native_utcb * const utcb = Thread_base::myself()->utcb();
-	if (utcb->msg.type != Msg_type::IPC) {
+	if (utcb->msg.type != Msg::Type::IPC) {
 		PERR("failed to receive reply");
 		throw Blocking_canceled();
 	}
@@ -169,7 +169,7 @@ void Ipc_server::_wait()
 	/* receive next request */
 	Kernel::wait_for_request();
 	Native_utcb * const utcb = Thread_base::myself()->utcb();
-	if (utcb->msg.type != Msg_type::IPC) {
+	if (utcb->msg.type != Msg::Type::IPC) {
 		PERR("failed to receive request");
 		throw Blocking_canceled();
 	}
@@ -202,7 +202,7 @@ void Ipc_server::_reply_wait()
 
 	/* fetch request */
 	Native_utcb * const utcb = Thread_base::myself()->utcb();
-	if (utcb->msg.type != Msg_type::IPC) {
+	if (utcb->msg.type != Msg::Type::IPC) {
 		PERR("failed to receive request");
 		throw Blocking_canceled();
 	}

base-hw/src/base/pager.cc

@@ -95,13 +95,8 @@ Pager_capability Pager_entrypoint::manage(Pager_object * const o)
 
 void Ipc_pager::wait_for_first_fault()
 {
-	Native_utcb * const utcb = Thread_base::myself()->utcb();
+	Kernel::wait_for_request();
-	while (1) {
+	_wait_for_fault();
-		Kernel::wait_for_request();
-		if (utcb->msg.type == Msg_type::IPC) { break; }
-		PERR("failed to receive fault");
-	}
-	_wait_for_fault(utcb->ipc_msg.size);
 }
 
 
@@ -110,62 +105,41 @@ void Ipc_pager::wait_for_fault()
 	Native_utcb * const utcb = Thread_base::myself()->utcb();
 	utcb->ipc_msg.size = 0;
 	Kernel::reply(1);
-	while (utcb->msg.type != Msg_type::IPC) {
+	_wait_for_fault();
-		PERR("failed to receive fault");
-		Kernel::wait_for_request();
-	}
-	_wait_for_fault(utcb->ipc_msg.size);
 }
 
 
-void Ipc_pager::_wait_for_fault(size_t s)
+void Ipc_pager::_wait_for_fault()
 {
+	Native_utcb * const utcb = Thread_base::myself()->utcb();
 	while (1)
 	{
-		/*
+		switch (utcb->msg.type) {
-		 * FIXME: the message size is a weak indicator for the message type
-		 */
-		switch (s)
-		{
-		case sizeof(Pagefault): {
 
-			/* message is a pagefault */
+		case Msg::Type::PAGEFAULT: {
-			Native_utcb * const utcb = Thread_base::myself()->utcb();
-			Pagefault * const pf = (Pagefault *)utcb->ipc_msg.data;
-			if (pf->valid())
-			{
-				/* give our caller the chance to handle the fault */
-				_pagefault = *pf;
-				return;
-			}
-			/* pagefault is invalid so get the next message */
-			else {
-				PERR("invalid pagefault");
-				continue;
-			}
-			continue; }
 
-		case sizeof(Pagefault_resolved): {
+			/* receive pagefault report */
+			_pagefault_msg = utcb->pagefault_msg;
+			return; }
 
-			/* message is a release request from a RM session */
+		case Msg::Type::IPC: {
+
+			/* receive release request from region manager */
 			Native_utcb * const utcb = Thread_base::myself()->utcb();
 			void * const msg_base = utcb->ipc_msg.data;
 			Pagefault_resolved * const msg = (Pagefault_resolved *)msg_base;
 
-			/* resume faulter, send ack to RM and get the next message */
+			/* resume faulter */
 			Kernel::resume_thread(msg->pager_object->badge());
 			utcb->ipc_msg.size = 0;
+
+			/* send ack to region manager and get next message */
 			Kernel::reply(1);
-			while (utcb->msg.type != Msg_type::IPC) {
-				PERR("failed to receive fault");
-				Kernel::wait_for_request();
-			}
-			s = utcb->ipc_msg.size;
 			continue; }
 
 		default: {
 
-			PERR("invalid message format");
+			PERR("unknown message type");
 			continue; }
 		}
 	}

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

@@ -24,7 +24,6 @@
 using namespace Kernel;
 
 typedef Genode::Thread_state Thread_state;
-typedef Genode::Msg_type     Msg_type;
 
 
 bool Thread::_core() const
@@ -91,8 +90,6 @@ void Thread::_received_ipc_request(size_t const s)
 {
 	switch (_state) {
 	case SCHEDULED:
-		_phys_utcb->msg.type = Msg_type::IPC;
-		_phys_utcb->ipc_msg.size = s;
 		return;
 	default:
 		PERR("wrong thread state to receive IPC");
@@ -122,8 +119,6 @@ void Thread::_await_ipc_succeeded(size_t const s)
 {
 	switch (_state) {
 	case AWAITS_IPC:
-		_phys_utcb->msg.type = Msg_type::IPC;
-		_phys_utcb->ipc_msg.size = s;
 		_schedule();
 		return;
 	case AWAITS_PAGER_IPC:
@@ -144,7 +139,6 @@ void Thread::_await_ipc_failed()
 {
 	switch (_state) {
 	case AWAITS_IPC:
-		_phys_utcb->msg.type = Msg_type::INVALID;
 		_schedule();
 		return;
 	case SCHEDULED:
@@ -340,10 +334,10 @@ void Thread::_mmu_exception()
 		PERR("unknown MMU exception");
 		return;
 	}
-	/* inform pager */
+	/* send pagefault message to pager */
-	_pagefault = Pagefault(id(), (Tlb *)tlb(), ip, va, w);
+	Pagefault_msg::init(&_pagefault_msg, id(), (Tlb *)tlb(), ip, va, w);
-	void * const base = &_pagefault;
+	void * const base = _pagefault_msg.base();
-	size_t const size = sizeof(_pagefault);
+	size_t const size = _pagefault_msg.size();
 	Ipc_node::send_request_await_reply(_pager, base, size, base, size);
 }
 
@@ -605,8 +599,9 @@ void Thread::_syscall_get_thread()
  */
 void Thread::_syscall_wait_for_request()
 {
-	void * const buf_base = _phys_utcb->ipc_msg.data;
+	void * buf_base;
-	size_t const buf_size = _phys_utcb->ipc_msg_max_size();
+	size_t buf_size;
+	_phys_utcb->syscall_wait_for_request(buf_base, buf_size);
 	Ipc_node::await_request(buf_base, buf_size);
 }
 
@@ -622,9 +617,14 @@ void Thread::_syscall_request_and_wait()
 		_await_ipc();
 		return;
 	}
-	Ipc_node::send_request_await_reply(
+	void * msg_base;
-		dst, _phys_utcb->ipc_msg.data, _phys_utcb->ipc_msg.size,
+	size_t msg_size;
-		_phys_utcb->ipc_msg.data, _phys_utcb->ipc_msg_max_size());
+	void * buf_base;
+	size_t buf_size;
+	_phys_utcb->syscall_request_and_wait(msg_base, msg_size,
+	                                     buf_base, buf_size);
+	Ipc_node::send_request_await_reply(dst, msg_base, msg_size,
+	                                   buf_base, buf_size);
 }
 
 
@@ -633,16 +633,12 @@ void Thread::_syscall_request_and_wait()
  */
 void Thread::_syscall_reply()
 {
-	bool const await_request = user_arg_1();
+	void * msg_base;
-	void * const msg_base = _phys_utcb->ipc_msg.data;
+	size_t msg_size;
-	size_t const msg_size = _phys_utcb->ipc_msg.size;
+	_phys_utcb->syscall_reply(msg_base, msg_size);
 	Ipc_node::send_reply(msg_base, msg_size);
-
+	bool const await_request = user_arg_1();
-	if (await_request) {
+	if (await_request) { _syscall_wait_for_request(); }
-		void * const buf_base = _phys_utcb->ipc_msg.data;
-		size_t const buf_size = _phys_utcb->ipc_msg_max_size();
-		Ipc_node::await_request(buf_base, buf_size);
-	} else { _phys_utcb->msg.type = Msg_type::INVALID; }
 }
 
 

base-hw/src/core/kernel/thread.h

@@ -32,9 +32,9 @@ namespace Kernel
 	class Thread;
 	class Pd;
 
-	typedef Genode::Cpu         Cpu;
+	typedef Genode::Cpu           Cpu;
-	typedef Genode::Pagefault   Pagefault;
+	typedef Genode::Pagefault_msg Pagefault_msg;
-	typedef Genode::Native_utcb Native_utcb;
+	typedef Genode::Native_utcb   Native_utcb;
 
 	void reset_lap_time();
 
@@ -82,7 +82,7 @@ class Kernel::Thread
 
 		Platform_thread * const _platform_thread;
 		State                   _state;
-		Pagefault               _pagefault;
+		Pagefault_msg           _pagefault_msg;
 		Thread *                _pager;
 		unsigned                _pd_id;
 		Native_utcb *           _phys_utcb;

base-hw/src/core/rm_session_support.cc

@@ -60,7 +60,7 @@ int Ipc_pager::resolve_and_wait_for_fault()
 		return -1;
 	}
 	/* prepare mapping */
-	Tlb * const tlb = _pagefault.tlb;
+	Tlb * const tlb = _pagefault_msg.tlb;
 	Page_flags::access_t const flags =
 	Page_flags::resolve_and_wait_for_fault(_mapping.writable,
 	                                       _mapping.write_combined,
@@ -91,7 +91,7 @@ int Ipc_pager::resolve_and_wait_for_fault()
 		}
 	}
 	/* wake up faulter */
-	Kernel::resume_faulter(_pagefault.thread_id);
+	Kernel::resume_faulter(_pagefault_msg.thread_id);
 
 	/* wait for next page fault */
 	wait_for_fault();