ARM-software / psa-arch-tests

Tests for verifying implementations of TBSA-v8M and the PSA Certified APIs
Apache License 2.0
67 stars 103 forks source link

Missing target_database.h #378

Closed UEWBot closed 4 months ago

UEWBot commented 5 months ago

I was building the IPC PSA Arch test for a new platform, and I got an error that target_database.h was missing while building val_target.c.

Digging into it, I was building the SPE code and the psa_generate_database rule was not being run because it is only a dependency of the NSPE build.

I applied this patch, which fixed the issue:

diff --git a/api-tests/CMakeLists.txt b/api-tests/CMakeLists.txt
index 602e6f6559cc..07a665f142b1 100644
--- a/api-tests/CMakeLists.txt
+++ b/api-tests/CMakeLists.txt
@@ -685,6 +685,7 @@ if(${SUITE} STREQUAL "IPC")
 add_dependencies(${PSA_TARGET_DRIVER_PARTITION_LIB}    ${PSA_TARGET_TEST_COMBINE_LIB})
 add_dependencies(${PSA_TARGET_CLIENT_PARTITION_LIB}    ${PSA_TARGET_DRIVER_PARTITION_LIB})
 add_dependencies(${PSA_TARGET_SERVER_PARTITION_LIB}    ${PSA_TARGET_CLIENT_PARTITION_LIB})
+add_dependencies(${PSA_TARGET_SERVER_PARTITION_LIB}    ${PSA_TARGET_GENERATE_DATABASE_POST})
 endif()

 # Include the files for make clean

Chris

avinaw01-arm commented 4 months ago

Hi @UEWBot,

I see that the 'psa_generate_database' rule is already running prior to this in the CMakeLists, irrespective of the suite being run. Please refer to lines 684 to 686 here: https://github.com/ARM-software/psa-arch-tests/blob/21f47a0e4a4fb1d3366402107632f636bb1933b4/api-tests/CMakeLists.txt#L684. I also ran regressions for the IPC suite locally to verify this; it was building fine. Therefore, I don't see a need to add the same functionality line again in the CMakeLists. I suggest that if it's working for your platform, you can keep this patch-set locally. Amending it in the main branch could affect other partner builds as well.

Also, if possible, please share the new platform you're using and the exact error you're facing during the build. I'll have a look on our end, but I don't see a reason to add this same functionality line again in the CMake.

Regards, Avi.

avinaw01-arm commented 4 months ago
Screenshot 2024-06-25 at 12 14 12 PM

Build is running fine for me.

UEWBot commented 4 months ago

It's exactly lines 686 and 687 in that cmakefile that I was looking at: add_dependencies(${PSA_TARGET_PAL_NSPE_LIB} ${PSA_TARGET_GENERATE_DATABASE_POST}) add_dependencies(${PSA_TARGET_VAL_NSPE_LIB} ${PSA_TARGET_PAL_NSPE_LIB}) so the database is only generated for an NSPE build, not for an SPE build.

But looking at api-tests/val/val_spe.cmake, lines 19-20: list(APPEND VAL_SRC_C_SPE ${PSA_ROOT_DIR}/val/spe/val_driver_service_apis.c ) and at val_driver_service_apis.c, line 20: `

include "val_target.c"

we see that val_target.c is needed for the *SPE* build as well as the NSPE build. And looking at val_target.c, line 19:

include "target_database.h"

` we see that it needs the target database. Therefore the target database is needed for the SPE build, not just the NSPE build. Hence my patch.

avinaw01-arm commented 4 months ago

Got your point, @UEWBot. Thank you for spotting and correcting this issue! We have merged the proposed changes in the repo., via triggering a P.R. #381.