[PATCH 2/2] efi: Add basic EFI_TCG2_PROTOCOL support

[Ilias Apalodimas]

On Wed, Nov 04, 2020 at 11:09:22PM +0100, Heinrich Schuchardt wrote:
> On 11/4/20 10:06 PM, Ilias Apalodimas wrote:
> > Hi Heinrich
> >>>
> > [...]
> >>> Why are we using COMMAND_BUFFER_SIZE throughout the TPM code if the
> >>> required buffer size for commands and responses can be read from the TPM
> >>> device?
> >>
> >> I think the logic is that 256b is enough for the basic commands we needed.
> >> I can change that here. Get the TPM response during efi_tcg2_register() and use that
> >> for the rest of the code?
> >
> > A clarification is needed here, which I forgot on my initial response.
> > The tpm library in U-boot is using the same buffer and length.
> > That's the reason I used the same response buffer size. The reply is copied from the
> > internal buffer defined in tpm2_get_capability() to our response buffer.
> > So unless we change the TPM internals changing the EFI part will make no difference.
> > That being said I don't mind changing the EFI code since it will be future-proof, against
> > the tpm code changes. Thoughts?
> 
> My question was not about EFI, it was about the lib/tpm-v2.c's usage of
> COMMAND_BUFFER_SIZE which would only be adequate if the specification
> explicitly forbids to implement a TPM with larger responses.

Right, that goes back to my initial response then.
People that contributed the TPM code are cc'ed so they might have a better answer, I'll 
just repeat myself here. I think the 256b was choosen because it was enough for the 
basic operations that were implemented.

> Types like TPML_PCR_SELECTION contain arrays. Why should we assume that
> these arrays fit into 256 bytes?

Well in practice they did. The spec used to say TPM2_NUM_PCR_BANKS = 3, so those specific
responses were meant to fit in there. The struct is:
typedef struct {
	UINT32              count;
	TPMS_PCR_SELECTION  pcrSelections[TPM2_NUM_PCR_BANKS];
} TPML_PCR_SELECTION;

As far as I understand this is going to change with a new revision and TPM2_NUM_PCR_BANKS will be
16 since newer hardware supports more PCR banks.

> 
> Should the logic in lib/tpm-v2.c be wrong, this needs to be fixed before
> building an EFI protocol upon it.

Why? The functionality it offsers is working *today*. As long as we make sure the added 
EFI protocol can use a configurable response buffer, acquired by the hardware we can go 
back and fix lib/tpm-v2.c when we need to. No?

Regards
/Ilias
> 
> Best regards
> 
> Heinrich