Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30826 )
Change subject: security/tpm/tss/tcg-2.0: Add TPM2 function tlcl_getcapability() ......................................................................
Patch Set 15: Code-Review-1
(3 comments)
Sorry, I still don't understand half of why this patch needs to do the stuff it does. Do we actually *need* to change tpm_process_command() for this? I don't see it.
Also adding Andrey who knows this code better.
https://review.coreboot.org/#/c/30826/15/src/security/tpm/tss/tcg-2.0/tss.c File src/security/tpm/tss/tcg-2.0/tss.c:
https://review.coreboot.org/#/c/30826/15/src/security/tpm/tss/tcg-2.0/tss.c@... PS15, Line 373: * Please note that the capabilityData is not unmarshalled. ...doesn't it? Aren't you adding that unmarshalling code to unmarshall_get_capability() in this very patch?
https://review.coreboot.org/#/c/30826/15/src/security/tpm/tss/tcg-2.0/tss.c@... PS15, Line 403: sizeof(TPMI_YES_NO) - sizeof(struct tpm_header)); I don't understand what this is for... you *are* unmarshalling the transfer buffer into the tpm2_response object, so why do you even need to add all this for passing out the response_size? TPMS_CAPABILITY_DATA has a well-known size, so why not just copy the whole thing?
https://review.coreboot.org/#/c/30826/15/src/security/tpm/tss/tcg-2.0/tss_st... File src/security/tpm/tss/tcg-2.0/tss_structures.h:
https://review.coreboot.org/#/c/30826/15/src/security/tpm/tss/tcg-2.0/tss_st... PS15, Line 235: #define PCR_SELECT_MAX ((IMPLEMENTATION_PCR + 7) / 8) Please use ALIGN_UP() for these.