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

[Alexey Kardashevskiy]

On Tue, 5 Nov 2024, at 03:10, Stefan Berger wrote:
> 
> 
> 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.

I guess I'll change it to:

if (cmd_size != be32_to_cpu(hdr->totlen))

and push it out. Sadly I just missed soft freeze :-( Thanks,

> 
> >>
> >>>
> >>>>> 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.
> 
>