userspace: assign thread IDs at build time

Kernel object metadata had an extra data field added recently to
store bounds for stack objects. Use this data field to assign
IDs to thread objects at build time. This has numerous advantages:

* Threads can be granted permissions on kernel objects before the
  thread is initialized. Previously, it was necessary to call
  k_thread_create() with a K_FOREVER delay, assign permissions, then
  start the thread. Permissions are still completely cleared when
  a thread exits.

* No need for runtime logic to manage thread IDs

* Build error if CONFIG_MAX_THREAD_BYTES is set too low

Signed-off-by: Andrew Boie <andrew.p.boie@intel.com>

include/kernel.h

@@ -398,11 +398,6 @@ struct _thread_base {
 	/* this thread's entry in a timeout queue */
 	struct _timeout timeout;
 #endif
-
-#ifdef CONFIG_USERSPACE
-	/* Bit position in kernel object permissions bitfield for this thread */
-	unsigned int perm_index;
-#endif
 };
 
 typedef struct _thread_base _thread_base_t;

kernel/include/kernel_structs.h

@@ -118,11 +118,6 @@ struct _kernel {
 	struct k_thread *threads; /* singly linked list of ALL threads */
 #endif
 
-#if defined(CONFIG_USERSPACE)
-	/* 0 bits for ids currently in use, 1 for free ids */
-	u8_t free_thread_ids[CONFIG_MAX_THREAD_BYTES];
-#endif
-
 	/* arch-specific part of _kernel */
 	struct _kernel_arch arch;
 };

kernel/init.c

@@ -259,18 +259,10 @@ static void prepare_multithreading(struct k_thread *dummy_thread)
 	dummy_thread->stack_info.start = 0;
 	dummy_thread->stack_info.size = 0;
 #endif
-#ifdef CONFIG_USERSPACE
-	dummy_thread->base.perm_index = 0;
-#endif
 #endif
 
 	/* _kernel.ready_q is all zeroes */
 
