[PATCH 1/2] tpm: Make response length of tpm2_get_capability() configurable

[Heinrich Schuchardt]

On 04.11.20 16:56, Ilias Apalodimas wrote:
> Hi Heinrich,
>
> [...]
>>> +++ b/lib/tpm-v2.c
>>> @@ -161,7 +161,7 @@ u32 tpm2_pcr_read(struct udevice *dev, u32 idx, unsigned int idx_min_sz,
>>>  }
>>>
>>>  u32 tpm2_get_capability(struct udevice *dev, u32 capability, u32 property,
>>> -			void *buf, size_t prop_count)
>>> +			void *buf, size_t prop_count, bool get_count)
>>
>> The implementation would be more stable if we would derive the offset
>> from field property instead of adding get_count.
>>
>
> We are not defining the tpmv2 internal structures anywhere in U-Boot.
> That's why the code is doing static sizeof(uX) instead of using offsetof.
> In the EFI part of the patchset, I've done exaclty that.
> Working with offsets on well defined struct is better, but out of scope for this
> patchset imho.
> We can look into refactoring the generic tpmv2 code once I add the rest of the EFI
> protocol parts?

Can't we add the structures that we need to tpm-v2.h and use their size
here?

Best regards

Heinrich

>
>>>  {
>>>  	u8 command_v2[COMMAND_BUFFER_SIZE] = {
>>
>> Shouldn't COMMAND_BUFFER_SIZE be changed to something with TPM in the
>> name, e.g TPM_COMMAND_BUFFER_SIZE?
>>
>>>  		tpm_u16(TPM2_ST_NO_SESSIONS),		/* TAG */
>>> @@ -181,13 +181,17 @@ u32 tpm2_get_capability(struct udevice *dev, u32 capability, u32 property,
>>>  	if (ret)
>>>  		return ret;
>>>
>>> +	/* When reading PCR properties we need the count */
>>> +	properties_off = sizeof(u16) + sizeof(u32) + sizeof(u32) +
>>> +			 sizeof(u8) + sizeof(u32);
>>>  	/*
>>>  	 * In the response buffer, the properties are located after the:
>>>  	 * tag (u16), response size (u32), response code (u32),
>>>  	 * YES/NO flag (u8), TPM_CAP (u32) and TPMU_CAPABILITIES (u32).
>>>  	 */
>>
>> This comment should be above 'properties_off ='. 'get_count' related
>> field should be mentioned.
>
> Sure, I'll fix this
>
> Regards
> /Ilias
>>
>> Best regards
>>
>> Heinrich
>>
>>> -	properties_off = sizeof(u16) + sizeof(u32) + sizeof(u32) +
>>> -			 sizeof(u8) + sizeof(u32) + sizeof(u32);
>>> +	if (!get_count)
>>> +		properties_off += sizeof(u32);
>>> +
>>>  	memcpy(buf, &response[properties_off], response_len - properties_off);
>>>
>>>  	return 0;
>>>
>>