From d4c0484e90d40634586a6e9104e0d32af014f283 Mon Sep 17 00:00:00 2001 From: Leandro Belli Date: Tue, 6 May 2025 14:40:58 +0100 Subject: [PATCH] mod/apcontext: Refactor context handling and add unit tests Refactors the AP context module to store the configuration pointer directly in the module context (`ctx.config`). This simplifies access throughout the module and avoids repeated calls to `fwk_module_get_data()`. Also adds a safeguard to prevent `wait_on_notifications` from being decremented below zero. Includes unit tests for init, start, and notification handling. Signed-off-by: Leandro Belli --- module/apcontext/src/mod_apcontext.c | 57 ++- module/apcontext/test/CMakeLists.txt | 29 ++ module/apcontext/test/fwk_module_idx.h | 24 ++ .../apcontext/test/mod_apcontext_unit_test.c | 363 ++++++++++++++++++ module/mock_sensor/CMakeLists.txt | 16 + module/mock_sensor/Module.cmake | 9 + unit_test/CMakeLists.txt | 1 + 7 files changed, 469 insertions(+), 30 deletions(-) create mode 100644 module/apcontext/test/CMakeLists.txt create mode 100644 module/apcontext/test/fwk_module_idx.h create mode 100644 module/apcontext/test/mod_apcontext_unit_test.c create mode 100644 module/mock_sensor/CMakeLists.txt create mode 100644 module/mock_sensor/Module.cmake diff --git a/module/apcontext/src/mod_apcontext.c b/module/apcontext/src/mod_apcontext.c index e494dcdf8..27a1a8cd9 100644 --- a/module/apcontext/src/mod_apcontext.c +++ b/module/apcontext/src/mod_apcontext.c @@ -1,6 +1,6 @@ /* * Arm SCP/MCP Software - * Copyright (c) 2018-2023, Arm Limited and Contributors. All rights reserved. + * Copyright (c) 2018-2025, Arm Limited and Contributors. All rights reserved. * * SPDX-License-Identifier: BSD-3-Clause */ @@ -13,7 +13,6 @@ #include #include #include -#include #include #include @@ -24,6 +23,9 @@ /* Module context structure */ struct apcontext_ctx { + /* Module configuration */ + const struct mod_apcontext_config *config; + /* * Number of notifications subscribed and to wait for before zeroing the * AP context memory region. @@ -36,17 +38,13 @@ static struct apcontext_ctx ctx; static void apcontext_zero(void) { - const struct mod_apcontext_config *config; - - config = fwk_module_get_data(fwk_module_id_apcontext); - FWK_LOG_INFO( MODULE_NAME " Zeroing AP context area [0x%" PRIxPTR " - 0x%" PRIxPTR "]", - config->base, - config->base + config->size); + ctx.config->base, + ctx.config->base + ctx.config->size); - memset((void *)config->base, 0, config->size); + memset((void *)ctx.config->base, 0, ctx.config->size); } /* @@ -56,45 +54,42 @@ static void apcontext_zero(void) static int apcontext_init(fwk_id_t module_id, unsigned int element_count, const void *data) { - const struct mod_apcontext_config *config = data; - - /* This module does not support elements */ - if (element_count != 0) + /* This module does not support elements or empty configuration. */ + if ((element_count != 0) || (data == NULL)) { return FWK_E_PARAM; + } - if (config->base == 0) - return FWK_E_DATA; - - if (config->size == 0) + ctx.config = (const struct mod_apcontext_config *)data; + if ((ctx.config->base == 0) || (ctx.config->size == 0)) { return FWK_E_DATA; + } return FWK_SUCCESS; } static int apcontext_start(fwk_id_t id) { - const struct mod_apcontext_config *config = - fwk_module_get_data(fwk_module_id_apcontext); int status; - if (!fwk_id_is_equal(config->clock_id, FWK_ID_NONE)) { + if (!fwk_id_is_equal(ctx.config->clock_id, FWK_ID_NONE)) { status = fwk_notification_subscribe( - mod_clock_notification_id_state_changed, config->clock_id, id); + mod_clock_notification_id_state_changed, ctx.config->clock_id, id); if (status != FWK_SUCCESS) { FWK_LOG_CRIT(MODULE_NAME "Failed to subscribe to clock " "notification!"); return status; } + ctx.wait_on_notifications++; } - if ((fwk_id_type_is_valid(config->platform_notification.source_id)) && + if ((fwk_id_type_is_valid(ctx.config->platform_notification.source_id)) && (!fwk_id_is_equal( - config->platform_notification.source_id, FWK_ID_NONE))) { + ctx.config->platform_notification.source_id, FWK_ID_NONE))) { status = fwk_notification_subscribe( - config->platform_notification.notification_id, - config->platform_notification.source_id, + ctx.config->platform_notification.notification_id, + ctx.config->platform_notification.source_id, id); if (status != FWK_SUCCESS) { FWK_LOG_CRIT(MODULE_NAME @@ -115,8 +110,6 @@ static int apcontext_start(fwk_id_t id) static int apcontext_process_notification(const struct fwk_event *event, struct fwk_event *resp_event) { - const struct mod_apcontext_config *config = - fwk_module_get_data(fwk_module_id_apcontext); struct clock_notification_params *params; int status; @@ -135,12 +128,14 @@ static int apcontext_process_notification(const struct fwk_event *event, "notification"); return status; } - ctx.wait_on_notifications--; + if (ctx.wait_on_notifications > 0) { + ctx.wait_on_notifications--; + } } } if (fwk_id_is_equal( - event->id, config->platform_notification.notification_id)) { + event->id, ctx.config->platform_notification.notification_id)) { /* Unsubscribe to the notification */ status = fwk_notification_unsubscribe( event->id, event->source_id, event->target_id); @@ -150,7 +145,9 @@ static int apcontext_process_notification(const struct fwk_event *event, "notification"); return status; } - ctx.wait_on_notifications--; + if (ctx.wait_on_notifications > 0) { + ctx.wait_on_notifications--; + } } if (ctx.wait_on_notifications == 0) { diff --git a/module/apcontext/test/CMakeLists.txt b/module/apcontext/test/CMakeLists.txt new file mode 100644 index 000000000..1ca8f47f7 --- /dev/null +++ b/module/apcontext/test/CMakeLists.txt @@ -0,0 +1,29 @@ +# +# Arm SCP/MCP Software +# Copyright (c) 2025, Arm Limited and Contributors. All rights reserved. +# +# SPDX-License-Identifier: BSD-3-Clause +# + +set(TEST_SRC mod_apcontext) +set(TEST_FILE mod_apcontext) + +set(UNIT_TEST_TARGET mod_${TEST_MODULE}_unit_test) + +set(MODULE_SRC ${MODULE_ROOT}/${TEST_MODULE}/src) +set(MODULE_INC ${MODULE_ROOT}/${TEST_MODULE}/include) +list(APPEND OTHER_MODULE_INC ${MODULE_ROOT}/clock/include) +set(MODULE_UT_SRC ${CMAKE_CURRENT_LIST_DIR}) +set(MODULE_UT_INC ${CMAKE_CURRENT_LIST_DIR}) + +list(APPEND MOCK_REPLACEMENTS fwk_id) +list(APPEND MOCK_REPLACEMENTS fwk_module) +list(APPEND MOCK_REPLACEMENTS fwk_notification) + +include(${SCP_ROOT}/unit_test/module_common.cmake) + +target_compile_definitions(${UNIT_TEST_TARGET} PUBLIC + "BUILD_HAS_NOTIFICATION") + +target_compile_definitions(${UNIT_TEST_TARGET} PUBLIC + "BUILD_HAS_MOD_CLOCK") diff --git a/module/apcontext/test/fwk_module_idx.h b/module/apcontext/test/fwk_module_idx.h new file mode 100644 index 000000000..772ec49d2 --- /dev/null +++ b/module/apcontext/test/fwk_module_idx.h @@ -0,0 +1,24 @@ +/* + * Arm SCP/MCP Software + * Copyright (c) 2025, Arm Limited and Contributors. All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +#ifndef TEST_FWK_MODULE_MODULE_IDX_H +#define TEST_FWK_MODULE_MODULE_IDX_H + +enum fwk_module_idx { + FWK_MODULE_IDX_APCONTEXT, + FWK_MODULE_IDX_CLOCK, + FWK_MODULE_IDX_FAKE, + FWK_MODULE_IDX_COUNT, +}; + +static const fwk_id_t fwk_module_id_apcontext = + FWK_ID_MODULE_INIT(FWK_MODULE_IDX_APCONTEXT); + +static const fwk_id_t fwk_module_id_fake = + FWK_ID_MODULE_INIT(FWK_MODULE_IDX_FAKE); + +#endif /* TEST_FWK_MODULE_MODULE_IDX_H */ diff --git a/module/apcontext/test/mod_apcontext_unit_test.c b/module/apcontext/test/mod_apcontext_unit_test.c new file mode 100644 index 000000000..992425978 --- /dev/null +++ b/module/apcontext/test/mod_apcontext_unit_test.c @@ -0,0 +1,363 @@ +/* + * Arm SCP/MCP Software + * Copyright (c) 2025, Arm Limited and Contributors. All rights reserved. + * + * SPDX-License-Identifier: BSD-3-Clause + */ + +#include "scp_unity.h" +#include "unity.h" + +#include +#include +#include + +#include +#include + +#include + +#include UNIT_TEST_SRC + +void setUp(void) +{ + memset(&ctx, 0, sizeof(ctx)); +} + +void tearDown(void) +{ + /* Do Nothing */ +} + +void test_apcontext_init_success(void) +{ + const struct mod_apcontext_config config = { + .base = 0x1000, + .size = 100, + }; + int status; + + status = apcontext_init(fwk_module_id_apcontext, 0, &config); + TEST_ASSERT_EQUAL(FWK_SUCCESS, status); +} + +void test_apcontext_init_with_elements_fail(void) +{ + int status = apcontext_init(fwk_module_id_apcontext, 1, NULL); + TEST_ASSERT_EQUAL(FWK_E_PARAM, status); +} + +void test_apcontext_init_with_null_config_fail(void) +{ + int status = apcontext_init(fwk_module_id_apcontext, 0, NULL); + TEST_ASSERT_EQUAL(FWK_E_PARAM, status); +} + +void test_apcontext_init_null_base_or_size_fail(void) +{ + struct mod_apcontext_config config = { 0 }; + int status; + + config.base = 0x0; + config.size = 100; + + status = apcontext_init(fwk_module_id_apcontext, 0, &config); + TEST_ASSERT_EQUAL(FWK_E_DATA, status); + + config.base = 0x1000; + config.size = 0; + + status = apcontext_init(fwk_module_id_apcontext, 0, &config); + TEST_ASSERT_EQUAL(FWK_E_DATA, status); +} + +void test_apcontext_start_with_notifications_subscribe_success(void) +{ + const struct mod_apcontext_config config = { + .base = 0x1000, + .size = 100, + .clock_id = FWK_ID_ELEMENT_INIT(FWK_MODULE_IDX_CLOCK, 0), + .platform_notification.notification_id = + FWK_ID_NOTIFICATION_INIT(FWK_MODULE_IDX_FAKE, 0), + .platform_notification.source_id = + FWK_ID_ELEMENT_INIT(FWK_MODULE_IDX_FAKE, 0), + }; + int status; + + ctx.config = &config; + + fwk_id_is_equal_ExpectAndReturn(config.clock_id, FWK_ID_NONE, false); + fwk_notification_subscribe_ExpectAnyArgsAndReturn(FWK_SUCCESS); + + fwk_id_type_is_valid_ExpectAndReturn( + config.platform_notification.source_id, true); + fwk_id_is_equal_ExpectAndReturn( + config.platform_notification.source_id, FWK_ID_NONE, false); + fwk_notification_subscribe_ExpectAnyArgsAndReturn(FWK_SUCCESS); + + status = apcontext_start(fwk_module_id_apcontext); + TEST_ASSERT_EQUAL(FWK_SUCCESS, status); + TEST_ASSERT_EQUAL(2, ctx.wait_on_notifications); +} + +void test_apcontext_start_with_no_subscribers_success(void) +{ + uint8_t buff[100], expected; + const struct mod_apcontext_config config = { + .base = (uintptr_t)buff, + .size = 100, + .clock_id = FWK_ID_NONE, + .platform_notification.source_id = FWK_ID_NONE, + }; + int status; + + ctx.config = &config; + + fwk_id_is_equal_ExpectAndReturn(config.clock_id, FWK_ID_NONE, true); + + fwk_id_type_is_valid_ExpectAndReturn( + config.platform_notification.source_id, true); + fwk_id_is_equal_ExpectAndReturn( + config.platform_notification.source_id, FWK_ID_NONE, true); + expected = 0; + + status = apcontext_start(fwk_module_id_apcontext); + TEST_ASSERT_EQUAL(FWK_SUCCESS, status); + TEST_ASSERT_EQUAL(0, ctx.wait_on_notifications); + TEST_ASSERT_EACH_EQUAL_UINT8(expected, buff, 100); +} + +void test_apcontext_start_with_clock_notifications_fail(void) +{ + const struct mod_apcontext_config config = { + .base = 0x1000, + .size = 100, + .clock_id = FWK_ID_ELEMENT_INIT(FWK_MODULE_IDX_CLOCK, 0), + .platform_notification.source_id = FWK_ID_NONE, + }; + int status; + + ctx.config = &config; + + fwk_id_is_equal_ExpectAndReturn(config.clock_id, FWK_ID_NONE, false); + fwk_notification_subscribe_ExpectAnyArgsAndReturn(FWK_E_PARAM); + + status = apcontext_start(fwk_module_id_apcontext); + TEST_ASSERT_EQUAL(FWK_E_PARAM, status); + TEST_ASSERT_EQUAL(0, ctx.wait_on_notifications); +} + +void test_apcontext_start_with_platform_notifications_fail(void) +{ + const struct mod_apcontext_config config = { + .base = 0x1000, + .size = 100, + .clock_id = FWK_ID_NONE, + .platform_notification.notification_id = + FWK_ID_NOTIFICATION_INIT(FWK_MODULE_IDX_FAKE, 0), + .platform_notification.source_id = + FWK_ID_ELEMENT_INIT(FWK_MODULE_IDX_FAKE, 0), + }; + int status; + + ctx.config = &config; + + fwk_id_is_equal_ExpectAndReturn(config.clock_id, FWK_ID_NONE, true); + + fwk_id_type_is_valid_ExpectAndReturn( + config.platform_notification.source_id, true); + fwk_id_is_equal_ExpectAndReturn( + config.platform_notification.source_id, FWK_ID_NONE, false); + fwk_notification_subscribe_ExpectAnyArgsAndReturn(FWK_E_PARAM); + + status = apcontext_start(fwk_module_id_apcontext); + TEST_ASSERT_EQUAL(FWK_E_PARAM, status); + TEST_ASSERT_EQUAL(0, ctx.wait_on_notifications); +} + +void test_apcontext_zero(void) +{ + uint8_t buff[100], expected; + const struct mod_apcontext_config config = { + .base = (uintptr_t)buff, + .size = 100, + }; + + ctx.config = &config; + + /* Fill buffer with some values and fill expected */ + memset(buff, 0xAA, 100); + expected = 0; + + apcontext_zero(); + TEST_ASSERT_EACH_EQUAL_UINT8(expected, buff, 100); +} + +void test_apcontext_process_clock_notification_success(void) +{ + uint8_t buff[100], expected; + const struct mod_apcontext_config config = { + .base = (uintptr_t)buff, + .size = 100, + }; + struct clock_notification_params params = { + .new_state = MOD_CLOCK_STATE_RUNNING, + }; + struct fwk_event clock_event = { + .id = mod_clock_notification_id_state_changed, + .source_id = FWK_ID_ELEMENT_INIT(FWK_MODULE_IDX_APCONTEXT, 0), + .target_id = fwk_module_id_apcontext, + }; + int status; + + ctx.config = &config; + ctx.wait_on_notifications = 1; + memcpy(&clock_event.params, ¶ms, sizeof(params)); + + fwk_id_is_type_ExpectAnyArgsAndReturn(true); + fwk_id_is_equal_ExpectAndReturn( + clock_event.id, mod_clock_notification_id_state_changed, true); + fwk_id_is_equal_ExpectAnyArgsAndReturn(false); + fwk_notification_unsubscribe_ExpectAnyArgsAndReturn(FWK_SUCCESS); + expected = 0; + + status = apcontext_process_notification(&clock_event, NULL); + TEST_ASSERT_EQUAL(FWK_SUCCESS, status); + TEST_ASSERT_EQUAL(0, ctx.wait_on_notifications); + TEST_ASSERT_EACH_EQUAL_UINT8(expected, buff, 100); +} + +void test_apcontext_process_platform_notification_success(void) +{ + uint8_t buff[100], expected; + const struct mod_apcontext_config config = { + .base = (uintptr_t)buff, + .size = 100, + .platform_notification.notification_id = + FWK_ID_NOTIFICATION_INIT(FWK_MODULE_IDX_FAKE, 0), + }; + struct fwk_event platform_event = { + .id = mod_clock_notification_id_state_changed, + .source_id = fwk_module_id_fake, + .target_id = fwk_module_id_apcontext, + }; + int status; + + ctx.config = &config; + ctx.wait_on_notifications = 1; + + fwk_id_is_type_ExpectAnyArgsAndReturn(true); + fwk_id_is_equal_ExpectAnyArgsAndReturn(false); + fwk_id_is_equal_ExpectAndReturn( + platform_event.id, config.platform_notification.notification_id, true); + fwk_notification_unsubscribe_ExpectAnyArgsAndReturn(FWK_SUCCESS); + + /* Fill buffer with some values and fill expected */ + memset(buff, 0xAA, 100); + expected = 0; + + status = apcontext_process_notification(&platform_event, NULL); + TEST_ASSERT_EQUAL(FWK_SUCCESS, status); + TEST_ASSERT_EQUAL(0, ctx.wait_on_notifications); + TEST_ASSERT_EACH_EQUAL_UINT8(expected, buff, 100); +} + +void test_apcontext_process_clock_notification_fail(void) +{ + uint8_t buff[100], expected; + const struct mod_apcontext_config config = { + .base = (uintptr_t)buff, + .size = 100, + }; + struct clock_notification_params params = { + .new_state = MOD_CLOCK_STATE_RUNNING, + }; + struct fwk_event clock_event = { + .id = mod_clock_notification_id_state_changed, + .source_id = FWK_ID_ELEMENT_INIT(FWK_MODULE_IDX_APCONTEXT, 0), + .target_id = fwk_module_id_apcontext, + }; + int status; + + ctx.config = &config; + ctx.wait_on_notifications = 1; + memcpy(&clock_event.params, ¶ms, sizeof(params)); + + fwk_id_is_type_ExpectAnyArgsAndReturn(true); + fwk_id_is_equal_ExpectAndReturn( + clock_event.id, mod_clock_notification_id_state_changed, true); + fwk_notification_unsubscribe_ExpectAnyArgsAndReturn(FWK_E_PARAM); + + /* Fill buffer with some values and fill expected */ + memset(buff, 0xAA, 100); + expected = 0xAA; + + status = apcontext_process_notification(&clock_event, NULL); + TEST_ASSERT_EQUAL(FWK_E_PARAM, status); + TEST_ASSERT_EQUAL(1, ctx.wait_on_notifications); + TEST_ASSERT_EACH_EQUAL_UINT8(expected, buff, 100); +} + +void test_apcontext_process_platform_notification_fail(void) +{ + uint8_t buff[100], expected; + const struct mod_apcontext_config config = { + .base = (uintptr_t)buff, + .size = 100, + .platform_notification.notification_id = + FWK_ID_NOTIFICATION_INIT(FWK_MODULE_IDX_FAKE, 0), + }; + struct fwk_event platform_event = { + .id = mod_clock_notification_id_state_changed, + .source_id = fwk_module_id_fake, + .target_id = fwk_module_id_apcontext, + }; + int status; + + ctx.config = &config; + ctx.wait_on_notifications = 1; + + fwk_id_is_type_ExpectAnyArgsAndReturn(true); + fwk_id_is_equal_ExpectAnyArgsAndReturn(false); + fwk_id_is_equal_ExpectAndReturn( + platform_event.id, config.platform_notification.notification_id, true); + fwk_notification_unsubscribe_ExpectAnyArgsAndReturn(FWK_E_PARAM); + + /* Fill buffer with some values and fill expected */ + memset(buff, 0xAA, 100); + expected = 0xAA; + + status = apcontext_process_notification(&platform_event, NULL); + TEST_ASSERT_EQUAL(FWK_E_PARAM, status); + TEST_ASSERT_EQUAL(1, ctx.wait_on_notifications); + TEST_ASSERT_EACH_EQUAL_UINT8(expected, buff, 100); +} + +int apcontext_test_main(void) +{ + UNITY_BEGIN(); + + RUN_TEST(test_apcontext_init_success); + RUN_TEST(test_apcontext_init_with_elements_fail); + RUN_TEST(test_apcontext_init_with_null_config_fail); + RUN_TEST(test_apcontext_init_null_base_or_size_fail); + + RUN_TEST(test_apcontext_start_with_notifications_subscribe_success); + RUN_TEST(test_apcontext_start_with_no_subscribers_success); + RUN_TEST(test_apcontext_start_with_clock_notifications_fail); + RUN_TEST(test_apcontext_start_with_platform_notifications_fail); + + RUN_TEST(test_apcontext_zero); + + RUN_TEST(test_apcontext_process_clock_notification_success); + RUN_TEST(test_apcontext_process_platform_notification_success); + RUN_TEST(test_apcontext_process_clock_notification_fail); + RUN_TEST(test_apcontext_process_platform_notification_fail); + + return UNITY_END(); +} + +int main(void) +{ + return apcontext_test_main(); +} diff --git a/module/mock_sensor/CMakeLists.txt b/module/mock_sensor/CMakeLists.txt new file mode 100644 index 000000000..5a816e64c --- /dev/null +++ b/module/mock_sensor/CMakeLists.txt @@ -0,0 +1,16 @@ +# +# Arm SCP/MCP Software +# Copyright (c) 2025, Arm Limited and Contributors. All rights reserved. +# +# SPDX-License-Identifier: BSD-3-Clause +# + +add_library(${SCP_MODULE_TARGET} SCP_MODULE) + +target_include_directories(${SCP_MODULE_TARGET} + PUBLIC "${CMAKE_CURRENT_SOURCE_DIR}/include") + +target_sources(${SCP_MODULE_TARGET} + PRIVATE "${CMAKE_CURRENT_SOURCE_DIR}/src/mod_mock_sensor.c") + +target_link_libraries(${SCP_MODULE_TARGET} PRIVATE module-timer) diff --git a/module/mock_sensor/Module.cmake b/module/mock_sensor/Module.cmake new file mode 100644 index 000000000..7627dbad8 --- /dev/null +++ b/module/mock_sensor/Module.cmake @@ -0,0 +1,9 @@ +# +# Arm SCP/MCP Software +# Copyright (c) 2025, Arm Limited and Contributors. All rights reserved. +# +# SPDX-License-Identifier: BSD-3-Clause +# + +set(SCP_MODULE "mock-sensor") +set(SCP_MODULE_TARGET "module-mock-sensor") diff --git a/unit_test/CMakeLists.txt b/unit_test/CMakeLists.txt index 5cc908755..f5732864d 100644 --- a/unit_test/CMakeLists.txt +++ b/unit_test/CMakeLists.txt @@ -111,6 +111,7 @@ list(APPEND FWK_SRC ${ARCH_SRC_ROOT}/arch_interrupt.c) list(APPEND SCP_UNITY_SRC ${TEST_ROOT}/unity_mocks/scp_unity.c) #Append common unit tests below here (alphabetical order) +list(APPEND UNIT_MODULE apcontext) list(APPEND UNIT_MODULE armv8r_mpu) list(APPEND UNIT_MODULE amu_mmap) list(APPEND UNIT_MODULE amu_smcf_drv) -- GitLab