[SLOF] [PATCH v4 00/33] Add vTPM support to SLOF

[Stefan Berger]

On 12/18/19 6:05 PM, Alexey Kardashevskiy wrote:
> Hey,
>
> It's been a while since the last attempt :) 33 is a lot! Comments below...
>
> On 12/12/2019 07:26, Stefan Berger wrote:
>> I am reposting this series of patches for adding vTPM support
>> to SLOF. The series has grown over time due to addition of
>> vTPM 2.0 support. The vTPM (1.2 & 2) SLOF code leans on the code I
> Where these versions are from? PAPR (I did not look deep though) or something else? Do we have to/want to implement

There's a firmware document for Power vTPM support. That's where some of 
the Power relevant parts are from, such as API support. Otherwise the 
code, as said, is very much leaning on the contributions I made to 
SeaBIOS and where Kevin also made changes to.


> anything but v2.0? What other pieces need SLOF to support v1.2 - grub, linux, qemu, aix, freebsd?


The issue, at least for me, is the stack of patches...


>
>
>> upstreamed to SeaBIOS and where Kevin O'Connor (cc'ed) has also made
>> changes to and given me permission to contribute the combined code to
>> SLOF under the BSD license. One goal is to keep the two code bases in
>> sync as much as possible.
>>
>> I expect that PAPR vTPM support will become available in QEMU 5.0.
> 01/33 refers to tpm-next+spapr.v3 and there is a newer v6 and now you say it is qemu 5.0, which statement is correct?

Over the last few days I worked on subsequent submission to qemu-devel. 
v3 works fine with this series. The intention is that vTPM support for 
pSeries will go into QEMU 5.0.


>
> 01/33 refers to libtpms/swtpm - are these standard libraries/tools available from distros (my ubuntu 18.04 does not have
> libtpms). Does qemu v5.0 depend on these? Or these can be added as submodules? Do we need both libraries? It must be
> documented then somewhere so you do not have to document it in 01/33. A few command line examples (one for qemu and one
> for swtpm) in the cover letter should be enough.

It's swtpm that needs libtpms. Swtpm is either started by libvirt or has 
to be started manually as described here (file will be converted to .rst):

https://github.com/qemu/qemu/blob/master/docs/specs/tpm.txt

It's been packaged for Fedora and will appear in RHAV. Not all distros 
may have it, yet.


> In general, it is too many small patches and later patches change what earlier patch in the series did, such as 26/33,
> 31/33, please avoid that - it might represent how you developed those but it is useless when bisecting.
> Either add commit logs into 29/33, 30/33, 31/33, 33/33, or (better) merge them in one patch.


Sure, I can do that.


>
> Also please organize the series as:
> 1. prerequisites (03/33, 28/33,...)
> 2. vtpm v1.2 driver (if we still need it)
> 3. vtpm v2.0 (avoid reorganising code from the previous step)
> 4. implement menu item

Ok, I will move the prerequisites to the top but would then like to 
build TPM 2 on top of TPM 1.2 support, meaning all TPM 1.2 patches come 
first, then we add TPM 2.


>
> btw do we really want the menu? Can this (whatever the menu does) be done by the QEMU command line, some properties of
> the new tpm-spapr device? Thanks,


There are aspects of TPM state that can only be controlled via a 
firmware menu after initialization of the TPM and before the firmware 
passes control to the OS loader. That's why physical machines have TPM 
menus in the firmware.

    Stefan