kernel: mem_domain: add/remove partition funcs to return errors

This changes both k_mem_domain_add_partition() and
k_mem_domain_remove_partition() to return errors instead of
asserting when errors are encountered. This gives the application
chance to recover.

The arch_mem_domain_parition_add()/_remove() will be modified
later together with all the other arch_mem_domain_*() changes
since the architecture code for partition addition and removal
functions usually cannot be separately changed.

Signed-off-by: Daniel Leung <daniel.leung@intel.com>

include/app_memory/mem_domain.h

@@ -151,8 +151,12 @@ extern int k_mem_domain_init(struct k_mem_domain *domain, uint8_t num_parts,
  *
  * @param domain The memory domain to be added a memory partition.
  * @param part The memory partition to be added
+ *
+ * @retval 0 if successful
+ * @retval -EINVAL if invalid parameters supplied
+ * @retval -ENOSPC if no free partition slots available
  */
-extern void k_mem_domain_add_partition(struct k_mem_domain *domain,
+extern int k_mem_domain_add_partition(struct k_mem_domain *domain,
 				      struct k_mem_partition *part);
 
 /**
@@ -162,8 +166,12 @@ extern void k_mem_domain_add_partition(struct k_mem_domain *domain,
  *
  * @param domain The memory domain to be removed a memory partition.
  * @param part The memory partition to be removed
+ *
+ * @retval 0 if successful
+ * @retval -EINVAL if invalid parameters supplied
+ * @retval -ENOENT if no matching partition found
  */
-extern void k_mem_domain_remove_partition(struct k_mem_domain *domain,
+extern int k_mem_domain_remove_partition(struct k_mem_domain *domain,
 					 struct k_mem_partition *part);
 
 /**

kernel/mem_domain.c

@@ -151,15 +151,23 @@ out:
 	return ret;
 }
 
-void k_mem_domain_add_partition(struct k_mem_domain *domain,
+int k_mem_domain_add_partition(struct k_mem_domain *domain,
 			       struct k_mem_partition *part)
 {
 	int p_idx;
 	k_spinlock_key_t key;
+	int ret = 0;
 
-	__ASSERT_NO_MSG(domain != NULL);
+	CHECKIF(domain == NULL) {
-	__ASSERT(check_add_partition(domain, part),
+		ret = -EINVAL;
-		 "invalid partition %p", part);
+		goto out;
+	}
+
+	CHECKIF(!check_add_partition(domain, part)) {
+		LOG_ERR("invalid partition %p", part);
+		ret = -EINVAL;
+		goto out;
+	}
 
 	key = k_spin_lock(&z_mem_domain_lock);
 
@@ -170,8 +178,11 @@ void k_mem_domain_add_partition(struct k_mem_domain *domain,
 		}
 	}
 
-	__ASSERT(p_idx < max_partitions,
+	CHECKIF(!(p_idx < max_partitions)) {
-		 "no free partition slots available");
+		LOG_ERR("no free partition slots available");
+		ret = -ENOSPC;
+		goto unlock_out;
+	}
 
 	LOG_DBG("add partition base %lx size %zu to domain %p\n",
 		part->start, part->size, domain);
@@ -185,17 +196,25 @@ void k_mem_domain_add_partition(struct k_mem_domain *domain,
 #ifdef CONFIG_ARCH_MEM_DOMAIN_SYNCHRONOUS_API
 	arch_mem_domain_partition_add(domain, p_idx);
 #endif
+
+unlock_out:
 	k_spin_unlock(&z_mem_domain_lock, key);
+
+out:
+	return ret;
 }
 
-void k_mem_domain_remove_partition(struct k_mem_domain *domain,
+int k_mem_domain_remove_partition(struct k_mem_domain *domain,
 				  struct k_mem_partition *part)
 {
 	int p_idx;
 	k_spinlock_key_t key;
+	int ret = 0;
 
-	__ASSERT_NO_MSG(domain != NULL);
+	CHECKIF((domain == NULL) || (part == NULL)) {
-	__ASSERT_NO_MSG(part != NULL);
+		ret = -EINVAL;
+		goto out;
+	}
 
 	key = k_spin_lock(&z_mem_domain_lock);
 
@@ -207,7 +226,11 @@ void k_mem_domain_remove_partition(struct k_mem_domain *domain,
 		}
 	}
 
-	__ASSERT(p_idx < max_partitions, "no matching partition found");
+	CHECKIF(!(p_idx < max_partitions)) {
+		LOG_ERR("no matching partition found");
+		ret = -ENOENT;
+		goto unlock_out;
+	}
 
 	LOG_DBG("remove partition base %lx size %zu from domain %p\n",
 		part->start, part->size, domain);
@@ -221,7 +244,11 @@ void k_mem_domain_remove_partition(struct k_mem_domain *domain,
 
 	domain->num_partitions--;
 
+unlock_out:
 	k_spin_unlock(&z_mem_domain_lock, key);
+
+out:
+	return ret;
 }
 
 static void add_thread_locked(struct k_mem_domain *domain,
@@ -301,7 +328,9 @@ static int init_mem_domain_module(const struct device *arg)
 	__ASSERT(ret == 0, "failed to init default mem domain");
 
 #ifdef Z_LIBC_PARTITION_EXISTS
-	k_mem_domain_add_partition(&k_mem_domain_default, &z_libc_partition);
+	ret = k_mem_domain_add_partition(&k_mem_domain_default,
+					 &z_libc_partition);
+	__ASSERT(ret == 0, "failed to add default libc mem partition");
 #endif /* Z_LIBC_PARTITION_EXISTS */
 
 	return 0;

samples/userspace/prod_consumer/src/app_b.c

@@ -76,12 +76,27 @@ static void processor_thread(void *p1, void *p2, void *p3)
 
 void app_b_entry(void *p1, void *p2, void *p3)
 {
+	int ret;
+
 	/* Much like how we are reusing the main thread as this application's
 	 * processor thread, we will re-use the default memory domain as the
 	 * domain for application B.
 	 */
-	k_mem_domain_add_partition(&k_mem_domain_default, &app_b_partition);
+	ret = k_mem_domain_add_partition(&k_mem_domain_default,
-	k_mem_domain_add_partition(&k_mem_domain_default, &shared_partition);
+					 &app_b_partition);
+	if (ret != 0) {
+		LOG_ERR("Failed to add app_b_partition to mem domain (%d)",
+			ret);
+		k_oops();
+	}
+
+	ret = k_mem_domain_add_partition(&k_mem_domain_default,
+					 &shared_partition);
+	if (ret != 0) {
+		LOG_ERR("Failed to add shared_partition to mem domain (%d)",
+			ret);
+		k_oops();
+	}
 
 	/* Assign a resource pool to serve for kernel-side allocations on
 	 * behalf of application A. Needed for k_queue_alloc_append().

samples/userspace/shared_mem/src/main.c

@@ -158,10 +158,20 @@ void main(void)
 	k_thread_access_grant(tCT, &allforone);
 	printk("CT Thread Created %p\n", tCT);
 	/* Re-using the default memory domain for CT */
-	k_mem_domain_add_partition(&k_mem_domain_default, &ct_part);
+	ret = k_mem_domain_add_partition(&k_mem_domain_default, &ct_part);
-	k_mem_domain_add_partition(&k_mem_domain_default, &blk_part);
+	if (ret != 0) {
+		printk("Failed to add ct_part to mem domain (%d)\n", ret);
+		k_oops();
+	}
 	printk("ct partitions installed\n");
 
+	ret = k_mem_domain_add_partition(&k_mem_domain_default, &blk_part);
+	if (ret != 0) {
+		printk("Failed to add blk_part to mem domain (%d)\n", ret);
+		k_oops();
+	}
+	printk("blk partitions installed\n");
+
 	k_thread_start(&enc_thread);
 	/* need to start all three threads.  let enc go first to perform init step */
 

subsys/testsuite/ztest/src/ztest.c

@@ -509,17 +509,29 @@ int main(void)
 void main(void)
 {
 #ifdef CONFIG_USERSPACE
+	int ret;
+
 	/* Partition containing globals tagged with ZTEST_DMEM and ZTEST_BMEM
 	 * macros. Any variables that user code may reference need to be
 	 * placed in this partition if no other memory domain configuration
 	 * is made.
 	 */
-	k_mem_domain_add_partition(&k_mem_domain_default,
+	ret = k_mem_domain_add_partition(&k_mem_domain_default,
 					 &ztest_mem_partition);
+	if (ret != 0) {
+		PRINT("ERROR: failed to add ztest_mem_partition to mem domain (%d)\n",
+		      ret);
+		k_oops();
+	}
 #ifdef Z_MALLOC_PARTITION_EXISTS
 	/* Allow access to malloc() memory */
-	k_mem_domain_add_partition(&k_mem_domain_default,
+	ret = k_mem_domain_add_partition(&k_mem_domain_default,
 					 &z_malloc_partition);
+	if (ret != 0) {
+		PRINT("ERROR: failed to add z_malloc_partition to mem domain (%d)\n",
+		      ret);
+		k_oops();
+	}
 #endif
 #endif /* CONFIG_USERSPACE */
 

tests/crypto/mbedtls/src/main.c

@@ -13,7 +13,12 @@ extern void test_mbedtls(void);
 void test_main(void)
 {
 #ifdef CONFIG_USERSPACE
-	k_mem_domain_add_partition(&k_mem_domain_default, &k_mbedtls_partition);
+	int ret = k_mem_domain_add_partition(&k_mem_domain_default,
+					     &k_mbedtls_partition);
+	if (ret != 0) {
+		printk("Failed to add memory partition (%d)\n", ret);
+		k_oops();
+	}
 #endif
 	ztest_test_suite(test_mbedtls_fn,
 		ztest_user_unit_test(test_mbedtls));

tests/kernel/mem_protect/mem_protect/src/mem_domain.c

@@ -66,7 +66,9 @@ void test_mem_domain_setup(void)
 			rw_bufs[i][j] = (j % 256U);
 		}
 
-		k_mem_domain_add_partition(&test_domain, &rw_parts[i]);
+		zassert_equal(
+			k_mem_domain_add_partition(&test_domain, &rw_parts[i]),
+			0, "cannot add memory partition");
 	}
 
 	for (unsigned int j = 0; j < MEM_REGION_ALLOC; j++) {
@@ -183,7 +185,9 @@ void test_mem_domain_no_writes_to_ro(void)
  */
 void test_mem_domain_remove_add_partition(void)
 {
-	k_mem_domain_remove_partition(&test_domain, &rw_parts[0]);
+	zassert_equal(
+		k_mem_domain_remove_partition(&test_domain, &rw_parts[0]),
+		0, "failed to remove memory partition");
 
 	/* Should still work, we didn't remove ro_part */
 	spawn_child_thread(ro_part_access, &test_domain, false);
@@ -192,7 +196,9 @@ void test_mem_domain_remove_add_partition(void)
 	spawn_child_thread(rw_part_access, &test_domain, true);
 
 	/* Restore test_domain contents so we don't mess up other tests */
-	k_mem_domain_add_partition(&test_domain, &rw_parts[0]);
+	zassert_equal(
+		k_mem_domain_add_partition(&test_domain, &rw_parts[0]),
+		0, "failed to add memory partition");
 
 	/* Should work again */
 	spawn_child_thread(rw_part_access, &test_domain, false);
@@ -216,12 +222,16 @@ static void mem_domain_init_entry(void *p1, void *p2, void *p3)
 
 static void mem_domain_add_partition_entry(void *p1, void *p2, void *p3)
 {
-	k_mem_domain_add_partition(&test_domain, &no_access_part);
+	zassert_equal(
+		k_mem_domain_add_partition(&test_domain, &no_access_part),
+		0, "failed to add memory partition");
 }
 
 static void mem_domain_remove_partition_entry(void *p1, void *p2, void *p3)
 {
-	k_mem_domain_remove_partition(&test_domain, &ro_part);
+	zassert_equal(
+		k_mem_domain_remove_partition(&test_domain, &ro_part),
+		0, "failed to remove memory partition");
 }
 
 static void mem_domain_add_thread_entry(void *p1, void *p2, void *p3)
@@ -382,9 +392,11 @@ void test_mem_domain_migration(void)
  */
 void test_mem_part_overlap(void)
 {
-	set_fault_valid(true);
+	set_fault_valid(false);
 
-	k_mem_domain_add_partition(&test_domain, &overlap_part);
+	zassert_not_equal(
+		k_mem_domain_add_partition(&test_domain, &overlap_part),
+		0, "should fail to add memory partition");
 }
 
 extern struct k_spinlock z_mem_domain_lock;
@@ -414,7 +426,7 @@ K_MEM_PARTITION_DEFINE(exceed_part, exceed_buf, sizeof(exceed_buf),
  * @details
  * - Add memory partitions one by one and more than architecture allows to add.
  * - When partitions added more than it is allowed by architecture, test that
- *   system assert for that case works correctly.
+ *   k_mem_domain_add_partition() returns non-zero.
  *
  * @ingroup kernel_memprotect_tests
  */
@@ -429,14 +441,13 @@ void test_mem_part_assert_add_overmax(void)
 			"domain still have room of partitions(%d).",
 			max_parts);
 
-	need_recover_spinlock = true;
+	need_recover_spinlock = false;
-	set_fault_valid(true);
+	set_fault_valid(false);
 
-	/* Add one more partition will trigger assert due to exceeding */
+	/* Add one more partition will fail due to exceeding */
-	k_mem_domain_add_partition(&test_domain, &exceed_part);
+	zassert_not_equal(
-
+		k_mem_domain_add_partition(&test_domain, &exceed_part),
-	/* should not reach here */
+		0, "should fail to add memory partition");
-	ztest_test_fail();
 }
 
 
@@ -448,10 +459,8 @@ K_MEM_PARTITION_DEFINE(find_no_part, misc_buf, sizeof(misc_buf),
 /**
  * @brief Test error case of removing memory partition fail
  *
- * @details Try to remove a partition not in the domain will cause
+ * @details Try to remove a partition not in the domain.
- * assertion, then triggher an expected fatal error.
+ * k_mem_domain_remove_partition() should return non-zero.
- * And while the fatal error happened, the memory domain spinlock
- * is held, we need to release them to make other follow test case.
  *
  * @ingroup kernel_memprotect_tests
  */
@@ -459,9 +468,12 @@ void test_mem_domain_remove_part_fail(void)
 {
 	struct k_mem_partition *no_parts = &find_no_part;
 
-	need_recover_spinlock = true;
+	need_recover_spinlock = false;
-	set_fault_valid(true);
+	set_fault_valid(false);
-	k_mem_domain_remove_partition(&test_domain, no_parts);
+
+	zassert_not_equal(
+		k_mem_domain_remove_partition(&test_domain, no_parts),
+		0, "should fail to remove memory partition");
 }
 #else
 void test_mem_domain_remove_part_fail(void)
@@ -495,17 +507,19 @@ void test_mem_domain_init_fail(void)
 /**
  * @brief Test error case of adding null memory partition fail
  *
- * @details Try to add a null partition to memory domain, then see
+ * @details Try to add a null partition to memory domain.
- * if an expected fatal error happens.
+ * k_mem_domain_add_partition() should return error.
  *
  * @ingroup kernel_memprotect_tests
  */
 void test_mem_part_add_error_null(void)
 {
-	/* add partition fail, expected fault happened */
+	/* add partition fail */
-	set_fault_valid(true);
+	set_fault_valid(false);
-	k_mem_domain_add_partition(&test_domain_fail, NULL);
+
-	ztest_test_fail();
+	zassert_not_equal(
+		k_mem_domain_add_partition(&test_domain_fail, NULL),
+		0, "should fail to add memory partition");
 }
 
 static volatile uint8_t __aligned(MEM_REGION_ALLOC) nosize_buf[MEM_REGION_ALLOC];
@@ -515,29 +529,30 @@ K_MEM_PARTITION_DEFINE(nonsize_part, nosize_buf, sizeof(nosize_buf),
 /**
  * @brief Test error case of adding zero sized memory partition fail
  *
- * @details Try to add a zero sized partition to memory domain, then see
+ * @details Try to add a zero sized partition to memory domain.
- * if an expected fatal error happens.
+ * k_mem_domain_add_partition() should return error.
  *
  * @ingroup kernel_memprotect_tests
  */
 void test_mem_part_add_error_zerosize(void)
 {
-
 	struct k_mem_partition *nosize_part = &nonsize_part;
 
 	nosize_part->size = 0U;
 
-	/* add partition fail, expected fault happened */
+	/* add partition fail */
-	set_fault_valid(true);
+	set_fault_valid(false);
-	k_mem_domain_add_partition(&test_domain_fail, nosize_part);
+
-	ztest_test_fail();
+	zassert_not_equal(
+		k_mem_domain_add_partition(&test_domain_fail, nosize_part),
+		0, "should fail to add memory partition");
 }
 
 /**
  * @brief Test error case of memory partition address wraparound
  *
- * @details Try to add a partition whose adddress is wraparound will cause
+ * @details Try to add a partition whose adddress is wraparound.
- * assertion, then triggher an expected fatal error.
+ * k_mem_domain_add_partition() should return error.
  *
  * @ingroup kernel_memprotect_tests
  */
@@ -551,19 +566,19 @@ void test_mem_part_error_wraparound(void)
 		       K_MEM_PARTITION_P_RO_U_RO);
 #endif
 
-	/* add partition fail, expected fault happened */
+	/* add partition fail */
-	set_fault_valid(true);
+	set_fault_valid(false);
-	k_mem_domain_add_partition(&test_domain_fail, &wraparound_part);
+
-	ztest_test_fail();
+	zassert_not_equal(
+		k_mem_domain_add_partition(&test_domain_fail, &wraparound_part),
+		0, "should fail to add memory partition");
 }
 
 /**
  * @brief Test error case of removing memory partition fail
  *
- * @details Try to remove a partition size mismatched will cause
+ * @details Try to remove a partition size mismatched will result
- * assertion, then triggher an expected fatal error.
+ * in k_mem_domain_remove_partition() returning error.
- * And while the fatal error happened, the memory domain spinlock
- * is held, we need to release them to make other follow test case.
  *
  * @ingroup kernel_memprotect_tests
  */
@@ -571,13 +586,21 @@ void test_mem_part_remove_error_zerosize(void)
 {
 	struct k_mem_partition *no_parts = &find_no_part;
 
-	k_mem_domain_remove_partition(&test_domain, &rw_parts[0]);
+	zassert_equal(
-	k_mem_domain_add_partition(&test_domain, no_parts);
+		k_mem_domain_remove_partition(&test_domain, &rw_parts[0]),
+		0, "failed to remove memory partition");
+
+	zassert_equal(
+		k_mem_domain_add_partition(&test_domain, no_parts),
+		0, "failed to add memory partition");
+
 	no_parts->size = 0U;
 
-	/* remove partition fail, expected fault happened */
+	/* remove partition fail */
-	need_recover_spinlock = true;
+	need_recover_spinlock = false;
-	set_fault_valid(true);
+	set_fault_valid(false);
-	k_mem_domain_remove_partition(&test_domain, no_parts);
+
-	ztest_test_fail();
+	zassert_not_equal(
+		k_mem_domain_remove_partition(&test_domain, no_parts),
+		0, "should fail to remove memory partition");
 }

tests/kernel/mem_protect/userspace/src/main.c

@@ -722,7 +722,11 @@ static void test_domain_add_thread_drop_to_user(void)
 static void test_domain_add_part_drop_to_user(void)
 {
 	clear_fault();
-	k_mem_domain_add_partition(&k_mem_domain_default, &alt_part);
+
+	zassert_equal(
+		k_mem_domain_add_partition(&k_mem_domain_default, &alt_part),
+		0, "failed to add memory partition");
+
 	drop_user(&alt_bool);
 }
 
@@ -738,7 +742,11 @@ static void test_domain_remove_part_drop_to_user(void)
 	 * remove it, and then try to access again.
 	 */
 	set_fault(K_ERR_CPU_EXCEPTION);
-	k_mem_domain_remove_partition(&k_mem_domain_default, &alt_part);
+
+	zassert_equal(
+		k_mem_domain_remove_partition(&k_mem_domain_default, &alt_part),
+		0, "failed to remove partition");
+
 	drop_user(&alt_bool);
 }
 
@@ -763,7 +771,11 @@ static void test_domain_add_thread_context_switch(void)
 static void test_domain_add_part_context_switch(void)
 {
 	clear_fault();
-	k_mem_domain_add_partition(&k_mem_domain_default, &alt_part);
+
+	zassert_equal(
+		k_mem_domain_add_partition(&k_mem_domain_default, &alt_part),
+		0, "failed to add memory partition");
+
 	spawn_user(&alt_bool);
 }
 
@@ -780,7 +792,11 @@ static void test_domain_remove_part_context_switch(void)
 	 * remove it, and then try to access again.
 	 */
 	set_fault(K_ERR_CPU_EXCEPTION);
-	k_mem_domain_remove_partition(&k_mem_domain_default, &alt_part);
+
+	zassert_equal(
+		k_mem_domain_remove_partition(&k_mem_domain_default, &alt_part),
+		0, "failed to remove memory partition");
+
 	spawn_user(&alt_bool);
 }
 
@@ -967,8 +983,14 @@ void test_tls_pointer(void)
 
 void test_main(void)
 {
+	int ret;
+
 	/* Most of these scenarios use the default domain */
-	k_mem_domain_add_partition(&k_mem_domain_default, &default_part);
+	ret = k_mem_domain_add_partition(&k_mem_domain_default, &default_part);
+	if (ret != 0) {
+		printk("Failed to add default memory partition (%d)\n", ret);
+		k_oops();
+	}
 
 #if defined(CONFIG_ARM64)
 	struct z_arm64_thread_stack_header *hdr;

tests/subsys/edac/ibecc/src/ibecc.c

@@ -326,7 +326,12 @@ void test_edac_dummy_api(void);
 void test_main(void)
 {
 #if defined(CONFIG_USERSPACE)
-	k_mem_domain_add_partition(&k_mem_domain_default, &default_part);
+	int ret = k_mem_domain_add_partition(&k_mem_domain_default,
+					     &default_part);
+	if (ret != 0) {
+		TC_PRINT("Failed to add to mem domain (%d)", ret);
+		k_oops();
+	}
 #endif
 
 	ztest_test_suite(ibecc,