Re: [SeaBIOS] [RFC PATCH v1 1/9] tpm: Extend TPM TIS with TPM 2 support.

[Kevin O'Connor]

On Fri, Jan 15, 2016 at 02:44:31PM -0500, Stefan Berger wrote:
> From: Stefan Berger
>
> Extend the probing of the interface with TPM 2 specifics.
>
> Use the new interface ID register of the TIS to check whether
> a TPM 1.2 or a TPM 2 is underneath.
>
> We select the TIS if possible and lock it so we can issue commands
> during S3 for example and prevent the OS from changing to CRB type
> of interface.
>
> The register is described in table 13 here:
>
> http://www.trustedcomputinggroup.org/resources/pc_client_platform_tpm_profil...
> Signed-off-by: Stefan Berger
>
> src/hw/tpm_drivers.c | 39 +++++++++++++++++++++++++++++++++++++++
>  src/hw/tpm_drivers.h |  7 +++++++
>  src/tcgbios.c        |  8 ++++++++
>  3 files changed, 54 insertions(+)
>
> diff --git a/src/hw/tpm_drivers.c b/src/hw/tpm_drivers.c
> index 08fd101..dd6b022 100644
> --- a/src/hw/tpm_drivers.c
> +++ b/src/hw/tpm_drivers.c
> @@ -23,6 +23,7 @@ struct tpm_driver {
>      u32 *durations;
>      void (*set_timeouts)(u32 timeouts[4], u32 durations[3]);
>      u32 (*probe)(void);
> TPMVersion (*get_tpm_version)(void);
> u32 (*init)(void);
> u32 (*activate)(u8 locty);
> u32 (*ready)(void);

BTW, what is this function table used for?  I think each function
pointer always points to a static function in the current code.  Is
there some hardware that is significantly different?
> @@ -69,6 +70,23 @@ static u32 tis_probe(void)
>      if ((didvid != 0) && (didvid != 0xffffffff))
>          rc = 1;
> /* TPM 2 has an interface register */
> u32 ifaceid = readl(TIS_REG(0, TIS_REG_IFACE_ID));
> if ((ifaceid & 0xf) != 0xf) {
>    if ((ifaceid & 0xf) == 1) {
>        /* CRB is active; no TIS */
>        return 0;
>    }
>    if ((ifaceid & (1 << 13)) == 0) {
>        /* TIS cannot be selected */
>        return 0;
>    }
>    /* write of 0 to bits 17-18 selects TIS */
>    writel(TIS_REG(0, TIS_REG_IFACE_ID), 0);
>    /* since we only support TIS, we lock it */
>    writel(TIS_REG(0, TIS_REG_IFACE_ID), (1 << 19));
> }
> return rc;
>
> }
>
> @@ -303,6 +321,19 @@ static u32 tis_waitrespready(enum tpmDurationType to_t)
>      return rc;
>  }
>
> +TPMVersion tis_get_tpm_version(void)
> +{
> u8 locty = tis_find_active_locality();
> /* TPM 2 has an interface register */
> u32 ifaceid = readl(TIS_REG(locty, TIS_REG_IFACE_ID));
> if ((ifaceid & 0xf) == 0) {
>    /* TPM 2 */
>    return TPM_VERSION_2;
> }
> return TPM_VERSION_1_2;
>
> +}
>
> struct tpm_driver tpm_drivers[TPM_NUM_DRIVERS] = {
>      [TIS_DRIVER_IDX] =
> @@ -312,6 +343,7 @@ struct tpm_driver tpm_drivers[TPM_NUM_DRIVERS] = {
>              .set_timeouts  = set_timeouts,
>              .probe         = tis_probe,
>              .init          = tis_init,
>        .get_tpm_version = tis_get_tpm_version,
>        .activate      = tis_activate,
>        .ready         = tis_ready,
>        .senddata      = tis_senddata,
>
> @@ -387,3 +419,10 @@ tpmhw_set_timeouts(u32 timeouts[4], u32 durations[3])
>      struct tpm_driver *td = &tpm_drivers[TPMHW_driver_to_use];
>      td->set_timeouts(timeouts, durations);
>  }
>
> +TPMVersion
> +tpmhw_get_tpm_version(void)
> +{
> struct tpm_driver *td = &tpm_drivers[TPMHW_driver_to_use];
> return td->get_tpm_version();
>
> +}

Instead of exporting an additional function from tpm_drivers.c, could
the version just be probed and returned from tpmhw_probe()?

-Kevin