-
Notifications
You must be signed in to change notification settings - Fork 22
Allow disabling the pthread support check #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow disabling the pthread support check #6
Conversation
In cases where this code is utlized only for the libriers in it (e.g. vTPM emulation in Coconut SVSM), we do not need to compile the simulator. So it would be convenient to have the ability to disable pthread checking, which in that case is not necassary since it is used only by the simulator. Also in Coconut SVSM we don't have pthread support, so it's also a requirement and for now we have our own fork to disable this check. By default pthread checking remains enabled, so there is no change. `--disable-pthread` is added to the configure to support this new use case. Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
|
@chrisfenner @bradlitterell @amycnelson any feedback on this? We have a workaround for now, although not very nice in COCONUT SVSM: https://github.com/coconut-svsm/svsm/blob/5efb4d21b6ce03b7d06dea59b69e814f8f0a13b3/libtcgtpm/Makefile#L170 |
Sorry this PR stayed dormant. It seems fine to me, so happy to approve and merge. However, I want to share a PSA that hopefully will not be terrible news: The build system is likely to change to cmake in the next published version of this code. |
Great, thanks!
Thanks for sharing. I think it's not that bad as news :-) We will need to change something, but I think we will find a way to adapt. |
|
@chrisfenner I guess it was a mistake but this PR was reverted by commit 9bc545f so I just opened a new PR #10 to re-apply this. PTAL. |
|
Bah, sorry about this Stefano. I must have made a mistake when I checked that these fixes were preserved across the big squash effort that we did for the 184 code last week. I'll take a look this week. |
@chrisfenner don't worry, let me know if I can help in some way! Thanks! |
See #6: this PR was accidentally merged as a merge commit, which got lost in the recent refactor+squash for 184. This change cherry-picks the actual commit from Stefano in that PR. --- In cases where this code is utlized only for the libriers in it (e.g. vTPM emulation in Coconut SVSM), we do not need to compile the simulator. So it would be convenient to have the ability to disable pthread checking, which in that case is not necassary since it is used only by the simulator. Also in Coconut SVSM we don't have pthread support, so it's also a requirement and for now we have our own fork to disable this check. By default pthread checking remains enabled, so there is no change. `--disable-pthread` is added to the configure to support this new use case. Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
|
Thanks for calling this out, I have re-commited your work in bc29a21. I believe what happened was:
I double checked your other recent PR (#9) and I think despite it also being a merge commit, the work was not accidentally undone. One more thing: we are full steam ahead on CMake now, so these files are gone in the latest internal tree. For v185 (in public review: https://trustedcomputinggroup.org/specifications-public-review/) I would say to expect the automake files to be gone. If I can be of help to your team in the transition please let me know! |
Thanks for the clarification! Much appreciated!
Yep, I confirm, I did the same yesterday before commenting here, thanks for double-checking!
I see, thanks for the update, I'll reach you if we have issue in our project (https://github.com/coconut-svsm/svsm/tree/main/libtcgtpm) |
The next TCG TPM reference implementation release (v185) will remove the support for autotools [1]. Currently, v184 supports both, so let's switch to cmake to be prepared. The main difference is that dependency on TpmBigNum and Math_Ossl was previously specified in the autotools files when producing libtpm.a and libplatform.a, but now it is only specified for the final product (e.g. the simulator in their project). Therefore, we must explicitly build and link them when producing libtcgtpm.a. [1] TrustedComputingGroup/TPM#6 (comment) Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
The next TCG TPM reference implementation release (v185) will remove the support for autotools [1]. Currently, v184 supports both, so let's switch to cmake to be prepared. The main difference is that dependency on TpmBigNum and Math_Ossl was previously specified in the autotools files when producing libtpm.a and libplatform.a, but now it is only specified for the final product (e.g. the simulator in their project). Therefore, we must explicitly build and link them when producing libtcgtpm.a. [1] TrustedComputingGroup/TPM#6 (comment) Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
The next TCG TPM reference implementation release (v185) will remove the support for autotools [1]. Currently, v184 supports both, so let's switch to cmake to be prepared. The main difference is that dependency on TpmBigNum and Math_Ossl was previously specified in the autotools files when producing libtpm.a and libplatform.a, but now it is only specified for the final product (e.g. the simulator in their project). Therefore, we must explicitly build and link them when producing libtcgtpm.a. [1] TrustedComputingGroup/TPM#6 (comment) Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
The next TCG TPM reference implementation release (v185) will remove the support for autotools [1]. Currently, v184 supports both, so let's switch to cmake to be prepared. The main difference is that dependency on TpmBigNum and Math_Ossl was previously specified in the autotools files when producing libtpm.a and libplatform.a, but now it is only specified for the final product (e.g. the simulator in their project). Therefore, we must explicitly build and link them when producing libtcgtpm.a. [1] TrustedComputingGroup/TPM#6 (comment) Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
|
@chrisfenner I switched to cmake in coconut-svsm/svsm#832, specifically in commit coconut-svsm/svsm@f4fadcc I tested it, and it works, but I'd really appreciate if you can check it if you have time. Feel free to suggest any improvement! |
The next TCG TPM reference implementation release (v185) will remove the support for autotools [1]. Currently, v184 supports both, so let's switch to cmake to be prepared. The main difference is that dependency on TpmBigNum and Math_Ossl was previously specified in the autotools files when producing libtpm.a and libplatform.a, but now it is only specified for the final product (e.g. the simulator in their project). Therefore, we must explicitly build and link them when producing libtcgtpm.a. [1] TrustedComputingGroup/TPM#6 (comment) Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
The next TCG TPM reference implementation release (v185) will remove the support for autotools [1]. Currently, v184 supports both, so let's switch to cmake to be prepared. The main difference is that dependency on TpmBigNum and Math_Ossl was previously specified in the autotools files when producing libtpm.a and libplatform.a, but now it is only specified for the final product (e.g. the simulator in their project). Therefore, we must explicitly build and link them when producing libtcgtpm.a. Update documentation and CI about deps to install to build TPM. `gcc-c++` is required by the cmake project, otherwise the "No CMAKE_CXX_COMPILER could be found" error is printed. [1] TrustedComputingGroup/TPM#6 (comment) Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
In cases where this code is utilized only for the libraries in it (e.g. vTPM emulation in Coconut SVSM), we do not need to compile the simulator.
So it would be convenient to have the ability to disable pthread checking, which in that case is not necassary since it is used only by the simulator. Also in Coconut SVSM we don't have pthread support, so it's also a requirement and for now we have our own fork to disable this check.
By default pthread checking remains enabled, so there is no change.
--disable-pthreadis added to the configure to support this new use case.