From c6e8acc037f331d4a6fb10863630eb36d118b2d1 Mon Sep 17 00:00:00 2001
From: Alexander Boettcher <alexander.boettcher@genode-labs.com>
Date: Sun, 9 Feb 2025 11:20:19 +0100
Subject: [PATCH] sel4: avoid unsynchronized vm state tracking

Remove _extra_dispatch_up which is not required and racy which may lead
to hangs between ep and vcpu thread.

Issue #5461
---
 repos/base-sel4/src/lib/base/x86/vm.cc | 83 +++++++++++---------------
 1 file changed, 34 insertions(+), 49 deletions(-)

diff --git a/repos/base-sel4/src/lib/base/x86/vm.cc b/repos/base-sel4/src/lib/base/x86/vm.cc
index a0119d8db5..adbc24cd74 100644
--- a/repos/base-sel4/src/lib/base/x86/vm.cc
+++ b/repos/base-sel4/src/lib/base/x86/vm.cc
@@ -80,8 +80,6 @@ struct Sel4_vcpu : Genode::Thread, Noncopyable
 		uint64_t                    _tsc_offset { 0 };
 		Semaphore                   _state_ready { 0 };
 		bool                        _dispatching { false };
-		bool                        _extra_dispatch_up { false };
-		void                       *_ep_handler    { nullptr };
 
 		Constructible<Sel4_native_rpc>     _rpc   { };
 
@@ -201,10 +199,7 @@ struct Sel4_vcpu : Genode::Thread, Noncopyable
 
 					_state_ready.up();
 
-					if (_extra_dispatch_up) {
-						_extra_dispatch_up = false;
-						_exit_handler.ready_semaphore().down();
-					}
+					_exit_handler.ready_semaphore().down();
 
 					continue;
 				}
@@ -244,17 +239,16 @@ struct Sel4_vcpu : Genode::Thread, Noncopyable
 
 				if (res != SEL4_VMENTER_RESULT_FAULT) {
 					Mutex::Guard guard(_remote_mutex);
-					if (_remote == PAUSE) {
+					if (_remote == PAUSE)
 						_remote = NONE;
-						_wake_up.down();
-					}
 				}
+
+				/* notify vCPU handler EP that state is valid */
 				_state_ready.up();
 
-				if (skip_dispatch)
-					continue;
-				/* notify VM handler */
-				Genode::Signal_transmitter(_exit_handler.signal_cap()).submit();
+				if (!skip_dispatch)
+					/* notify VM handler */
+					Genode::Signal_transmitter(_exit_handler.signal_cap()).submit();
 
 				/*
 				 * Wait until VM handler is really really done,
@@ -768,9 +762,14 @@ struct Sel4_vcpu : Genode::Thread, Noncopyable
 
 		void _wrapper_dispatch()
 		{
-			_dispatching = true;
-			_vcpu_handler.dispatch(1);
-			_dispatching = false;
+			try {
+				_dispatching = true;
+				_vcpu_handler.dispatch(1);
+				_dispatching = false;
+			} catch (...) {
+				_dispatching = false;
+				throw;
+			}
 		}
 
 	public:
@@ -787,7 +786,6 @@ struct Sel4_vcpu : Genode::Thread, Noncopyable
 
 			/* wait until thread is alive, e.g. Thread::cap() is valid */
 			_startup.block();
-			_ep_handler = reinterpret_cast<Thread *>(&handler.rpc_ep());
 
 			_rpc.construct(vm, this->cap(), *this);
 
@@ -811,48 +809,35 @@ struct Sel4_vcpu : Genode::Thread, Noncopyable
 
 		void with_state(auto const &fn)
 		{
-			if (!_dispatching) {
-				if (Thread::myself() != _ep_handler) {
-					error("vCPU state requested outside of vcpu_handler EP");
-					sleep_forever();
-				}
+			if (_dispatching) {
 
-				_remote_mutex.acquire();
+				/* wait for vCPU thread until it provides the state */
+				_state_ready.down();
+
+				if (fn(_state))
+					resume();
+
+				return;
+			}
+
+			{
+				Mutex::Guard guard(_remote_mutex);
 
 				/* Trigger pause exit */
 				_remote = PAUSE;
+
 				seL4_Signal(_recall);
 				_wake_up.up();
-
-				_remote_mutex.release();
-				_state_ready.down();
-
-				/*
-				 * We're in the async dispatch, yet processing a non-pause exit.
-				 * Signal that we have to wrap the dispatch loop around.
-				 */
-				if (_state.exit_reason != VMEXIT_RECALL) {
-					_extra_dispatch_up = true;
-				}
-			} else {
-				_state_ready.down();
 			}
 
-			if (fn(_state)
-			    || _extra_dispatch_up)
+			/* wait for vCPU thread until it provides the state */
+			_state_ready.down();
+
+			if (fn(_state))
 				resume();
 
-			/*
-			 * The regular exit was handled by the asynchronous dispatch handler
-			 * triggered by the pause request.
-			 *
-			 * Fake finishing the exit dispatch so that the vCPU loop
-			 * processes the asynchronously dispatched exit and provides
-			 * the VMEXIT_RECALL to the already pending dispatch function
-			 * for the exit code.
-			 */
-			if (!_dispatching && _extra_dispatch_up)
-				_exit_handler.ready_semaphore().up();
+			/* notify vCPU thread that we are done handling the state. */
+			_exit_handler.ready_semaphore().up();
 		}
 
 		Sel4_native_rpc * rpc()   { return &*_rpc; }