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

Stefan Berger posted 3 patches 7 years, 3 months ago
There is a newer version of this series
[SeaBIOS] [PATCH v2 3/3] tcgbios: extend Physical Presence interface with more functions
Posted by Stefan Berger 7 years, 3 months ago
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@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;
+            ret = tpm12_enable_activate(1, verbose, returnCode);
+            /* no reboot happened */
+            if (!ret) {
+                 *nextStep = TPM_PPI_OP_NOOP;
+                ret = tpm12_force_clear(0, 1, verbose, returnCode);
+            }
+            break;
+
         default:
             break;
     }
@@ -1783,11 +1833,12 @@ tpm20_process_cfg(tpm_ppi_code msgCode, int verbose, u32 *returnCode)
 }
 
 static int
-tpm_process_cfg(tpm_ppi_code msgCode, int verbose, u32 *returnCode)
+tpm_process_cfg(tpm_ppi_code msgCode, int verbose, u32 *returnCode,
+                u8 *nextStep)
 {
     switch (TPM_version) {
     case TPM_VERSION_1_2:
-        return tpm12_process_cfg(msgCode, verbose, returnCode);
+        return tpm12_process_cfg(msgCode, verbose, returnCode, nextStep);
     case TPM_VERSION_2:
         return tpm20_process_cfg(msgCode, verbose, returnCode);
     }
@@ -1959,7 +2010,8 @@ tpm12_menu(void)
                     break;
 
                 if (next_scancodes[i] == scancode) {
-                    tpm12_process_cfg(msgCode, 1, NULL);
+                    u8 ignore;
+                    tpm12_process_cfg(msgCode, 1, NULL, &ignore);
                     waitkey = 0;
                     break;
                 }
@@ -2049,8 +2101,15 @@ static const u8 tpm12_ppi_funcs[] = {
     [TPM_PPI_OP_ACTIVATE] = FLAGS,
     [TPM_PPI_OP_DEACTIVATE] = FLAGS,
     [TPM_PPI_OP_CLEAR] = FLAGS,
+    [TPM_PPI_OP_ENABLE_ACTIVATE] = FLAGS,
+    [TPM_PPI_OP_DEACTIVATE_DISABLE] = FLAGS,
     [TPM_PPI_OP_SET_OWNERINSTALL_TRUE] = FLAGS,
     [TPM_PPI_OP_SET_OWNERINSTALL_FALSE] = FLAGS,
+    [TPM_PPI_OP_ENABLE_ACTIVATE_SET_OWNERINSTALL_TRUE] = FLAGS,
+    [TPM_PPI_OP_SET_OWNERINSTALL_FALSE_DEACTIVATE_DISABLE] = FLAGS,
+    [TPM_PPI_OP_CLEAR_ENABLE_ACTIVATE] = FLAGS,
+    [TPM_PPI_OP_ENABLE_ACTIVATE_CLEAR] = FLAGS,
+    [TPM_PPI_OP_ENABLE_ACTIVATE_CLEAR_ENABLE_ACTIVATE] = FLAGS,
 };
 
 static const u8 tpm2_ppi_funcs[] = {
@@ -2116,7 +2175,7 @@ tpm_ppi_process(void)
             tp->pprq = 0;
 
             printf("Processing TPM PPI opcode %d\n", op);
-            tpm_process_cfg(op, 0, &tp->pprp);
+            tpm_process_cfg(op, 0, &tp->pprp, &nextStep);
         }
    }
 }
-- 
2.5.5


_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH v2 3/3] tcgbios: extend Physical Presence interface with more functions
Posted by Kevin O'Connor 7 years, 3 months ago
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@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.

-Kevin

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios
Re: [SeaBIOS] [PATCH v2 3/3] tcgbios: extend Physical Presence interface with more functions
Posted by Stefan Berger 7 years, 3 months ago
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@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
>


_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
https://mail.coreboot.org/mailman/listinfo/seabios