[SLOF] [PATCH v2 00/20] Add vTPM support to SLOF

[Stefan Berger]

On 11/19/2015 06:52 AM, Thomas Huth wrote:
> On 17/11/15 18:02, Stefan Berger wrote:
>> The following series of patches adds TPM support to SLOF.
>> In particular it adds the following:
>>
>> - TPM drivers for hardware interface and CRQ interface
>> - TPM initialization
>> - TPM logging area and firmware API to transfer it to the OS
>>    (measurements are visible in sysfs)
>> - Some measurement code (Static Core Root Of Trust)
>> - TPM menu (accessible via 't' key during boot if TPM is available)
>> - Firmware API extensions following Power Firmware Doc
>>    (to make trusted grub work)
>>
>>
>> Having a vTPM attached to a VM provides the following benefits:
>>
>> - enablement of trusted boot; this allow us to eventually extend the chain
>>    of trust from the hypervisor to the guests
>> - enablement of attestation so that one can verify what software is
>>    running on a machine
>> - provides TPM functionality to VMs, which includes a standardized
>>    mechanism to store keys and other blobs
>>    (Linux trusted keys, GNU TLS's TPM extensions)
>>
>> Necessarily, some of its parts are written in Forth, many are written
>> in 'C'. The extensions are known to work with QEMU for ppc64 running Linux.
> Ok, summary time: After scanning through the patch series, most of it
> sounds basically sane to me, but there are a couple of things I really
> don't like and where I'd like to ask you to rework it:
>
> - The "struct tpm_driver" function pointers seem over-engineered, I
>    think that could be made much easier by direct calls (unless you
>    really plan multiple drivers in the near future)
>
> - In the second half of the patch series, you add a lot of functions
>    (or rather one function and two wrappers) three times: One time in
>    vio-vtpm-cdriver.fs, one time in tpm-static.fs and one time in
>    vtpm-sml.fs. Is it really necessary to have those functions in both
>    device tree nodes, /vdevice/vtpm and /ibm,vtpm ? What does the spec
>    say?
>    Anyway, I think you should at least get rid of the functions
>    in tpm-static.fs and implement the stuff only in /ibm,vtpm instead.

Here is what the spec  (V1.2) says in section 1.3.2:

"This /vdevice/vtpm node is provided for use by the client – it simply 
passes commands on to the real /ibm,vtpm node."

We would have to keep it.

>
> - Also for the other functions in tpm-static.fs, I think it would be
>    nicer if you would move them to /ibm,vtpm instead to get rid of
>    tpm-static.fs completely.

See above. I do not think we can.

> - Since your patch series is already really huge, please try to focus
>    on the basics first and omit things like the menu in this initial
>    series. Send these things later once the basics have been included.
>
>   Thomas


Stefan