add ipc_notify testcase, remove ipc_ghost

+ A bit easier to implement just ignoring the ipc_resp() arguments than
  try to enforce that a notification is exited from by ipc_ghost(),
  especially in cases where the root process has been killed.

11 files changed, 111 insertions, 65 deletions

diff --git a/include/kmi/attrs.h b/include/kmi/attrs.h
index 56aca34..9038690 100644
--- a/include/kmi/attrs.h
+++ b/include/kmi/attrs.h
@@ -50,6 +50,7 @@
 /** Don't inline function. */
 #define __noinline __attribute__((noinline))
 
+/** Always inline function. */
 #define __inline inline __attribute__((always_inline))
 
 /** Function should not return. */
diff --git a/include/kmi/notify.h b/include/kmi/notify.h
index c0c5d8b..4e5b8b6 100644
--- a/include/kmi/notify.h
+++ b/include/kmi/notify.h
@@ -14,8 +14,8 @@
 #include <kmi/tcb.h>
 
 /**
- * Try to send a notification to a thread. Sets up a new rpc call, should be
- * returned from with \ref ipc_ghost() to restore register state.
+ * Try to send a notification to a thread. Sets up a new rpc call and sets
+ * notify flag of the frame to true.
  *
  * If the thread is idle, immediately swap to it.
  *
diff --git a/include/kmi/syscalls.h b/include/kmi/syscalls.h
index e3d5dad..28e7505 100644
--- a/include/kmi/syscalls.h
+++ b/include/kmi/syscalls.h
@@ -102,11 +102,8 @@ enum sys_code {
 	/** IPC response from server. */
 	SYS_IPC_RESP,
 
-	/** IPC return without visible side effects. */
-	SYS_IPC_GHOST,
-
 	/** Notify thread, essentially interrupt or signal. */
-	SYS_NOTIFY,
+	SYS_IPC_NOTIFY,
 	/** @} */
 
 	/** @name Process management. */
