[SLOF] [PATCH v2 02/20] Add TPM initialization support

[Stefan Berger]

On 11/22/2015 10:58 PM, Nikunj A Dadhania wrote:
> Thomas Huth <thuth at redhat.com> writes:
>
>> On 19/11/15 13:15, Stefan Berger wrote:
>>> On 11/19/2015 04:04 AM, Thomas Huth wrote:
>>>> On 17/11/15 18:02, Stefan Berger wrote:
>> [...]
>>>>> diff --git a/board-qemu/slof/vtpm-sml.fs b/board-qemu/slof/vtpm-sml.fs
>>>>> new file mode 100644
>>>>> index 0000000..40f1b7e
>>>>> --- /dev/null
>>>>> +++ b/board-qemu/slof/vtpm-sml.fs
>>>>> @@ -0,0 +1,49 @@
>>>>> +\
>>>>> *****************************************************************************
>>>>>
>>>>> +\ * Copyright (c) 2015 IBM Corporation
>>>>> +\ * All rights reserved.
>>>>> +\ * This program and the accompanying materials
>>>>> +\ * are made available under the terms of the BSD License
>>>>> +\ * which accompanies this distribution, and is available at
>>>>> +\ * http://www.opensource.org/licenses/bsd-license.php
>>>>> +\ *
>>>>> +\ * Contributors:
>>>>> +\ *     IBM Corporation - initial implementation
>>>>> +\
>>>>> ****************************************************************************/
>>>>>
>>>>> +
>>>>> +\ KVM/qemu TPM SML entries in /ibm,vtpm
>>>> What does SML mean? ... being a little bit more verbose the first time
>>>> you use TLAs (Three Letter Acronyms) would be nice.
>>> Stored Measurement Log.
>>>
>>>>> +" /" find-device
>>>>> +
>>>>> +new-device
>>>>> +
>>>>> +false VALUE    vtpm-debug?
>>>> Again, yet another vtpm-debug? variable that shadows the global one?
>>> What is preferable ? One global one or local ones?
>> Depends on what you want ... but I'd rather expect one knob only to turn
>> on debugging instead of three.
>>
>>>>> diff --git a/lib/libtpm/tcgbios.c b/lib/libtpm/tcgbios.c
>>>>> new file mode 100644
>>>>> index 0000000..0dae810
>>>>> --- /dev/null
>>>>> +++ b/lib/libtpm/tcgbios.c
>> [...]
>>>>> +/********************************************************
>>>>> +  Extensions for TCG-enabled BIOS
>>>>> + *******************************************************/
>>>>> +
>>>>> +static bool is_tpm_present(void)
>>>>> +{
>>>>> +    bool rc = false;
>>>>> +    unsigned int i;
>>>>> +
>>>>> +    for (i = 0; i < TPM_NUM_DRIVERS; i++) {
>>>>> +        struct tpm_driver *td = &tpm_drivers[i];
>>>>> +        if (td->probe()) {
>>>>> +            td->init();
>>>>> +            tpm_state.tpm_drv = td;
>>>>> +            rc = true;
>>>>> +            break;
>>>>> +        }
>>>>> +    }
>>>>> +
>>>>> +    return rc;
>>>>> +}
>>>> Uh, unless you really really want to support multiple TPM drivers in the
>>>> near future, I really would prefer if you could keep this simple instead
>>>> and get rid of that indirect "struct tpm_driver *" function pointer
>>>> magic. That really looks over-engineered to me right now.
>>> ... is this really necessary?
>> Sounds like a question for the maintainers ... Nikunj? Alexey?
> If we dont intend to support more than 3 drivers, i dont see an
> advantage here. So I agree here with Thomas, we can get rid of the
> function pointer stuff.

I converted that part now as well.

    Stefan