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

[Stefan Berger]

On 11/19/2015 11:11 AM, Thomas Huth wrote:
> On 19/11/15 16:47, Stefan Berger wrote:
>> On 11/19/2015 06:52 AM, Thomas Huth wrote:
> [...]
>>> - 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."
> That's a strange design. But ok, if it's already in the spec this way,
> you likely can hardly do anything against this.
>
>>> - 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.
> Why not? The functions in tpm-static.fs aren't mandated by the spec, are
> they? I'd expect that only the device tree nodes are defined by the
> spec, so it should be fine to remove tpm-static.fs and keep all
> functions in the device tree node instead.
>
> For example vtpm-measure-hdd-mbr - if you move that function to
> /ibm,vtpm you could do something like this in disk-label instead:
>
>   : load-from-boot-partition ( addr -- size )
>      debug-disk-label? IF ." Trying DOS boot " .s cr THEN
>      dup load-from-dos-boot-partition ?dup 0 <> IF
>         nip
>         block s" /ibm,vtpm" find-node dup IF
>            s" vtpm-measure-hdd-mbr" rot $call-static
>         ELSE
>            2drop
>         THEN
>         EXIT
>      THEN
>      ...
>
> That way you also do not have to worry about including a wrapper for
> board-js2x anymore.

You guys are more familiar with this code. I personally liked my simple 
one-line addition to the code:

: load-from-boot-partition ( addr -- size )
    debug-disk-label? IF ." Trying DOS boot " .s cr THEN
    dup load-from-dos-boot-partition ?dup 0 <> IF
       nip
       block vtpm-measure-hdd-mbr
       EXIT
    THEN
[...]
;

But if you guys tell me the above is the SLOF-way of doing things, I 
will convert it...

Unfortunately the JS21 is old, but at least it had a TPM built-in. Well, 
that would have been a reason to keep the driver architecture as-is and 
dig out the TPM TIS driver that can control its hardware TPM. Probing 
would presumably fail on the PAPR interface and succeed on the TPM TIS 
interface and then choose that driver. I do not have a JS2x machine nor 
do I know what the runtime failures are in the Forth code now due to me 
adding all this TPM code here..

     Stefan


>
>   Thomas
>