-#ifdef CONFIG_USERSPACE
-	/* Mark all potential IDs as available */
-	memset(_kernel.free_thread_ids, 0xFF, CONFIG_MAX_THREAD_BYTES);
-#endif
-
 	/*
 	 * The interrupt library needs to be initialized early since a series
 	 * of handlers are installed into the interrupt table to catch

kernel/thread.c

@@ -28,64 +28,6 @@
 extern struct _static_thread_data _static_thread_data_list_start[];
 extern struct _static_thread_data _static_thread_data_list_end[];
 
-#ifdef CONFIG_USERSPACE
-static int thread_count;
-
-/*
- * Fetch an unused thread ID. Returns -1 if all thread IDs are in use
- */
-static int get_next_thread_index(void)
-{
-	int key, pos = -1;
-
-	key = irq_lock();
-
-	if (thread_count == CONFIG_MAX_THREAD_BYTES * 8) {
-		/* We have run out of thread IDs! */
-		goto out;
-	}
-
-	/* find an unused bit in the kernel's bitfield of in-use thread IDs */
-	for (int i = 0; i < CONFIG_MAX_THREAD_BYTES; i++) {
-		int fs;
-
-		fs = find_lsb_set(_kernel.free_thread_ids[i]);
-		if (fs) {
-			/* find_lsb_set counts bit positions starting at 1 */
-			--fs;
-			_kernel.free_thread_ids[i] &= ~(1 << fs);
-			pos = fs + (i * 8);
-			break;
-		}
-	}
-
-	thread_count++;
-out:
-	irq_unlock(key);
-
-	return pos;
-}
-
-static void free_thread_index(int id)
-{
-	int index, key;
-	u8_t bit;
-
-	if (id == -1) {
-		return;
-	}
-
-	key = irq_lock();
-
-	thread_count--;
-	index = id / 8;
-	bit = 1 << (id % 8);
-	_kernel.free_thread_ids[index] |= bit;
-
-	irq_unlock(key);
-}
-#endif
-
 #define _FOREACH_STATIC_THREAD(thread_data)              \
 	for (struct _static_thread_data *thread_data =   \
 	     _static_thread_data_list_start;             \
@@ -324,7 +266,6 @@ void _setup_new_thread(struct k_thread *new_thread,
 	_new_thread(new_thread, stack, stack_size, entry, p1, p2, p3,
 		    prio, options);
 #ifdef CONFIG_USERSPACE
-	new_thread->base.perm_index = get_next_thread_index();
 	_k_object_init(new_thread);
 	_k_object_init(stack);
 	new_thread->stack_obj = stack;
@@ -417,11 +358,6 @@ _SYSCALL_HANDLER(k_thread_create,
 			  (void *)margs->arg6, (void *)margs->arg7, prio,
 			  options);
 
-	if (new_thread->base.perm_index == -1) {
-		k_thread_abort(new_thread);
-		_SYSCALL_VERIFY_MSG(0, "too many threads created");
-	}
-
 	if (delay != K_FOREVER) {
 		schedule_new_thread(new_thread, delay);
 	}
@@ -580,12 +516,8 @@ void _k_thread_single_abort(struct k_thread *thread)
 	_k_object_uninit(thread->stack_obj);
 	_k_object_uninit(thread);
 
-	if (thread->base.perm_index != -1) {
-		free_thread_index(thread->base.perm_index);
-
-		/* Revoke permissions on thread's ID so that it may be recycled */
-		_thread_perms_all_clear(thread);
-	}
+	/* Revoke permissions on thread's ID so that it may be recycled */
+	_thread_perms_all_clear(thread);
 #endif
 }
 

kernel/userspace.c

@@ -92,6 +92,19 @@ struct perm_ctx {
 	struct k_thread *parent;
 };
 
+static int thread_index_get(struct k_thread *t)
+{
+	struct _k_object *ko;
+
+	ko = _k_object_find(t);
+
+	if (!ko) {
+		return -1;
+	}
+
+	return ko->data;
+}
+
 static void wordlist_cb(struct _k_object *ko, void *ctx_ptr)
 {
 	struct perm_ctx *ctx = (struct perm_ctx *)ctx_ptr;
@@ -105,30 +118,31 @@ static void wordlist_cb(struct _k_object *ko, void *ctx_ptr)
 void _thread_perms_inherit(struct k_thread *parent, struct k_thread *child)
 {
 	struct perm_ctx ctx = {
-		parent->base.perm_index,
-		child->base.perm_index,
+		thread_index_get(parent),
+		thread_index_get(child),
 		parent
 	};
 
-	if ((ctx.parent_id < MAX_THREAD_BITS) &&
-	    (ctx.child_id < MAX_THREAD_BITS)) {
+	if ((ctx.parent_id != -1) && (ctx.child_id != -1)) {
 		_k_object_wordlist_foreach(wordlist_cb, &ctx);
 	}
 }
 
 void _thread_perms_set(struct _k_object *ko, struct k_thread *thread)
 {
-	if (thread->base.perm_index < MAX_THREAD_BITS) {
-		sys_bitfield_set_bit((mem_addr_t)&ko->perms,
-				     thread->base.perm_index);
+	int index = thread_index_get(thread);
+
+	if (index != -1) {
+		sys_bitfield_set_bit((mem_addr_t)&ko->perms, index);
 	}
 }
 
 void _thread_perms_clear(struct _k_object *ko, struct k_thread *thread)
 {
-	if (thread->base.perm_index < MAX_THREAD_BITS) {
-		sys_bitfield_clear_bit((mem_addr_t)&ko->perms,
-				       thread->base.perm_index);
+	int index = thread_index_get(thread);
+
+	if (index != -1) {
+		sys_bitfield_clear_bit((mem_addr_t)&ko->perms, index);
 	}
 }
 
@@ -141,29 +155,33 @@ static void clear_perms_cb(struct _k_object *ko, void *ctx_ptr)
 
 void _thread_perms_all_clear(struct k_thread *thread)
 {
-	if (thread->base.perm_index < MAX_THREAD_BITS) {
-		_k_object_wordlist_foreach(clear_perms_cb,
-					   (void *)thread->base.perm_index);
+	int index = thread_index_get(thread);
+
+	if (index != -1) {
+		_k_object_wordlist_foreach(clear_perms_cb, (void *)index);
 	}
 }
 
 static int thread_perms_test(struct _k_object *ko)
 {
+	int index;
+
 	if (ko->flags & K_OBJ_FLAG_PUBLIC) {
 		return 1;
 	}
 
-	if (_current->base.perm_index < MAX_THREAD_BITS) {
-		return sys_bitfield_test_bit((mem_addr_t)&ko->perms,
-					     _current->base.perm_index);
+	index = thread_index_get(_current);
+	if (index != -1) {
+		return sys_bitfield_test_bit((mem_addr_t)&ko->perms, index);
 	}
 	return 0;
 }
 
 static void dump_permission_error(struct _k_object *ko)
 {
+	int index = thread_index_get(_current);
 	printk("thread %p (%d) does not have permission on %s %p [",
-	       _current, _current->base.perm_index,
+	       _current, index,
 	       otype_to_str(ko->type), ko->name);
 	for (int i = CONFIG_MAX_THREAD_BYTES - 1; i >= 0; i--) {
 		printk("%02x", ko->perms[i]);

kernel/userspace_handler.c

@@ -39,7 +39,7 @@ _SYSCALL_HANDLER(k_object_access_grant, object, thread)
 {
 	struct _k_object *ko;
 
-	_SYSCALL_OBJ(thread, K_OBJ_THREAD);
+	_SYSCALL_OBJ_INIT(thread, K_OBJ_THREAD);
 	ko = validate_any_object((void *)object);
 	_SYSCALL_VERIFY_MSG(ko, "object %p access denied", (void *)object);
 	_thread_perms_set(ko, (struct k_thread *)thread);
@@ -51,7 +51,7 @@ _SYSCALL_HANDLER(k_object_access_revoke, object, thread)
 {
 	struct _k_object *ko;
 
-	_SYSCALL_OBJ(thread, K_OBJ_THREAD);
+	_SYSCALL_OBJ_INIT(thread, K_OBJ_THREAD);
 	ko = validate_any_object((void *)object);
 	_SYSCALL_VERIFY_MSG(ko, "object %p access denied", (void *)object);
 	_thread_perms_clear(ko, (struct k_thread *)thread);

scripts/gen_kobject_list.py

@@ -62,6 +62,7 @@ def kobject_to_enum(ko):
 DW_OP_addr = 0x3
 DW_OP_fbreg = 0x91
 STACK_TYPE = "_k_thread_stack_element"
+thread_counter = 0
 
 # Global type environment. Populated by pass 1.
 type_env = {}
@@ -90,12 +91,22 @@ def debug_die(die, text):
 
 class KobjectInstance:
     def __init__(self, type_obj, addr):
+        global thread_counter
+
         self.addr = addr
         self.type_obj = type_obj
 
-        # these two are set later
+        # Type name determined later since drivers needs to look at the
+        # API struct address
         self.type_name = None
-        self.data = 0
+
+        if self.type_obj.name == "k_thread":
+            # Assign an ID for this thread object, used to track its
+            # permissions to other kernel objects
+            self.data = thread_counter
+            thread_counter = thread_counter + 1
+        else:
+            self.data = 0
 
 
 class KobjectType:
@@ -587,8 +598,15 @@ def main():
         elf = ELFFile(fp)
         args.little_endian = elf.little_endian
         syms = get_symbols(elf)
+        max_threads = syms["CONFIG_MAX_THREAD_BYTES"] * 8
         objs = find_kobjects(elf, syms)
 
+    if thread_counter > max_threads:
+        sys.stderr.write("Too many thread objects (%d)\n" % thread_counter)
+        sys.stderr.write("Increase CONFIG_MAX_THREAD_BYTES to %d\n",
+                -(-thread_counter // 8));
+        sys.exit(1)
+
     with open(args.output, "w") as fp:
         write_gperf_table(fp, objs, syms["_static_kernel_objects_begin"],
                 syms["_static_kernel_objects_end"])