Hi all It seems to me like following check
# FWU_DEVICE_CONFIG_FILE exists and is a file if(NOT EXISTS ${FWU_DEVICE_CONFIG_FILE}) message(FATAL_ERROR "FWU_DEVICE_CONFIG_FILE:${FWU_DEVICE_CONFIG_FILE} does not exist.") elseif(IS_DIRECTORY ${FWU_DEVICE_CONFIG_FILE}) message(FATAL_ERROR "FWU_DEVICE_CONFIG_FILE:${FWU_DEVICE_CONFIG_FILE} is a folder while a file is expected.") endif()
in secure_fw/partitions/firmware_update/CMakeLists.txt is redundant as FWU_DEVICE_CONFIG_FILE may be generated, thus not present when cmake performs EXISTS check (note that by default FWU_DEVICE_CONFIG_FILE is generated so I dont see point in limiting user from using generated file)
So i propose to remove this check.
Regards, Bohdan Hunko
Cypress Semiconductor Ukraine Engineer CSUKR CSS ICW SW FW Mobile: +38099 50 19 714 Bohdan.Hunko@infineon.commailto:Bohdan.Hunko@infineon.com
Hi Bohdan,
The code you posted is in the else routine of FWU_DEVICE_CONFIG_FILE not defined condition(see https://git.trustedfirmware.org/TF-M/trusted-firmware-m.git/tree/secure_fw/p...):
if (NOT FWU_DEVICE_CONFIG_FILE) configure_file(${CMAKE_SOURCE_DIR}/interface/include/psa/fwu_config.h.in ${CMAKE_BINARY_DIR}/generated/interface/include/psa/fwu_config.h @ONLY) set(FWU_DEVICE_CONFIG_FILE "${CMAKE_BINARY_DIR}/generated/interface/include/psa/fwu_config.h") else() # FWU_DEVICE_CONFIG_FILE exists and is a file if(NOT EXISTS ${FWU_DEVICE_CONFIG_FILE}) message(FATAL_ERROR "FWU_DEVICE_CONFIG_FILE:${FWU_DEVICE_CONFIG_FILE} does not exist.") elseif(IS_DIRECTORY ${FWU_DEVICE_CONFIG_FILE}) message(FATAL_ERROR "FWU_DEVICE_CONFIG_FILE:${FWU_DEVICE_CONFIG_FILE} is a folder while a file is expected.") endif() endif()
If FWU_DEVICE_CONFIG_FILE is defined, it should be an existing file. The default routine is FWU_DEVICE_CONFIG_FILE not-defined(the if routine).
Regards, Sherry Zhang
From: Bohdan.Hunko--- via TF-M tf-m@lists.trustedfirmware.org Sent: Tuesday, November 14, 2023 11:43 PM To: tf-m@lists.trustedfirmware.org Subject: [TF-M] Redundant check on FWU_DEVICE_CONFIG_FILE
Hi all It seems to me like following check
# FWU_DEVICE_CONFIG_FILE exists and is a file if(NOT EXISTS ${FWU_DEVICE_CONFIG_FILE}) message(FATAL_ERROR "FWU_DEVICE_CONFIG_FILE:${FWU_DEVICE_CONFIG_FILE} does not exist.") elseif(IS_DIRECTORY ${FWU_DEVICE_CONFIG_FILE}) message(FATAL_ERROR "FWU_DEVICE_CONFIG_FILE:${FWU_DEVICE_CONFIG_FILE} is a folder while a file is expected.") endif()
in secure_fw/partitions/firmware_update/CMakeLists.txt is redundant as FWU_DEVICE_CONFIG_FILE may be generated, thus not present when cmake performs EXISTS check (note that by default FWU_DEVICE_CONFIG_FILE is generated so I dont see point in limiting user from using generated file)
So i propose to remove this check.
Regards, Bohdan Hunko
Cypress Semiconductor Ukraine Engineer CSUKR CSS ICW SW FW Mobile: +38099 50 19 714 Bohdan.Hunko@infineon.commailto:Bohdan.Hunko@infineon.com
Hi Sherry,
Yes, you are correct - the check is only done if FWU_DEVICE_CONFIG_FILE is not defined.
BUT this is still a problem because even if FWU_DEVICE_CONFIG_FILE is defined it still may be generated (same way as if FWU_DEVICE_CONFIG_FILE is not defined it is generated). So default TFM implementation use generated file as FWU_DEVICE_CONFIG_FILE but restricts user from doing so.
So in current implementation FWU_DEVICE_CONFIG_FILE must be present on cmake config time and can not be generated. I see this as unnecessary limitation.
Actually this is not hypothetical problem, I only brought this problem up because I hit this issue when trying to use custom FWU_DEVICE_CONFIG_FILE and this custom file is generated (same way as default one is).
So my proposal is to remove this check, this will make the implementation more flexible.
Regards, Bohdan Hunko
Cypress Semiconductor Ukraine Engineer CSUKR CSS ICW SW FW Mobile: +38099 50 19 714 Bohdan.Hunko@infineon.commailto:Bohdan.Hunko@infineon.com
From: Sherry Zhang Sherry.Zhang2@arm.com Sent: Wednesday, November 15, 2023 04:32 To: Hunko Bohdan (CSS ICW SW FW 3) Bohdan.Hunko@infineon.com; tf-m@lists.trustedfirmware.org Cc: nd nd@arm.com Subject: RE: Redundant check on FWU_DEVICE_CONFIG_FILE
Caution: This e-mail originated outside Infineon Technologies. Do not click on links or open attachments unless you validate it is safehttps://intranet-content.infineon.com/explore/aboutinfineon/rules/informationsecurity/ug/SocialEngineering/Pages/SocialEngineeringElements_en.aspx.
Hi Bohdan,
The code you posted is in the else routine of FWU_DEVICE_CONFIG_FILE not defined condition(see https://git.trustedfirmware.org/TF-M/trusted-firmware-m.git/tree/secure_fw/p...):
if (NOT FWU_DEVICE_CONFIG_FILE) configure_file(${CMAKE_SOURCE_DIR}/interface/include/psa/fwu_config.h.in ${CMAKE_BINARY_DIR}/generated/interface/include/psa/fwu_config.h @ONLY) set(FWU_DEVICE_CONFIG_FILE "${CMAKE_BINARY_DIR}/generated/interface/include/psa/fwu_config.h") else() # FWU_DEVICE_CONFIG_FILE exists and is a file if(NOT EXISTS ${FWU_DEVICE_CONFIG_FILE}) message(FATAL_ERROR "FWU_DEVICE_CONFIG_FILE:${FWU_DEVICE_CONFIG_FILE} does not exist.") elseif(IS_DIRECTORY ${FWU_DEVICE_CONFIG_FILE}) message(FATAL_ERROR "FWU_DEVICE_CONFIG_FILE:${FWU_DEVICE_CONFIG_FILE} is a folder while a file is expected.") endif() endif()
If FWU_DEVICE_CONFIG_FILE is defined, it should be an existing file. The default routine is FWU_DEVICE_CONFIG_FILE not-defined(the if routine).
Regards, Sherry Zhang
From: Bohdan.Hunko--- via TF-M <tf-m@lists.trustedfirmware.orgmailto:tf-m@lists.trustedfirmware.org> Sent: Tuesday, November 14, 2023 11:43 PM To: tf-m@lists.trustedfirmware.orgmailto:tf-m@lists.trustedfirmware.org Subject: [TF-M] Redundant check on FWU_DEVICE_CONFIG_FILE
Hi all It seems to me like following check
# FWU_DEVICE_CONFIG_FILE exists and is a file if(NOT EXISTS ${FWU_DEVICE_CONFIG_FILE}) message(FATAL_ERROR "FWU_DEVICE_CONFIG_FILE:${FWU_DEVICE_CONFIG_FILE} does not exist.") elseif(IS_DIRECTORY ${FWU_DEVICE_CONFIG_FILE}) message(FATAL_ERROR "FWU_DEVICE_CONFIG_FILE:${FWU_DEVICE_CONFIG_FILE} is a folder while a file is expected.") endif()
in secure_fw/partitions/firmware_update/CMakeLists.txt is redundant as FWU_DEVICE_CONFIG_FILE may be generated, thus not present when cmake performs EXISTS check (note that by default FWU_DEVICE_CONFIG_FILE is generated so I dont see point in limiting user from using generated file)
So i propose to remove this check.
Regards, Bohdan Hunko
Cypress Semiconductor Ukraine Engineer CSUKR CSS ICW SW FW Mobile: +38099 50 19 714 Bohdan.Hunko@infineon.commailto:Bohdan.Hunko@infineon.com
Hi Bohdan,
FWU_DEVICE_CONFIG_FILE should be defined as the header file which is included in the source file. May I know what kind of value does FWU_DEVICE_CONFIG_FILE is defined to in your case? And why it is defined while it still needs to be generated?
Regards, Sherry Zhang
From: Bohdan.Hunko@infineon.com Bohdan.Hunko@infineon.com Sent: Monday, November 27, 2023 9:37 PM To: Sherry Zhang Sherry.Zhang2@arm.com; tf-m@lists.trustedfirmware.org Cc: nd nd@arm.com Subject: RE: Redundant check on FWU_DEVICE_CONFIG_FILE
Hi Sherry,
Yes, you are correct - the check is only done if FWU_DEVICE_CONFIG_FILE is not defined.
BUT this is still a problem because even if FWU_DEVICE_CONFIG_FILE is defined it still may be generated (same way as if FWU_DEVICE_CONFIG_FILE is not defined it is generated). So default TFM implementation use generated file as FWU_DEVICE_CONFIG_FILE but restricts user from doing so.
So in current implementation FWU_DEVICE_CONFIG_FILE must be present on cmake config time and can not be generated. I see this as unnecessary limitation.
Actually this is not hypothetical problem, I only brought this problem up because I hit this issue when trying to use custom FWU_DEVICE_CONFIG_FILE and this custom file is generated (same way as default one is).
So my proposal is to remove this check, this will make the implementation more flexible.
Regards, Bohdan Hunko
Cypress Semiconductor Ukraine Engineer CSUKR CSS ICW SW FW Mobile: +38099 50 19 714 Bohdan.Hunko@infineon.commailto:Bohdan.Hunko@infineon.com
From: Sherry Zhang <Sherry.Zhang2@arm.commailto:Sherry.Zhang2@arm.com> Sent: Wednesday, November 15, 2023 04:32 To: Hunko Bohdan (CSS ICW SW FW 3) <Bohdan.Hunko@infineon.commailto:Bohdan.Hunko@infineon.com>; tf-m@lists.trustedfirmware.orgmailto:tf-m@lists.trustedfirmware.org Cc: nd <nd@arm.commailto:nd@arm.com> Subject: RE: Redundant check on FWU_DEVICE_CONFIG_FILE
Caution: This e-mail originated outside Infineon Technologies. Do not click on links or open attachments unless you validate it is safehttps://intranet-content.infineon.com/explore/aboutinfineon/rules/informationsecurity/ug/SocialEngineering/Pages/SocialEngineeringElements_en.aspx.
Hi Bohdan,
The code you posted is in the else routine of FWU_DEVICE_CONFIG_FILE not defined condition(see https://git.trustedfirmware.org/TF-M/trusted-firmware-m.git/tree/secure_fw/p...):
if (NOT FWU_DEVICE_CONFIG_FILE) configure_file(${CMAKE_SOURCE_DIR}/interface/include/psa/fwu_config.h.in ${CMAKE_BINARY_DIR}/generated/interface/include/psa/fwu_config.h @ONLY) set(FWU_DEVICE_CONFIG_FILE "${CMAKE_BINARY_DIR}/generated/interface/include/psa/fwu_config.h") else() # FWU_DEVICE_CONFIG_FILE exists and is a file if(NOT EXISTS ${FWU_DEVICE_CONFIG_FILE}) message(FATAL_ERROR "FWU_DEVICE_CONFIG_FILE:${FWU_DEVICE_CONFIG_FILE} does not exist.") elseif(IS_DIRECTORY ${FWU_DEVICE_CONFIG_FILE}) message(FATAL_ERROR "FWU_DEVICE_CONFIG_FILE:${FWU_DEVICE_CONFIG_FILE} is a folder while a file is expected.") endif() endif()
If FWU_DEVICE_CONFIG_FILE is defined, it should be an existing file. The default routine is FWU_DEVICE_CONFIG_FILE not-defined(the if routine).
Regards, Sherry Zhang
From: Bohdan.Hunko--- via TF-M <tf-m@lists.trustedfirmware.orgmailto:tf-m@lists.trustedfirmware.org> Sent: Tuesday, November 14, 2023 11:43 PM To: tf-m@lists.trustedfirmware.orgmailto:tf-m@lists.trustedfirmware.org Subject: [TF-M] Redundant check on FWU_DEVICE_CONFIG_FILE
Hi all It seems to me like following check
# FWU_DEVICE_CONFIG_FILE exists and is a file if(NOT EXISTS ${FWU_DEVICE_CONFIG_FILE}) message(FATAL_ERROR "FWU_DEVICE_CONFIG_FILE:${FWU_DEVICE_CONFIG_FILE} does not exist.") elseif(IS_DIRECTORY ${FWU_DEVICE_CONFIG_FILE}) message(FATAL_ERROR "FWU_DEVICE_CONFIG_FILE:${FWU_DEVICE_CONFIG_FILE} is a folder while a file is expected.") endif()
in secure_fw/partitions/firmware_update/CMakeLists.txt is redundant as FWU_DEVICE_CONFIG_FILE may be generated, thus not present when cmake performs EXISTS check (note that by default FWU_DEVICE_CONFIG_FILE is generated so I dont see point in limiting user from using generated file)
So i propose to remove this check.
Regards, Bohdan Hunko
Cypress Semiconductor Ukraine Engineer CSUKR CSS ICW SW FW Mobile: +38099 50 19 714 Bohdan.Hunko@infineon.commailto:Bohdan.Hunko@infineon.com
Hi Sherry,
Let me explain what I am doing.
Here is what is done in default use case (FWU_DEVICE_CONFIG_FILE is not defined):
* fwu_config.h is set to be generated from fwu_config.h.in * fwu_config.h is set to be used as FWU_DEVICE_CONFIG_FILE
By default fwu_config.h only has 2 component IDs (S and NS) and In my case I have more components, thus I want to define more IDs, so I need custom fwu_config.h. Keep in mind that I still use everything, other than image IDs, same as in default case (MCUboot, ...)
So I have done the following:
1. Copied fwu_config.h.in to my platform folder 2. Changed image IDs in fwu_config.h.in 3. fwu_config.h is set to be generated from fwu_config.h.in (This is exactly the same as what is done in default case) 4. fwu_config.h is set to be used as FWU_DEVICE_CONFIG_FILE (This is exactly the same as what is done in default case) 5. BUT I get an error which says that fwu_config.h does not exist. And it is true, it will only be generated after CMake is done and it will be a valid .h file.
I hope this shown you the problem I am pointing to - In default case FWU_DEVICE_CONFIG_FILE is generated (thus not present in Cmake config time) BUT for some reasons TFM restricts user from having his FWU_DEVICE_CONFIG_FILE generated (because TFM code has a check to verify that FWU_DEVICE_CONFIG_FILE exists, which may not be the case if FWU_DEVICE_CONFIG_FILE is generated).
Regards, Bohdan Hunko
Cypress Semiconductor Ukraine Engineer CSUKR CSS ICW SW FW Mobile: +38099 50 19 714 Bohdan.Hunko@infineon.commailto:Bohdan.Hunko@infineon.com
From: Sherry Zhang Sherry.Zhang2@arm.com Sent: Tuesday, November 28, 2023 07:46 To: Hunko Bohdan (CSS ICW SW FW 3) Bohdan.Hunko@infineon.com; tf-m@lists.trustedfirmware.org Cc: nd nd@arm.com Subject: RE: Redundant check on FWU_DEVICE_CONFIG_FILE
Caution: This e-mail originated outside Infineon Technologies. Do not click on links or open attachments unless you validate it is safehttps://intranet-content.infineon.com/explore/aboutinfineon/rules/informationsecurity/ug/SocialEngineering/Pages/SocialEngineeringElements_en.aspx.
Hi Bohdan,
FWU_DEVICE_CONFIG_FILE should be defined as the header file which is included in the source file. May I know what kind of value does FWU_DEVICE_CONFIG_FILE is defined to in your case? And why it is defined while it still needs to be generated?
Regards, Sherry Zhang
From: Bohdan.Hunko@infineon.commailto:Bohdan.Hunko@infineon.com <Bohdan.Hunko@infineon.commailto:Bohdan.Hunko@infineon.com> Sent: Monday, November 27, 2023 9:37 PM To: Sherry Zhang <Sherry.Zhang2@arm.commailto:Sherry.Zhang2@arm.com>; tf-m@lists.trustedfirmware.orgmailto:tf-m@lists.trustedfirmware.org Cc: nd <nd@arm.commailto:nd@arm.com> Subject: RE: Redundant check on FWU_DEVICE_CONFIG_FILE
Hi Sherry,
Yes, you are correct - the check is only done if FWU_DEVICE_CONFIG_FILE is not defined.
BUT this is still a problem because even if FWU_DEVICE_CONFIG_FILE is defined it still may be generated (same way as if FWU_DEVICE_CONFIG_FILE is not defined it is generated). So default TFM implementation use generated file as FWU_DEVICE_CONFIG_FILE but restricts user from doing so.
So in current implementation FWU_DEVICE_CONFIG_FILE must be present on cmake config time and can not be generated. I see this as unnecessary limitation.
Actually this is not hypothetical problem, I only brought this problem up because I hit this issue when trying to use custom FWU_DEVICE_CONFIG_FILE and this custom file is generated (same way as default one is).
So my proposal is to remove this check, this will make the implementation more flexible.
Regards, Bohdan Hunko
Cypress Semiconductor Ukraine Engineer CSUKR CSS ICW SW FW Mobile: +38099 50 19 714 Bohdan.Hunko@infineon.commailto:Bohdan.Hunko@infineon.com
From: Sherry Zhang <Sherry.Zhang2@arm.commailto:Sherry.Zhang2@arm.com> Sent: Wednesday, November 15, 2023 04:32 To: Hunko Bohdan (CSS ICW SW FW 3) <Bohdan.Hunko@infineon.commailto:Bohdan.Hunko@infineon.com>; tf-m@lists.trustedfirmware.orgmailto:tf-m@lists.trustedfirmware.org Cc: nd <nd@arm.commailto:nd@arm.com> Subject: RE: Redundant check on FWU_DEVICE_CONFIG_FILE
Caution: This e-mail originated outside Infineon Technologies. Do not click on links or open attachments unless you validate it is safehttps://intranet-content.infineon.com/explore/aboutinfineon/rules/informationsecurity/ug/SocialEngineering/Pages/SocialEngineeringElements_en.aspx.
Hi Bohdan,
The code you posted is in the else routine of FWU_DEVICE_CONFIG_FILE not defined condition(see https://git.trustedfirmware.org/TF-M/trusted-firmware-m.git/tree/secure_fw/p...):
if (NOT FWU_DEVICE_CONFIG_FILE) configure_file(${CMAKE_SOURCE_DIR}/interface/include/psa/fwu_config.h.in ${CMAKE_BINARY_DIR}/generated/interface/include/psa/fwu_config.h @ONLY) set(FWU_DEVICE_CONFIG_FILE "${CMAKE_BINARY_DIR}/generated/interface/include/psa/fwu_config.h") else() # FWU_DEVICE_CONFIG_FILE exists and is a file if(NOT EXISTS ${FWU_DEVICE_CONFIG_FILE}) message(FATAL_ERROR "FWU_DEVICE_CONFIG_FILE:${FWU_DEVICE_CONFIG_FILE} does not exist.") elseif(IS_DIRECTORY ${FWU_DEVICE_CONFIG_FILE}) message(FATAL_ERROR "FWU_DEVICE_CONFIG_FILE:${FWU_DEVICE_CONFIG_FILE} is a folder while a file is expected.") endif() endif()
If FWU_DEVICE_CONFIG_FILE is defined, it should be an existing file. The default routine is FWU_DEVICE_CONFIG_FILE not-defined(the if routine).
Regards, Sherry Zhang
From: Bohdan.Hunko--- via TF-M <tf-m@lists.trustedfirmware.orgmailto:tf-m@lists.trustedfirmware.org> Sent: Tuesday, November 14, 2023 11:43 PM To: tf-m@lists.trustedfirmware.orgmailto:tf-m@lists.trustedfirmware.org Subject: [TF-M] Redundant check on FWU_DEVICE_CONFIG_FILE
Hi all It seems to me like following check
# FWU_DEVICE_CONFIG_FILE exists and is a file if(NOT EXISTS ${FWU_DEVICE_CONFIG_FILE}) message(FATAL_ERROR "FWU_DEVICE_CONFIG_FILE:${FWU_DEVICE_CONFIG_FILE} does not exist.") elseif(IS_DIRECTORY ${FWU_DEVICE_CONFIG_FILE}) message(FATAL_ERROR "FWU_DEVICE_CONFIG_FILE:${FWU_DEVICE_CONFIG_FILE} is a folder while a file is expected.") endif()
in secure_fw/partitions/firmware_update/CMakeLists.txt is redundant as FWU_DEVICE_CONFIG_FILE may be generated, thus not present when cmake performs EXISTS check (note that by default FWU_DEVICE_CONFIG_FILE is generated so I dont see point in limiting user from using generated file)
So i propose to remove this check.
Regards, Bohdan Hunko
Cypress Semiconductor Ukraine Engineer CSUKR CSS ICW SW FW Mobile: +38099 50 19 714 Bohdan.Hunko@infineon.commailto:Bohdan.Hunko@infineon.com
Hi Bohdan,
I learnt your usage case now. Thanks for the information! The FWU_DEVICE_CONFIG_FILE is intended to be defined as the .h file directly. So, the configure_file is only conducted when it is not defined. And, that is why the file existing is checked when it is not defined. In your case, it is defined as the .h.in file. Taking your usage case into consideration, which should be a typical one, defining FWU_DEVICE_CONFIG_FILE as a .h.in file seems to be fair enough. I created https://review.trustedfirmware.org/c/TF-M/trusted-firmware-m/+/25230 to make the configuration either a .h.in file or a .h file. It should solve your issue. Can you have review it?
Thanks,
Regards, Sherry Zhang
From: Bohdan.Hunko@infineon.com Bohdan.Hunko@infineon.com Sent: Tuesday, November 28, 2023 8:01 PM To: Sherry Zhang Sherry.Zhang2@arm.com; tf-m@lists.trustedfirmware.org Cc: nd nd@arm.com Subject: RE: Redundant check on FWU_DEVICE_CONFIG_FILE
Hi Sherry,
Let me explain what I am doing.
Here is what is done in default use case (FWU_DEVICE_CONFIG_FILE is not defined):
* fwu_config.h is set to be generated from fwu_config.h.in * fwu_config.h is set to be used as FWU_DEVICE_CONFIG_FILE
By default fwu_config.h only has 2 component IDs (S and NS) and In my case I have more components, thus I want to define more IDs, so I need custom fwu_config.h. Keep in mind that I still use everything, other than image IDs, same as in default case (MCUboot, ...)
So I have done the following:
1. Copied fwu_config.h.in to my platform folder 2. Changed image IDs in fwu_config.h.in 3. fwu_config.h is set to be generated from fwu_config.h.in (This is exactly the same as what is done in default case) 4. fwu_config.h is set to be used as FWU_DEVICE_CONFIG_FILE (This is exactly the same as what is done in default case) 5. BUT I get an error which says that fwu_config.h does not exist. And it is true, it will only be generated after CMake is done and it will be a valid .h file.
I hope this shown you the problem I am pointing to - In default case FWU_DEVICE_CONFIG_FILE is generated (thus not present in Cmake config time) BUT for some reasons TFM restricts user from having his FWU_DEVICE_CONFIG_FILE generated (because TFM code has a check to verify that FWU_DEVICE_CONFIG_FILE exists, which may not be the case if FWU_DEVICE_CONFIG_FILE is generated).
Regards, Bohdan Hunko
Cypress Semiconductor Ukraine Engineer CSUKR CSS ICW SW FW Mobile: +38099 50 19 714 Bohdan.Hunko@infineon.commailto:Bohdan.Hunko@infineon.com
From: Sherry Zhang <Sherry.Zhang2@arm.commailto:Sherry.Zhang2@arm.com> Sent: Tuesday, November 28, 2023 07:46 To: Hunko Bohdan (CSS ICW SW FW 3) <Bohdan.Hunko@infineon.commailto:Bohdan.Hunko@infineon.com>; tf-m@lists.trustedfirmware.orgmailto:tf-m@lists.trustedfirmware.org Cc: nd <nd@arm.commailto:nd@arm.com> Subject: RE: Redundant check on FWU_DEVICE_CONFIG_FILE
Caution: This e-mail originated outside Infineon Technologies. Do not click on links or open attachments unless you validate it is safehttps://intranet-content.infineon.com/explore/aboutinfineon/rules/informationsecurity/ug/SocialEngineering/Pages/SocialEngineeringElements_en.aspx.
Hi Bohdan,
FWU_DEVICE_CONFIG_FILE should be defined as the header file which is included in the source file. May I know what kind of value does FWU_DEVICE_CONFIG_FILE is defined to in your case? And why it is defined while it still needs to be generated?
Regards, Sherry Zhang
From: Bohdan.Hunko@infineon.commailto:Bohdan.Hunko@infineon.com <Bohdan.Hunko@infineon.commailto:Bohdan.Hunko@infineon.com> Sent: Monday, November 27, 2023 9:37 PM To: Sherry Zhang <Sherry.Zhang2@arm.commailto:Sherry.Zhang2@arm.com>; tf-m@lists.trustedfirmware.orgmailto:tf-m@lists.trustedfirmware.org Cc: nd <nd@arm.commailto:nd@arm.com> Subject: RE: Redundant check on FWU_DEVICE_CONFIG_FILE
Hi Sherry,
Yes, you are correct - the check is only done if FWU_DEVICE_CONFIG_FILE is not defined.
BUT this is still a problem because even if FWU_DEVICE_CONFIG_FILE is defined it still may be generated (same way as if FWU_DEVICE_CONFIG_FILE is not defined it is generated). So default TFM implementation use generated file as FWU_DEVICE_CONFIG_FILE but restricts user from doing so.
So in current implementation FWU_DEVICE_CONFIG_FILE must be present on cmake config time and can not be generated. I see this as unnecessary limitation.
Actually this is not hypothetical problem, I only brought this problem up because I hit this issue when trying to use custom FWU_DEVICE_CONFIG_FILE and this custom file is generated (same way as default one is).
So my proposal is to remove this check, this will make the implementation more flexible.
Regards, Bohdan Hunko
Cypress Semiconductor Ukraine Engineer CSUKR CSS ICW SW FW Mobile: +38099 50 19 714 Bohdan.Hunko@infineon.commailto:Bohdan.Hunko@infineon.com
From: Sherry Zhang <Sherry.Zhang2@arm.commailto:Sherry.Zhang2@arm.com> Sent: Wednesday, November 15, 2023 04:32 To: Hunko Bohdan (CSS ICW SW FW 3) <Bohdan.Hunko@infineon.commailto:Bohdan.Hunko@infineon.com>; tf-m@lists.trustedfirmware.orgmailto:tf-m@lists.trustedfirmware.org Cc: nd <nd@arm.commailto:nd@arm.com> Subject: RE: Redundant check on FWU_DEVICE_CONFIG_FILE
Caution: This e-mail originated outside Infineon Technologies. Do not click on links or open attachments unless you validate it is safehttps://intranet-content.infineon.com/explore/aboutinfineon/rules/informationsecurity/ug/SocialEngineering/Pages/SocialEngineeringElements_en.aspx.
Hi Bohdan,
The code you posted is in the else routine of FWU_DEVICE_CONFIG_FILE not defined condition(see https://git.trustedfirmware.org/TF-M/trusted-firmware-m.git/tree/secure_fw/p...):
if (NOT FWU_DEVICE_CONFIG_FILE) configure_file(${CMAKE_SOURCE_DIR}/interface/include/psa/fwu_config.h.in ${CMAKE_BINARY_DIR}/generated/interface/include/psa/fwu_config.h @ONLY) set(FWU_DEVICE_CONFIG_FILE "${CMAKE_BINARY_DIR}/generated/interface/include/psa/fwu_config.h") else() # FWU_DEVICE_CONFIG_FILE exists and is a file if(NOT EXISTS ${FWU_DEVICE_CONFIG_FILE}) message(FATAL_ERROR "FWU_DEVICE_CONFIG_FILE:${FWU_DEVICE_CONFIG_FILE} does not exist.") elseif(IS_DIRECTORY ${FWU_DEVICE_CONFIG_FILE}) message(FATAL_ERROR "FWU_DEVICE_CONFIG_FILE:${FWU_DEVICE_CONFIG_FILE} is a folder while a file is expected.") endif() endif()
If FWU_DEVICE_CONFIG_FILE is defined, it should be an existing file. The default routine is FWU_DEVICE_CONFIG_FILE not-defined(the if routine).
Regards, Sherry Zhang
From: Bohdan.Hunko--- via TF-M <tf-m@lists.trustedfirmware.orgmailto:tf-m@lists.trustedfirmware.org> Sent: Tuesday, November 14, 2023 11:43 PM To: tf-m@lists.trustedfirmware.orgmailto:tf-m@lists.trustedfirmware.org Subject: [TF-M] Redundant check on FWU_DEVICE_CONFIG_FILE
Hi all It seems to me like following check
# FWU_DEVICE_CONFIG_FILE exists and is a file if(NOT EXISTS ${FWU_DEVICE_CONFIG_FILE}) message(FATAL_ERROR "FWU_DEVICE_CONFIG_FILE:${FWU_DEVICE_CONFIG_FILE} does not exist.") elseif(IS_DIRECTORY ${FWU_DEVICE_CONFIG_FILE}) message(FATAL_ERROR "FWU_DEVICE_CONFIG_FILE:${FWU_DEVICE_CONFIG_FILE} is a folder while a file is expected.") endif()
in secure_fw/partitions/firmware_update/CMakeLists.txt is redundant as FWU_DEVICE_CONFIG_FILE may be generated, thus not present when cmake performs EXISTS check (note that by default FWU_DEVICE_CONFIG_FILE is generated so I dont see point in limiting user from using generated file)
So i propose to remove this check.
Regards, Bohdan Hunko
Cypress Semiconductor Ukraine Engineer CSUKR CSS ICW SW FW Mobile: +38099 50 19 714 Bohdan.Hunko@infineon.commailto:Bohdan.Hunko@infineon.com
Hi Sherry,
Thanks, please see my comments on the review.
Regards, Bohdan Hunko
Cypress Semiconductor Ukraine Engineer CSUKR CSS ICW SW FW Mobile: +38099 50 19 714 Bohdan.Hunko@infineon.commailto:Bohdan.Hunko@infineon.com
From: Sherry Zhang Sherry.Zhang2@arm.com Sent: Wednesday, November 29, 2023 06:18 To: Hunko Bohdan (CSS ICW SW FW 3) Bohdan.Hunko@infineon.com; tf-m@lists.trustedfirmware.org Cc: nd nd@arm.com Subject: RE: Redundant check on FWU_DEVICE_CONFIG_FILE
Caution: This e-mail originated outside Infineon Technologies. Do not click on links or open attachments unless you validate it is safehttps://intranet-content.infineon.com/explore/aboutinfineon/rules/informationsecurity/ug/SocialEngineering/Pages/SocialEngineeringElements_en.aspx.
Hi Bohdan,
I learnt your usage case now. Thanks for the information! The FWU_DEVICE_CONFIG_FILE is intended to be defined as the .h file directly. So, the configure_file is only conducted when it is not defined. And, that is why the file existing is checked when it is not defined. In your case, it is defined as the .h.in file. Taking your usage case into consideration, which should be a typical one, defining FWU_DEVICE_CONFIG_FILE as a .h.in file seems to be fair enough. I created https://review.trustedfirmware.org/c/TF-M/trusted-firmware-m/+/25230 to make the configuration either a .h.in file or a .h file. It should solve your issue. Can you have review it?
Thanks,
Regards, Sherry Zhang
From: Bohdan.Hunko@infineon.commailto:Bohdan.Hunko@infineon.com <Bohdan.Hunko@infineon.commailto:Bohdan.Hunko@infineon.com> Sent: Tuesday, November 28, 2023 8:01 PM To: Sherry Zhang <Sherry.Zhang2@arm.commailto:Sherry.Zhang2@arm.com>; tf-m@lists.trustedfirmware.orgmailto:tf-m@lists.trustedfirmware.org Cc: nd <nd@arm.commailto:nd@arm.com> Subject: RE: Redundant check on FWU_DEVICE_CONFIG_FILE
Hi Sherry,
Let me explain what I am doing.
Here is what is done in default use case (FWU_DEVICE_CONFIG_FILE is not defined):
* fwu_config.h is set to be generated from fwu_config.h.in * fwu_config.h is set to be used as FWU_DEVICE_CONFIG_FILE
By default fwu_config.h only has 2 component IDs (S and NS) and In my case I have more components, thus I want to define more IDs, so I need custom fwu_config.h. Keep in mind that I still use everything, other than image IDs, same as in default case (MCUboot, ...)
So I have done the following:
1. Copied fwu_config.h.in to my platform folder 2. Changed image IDs in fwu_config.h.in 3. fwu_config.h is set to be generated from fwu_config.h.in (This is exactly the same as what is done in default case) 4. fwu_config.h is set to be used as FWU_DEVICE_CONFIG_FILE (This is exactly the same as what is done in default case) 5. BUT I get an error which says that fwu_config.h does not exist. And it is true, it will only be generated after CMake is done and it will be a valid .h file.
I hope this shown you the problem I am pointing to - In default case FWU_DEVICE_CONFIG_FILE is generated (thus not present in Cmake config time) BUT for some reasons TFM restricts user from having his FWU_DEVICE_CONFIG_FILE generated (because TFM code has a check to verify that FWU_DEVICE_CONFIG_FILE exists, which may not be the case if FWU_DEVICE_CONFIG_FILE is generated).
Regards, Bohdan Hunko
Cypress Semiconductor Ukraine Engineer CSUKR CSS ICW SW FW Mobile: +38099 50 19 714 Bohdan.Hunko@infineon.commailto:Bohdan.Hunko@infineon.com
From: Sherry Zhang <Sherry.Zhang2@arm.commailto:Sherry.Zhang2@arm.com> Sent: Tuesday, November 28, 2023 07:46 To: Hunko Bohdan (CSS ICW SW FW 3) <Bohdan.Hunko@infineon.commailto:Bohdan.Hunko@infineon.com>; tf-m@lists.trustedfirmware.orgmailto:tf-m@lists.trustedfirmware.org Cc: nd <nd@arm.commailto:nd@arm.com> Subject: RE: Redundant check on FWU_DEVICE_CONFIG_FILE
Caution: This e-mail originated outside Infineon Technologies. Do not click on links or open attachments unless you validate it is safehttps://intranet-content.infineon.com/explore/aboutinfineon/rules/informationsecurity/ug/SocialEngineering/Pages/SocialEngineeringElements_en.aspx.
Hi Bohdan,
FWU_DEVICE_CONFIG_FILE should be defined as the header file which is included in the source file. May I know what kind of value does FWU_DEVICE_CONFIG_FILE is defined to in your case? And why it is defined while it still needs to be generated?
Regards, Sherry Zhang
From: Bohdan.Hunko@infineon.commailto:Bohdan.Hunko@infineon.com <Bohdan.Hunko@infineon.commailto:Bohdan.Hunko@infineon.com> Sent: Monday, November 27, 2023 9:37 PM To: Sherry Zhang <Sherry.Zhang2@arm.commailto:Sherry.Zhang2@arm.com>; tf-m@lists.trustedfirmware.orgmailto:tf-m@lists.trustedfirmware.org Cc: nd <nd@arm.commailto:nd@arm.com> Subject: RE: Redundant check on FWU_DEVICE_CONFIG_FILE
Hi Sherry,
Yes, you are correct - the check is only done if FWU_DEVICE_CONFIG_FILE is not defined.
BUT this is still a problem because even if FWU_DEVICE_CONFIG_FILE is defined it still may be generated (same way as if FWU_DEVICE_CONFIG_FILE is not defined it is generated). So default TFM implementation use generated file as FWU_DEVICE_CONFIG_FILE but restricts user from doing so.
So in current implementation FWU_DEVICE_CONFIG_FILE must be present on cmake config time and can not be generated. I see this as unnecessary limitation.
Actually this is not hypothetical problem, I only brought this problem up because I hit this issue when trying to use custom FWU_DEVICE_CONFIG_FILE and this custom file is generated (same way as default one is).
So my proposal is to remove this check, this will make the implementation more flexible.
Regards, Bohdan Hunko
Cypress Semiconductor Ukraine Engineer CSUKR CSS ICW SW FW Mobile: +38099 50 19 714 Bohdan.Hunko@infineon.commailto:Bohdan.Hunko@infineon.com
From: Sherry Zhang <Sherry.Zhang2@arm.commailto:Sherry.Zhang2@arm.com> Sent: Wednesday, November 15, 2023 04:32 To: Hunko Bohdan (CSS ICW SW FW 3) <Bohdan.Hunko@infineon.commailto:Bohdan.Hunko@infineon.com>; tf-m@lists.trustedfirmware.orgmailto:tf-m@lists.trustedfirmware.org Cc: nd <nd@arm.commailto:nd@arm.com> Subject: RE: Redundant check on FWU_DEVICE_CONFIG_FILE
Caution: This e-mail originated outside Infineon Technologies. Do not click on links or open attachments unless you validate it is safehttps://intranet-content.infineon.com/explore/aboutinfineon/rules/informationsecurity/ug/SocialEngineering/Pages/SocialEngineeringElements_en.aspx.
Hi Bohdan,
The code you posted is in the else routine of FWU_DEVICE_CONFIG_FILE not defined condition(see https://git.trustedfirmware.org/TF-M/trusted-firmware-m.git/tree/secure_fw/p...):
if (NOT FWU_DEVICE_CONFIG_FILE) configure_file(${CMAKE_SOURCE_DIR}/interface/include/psa/fwu_config.h.in ${CMAKE_BINARY_DIR}/generated/interface/include/psa/fwu_config.h @ONLY) set(FWU_DEVICE_CONFIG_FILE "${CMAKE_BINARY_DIR}/generated/interface/include/psa/fwu_config.h") else() # FWU_DEVICE_CONFIG_FILE exists and is a file if(NOT EXISTS ${FWU_DEVICE_CONFIG_FILE}) message(FATAL_ERROR "FWU_DEVICE_CONFIG_FILE:${FWU_DEVICE_CONFIG_FILE} does not exist.") elseif(IS_DIRECTORY ${FWU_DEVICE_CONFIG_FILE}) message(FATAL_ERROR "FWU_DEVICE_CONFIG_FILE:${FWU_DEVICE_CONFIG_FILE} is a folder while a file is expected.") endif() endif()
If FWU_DEVICE_CONFIG_FILE is defined, it should be an existing file. The default routine is FWU_DEVICE_CONFIG_FILE not-defined(the if routine).
Regards, Sherry Zhang
From: Bohdan.Hunko--- via TF-M <tf-m@lists.trustedfirmware.orgmailto:tf-m@lists.trustedfirmware.org> Sent: Tuesday, November 14, 2023 11:43 PM To: tf-m@lists.trustedfirmware.orgmailto:tf-m@lists.trustedfirmware.org Subject: [TF-M] Redundant check on FWU_DEVICE_CONFIG_FILE
Hi all It seems to me like following check
# FWU_DEVICE_CONFIG_FILE exists and is a file if(NOT EXISTS ${FWU_DEVICE_CONFIG_FILE}) message(FATAL_ERROR "FWU_DEVICE_CONFIG_FILE:${FWU_DEVICE_CONFIG_FILE} does not exist.") elseif(IS_DIRECTORY ${FWU_DEVICE_CONFIG_FILE}) message(FATAL_ERROR "FWU_DEVICE_CONFIG_FILE:${FWU_DEVICE_CONFIG_FILE} is a folder while a file is expected.") endif()
in secure_fw/partitions/firmware_update/CMakeLists.txt is redundant as FWU_DEVICE_CONFIG_FILE may be generated, thus not present when cmake performs EXISTS check (note that by default FWU_DEVICE_CONFIG_FILE is generated so I dont see point in limiting user from using generated file)
So i propose to remove this check.
Regards, Bohdan Hunko
Cypress Semiconductor Ukraine Engineer CSUKR CSS ICW SW FW Mobile: +38099 50 19 714 Bohdan.Hunko@infineon.commailto:Bohdan.Hunko@infineon.com
tf-m@lists.trustedfirmware.org
-
Bohdan.Hunko＠infineon.com -
Sherry Zhang