diff --git a/include/app_memory/mem_domain.h b/include/app_memory/mem_domain.h index f50366020ae..f53dd8f98d9 100644 --- a/include/app_memory/mem_domain.h +++ b/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); /** diff --git a/kernel/mem_domain.c b/kernel/mem_domain.c index 1f1118f7c51..6cbd5ae1400 100644 --- a/kernel/mem_domain.c +++ b/kernel/mem_domain.c @@ -151,15 +151,23 @@ out: return ret; } -void k_mem_domain_add_partition(struct k_mem_domain *domain, - struct k_mem_partition *part) +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); - __ASSERT(check_add_partition(domain, part), - "invalid partition %p", part); + CHECKIF(domain == NULL) { + ret = -EINVAL; + 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, - "no free partition slots available"); + CHECKIF(!(p_idx < max_partitions)) { + 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); - __ASSERT_NO_MSG(part != NULL); + CHECKIF((domain == NULL) || (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; diff --git a/samples/userspace/prod_consumer/src/app_b.c b/samples/userspace/prod_consumer/src/app_b.c index 29ddfe0aede..741bd808ee4 100644 --- a/samples/userspace/prod_consumer/src/app_b.c +++ b/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); - k_mem_domain_add_partition(&k_mem_domain_default, &shared_partition); + ret = k_mem_domain_add_partition(&k_mem_domain_default, + &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(). diff --git a/samples/userspace/shared_mem/src/main.c b/samples/userspace/shared_mem/src/main.c index 50775ec5044..7cdd499546f 100644 --- a/samples/userspace/shared_mem/src/main.c +++ b/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); - k_mem_domain_add_partition(&k_mem_domain_default, &blk_part); + ret = k_mem_domain_add_partition(&k_mem_domain_default, &ct_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 */ diff --git a/subsys/testsuite/ztest/src/ztest.c b/subsys/testsuite/ztest/src/ztest.c index 5c0a2c9b2ff..dad2f4b53f2 100644 --- a/subsys/testsuite/ztest/src/ztest.c +++ b/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, - &ztest_mem_partition); + 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, - &z_malloc_partition); + 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 */ diff --git a/tests/crypto/mbedtls/src/main.c b/tests/crypto/mbedtls/src/main.c index 38db964ab25..b1c006c855e 100644 --- a/tests/crypto/mbedtls/src/main.c +++ b/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)); diff --git a/tests/kernel/mem_protect/mem_protect/src/mem_domain.c b/tests/kernel/mem_protect/mem_protect/src/mem_domain.c index c91505d74d5..5a582eb75df 100644 --- a/tests/kernel/mem_protect/mem_protect/src/mem_domain.c +++ b/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; - set_fault_valid(true); + need_recover_spinlock = false; + set_fault_valid(false); - /* Add one more partition will trigger assert due to exceeding */ - k_mem_domain_add_partition(&test_domain, &exceed_part); - - /* should not reach here */ - ztest_test_fail(); + /* Add one more partition will fail due to exceeding */ + zassert_not_equal( + k_mem_domain_add_partition(&test_domain, &exceed_part), + 0, "should fail to add memory partition"); } @@ -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 - * assertion, then triggher an expected fatal error. - * And while the fatal error happened, the memory domain spinlock - * is held, we need to release them to make other follow test case. + * @details Try to remove a partition not in the domain. + * k_mem_domain_remove_partition() should return non-zero. * * @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; - set_fault_valid(true); - k_mem_domain_remove_partition(&test_domain, no_parts); + need_recover_spinlock = false; + set_fault_valid(false); + + 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 - * if an expected fatal error happens. + * @details Try to add a null partition to memory domain. + * 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 */ - set_fault_valid(true); - k_mem_domain_add_partition(&test_domain_fail, NULL); - ztest_test_fail(); + /* add partition fail */ + set_fault_valid(false); + + 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 - * if an expected fatal error happens. + * @details Try to add a zero sized partition to memory domain. + * 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 */ - set_fault_valid(true); - k_mem_domain_add_partition(&test_domain_fail, nosize_part); - ztest_test_fail(); + /* add partition fail */ + set_fault_valid(false); + + 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 - * assertion, then triggher an expected fatal error. + * @details Try to add a partition whose adddress is wraparound. + * 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 */ - set_fault_valid(true); - k_mem_domain_add_partition(&test_domain_fail, &wraparound_part); - ztest_test_fail(); + /* add partition fail */ + set_fault_valid(false); + + 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 - * assertion, then triggher an expected fatal error. - * And while the fatal error happened, the memory domain spinlock - * is held, we need to release them to make other follow test case. + * @details Try to remove a partition size mismatched will result + * in k_mem_domain_remove_partition() returning error. * * @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]); - k_mem_domain_add_partition(&test_domain, no_parts); + zassert_equal( + 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 */ - need_recover_spinlock = true; - set_fault_valid(true); - k_mem_domain_remove_partition(&test_domain, no_parts); - ztest_test_fail(); + /* remove partition fail */ + need_recover_spinlock = false; + set_fault_valid(false); + + zassert_not_equal( + k_mem_domain_remove_partition(&test_domain, no_parts), + 0, "should fail to remove memory partition"); } diff --git a/tests/kernel/mem_protect/userspace/src/main.c b/tests/kernel/mem_protect/userspace/src/main.c index c4b5d354947..2e748c57bcc 100644 --- a/tests/kernel/mem_protect/userspace/src/main.c +++ b/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; diff --git a/tests/subsys/edac/ibecc/src/ibecc.c b/tests/subsys/edac/ibecc/src/ibecc.c index a0ded2b4786..b6044370b62 100644 --- a/tests/subsys/edac/ibecc/src/ibecc.c +++ b/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,