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

I addressed the above comments now. To address the last item, I am 
rearranging the order of the patches. Also some patches will have to be 
merged to avoid gcc warnings related to missing users of functions, so 
some patches will become larger.
One obstacle is certainly that the QEMU patches haven't been accepted, 
yet. Nevertheless I would like to get this series of patches into 
acceptable state.

Regards,
     Stefan