[SeaBIOS] [PATCH v2 2/3] tcgbios: Add TPM Physical Presence interface support

Kevin O'Connor kevin at koconnor.net
Tue Jan 16 19:36:58 CET 2018


On Tue, Jan 16, 2018 at 11:41:02AM -0500, Stefan Berger wrote:
> Add support for TPM 1.2 and TPM 2 Physical Presence interface (PPI).
> A shared memory structure is located at 0xfffe f000 - 0xfffe f3ff
> that SeaBIOS initializes (unless it has already been intialized) and
> then searches for a code it is supposed to act upon. A code typically
> requires that one or more TPM commands are being sent.

If I'm understanding the code correctly, it no longer hardcodes
0xfffef000 (great!).  The commit comment should also be updated.

> 
> The underlying spec can be accessed from this page here:
> 
> https://trustedcomputinggroup.org/tcg-physical-presence-interface-specification/
> 
> Version 1.30 is implemented.
> 
> Signed-off-by: Stefan Berger <stefanb at linux.vnet.ibm.com>
> ---
>  src/post.c     |  4 +++
>  src/std/acpi.h | 10 ++++++
>  src/std/tcg.h  | 31 ++++++++++++++++++
>  src/tcgbios.c  | 99 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/tcgbios.h  |  3 ++
>  5 files changed, 147 insertions(+)
> 
> diff --git a/src/post.c b/src/post.c
> index f93106a..f451013 100644
> --- a/src/post.c
> +++ b/src/post.c
> @@ -201,6 +201,7 @@ maininit(void)
>  
>      // Setup platform devices.
>      platform_hardware_setup();
> +    tpm_ppi_init();
>  
>      // Start hardware initialization (if threads allowed during optionroms)
>      if (threads_during_optionroms())
> @@ -220,6 +221,9 @@ maininit(void)
>      // Run option roms
>      optionrom_setup();
>  
> +    // Process user-requested TPM state change
> +    tpm_ppi_process();

I think it would be better if these two calls were added to the
existing tpm_setup() call.

Also, function suffixes of "_init" and "_setup" have specific meaning
in seabios - see docs/Execution_and_code_flow.md - so you should not
export a function with those sufixes unless they follow the
convention.

[...]
> --- a/src/std/acpi.h
> +++ b/src/std/acpi.h
> @@ -320,4 +320,14 @@ struct tpm2_descriptor_rev2
>      u64  log_area_start_address;
>  } PACKED;
>  
> +#define QEMU_SIGNATURE 0x554d4551
> +struct qemu_descriptor
> +{
> +    ACPI_TABLE_HEADER_DEF
> +    u32 tpmppi_address;
> +    u8 tpm_version; /* 1 = 1.2, 2 = 2 */
> +    u8 tpmppi_version;
> +#define TPM_PPI_VERSION_1_30   1
> +} PACKED;

I'm confused at the purpose of this acpi table.  If I'm understanding
it correctly, it is purely to pass information from QEMU to SeaBIOS
(and perhaps OVMF?).  If so, I don't think this is a good way to do it
- a regular fw_cfg setting seems simpler (and less likely to cause
problems with OSes).

> +
>  #endif // acpi.h
> diff --git a/src/std/tcg.h b/src/std/tcg.h
> index 09a92d8..22353a9 100644
> --- a/src/std/tcg.h
> +++ b/src/std/tcg.h
> @@ -551,4 +551,35 @@ struct pcctes_romex
>  #define TPM_PPI_OP_SET_OWNERINSTALL_TRUE 8
>  #define TPM_PPI_OP_SET_OWNERINSTALL_FALSE 9
>  
> +struct tpm_ppi {
> +    u8 ppin;            /*  0: 1 = initialized */
> +    u32 ppip;           /*  1: not used */
> +    u32 pprp;           /*  5: response from TPM; set by BIOS */
> +    u32 pprq;           /*  9: opcode; set by ACPI */
> +    u32 pprm;           /* 13: parameter for opcode; set by ACPI */
> +    u32 lppr;           /* 17: last opcode; set by BIOS */
> +    u32 fret;           /* 21: not used */
> +    u8 res1;            /* 25: reserved */
> +    u32 res2[4];        /* 26: reserved */
> +    u8 res3[214];       /* 42: reserved */
> +    u8 func[256];       /* 256: per function implementation flags; set by BIOS */
> +/* indication whether function is implemented; bit 0 */
> +#define TPM_PPI_FUNC_IMPLEMENTED       (1 << 0)
> +/* actions OS should take to transition to the pre-OS env.; bits 1, 2 */
> +#define TPM_PPI_FUNC_ACTION_SHUTDOWN   (1 << 1)
> +#define TPM_PPI_FUNC_ACTION_REBOOT     (2 << 1)
> +#define TPM_PPI_FUNC_ACTION_VENDOR     (3 << 1)
> +#define TPM_PPI_FUNC_ACTION_MASK       (3 << 1)
> +/* whether function is blocked by BIOS settings; bits 3,4,5 */
> +#define TPM_PPI_FUNC_NOT_IMPLEMENTED     (0 << 3)
> +#define TPM_PPI_FUNC_BIOS_ONLY           (1 << 3)
> +#define TPM_PPI_FUNC_BLOCKED             (2 << 3)
> +#define TPM_PPI_FUNC_ALLOWED_USR_REQ     (3 << 3)
> +#define TPM_PPI_FUNC_ALLOWED_USR_NOT_REQ (4 << 3)
> +#define TPM_PPI_FUNC_MASK                (7 << 3)
> +} PACKED;
> +
> +void tpm_ppi_init(void);
> +void tpm_ppi_process(void);

