[coreboot-gerrit] [M] Change in coreboot[main]: security/tpm: Add TPM2 NV_ReadPublic command support

[Filip Lewiński (Code Review)]

Attention is currently required from: Arthur Heymans, Christian Walter, Felix Held, Julius Werner, Krystian Hebel, Martin Roth, Matt DeVillier, Michał Kopeć, Michał Żygowski.

Filip Lewiński has posted comments on this change by Filip Lewiński. ( https://review.coreboot.org/c/coreboot/+/82037?usp=email )

Change subject: security/tpm: Add TPM2 NV_ReadPublic command support
......................................................................

Patch Set 28:

(2 comments)

This change is ready for review.

File src/security/tpm/tss/tcg-2.0/tss.c:

https://review.coreboot.org/c/coreboot/+/82037/comment/95b5a580_6de47ea1?usp... :
PS16, Line 395: struct nv_read_public_response *nvrp_resp
> These TPM spec specific structures are usually only for use inside the TSS layer, and we prefer to u […]

@[e-mail] I've [refactored](https://review.coreboot.org/c/coreboot/+/82037/28/src/security/tpm/tss/tcg-2...) the function and its uses to work on separate pointers.

File src/security/tpm/tss/tcg-2.0/tss_structures.h:

https://review.coreboot.org/c/coreboot/+/82037/comment/fad6140c_6c4d671d?usp... :
PS16, Line 330: 	uint8_t sha512[SHA512_DIGEST_SIZE];
> I don't really understand why this digest is inline in the union while in all other cases we use a p […]

@[e-mail] Hmmm, yes, you are right that this structure differs from others that use pointers to external buffers.

However in this case, the use of inline arrays in TPMU_HA (and by extension TPMT_HA, TPML_DIGEST_VALUES, and TPMU_NAME) actually follows the TPM 2.0 Library Spec — specifically, Table 71 and Table 72 in Part 2: Structures. The spec defines the union members as fixed-size arrays, and hashAlg serves as the selector for which one is valid.

I agree it might make sense to unify how digests are handled throughout the codebase, especially if we're aiming for a consistent pointer-based model. That said, this feels like a broader refactor that probably deserves its own dedicated patch, since it's a bit outside the scope of the changes here (which are focused on NV_ReadPublic support).