Skip to content
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

sapi: allow for per-function configuration #2369

Closed

Conversation

williamcroberts
Copy link
Member

To reduce the size of the system API, allow for a KConfig like system
where one can select required functionality.

The selected functionality can be included through a YAML list file of
the functions needed. The build tooling, scripts/tssconf.py is smart
enough to look through and find all the leaf functions for internal
tpm2-tss calls that we care about, like MU calls. For instance with a
config file of:

  • Tss2_Sys_GetRandom

The functions to enable are:

  • Tss2_Sys_GetRandom
  • Tss2_Sys_GetRandom_Prepare
  • Tss2_Sys_GetRandom_Complete

It will also output the required MU function defines, but libMU is
currently not instrumented to handle that.

For instance, I was able to build a SAPI as small as 6k with very
limited functionality and 10k with the required functionality for my
environment. The original SAPI object code for functions was 98k.

William Roberts added 5 commits June 8, 2022 12:52
log strings should be scoped to where its used for clarity on use but it
also reduces the object code size by not needing the log string table
always included.

Signed-off-by: William Roberts <william.c.roberts@intel.com>
If disabling loging, don't compile in backend logging support which
bloats the object code.

Signed-off-by: William Roberts <william.c.roberts@intel.com>
Scope command handles to where they are used.

Signed-off-by: William Roberts <william.c.roberts@intel.com>
Don't provide empty functions when disabling validation fuinctionality,
just provide the RC success value. This reduces object code by about 2k.

Signed-off-by: William Roberts <william.c.roberts@intel.com>
Its just needs log.c, so just bring that in.
This saves about 1.5k in dead object code due to all
the socket and write_all, read_all, etc functionality.

Not all linkers support dead code stripping, or do a good job, so lets
just avoid it.

Signed-off-by: William Roberts <william.c.roberts@intel.com>
@williamcroberts williamcroberts force-pushed the sapi-configure branch 2 times, most recently from 9c73227 to 479785b Compare June 9, 2022 18:31
To reduce the size of the system API, allow for a KConfig like system
where one can select required functionality.

The selected functionality can be included through a YAML list file of
the functions needed. The build tooling, scripts/tssconf.py is smart
enough to look through and find all the leaf functions for internal
tpm2-tss calls that we care about, like MU calls. For instance with a
config file of:
 - Tss2_Sys_GetRandom

The functions to enable are:
  - Tss2_Sys_GetRandom
  - Tss2_Sys_GetRandom_Prepare
  - Tss2_Sys_GetRandom_Complete

It will also output the required MU function defines, but libMU is
currently not instrumented to handle that.

For instance, I was able to build a SAPI as small as 6k with very
limited functionality and 10k with the required functionality for my
environment. The original SAPI object code for functions was 98k.

Signed-off-by: William Roberts <william.c.roberts@intel.com>
@codecov
Copy link

codecov bot commented Jun 9, 2022

Codecov Report

Merging #2369 (4d25152) into master (19a0634) will increase coverage by 0.24%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #2369      +/-   ##
==========================================
+ Coverage   83.41%   83.66%   +0.24%     
==========================================
  Files         352      351       -1     
  Lines       37900    37773     -127     
==========================================
- Hits        31614    31601      -13     
+ Misses       6286     6172     -114     
Impacted Files Coverage Δ
src/tss2-sys/api/Tss2_Sys_ACT_SetTimeout.c 45.45% <ø> (ø)
src/tss2-sys/api/Tss2_Sys_AC_GetCapability.c 0.00% <ø> (ø)
src/tss2-sys/api/Tss2_Sys_AC_Send.c 0.00% <ø> (ø)
src/tss2-sys/api/Tss2_Sys_ActivateCredential.c 59.25% <ø> (ø)
src/tss2-sys/api/Tss2_Sys_Certify.c 58.18% <ø> (ø)
src/tss2-sys/api/Tss2_Sys_CertifyCreation.c 60.31% <ø> (ø)
src/tss2-sys/api/Tss2_Sys_CertifyX509.c 64.40% <ø> (ø)
src/tss2-sys/api/Tss2_Sys_ChangeEPS.c 79.31% <ø> (ø)
src/tss2-sys/api/Tss2_Sys_ChangePPS.c 79.31% <ø> (ø)
src/tss2-sys/api/Tss2_Sys_Clear.c 82.75% <ø> (ø)
... and 134 more

📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more

@AndreasFuchsTPM
Copy link
Member

I'm not quite sure, whether this is overkill.
If you need a really tiny bit of SAPI only, then why not just statically link so only used stuff gets pulled in ?
Is there really a use case of a minimized shared library ?

@williamcroberts
Copy link
Member Author

I'm not quite sure, whether this is overkill. If you need a really tiny bit of SAPI only, then why not just statically link so only used stuff gets pulled in ? Is there really a use case of a minimized shared library ?

Yes, sigh, but sadly not all linkers in every environment can and do strip dead code.

@AndreasFuchsTPM
Copy link
Member

Yes, sigh, but sadly not all linkers in every environment can and do strip dead code.

You're joking, right ? April fools or something ?

Still, are such linkers common enough for us to maintain this chunk of code instead of telling people to include the needed C-Files one-by-one as needed ?

If you say "yes", I'll be happy to review though.

@williamcroberts
Copy link
Member Author

Yes, sigh, but sadly not all linkers in every environment can and do strip dead code.

You're joking, right ? April fools or something ?

I really wish I was.

Still, are such linkers common enough for us to maintain this chunk of code instead of telling people to include the needed C-Files one-by-one as needed ?

You could, and I thought about that approach. The problem is when you enable ESAPI, you don't need the oneshot variants, just the prepare and complete SAPI calls, which consumes about 500 bytes per oneshot call on average. I figured if a semi-intelligent KConfig system was hard, i'd abandon it but its simple enough.

If you say "yes", I'll be happy to review though.

I'd like to, because then updates are as easy as copy+paste+build. Given that in constrained environments updates are slow because of the pain of updating, I'd like to keep this as streamlined as possible rather than having folks have a pile of merge conflicts or other issues.

I need to #ifdef ESAPI and MU still. But I figured i'd post his early for feedback.


#if !defined(CONFIGURATOR) || defined(ENABLE_TSS2_SYS_ACT_SETTIMEOUT_COMPLETE)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd be more happy with a

#if !defined(DISABLE_TSS2_SYS_ACT_SETTIMEOUT_COMPLETE)

and similarly for the rest.

The configurator.h could include said macros then.

@@ -12,6 +12,11 @@
#include "tss2_mu.h"
#include "sysapi_util.h"

#ifdef CONFIGURATOR
#include "configurator.h"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really like this new include. It's a bit unintuitive.
Could we maybe instead get this into config.h somehow ?
Or maybe add the include on configurator.h into config.h ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really like this new include. It's a bit unintuitive. Could we maybe instead get this into config.h somehow ? Or maybe add the include on configurator.h into config.h ?

I've tried to figure out how to add things into config.h and it doesn't like my attempts to modify config.h since it takes control of it through autoconf magic.

The name does suck, I was going to rename everything. I was going to call everything BUILD_CONFIG and use that namespace everywhere as well as name the heade build_config.h.

Copy link
Member

@AndreasFuchsTPM AndreasFuchsTPM Jun 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I found something:

Macro: AH_TOP (text)
Include text at the top of the header template file.

Macro: AH_BOTTOM (text)
Include text at the bottom of the header template file.

https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.71/autoconf.html#index-AH_005fTOP-1

So