The files in the std/ directory are for well defined specifications.
It should not be used to export seabios functions.  Also, it's not
clear to me if 'struct tpm_ppi' and the TPM_PPI_FUNC_* defines are a
qemu/acpi/seabios interface or a TPM standard.

> +
>  #endif // tcg.h
> diff --git a/src/tcgbios.c b/src/tcgbios.c
> index 730b5e7..c8e6ca2 100644
> --- a/src/tcgbios.c
> +++ b/src/tcgbios.c
> @@ -1783,6 +1783,18 @@ tpm20_process_cfg(tpm_ppi_code msgCode, int verbose, u32 *returnCode)
>  }
>  
>  static int
> +tpm_process_cfg(tpm_ppi_code msgCode, int verbose, u32 *returnCode)
> +{
> +    switch (TPM_version) {
> +    case TPM_VERSION_1_2:
> +        return tpm12_process_cfg(msgCode, verbose, returnCode);
> +    case TPM_VERSION_2:
> +        return tpm20_process_cfg(msgCode, verbose, returnCode);
> +    }
> +    return -1;
> +}
> +
> +static int
>  tpm12_get_tpm_state(void)
>  {
>      int state = 0;
> @@ -2021,3 +2033,90 @@ tpm_can_show_menu(void)
>      }
>      return 0;
>  }
> +
> +static struct tpm_ppi *tp;
> +static u8 nextStep = TPM_PPI_OP_NOOP; /* opcode to execute after reboot */
> +
> +#define FLAGS (TPM_PPI_FUNC_IMPLEMENTED | \
> +               TPM_PPI_FUNC_ACTION_REBOOT | \
> +               TPM_PPI_FUNC_ALLOWED_USR_NOT_REQ)
> +
> +static const u8 tpm12_ppi_funcs[] = {
> +    [TPM_PPI_OP_NOOP] = TPM_PPI_FUNC_IMPLEMENTED |
> +                        TPM_PPI_FUNC_ALLOWED_USR_NOT_REQ,
> +    [TPM_PPI_OP_ENABLE]  = FLAGS,
> +    [TPM_PPI_OP_DISABLE] = FLAGS,
> +    [TPM_PPI_OP_ACTIVATE] = FLAGS,
> +    [TPM_PPI_OP_DEACTIVATE] = FLAGS,
> +    [TPM_PPI_OP_CLEAR] = FLAGS,
> +    [TPM_PPI_OP_SET_OWNERINSTALL_TRUE] = FLAGS,
> +    [TPM_PPI_OP_SET_OWNERINSTALL_FALSE] = FLAGS,
> +};
> +
> +static const u8 tpm2_ppi_funcs[] = {
> +    [TPM_PPI_OP_CLEAR] = FLAGS,
> +};
> +
> +void
> +tpm_ppi_init(void)
> +{
> +    struct qemu_descriptor *qemu = NULL;
> +
> +    while (1) {
> +        qemu  = find_acpi_table_iter(QEMU_SIGNATURE, qemu);
> +        if (!qemu)
> +            return;
> +        if (!memcmp("QEMU", qemu->oem_id, 5) && !memcmp("CONF", qemu->oem_table_id, 5))
> +            break;
> +    }
> +
> +    tp = (struct tpm_ppi *)(u32)qemu->tpmppi_address;
> +    dprintf(DEBUG_tcg, "TCGBIOS: TPM PPI struct at %p\n", tp);
> +
> +    memset(&tp->func, 0, sizeof(tp->func));
> +    switch (qemu->tpmppi_version) {
> +    case TPM_PPI_VERSION_1_30:
> +        switch (qemu->tpm_version) {
> +        case TPM_VERSION_1_2:
> +            memcpy(&tp->func, tpm12_ppi_funcs, sizeof(tpm12_ppi_funcs));
> +            break;
> +        case TPM_VERSION_2:
> +            memcpy(&tp->func, tpm2_ppi_funcs, sizeof(tpm2_ppi_funcs));
> +            break;
> +        }

Can you elaborate on what this does?  Why is SeaBIOS filling a memory
addreses created by QEMU?  Why wouldn't QEMU just fill it with what it
wants directly?

-Kevin


> +        break;
> +    }
> +
> +    if (!tp->ppin) {
> +        tp->ppin = 1;
> +        tp->pprq = 0;
> +        tp->lppr = 0;
> +    }
> +}
> +
> +void
> +tpm_ppi_process(void)
> +{
> +   tpm_ppi_code op;
> +
> +   if (tp) {
> +        op = tp->pprq;
> +        if (!op) {
> +            /* intermediate step after a reboot? */
> +            op = nextStep;
> +        } else {
> +            /* last full opcode */
> +            tp->lppr = op;
> +        }
> +        if (op) {
> +            /*
> +             * Reset the opcode so we don't permanently reboot upon
> +             * code 3 (Activate).
> +             */
> +            tp->pprq = 0;
> +
> +            printf("Processing TPM PPI opcode %d\n", op);
> +            tpm_process_cfg(op, 0, &tp->pprp);
> +        }
> +   }
> +}
> diff --git a/src/tcgbios.h b/src/tcgbios.h
> index 32fb941..52b86f2 100644
> --- a/src/tcgbios.h
> +++ b/src/tcgbios.h
> @@ -16,4 +16,7 @@ void tpm_option_rom(const void *addr, u32 len);
>  int tpm_can_show_menu(void);
>  void tpm_menu(void);
>  
> +void tpm_ppi_init(void);
> +void tpm_ppi_process(void);
> +
>  #endif /* TCGBIOS_H */
> -- 
> 2.5.5
> 



More information about the SeaBIOS mailing list