[rom_ext] Remove dbg print in imm_section #26532
Conversation
sw/device/silicon_creator/lib/BUILD
Outdated
| @@ -257,6 +257,14 @@ cc_test( | |||
| ], | |||
| ) | |||
|
|
|||
| cc_library( | |||
There was a problem hiding this comment.
Why do you need a lib to disable dbg_prints in the immutable section? I think you should just remove all calls to dbg_printf().
@cfrantz what would you prefer here?
There was a problem hiding this comment.
I'd prefer removing the dbg_{printf,puts} completely from the code that lands in the immutable section.
If there are some dbg prints that are getting linked in unexpectedly, lets have a discussion about them.
There was a problem hiding this comment.
I have removed all the instances.
That includes two messages from common libraries, which means they’re also removed from the mutable section.
- otbn_boot_service - Warning: Attestation key seed flash info page not provisioned.
- dice_chain - info: flushed dice cert page
Prints are removed due to: * Pen Testing * Boot timing * Code size Change-Id: Icce141e2bb4a2658ee5ef6e061c6a54346cc8d7d Signed-off-by: Yi-Hsuan Deng <[email protected]>
| @@ -328,7 +328,6 @@ rom_error_t dice_chain_attestation_creator( | |||
| // Check if the current CDI_0 cert is valid. | |||
| RETURN_IF_ERROR(dice_chain_load_cert_obj("CDI_0", /*name_size=*/6)); | |||
| if (dice_chain.cert_valid == kHardenedBoolFalse) { | |||
| dbg_puts("warning: CDI_0 certificate not valid; updating\r\n"); | |||
There was a problem hiding this comment.
We still want to have the mutable portion of the ROM_EXT print if certs were updated. We have tests that depend on these (e.g., https://cs.opensource.google/opentitan/opentitan/+/earlgrey_1.0.0:sw/device/silicon_creator/rom_ext/e2e/dice_chain/BUILD;l=38)
@cfrantz and I spoke, we would like to have a block of dbg_printf's here (https://cs.opensource.google/opentitan/opentitan/+/earlgrey_1.0.0:sw/device/silicon_creator/rom_ext/rom_ext.c;l=428) that prints the following:
- if UDS was invalid
- if CDI_0 was updated
- if CDI_1 was updated
Would this be possible? I.e., could you save the state of which certs are valid in the static_dice section maybe? and then just have a block of prints after the certs are flushed to flash?
Prints are removed due to: