[SLOF] [PATCH v2 12/20] Add TPM firmware API calls hash-all, log-event, hash-log-extend-event

[Nikunj A Dadhania]

Thomas Huth <thuth at redhat.com> writes:

> On 19/11/15 19:20, Stefan Berger wrote:
>> On 11/19/2015 06:30 AM, Thomas Huth wrote:
>>> On 17/11/15 18:02, Stefan Berger wrote:
>>>> From: Stefan Berger <stefanb at linux.vnet.ibm.com>
>>>>
>>>> +\ firmware API function
>>>> +: vtpm-hash-all ( data-ptr data-len hash-ptr -- )
>>>> +    vtpm-available? IF
>>>> +        tpm-hash-all                               ( -- errcode )
>>>> +        dup 0<> IF
>>>> +            ." VTPM: Error code from tpm-hash-all: " . cr
>>>> +        ELSE
>>>> +            drop
>>>> +        THEN
>>>> +    ELSE
>>>> +        3drop
>>>> +    THEN
>>>> +;
>>> Why do you need wrappers for these in tpm-static.fs at all? The
>>> functions only seem to be necessary from vtpm-sml.fs, so you could
>>> directly implement them only there instead.
>> 
>> 
>> Some of the functions here in tpm-static.fs will be called directly,
>> others only via the firmware API and node. All the functions in this
>> file follow a similar pattern:
>> 
>> : vtpm-xyz
>>     vtpm-available? IF
>>         tpm-xyz  \ invoke C code
>>         [...]
>>     THEN
>> ;
>> 
>> I am not sure whether partially putting this pattern into other files
>> helps much.
>
> I guess I should explain my thoughts a little bit: Since Forth in SLOF
> is interpreted, each file that you add to the boot flow unconditionally
> (like tpm-static.fs) will slow down the boot process a little bit - and
> additional Forth functions will clutter the common Forth dictionary (try
> to type "words" at the SLOF prompt to see what I mean).
>
> So if we've got the possibility to put the code in device tree nodes
> instead, whose .fs files are only included if the device is really
> there, this is IMHO the better solution.

I agree, only include this code when the device is present.

>
> But as always - it's a question what the maintainers prefer... Nikunj?
> Alexey?

Regards,
Nikunj