[SLOF] [PATCH v3 2/2] tpm: Implement firmware API call pass-through-to-tpm

[Stefan Berger]

On 11/3/24 5:16 AM, Alexey Kardashevskiy wrote:
> 
> 
> On Sat, 2 Nov 2024, at 23:50, Stefan Berger wrote:
>>
>>
>> On 11/1/24 11:54 PM, Alexey Kardashevskiy wrote:
>>>
>>>
>>> On Fri, 1 Nov 2024, at 22:47, Stefan Berger wrote:
>>>>
>>>>
>>>> On 10/31/24 9:52 PM, Alexey Kardashevskiy wrote:
>>>>>
>>>>>
>>>>> On Tue, 29 Oct 2024, at 23:49, Stefan Berger wrote:
>>>>>> From: Stefan Berger <stefanb at linux.ibm.com>
>>>>>>
>>>>>> Implement the firmware API call pass-through-to-tpm that allows a caller
>>>>>> to pass a TPM command to the TPM. Since the buffer provided by the user
>>>>>> will be used for returning the TPM's response it must be sufficiently
>>>>>> large. To be safe, it should be of the size returned by the firmware API
>>>>>> call tpm-get-maximum-cmd-size.
>>>>>>
>>>>>> Signed-off-by: Stefan Berger <stefanb at linux.ibm.com>
>>>>>> ---
>>>>>> board-qemu/slof/vio-vtpm-cdriver.fs | 11 +++++++++++
>>>>>> lib/libtpm/tcgbios.c                | 25 +++++++++++++++++++++++++
>>>>>> lib/libtpm/tcgbios.h                |  1 +
>>>>>> lib/libtpm/tpm.code                 | 11 +++++++++++
>>>>>> lib/libtpm/tpm.in                   |  1 +
>>>>>> 5 files changed, 49 insertions(+)
>>>>>>
>>>>>> diff --git a/board-qemu/slof/vio-vtpm-cdriver.fs b/board-qemu/slof/vio-vtpm-cdriver.fs
>>>>>> index 21c2190..ced2ac0 100644
>>>>>> --- a/board-qemu/slof/vio-vtpm-cdriver.fs
>>>>>> +++ b/board-qemu/slof/vio-vtpm-cdriver.fs
>>>>>> @@ -57,6 +57,17 @@ LOG-SIZE BUFFER: log-base
>>>>>>         THEN
>>>>>> ;
>>>>>>     
>>>>>> +\ firmware API call
>>>>>> +: pass-through-to-tpm ( buf-addr cmd-size -- rsp-size )
>>>>>> +    vtpm-debug? IF
>>>>>> +        ." Call to pass-through-to-tpm" cr
>>>>>> +    THEN
>>>>>> +    tpm-pass-through-to-tpm                ( rsp-size )
>>>>>> +    vtpm-debug? IF
>>>>>> +        ." VTPM: tpm-pass-through-to-tpm returned size: " dup . cr
>>>>>> +    THEN
>>>>>> +;
>>>>>> +
>>>>>> \ firmware API call
>>>>>> : get-maximum-cmd-size ( -- max-size )
>>>>>>         vtpm-debug? IF
>>>>>> diff --git a/lib/libtpm/tcgbios.c b/lib/libtpm/tcgbios.c
>>>>>> index a64afde..366eda9 100644
>>>>>> --- a/lib/libtpm/tcgbios.c
>>>>>> +++ b/lib/libtpm/tcgbios.c
>>>>>> @@ -972,6 +972,31 @@ uint32_t tpm_get_maximum_cmd_size(void)
>>>>>> return PAPR_VTPM_MAX_BUFFER_SIZE;
>>>>>> }
>>>>>>     
>>>>>> +uint32_t tpm_pass_through_to_tpm(void *buffer, uint32_t cmd_size)
>>>>>> +{
>>>>>> + unsigned char respbuffer[PAPR_VTPM_MAX_BUFFER_SIZE];
>>>>>> + uint32_t respbufferlen = sizeof(respbuffer);
>>>>>> + struct tpm_req_header *hdr = buffer;
>>>>>> + uint32_t totlen;
>>>>>
>>>>> uint32_t totlen = be32_to_cpu(hdr->totlen);
>>>>> and drop memcpy(&totlen,...) below?
>>>>
>>>> I did this not wanting to assume alignment.
>>>
>>> Why would unaligned access not work here?
>>
>> If hdr->totlen was on an odd address we'd probably get an exception.
> 
> Seems unlikely as modern POWER should be able handle it but I do not have the hw to try (just for fun) adding "1" to the address and see if it throws an exception.

It can handle it at odd addresses as well. Still, removing the check now 
but would pass the cmd_size all the way down into this C function but 
leave it unused.

>>
>>>
>>>>> Is it a total size of the buffer (and then passing @cmd_size makes sense but checking cmd_size!=totlen does not) or a size of a TPM command (looks like this is the case but then this function does not know if @buffer is big enough for the response)?
>>>>
>>>> The spec says: "The method takes as arguments the address and size of a
>>>> fully formed TPM command – which is passed to the vTPM. The entire
>>>> response is copied back over the input buffer – which must be large
>>>> enough to accept the expected response or else the buffer will be overrun ."
>>>
>>> What a lovely spec :) What does "large enough" mean there, a page? I can see 1K is the bare minimum for spapr_vtpm.buffer_size, can we use this parameter, or it is a wrong buffer size? I just think we better not allow buffer overrun even if the spec is fine about it. Thanks,
>>
>> It basically means the caller should use a buffer of size returned by
>> tpm-get-maximum-cmd-size, as described in the commit message.
>> There's no size of the callers' buffer given via API so that we could
>> restrict the copy size.
> 
> Have a link to the spec handy? I could not easily google one, and the one I have does not mention  tpm-pass-through-to-tpm. Thanks,

Afaik it's not public. I am asking whether it can be published.