[SeaBIOS] [PATCH v2 3/3] tcgbios: extend Physical Presence interface with more functions

[Stefan Berger]

On 01/16/2018 01:58 PM, Kevin O'Connor wrote:
> On Tue, Jan 16, 2018 at 11:41:03AM -0500, Stefan Berger wrote:
>> Implement more functions of the TPM Physical Presence interface.
>> Some of the added functions will automatically reboot the machine.
>> Thus we need to save the next step after the reboot in an additional
>> variable.
>>
>> Signed-off-by: Stefan Berger <stefanb at linux.vnet.ibm.com>
>> ---
>>   src/std/tcg.h |  7 ++++++
>>   src/tcgbios.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-----
>>   2 files changed, 71 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/std/tcg.h b/src/std/tcg.h
>> index 22353a9..aeee689 100644
>> --- a/src/std/tcg.h
>> +++ b/src/std/tcg.h
>> @@ -548,8 +548,15 @@ struct pcctes_romex
>>   #define TPM_PPI_OP_ACTIVATE 3
>>   #define TPM_PPI_OP_DEACTIVATE 4
>>   #define TPM_PPI_OP_CLEAR 5
>> +#define TPM_PPI_OP_ENABLE_ACTIVATE 6
>> +#define TPM_PPI_OP_DEACTIVATE_DISABLE 7
>>   #define TPM_PPI_OP_SET_OWNERINSTALL_TRUE 8
>>   #define TPM_PPI_OP_SET_OWNERINSTALL_FALSE 9
>> +#define TPM_PPI_OP_ENABLE_ACTIVATE_SET_OWNERINSTALL_TRUE 10
>> +#define TPM_PPI_OP_SET_OWNERINSTALL_FALSE_DEACTIVATE_DISABLE 11
>> +#define TPM_PPI_OP_CLEAR_ENABLE_ACTIVATE 14
>> +#define TPM_PPI_OP_ENABLE_ACTIVATE_CLEAR 21
>> +#define TPM_PPI_OP_ENABLE_ACTIVATE_CLEAR_ENABLE_ACTIVATE 22
>>   
>>   struct tpm_ppi {
>>       u8 ppin;            /*  0: 1 = initialized */
>> diff --git a/src/tcgbios.c b/src/tcgbios.c
>> index c8e6ca2..e074d42 100644
>> --- a/src/tcgbios.c
>> +++ b/src/tcgbios.c
>> @@ -1655,7 +1655,8 @@ tpm12_set_owner_install(int allow, int verbose, u32 *returnCode)
>>   }
>>   
>>   static int
>> -tpm12_process_cfg(tpm_ppi_code msgCode, int verbose, u32 *returnCode)
>> +tpm12_process_cfg(tpm_ppi_code msgCode, int verbose, u32 *returnCode,
>> +                  u8 *nextStep)
>>   {
>>       int ret = 0;
>>   
>> @@ -1683,6 +1684,18 @@ tpm12_process_cfg(tpm_ppi_code msgCode, int verbose, u32 *returnCode)
>>               ret = tpm12_force_clear(1, 0, verbose, returnCode);
>>               break;
>>   
>> +        case TPM_PPI_OP_ENABLE_ACTIVATE:
>> +            ret = tpm12_enable_tpm(1, verbose, returnCode);
>> +            if (!ret)
>> +                ret = tpm12_activate_tpm(1, 1, verbose, returnCode);
>> +            break;
>> +
>> +        case TPM_PPI_OP_DEACTIVATE_DISABLE:
>> +            ret = tpm12_activate_tpm(0, 1, verbose, returnCode);
>> +            if (!ret)
>> +                ret = tpm12_enable_tpm(0, verbose, returnCode);
>> +            break;
>> +
>>           case TPM_PPI_OP_SET_OWNERINSTALL_TRUE:
>>               ret = tpm12_set_owner_install(1, verbose, returnCode);
>>               break;
>> @@ -1691,6 +1704,43 @@ tpm12_process_cfg(tpm_ppi_code msgCode, int verbose, u32 *returnCode)
>>               ret = tpm12_set_owner_install(0, verbose, returnCode);
>>               break;
>>   
>> +        case TPM_PPI_OP_ENABLE_ACTIVATE_SET_OWNERINSTALL_TRUE:
>> +            *nextStep = TPM_PPI_OP_SET_OWNERINSTALL_TRUE;
>> +            ret = tpm12_enable_activate(1, verbose, returnCode);
>> +            if (!ret)
>> +                ret = tpm12_set_owner_install(1, verbose, returnCode);
>> +            break;
>> +
>> +        case TPM_PPI_OP_SET_OWNERINSTALL_FALSE_DEACTIVATE_DISABLE:
>> +            ret = tpm12_set_owner_install(0, verbose, returnCode);
>> +            if (!ret)
>> +                ret = tpm12_activate_tpm(0, 0, verbose, returnCode);
>> +            if (!ret)
>> +                ret = tpm12_enable_tpm(0, verbose, returnCode);
>> +            break;
>> +
>> +        case TPM_PPI_OP_CLEAR_ENABLE_ACTIVATE:
>> +            ret = tpm12_force_clear(0, 1, verbose, returnCode);
>> +            break;
>> +
>> +        case TPM_PPI_OP_ENABLE_ACTIVATE_CLEAR:
>> +            *nextStep = TPM_PPI_OP_CLEAR;
>> +            ret = tpm12_enable_activate(1, verbose, returnCode);
>> +            /* no reboot happened */
>> +            if (!ret)
>> +                ret = tpm12_force_clear(0, 0, verbose, returnCode);
>> +            break;
>> +
>> +        case TPM_PPI_OP_ENABLE_ACTIVATE_CLEAR_ENABLE_ACTIVATE:
>> +            *nextStep = TPM_PPI_OP_CLEAR_ENABLE_ACTIVATE;
> I'm strugging to understand "nextStep".  I think it would be clearer
> if the code could do a switch on the request and then execute all the
> details of that request without the need to set flags indicating the
> function should be rerun.  tpm12_process_cfg() can call itself
> recursively if it needs to.

Some of the opcodes we are executing here need to do a reboot and then 
execute more steps after the reboot. The last one of the above sequences 
does this:

enable + activate -> reboot -> clear + enable + activate -> reboot

The post-reboot sequence clear + enable + activate exists with 
TPM_PPI_OP_CLEAR_ENABLE_ACTIVATE, so we can execute that as the 2nd part.

    Stefan

>
> -Kevin
>