@@ -191,7 +188,7 @@ enum sys_user {
 
 /** Which notifications have arrived. */
 enum notify_flag {
-	/** A signal (\ref sys_notify()). */
+	/** A signal (\ref sys_ipc_notify()). */
 	NOTIFY_SIGNAL = (1 << 0),
 
 	/** A timer has expired. */
diff --git a/include/kmi/tcb.h b/include/kmi/tcb.h
index a3f119b..6cc63b5 100644
--- a/include/kmi/tcb.h
+++ b/include/kmi/tcb.h
@@ -115,7 +115,7 @@ struct tcb {
 	/** Memory mapping data. Only relevant in root thread. */
 	struct uvmem uvmem;
 
-	/** Address of callback function in servers. */
+	/** Address of callback function. */
 	vm_t callback;
 
 	/** Address of this thread's stack base. */
diff --git a/include/kmi/uapi.h b/include/kmi/uapi.h
index 95c7000..6e5f865 100644
--- a/include/kmi/uapi.h
+++ b/include/kmi/uapi.h
@@ -552,7 +552,13 @@ SYSCALL_DECLARE5(ipc_tail, pid, d0, d1, d2, d3);
 SYSCALL_DECLARE5(ipc_kick, pid, d0, d1, d2, d3);
 
 /**
- * Response syscall.
+ * Response syscall. If the current rpc frame is a notification frame, the
+ * arguments will be ignored, turned out to be easier to implement than try to
+ * enforce an ipc_ghost(), especially in cases where the root process has
+ * already been killed. Generally this shouldn't be an issue, as the
+ * notification handler is always aware of when it's being called to handle a
+ * notification, and can tailor its ipc_resp() to return zero arguments or
+ * whatever.
  *
  * @param t Current tcb.
  * @param d0 First response argument.
@@ -566,22 +572,6 @@ SYSCALL_DECLARE5(ipc_kick, pid, d0, d1, d2, d3);
 SYSCALL_DECLARE4(ipc_resp, d0, d1, d2, d3);
 
 /**
- * Returns to caller but without changing any visible state. Primarily used when
- * returning from interrupt handlers, like notifications, timers or external
- * devices.
- *
- * @param t Current tcb.
- * @param a Unused.
- * @param b Unused.
- * @param c Unused.
- * @param d Unused.
- * @param e Unused.
- *
- * Return OK.
- */
-SYSCALL_DECLARE0(ipc_ghost);
-
-/**
  * Notify thread syscall.
  *
  * @param t Current tcb.
@@ -593,7 +583,7 @@ SYSCALL_DECLARE0(ipc_ghost);
  *
  * Returns \ref OK and 0.
  */
-SYSCALL_DECLARE1(notify, tid);
+SYSCALL_DECLARE1(ipc_notify, tid);
 /** @} */
 
 /** @name Process handling syscalls. */
diff --git a/src/uapi/dispatch.c b/src/uapi/dispatch.c
index 399f02b..859543a 100644
--- a/src/uapi/dispatch.c
+++ b/src/uapi/dispatch.c
@@ -65,9 +65,8 @@ void handle_syscall(sys_arg_t syscall, sys_arg_t a, sys_arg_t b,
 	case SYS_IPC_TAIL: sys_ipc_tail(t, a, b, c, d, e); break;
 	case SYS_IPC_KICK: sys_ipc_kick(t, a, b, c, d, e); break;
 	case SYS_IPC_RESP: sys_ipc_resp(t, a, b, c, d, e); break;
-	case SYS_IPC_GHOST: sys_ipc_ghost(t, a, b, c, d, e); break;
+	case SYS_IPC_NOTIFY: sys_ipc_notify(t, a, b, c, d, e); break;
 	case SYS_SET_HANDLER: sys_set_handler(t, a, b, c, d, e); break;
-	case SYS_NOTIFY: sys_notify(t, a, b, c, d, e); break;
 	case SYS_CREATE: sys_create(t, a, b, c, d, e); break;
 	case SYS_FORK: sys_fork(t, a, b, c, d, e); break;
 	case SYS_EXEC: sys_exec(t, a, b, c, d, e); break;
diff --git a/src/uapi/ipc.c b/src/uapi/ipc.c
index 3d77639..850178c 100644
--- a/src/uapi/ipc.c
+++ b/src/uapi/ipc.c
@@ -31,6 +31,10 @@ struct call_ctx {
 
 	/** Current process ID. */
 	id_t pid;
+
+	/** If this frame was due to a notification, which means leaving the
+	 * frame must restore registers as they were */
+	bool notify;
 };
 
 /**
@@ -51,7 +55,8 @@ enum ipc_flags {
 	 * process. */
 	IPC_TAIL = (1 << 0),
 	/** Don't update the effective ID. */
-	IPC_FORWARD = (1 << 1)
+	IPC_FORWARD = (1 << 1),
+	IPC_NOTIFY = (1 << 2),
 };
 
 /**
@@ -102,6 +107,7 @@ static inline vm_t enter_rpc(struct tcb *t, struct sys_ret a,
 	ctx->exec = t->exec;
 	ctx->pid = t->pid;
 	ctx->eid = t->eid;
+	ctx->notify = flags & IPC_NOTIFY;
 	ctx->rpc_stack = rpc_stack;
 
 	/** @todo if we run out of rpc_stack space we should just stop, likely
@@ -173,7 +179,7 @@ static __noreturn void __run_notify(struct tcb *t, struct tcb *r)
 	 * ("pid 0"), and we are notifying the current thread */
 	vm_t s = enter_rpc(t,
 	                   SYS_RET5(0, t->tid, code, flags, t->eid),
-	                   0);
+	                   IPC_NOTIFY);
 
 	finalize_rpc(t, r, s);
 
@@ -239,7 +245,8 @@ static void leave_rpc(struct tcb *t, struct sys_ret a)
 
 	t->regs = ctx->regs;
 	/* again, get rid of args as fast as possible */
-	set_ret(t, 6, a);
+	if (!ctx->notify)
+		set_ret(t, 6, a);
 
 	struct tcb *r = get_tcb(ctx->pid);
 	while (!r || !is_proc(r) || zombie(r)) {
@@ -252,12 +259,13 @@ static void leave_rpc(struct tcb *t, struct sys_ret a)
 
 		rpc_stack = ctx->rpc_stack + BASE_PAGE_SIZE;
 		ctx = (struct call_ctx *)(rpc_stack) - 1;
+		t->regs = ctx->regs;
 
 		r = get_tcb(ctx->pid);
 		/* equivalent to return_args1 but without returning so we can
 		 * handle other cases in the loop. */
-		/** @todo is set_args useful? */
-		set_ret2(t, 0, ERR_NF);
+		if (!ctx->notify)
+			set_args1(t, ERR_NF);
 	}
 
 	if (orphan(t) && !is_rpc(t))
@@ -442,21 +450,6 @@ SYSCALL_DEFINE4(ipc_resp)(struct tcb *t, sys_arg_t d0, sys_arg_t d1,
 }
 
 /**
- * Ghost return, resetting register state.
- *
- * @param t Current tcb.
- * @return The previous registers of thread.
- */
-SYSCALL_DEFINE0(ipc_ghost)(struct tcb *t)
-{
-	if (unlikely(!is_rpc(t)))
-		return_args1(t, ERR_MISC);
-
-	enable_irqs();
-	leave_rpc(t, get_ret(t));
-}
-
-/**
  * Notify syscall handler.
  *
  * \todo Implement.
@@ -465,7 +458,7 @@ SYSCALL_DEFINE0(ipc_ghost)(struct tcb *t)
  * @param tid Thread ID to notify.
  * @return \ref OK and 0.
  */
-SYSCALL_DEFINE1(notify)(struct tcb *t, sys_arg_t tid){
+SYSCALL_DEFINE1(ipc_notify)(struct tcb *t, sys_arg_t tid){
 	if (t->tid != tid && !has_cap(t->caps, CAP_NOTIFY))
 		return_args1(t, ERR_PERM);
 
diff --git a/tests/common/test.h b/tests/common/test.h
index 2738bcd..18ac388 100644
--- a/tests/common/test.h
+++ b/tests/common/test.h
@@ -177,27 +177,45 @@ static inline id_t sys_req_abs_timer(uint64_t ticks)
 #define sys_ipc_kick3(pid, d0, d1, d2) syscall4(SYS_IPC_KICK, pid, d0, d1, d2)
 #define sys_ipc_kick4(pid, d0, d1, d2, d3) syscall5(SYS_IPC_KICK, pid, d0, d1, d2, d3)
 
-#define sys_ipc_resp0() syscall0(SYS_IPC_RESP)
-#define sys_ipc_resp1(d0) syscall1(SYS_IPC_RESP, d0)
-#define sys_ipc_resp2(d0, d1) syscall2(SYS_IPC_RESP, d0, d1)
-#define sys_ipc_resp3(d0, d1, d2) syscall3(SYS_IPC_RESP, d0, d1, d2)
-#define sys_ipc_resp4(d0, d1, d2, d3) syscall4(SYS_IPC_RESP, d0, d1, d2, d3)
-
-#define sys_ipc_ghost0() syscall1(SYS_IPC_GHOST)
-#define sys_ipc_ghost1(d0) syscall2(SYS_IPC_GHOST, d0)
-#define sys_ipc_ghost2(d0, d1) syscall3(SYS_IPC_GHOST, d0, d1)
-#define sys_ipc_ghost3(d0, d1, d2) syscall4(SYS_IPC_GHOST, d0, d1, d2)
-#define sys_ipc_ghost4(d0, d1, d2, d3) syscall5(SYS_IPC_GHOST, d0, d1, d2, d3)
-
 static inline enum sys_status sys_set_handler(id_t tid, id_t pid)
 {
 	struct sys_ret r = syscall2(SYS_SET_HANDLER, tid, pid);
 	return r.s;
 }
 
-static inline enum sys_status sys_notify(id_t tid)
+static inline enum sys_status sys_ipc_notify(id_t tid)
+{
+	struct sys_ret r = syscall1(SYS_IPC_NOTIFY, tid);
+	return r.s;
+}
+
+static inline enum sys_status sys_ipc_resp0()
+{
+	struct sys_ret r = syscall0(SYS_IPC_RESP);
+	return r.s;
+}
+
+static inline enum sys_status sys_ipc_resp1(sys_arg_t a)
+{
+	struct sys_ret r = syscall1(SYS_IPC_RESP, a);
+	return r.s;
+}
+
+static inline enum sys_status sys_ipc_resp2(sys_arg_t a, sys_arg_t b)
+{
+	struct sys_ret r = syscall2(SYS_IPC_RESP, a, b);
+	return r.s;
+}
+
+static inline enum sys_status sys_ipc_resp3(sys_arg_t a, sys_arg_t b, sys_arg_t c)
+{
+	struct sys_ret r = syscall3(SYS_IPC_RESP, a, b, c);
+	return r.s;
+}
+
+static inline enum sys_status sys_ipc_resp4(sys_arg_t a, sys_arg_t b, sys_arg_t c, sys_arg_t d)
 {
-	struct sys_ret r = syscall1(SYS_NOTIFY, tid);
+	struct sys_ret r = syscall4(SYS_IPC_RESP, a, b, c, d);
 	return r.s;
 }
 
@@ -314,11 +332,11 @@ static inline enum sys_status sys_exit()
 int printf(const char *fmt, ...) __printf;
 
 #define error(x, ...)\
-	printf("ERROR: " x # __VA_ARGS__)
+	printf("ERROR: " x, ## __VA_ARGS__)
 
 #define check(x, y, ...)\
 	if (!(x)) {\
-		error(y #__VA_ARGS__);\
+		error(y, ##__VA_ARGS__);\
 		sys_poweroff(SYS_SHUTDOWN);\
 	}
 
diff --git a/tests/ipc-notify/check.mk b/tests/ipc-notify/check.mk
new file mode 100644
index 0000000..54da852
--- /dev/null
+++ b/tests/ipc-notify/check.mk
@@ -0,0 +1,4 @@
+ipc-notify: do-ipc-notify
+	@grep 'BUG' reports/ipc-notify/log \
+		&& echo 'BUG' > reports/ipc-notify/OK \
+		|| tail -n1 reports/ipc-notify/log | tr -d '\r' > reports/ipc-notify/OK
diff --git a/tests/ipc-notify/init.c b/tests/ipc-notify/init.c
new file mode 100644
index 0000000..4626f10
--- /dev/null
+++ b/tests/ipc-notify/init.c
@@ -0,0 +1,42 @@
+#include <common/test.h>
+
+START(pid, tid, d0, d1, d2, d3)
+{
+	UNUSED(d3);
+	check(pid == 0, "illegal pid for init\n");
+	check(d0 == SYS_USER_NOTIFY || d0 == SYS_USER_SPAWNED,
+			"illegal d0 for init\n");
+
+	if (d0 == SYS_USER_SPAWNED) {
+		/* send request to ourselves */
+		printf("sending ipc notify to ourselves\n");
+		check(tid == 1, "illegal init thread ID\n");
+		struct sys_ret r = syscall5(SYS_IPC_NOTIFY, 1, 1, 2, 3, 4);
+		printf("returned ipc notify to ourselves\n");
+
+		check(r.s == OK, "illegal notify status\n");
+		check(r.id == 0, "illegal notify id\n");
+		check(r.a0 != 10, "ipc resp leaked through?\n");
+		check(r.a1 != 11, "ipc resp leaked through?\n");
+		check(r.a2 != 12, "ipc resp leaked through?\n");
+		check(r.a3 != 13, "ipc resp leaked through?\n");
+
+		/* try to notify to non-existing proc */
+		enum sys_status s = sys_ipc_notify(200);
+		check(s != OK, "got OK return for illegal pid\n");
+	}
+	else if (d0 == SYS_USER_NOTIFY) {
+		printf("caught ipc notify\n");
+		check(tid == 1, "illegal tid source\n");
+		check(d1 & NOTIFY_SIGNAL, "illegal d1\n");
+		check(d2 == 1, "illegal d2\n");
+
+		printf("doing ipc resp in notification\n");
+		/* whatever we respond here should not be visible in whoever ran
+		 * the notification */
+		sys_ipc_resp4(10, 11, 12, 13);
+		check(0, "ipc resp failed\n");
+	}
+
+	ok();
+}
diff --git a/tests/ipc-notify/source.mk b/tests/ipc-notify/source.mk
new file mode 100644
index 0000000..e154996
--- /dev/null
+++ b/tests/ipc-notify/source.mk
@@ -0,0 +1,2 @@
+DO	!= ./scripts/gen-prog -n ipc-notify -p init init.c
+DO	!= ./scripts/gen-simple -n ipc-notify -p init