From 945cc06e4a5e371a2cb73d181ac22d6474fa730b Mon Sep 17 00:00:00 2001 From: Mohamed Omar Asaker Date: Sat, 28 Jun 2025 13:57:32 +0100 Subject: [PATCH 1/2] mod/scmi_power_capping: fix check on cap step vs cap range The check answer the validity of the step size compared to the range (min and max). but step cannot be zero only if min and max power cap are different. Add ut for valid cap_step = 0. Signed-off-by: Mohamed Omar Asaker --- .../src/scmi_power_capping_core.c | 10 ++++++---- .../test/scmi_power_capping_core_unit_test.c | 16 ++++++++++++++++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/module/scmi_power_capping/src/scmi_power_capping_core.c b/module/scmi_power_capping/src/scmi_power_capping_core.c index 6daa42aa..ef347970 100644 --- a/module/scmi_power_capping/src/scmi_power_capping_core.c +++ b/module/scmi_power_capping/src/scmi_power_capping_core.c @@ -75,10 +75,12 @@ static int pcapping_core_check_domain_configuration( return FWK_E_DATA; } - if (config->min_power_cap != config->max_power_cap) { - if (config->power_cap_step == (uint32_t)0) { - return FWK_E_DATA; - } + if (config->min_power_cap == config->max_power_cap) { + return FWK_SUCCESS; + } + + if (config->power_cap_step == (uint32_t)0) { + return FWK_E_DATA; } if ((config->max_power_cap - config->min_power_cap) % diff --git a/module/scmi_power_capping/test/scmi_power_capping_core_unit_test.c b/module/scmi_power_capping/test/scmi_power_capping_core_unit_test.c index cfc3a374..24a587b8 100644 --- a/module/scmi_power_capping/test/scmi_power_capping_core_unit_test.c +++ b/module/scmi_power_capping/test/scmi_power_capping_core_unit_test.c @@ -127,6 +127,20 @@ void utest_pcapping_core_check_domain_configuration_invalid_cap_step_zero(void) TEST_ASSERT_EQUAL(status, FWK_E_DATA); } +void utest_pcapping_core_check_domain_configuration_valid_cap_step_zero(void) +{ + int status; + + struct mod_scmi_power_capping_domain_config config = { + .min_power_cap = 10u, + .max_power_cap = 10u, + .power_cap_step = 0u, + }; + + status = pcapping_core_check_domain_configuration(&config); + TEST_ASSERT_EQUAL(status, FWK_SUCCESS); +} + void utest_pcapping_core_check_domain_configuration_success(void) { int status; @@ -921,6 +935,8 @@ int scmi_test_main(void) RUN_TEST(utest_pcapping_core_check_domain_configuration_max_cap_0); RUN_TEST( utest_pcapping_core_check_domain_configuration_invalid_cap_step_zero); + RUN_TEST( + utest_pcapping_core_check_domain_configuration_valid_cap_step_zero); RUN_TEST(utest_pcapping_core_check_domain_configuration_success); RUN_TEST(utest_pcapping_core_bind); RUN_TEST(utest_pcapping_core_init); -- GitLab From 2f15a792e1a6b37517de7500929a26710e6e0759 Mon Sep 17 00:00:00 2001 From: Mohamed Omar Asaker Date: Fri, 27 Jun 2025 19:23:10 +0100 Subject: [PATCH 2/2] mod/scmi_power_capping: fix use of uninitialized domain config Fixes a bug where `domain_ctx->config` was passed to the validation function before being assigned. The correct behavior is to validate the `config` parameter first, then assign it to `domain_ctx->config` only if valid. This change updates `pcapping_core_domain_init()` to pass the received `config` parameter to `pcapping_core_check_domain_configuration()`. Assignment to `domain_ctx->config` is performed only after successful validation. A unit test has been added to ensure domain initialization fails when invalid configuration values are provided, and that the domain context remains uninitialized in such cases. Signed-off-by: Mohamed Omar Asaker --- .../src/scmi_power_capping_core.c | 2 +- .../test/scmi_power_capping_core_unit_test.c | 23 +++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/module/scmi_power_capping/src/scmi_power_capping_core.c b/module/scmi_power_capping/src/scmi_power_capping_core.c index ef347970..e66b18ff 100644 --- a/module/scmi_power_capping/src/scmi_power_capping_core.c +++ b/module/scmi_power_capping/src/scmi_power_capping_core.c @@ -315,7 +315,7 @@ int pcapping_core_domain_init( return status; } - status = pcapping_core_check_domain_configuration(domain_ctx->config); + status = pcapping_core_check_domain_configuration(config); if (!fwk_expect(status == FWK_SUCCESS)) { return status; diff --git a/module/scmi_power_capping/test/scmi_power_capping_core_unit_test.c b/module/scmi_power_capping/test/scmi_power_capping_core_unit_test.c index 24a587b8..8c841a4a 100644 --- a/module/scmi_power_capping/test/scmi_power_capping_core_unit_test.c +++ b/module/scmi_power_capping/test/scmi_power_capping_core_unit_test.c @@ -202,6 +202,27 @@ void utest_pcapping_core_domain_init(void) TEST_ASSERT_EQUAL(status, FWK_SUCCESS); } +void utest_pcapping_core_domain_init_invalid_config(void) +{ + int status; + uint32_t domain_idx = FAKE_POWER_CAPPING_IDX_COUNT - 1u; + fwk_id_t none_id = FWK_ID_NONE; + struct mod_scmi_power_capping_domain_config invalid_configs = { + .min_power_cap = 9u, + .max_power_cap = 100u, + .power_cap_step = 2u, + }; + (void)none_id; + + struct mod_scmi_power_capping_domain_context *ctx = + &pcapping_core_ctx.power_capping_domain_ctx_table[domain_idx]; + + status = pcapping_core_domain_init(domain_idx, &invalid_configs); + + TEST_ASSERT_EQUAL(FWK_E_DATA, status); + TEST_ASSERT(ctx->config != &invalid_configs); +} + void utest_pcapping_core_start(void) { int status; @@ -938,6 +959,8 @@ int scmi_test_main(void) RUN_TEST( utest_pcapping_core_check_domain_configuration_valid_cap_step_zero); RUN_TEST(utest_pcapping_core_check_domain_configuration_success); + RUN_TEST(utest_pcapping_core_domain_init); + RUN_TEST(utest_pcapping_core_domain_init_invalid_config); RUN_TEST(utest_pcapping_core_bind); RUN_TEST(utest_pcapping_core_init); RUN_TEST(utest_pcapping_core_start); -- GitLab