AS_IF(...
    AH_BOTTOM(#include "configurator.h")
    )

@williamcroberts
Copy link
Member Author

Example config file:

- Tss2_Sys_GetRandom
- Esys_TR_FromPublic

@williamcroberts
Copy link
Member Author

Closing, I think an easier way is to remove API files from the build, libmu being complete doesn't seem like too much. If that granularity is needed I will reopen.

@tomoveu
Copy link
Contributor

tomoveu commented Oct 26, 2022

easier way is to remove API files from the build

@williamcroberts is there a better way than manually editing makefiles for doing this?

@williamcroberts
Copy link
Member Author

easier way is to remove API files from the build

@williamcroberts is there a better way than manually editing makefiles for doing this?

No, but my specific use case has it's own build system so I cannot re-use the makefiles. Do you have a specific use case for this feature?

@tomoveu
Copy link
Contributor

tomoveu commented Oct 26, 2022

easier way is to remove API files from the build

@williamcroberts is there a better way than manually editing makefiles for doing this?

No, but my specific use case has it's own build system so I cannot re-use the makefiles. Do you have a specific use case for this feature?

Embedded Systems where we need to fit in a memory constrained environment.

@williamcroberts
Copy link
Member Author

easier way is to remove API files from the build

@williamcroberts is there a better way than manually editing makefiles for doing this?

No, but my specific use case has it's own build system so I cannot re-use the makefiles. Do you have a specific use case for this feature?

Embedded Systems where we need to fit in a memory constrained environment.

I've got 256kb to fit my code and everyone else's into and just picking and choosing which API files seems to be fine at the moment. The only thing this really provides is being able to break up libmu which is pretty tiny (i don't have hard numbers offhand).

We just pulled in the SPI TCTI driver skeleton and those folks haven't needed any mods in this area. If someone was actually up against a real limitation i'd be willing to dust this off.

@tomoveu
Copy link
Contributor

tomoveu commented Oct 27, 2022

@williamcroberts could you share your build setup/makefile for this 256kB version?

In some cases, we have MCU with RTOS and 256kB total flash memory :)

Of course, nowadays, we see more often 1MB total flash, but split this for rollback update, leaves us just 512kb. 50% for tpm2-tss is a lot.

@williamcroberts
Copy link
Member Author

@williamcroberts could you share your build setup/makefile for this 256kB version?

I can't share the Makefiles but I configure out a lot of things like logging, tctildr, all tctis (we have a custom one),
tss2-rc, etc. I just build the bare minimum.

In some cases, we have MCU with RTOS and 256kB total flash memory :)

We're bare metal with a 256kb slot.

Of course, nowadays, we see more often 1MB total flash, but split this for rollback update, leaves us just 512kb. 50% for tpm2-tss is a lot.

Yeah.

I'll implement the feature if folks actually say, "I can't get this to work in my environment". But I've never had anyone say that yet.

@tomoveu
Copy link
Contributor

tomoveu commented Oct 27, 2022

This is okay, Bill. Could you please share your configure line then?

How to reproduce your 256kB build 😃

@williamcroberts
Copy link
Member Author

This is okay, Bill. Could you please share your configure line then?

How to reproduce your 256kB build smiley

It's all custom. I don't use auto tools for it so their is no configure line. I took a generated config.h and modified it to disable things I didn't want. Then I just added the src files I needed to the build.

For src/tss2-sys you need sysapi_util.c and then pick whatever api file you need for interfaces.

For src/tss2-esys you need esys_context.c. esys_crypto.c esys_free.c esys_iutil.c esys_mu.c esys_tr.c and your choice of backend for crypto esys_crypto_mbed.c OR esys_crypto_ossl.c. I don't use a crypto back-end, ours is implementation specific and plugged in at run time using the new Esys_SetCryptoCallbacks.

for src/tpm2-mu I just build all the c files.

I build with these flags:
-DINTERNALBUILD=1 -DHAVE_CONFIG_H=1 -DNO_ESYS_TCTI

And the config.h is just hand modified generated one...

@tomoveu
Copy link
Contributor

tomoveu commented Oct 28, 2022

Thank you for the detailed instructions, Bill. What you described is one of the main reasons we do not use this stack in embedded systems. I do not want to review this PR, just sharing my experience.

If it is not too much bother, please pack a ZIP with this custom build and upload it somewhere :)

@williamcroberts
Copy link
Member Author

Thank you for the detailed instructions, Bill. What you described is one of the main reasons we do not use this stack in embedded systems. I do not want to review this PR, just sharing my experience.

You use autotools to build?

I know lot's of folks that use this in embedded systems. But almost every embedded system has it's own build system that folks integrate into.

If it is not too much bother, please pack a ZIP with this custom build and upload it somewhere :)

I can't :-p

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants