Re: [SeaBIOS] [PATCH v8 3/8] Implementation of the TCG BIOS extensions

[Kevin O'Connor]

On Wed, Jul 02, 2014 at 11:38:47AM -0400, Stefan Berger wrote:
> This patch implements the main part of the TCG BIOS extensions. It provides
> the following functionality:
> initialization of the TCPA ACPI table used for logging of measurements
> initialization of the TPM by sending a sequence of commands to it
> proper setup of the TPM once the BIOS hands over control to the bootloader
> support for S3 resume; BIOS sends TPM_Startup(ST_STATE) to TPM
> enable configuration of SeaBIOS to be built with TCGBIOS extensions
> depending on COREBOOT not being selected
> All TCG BIOS extensions are activated with CONFIG_TCGBIOS.
>
> Structures that are needed in subsequent patches are also included in
> tcgbios.h at this point.
>
> The effect of this patch is that it initialized the TPM upon VM start
> and S3 resume.
>
> v6:
> passing durations of commands to the transmission function
> acquire timeouts and durations from TPM and use them
>
> v5:
> adding the lock flag to the 'not present' Physcial_presence_NOT_PRESENT
> structure
>
> v4:
> return TCG_GENERAL_ERROR if ! has_working_tpm()
>
> v3:
> upon S3 resume call timer_setup()
>
> v2:
> replace mssleep() with calls to msleep()
> Moving Kconfig patch to this file
> converting code to call dprintf(DEBUG_tcg, ...)
> use the get_rsdp call to get hold of the RSDP
> use util.c:checksum()
> Adapting tcgbios.c to be under LGPLv3
> using if (!CONFIG_TCGBIOS) everywhere
> Signed-off-by: Stefan Berger
>
> src/Kconfig          |   7 +
>  src/boot.c           |   2 +
>  src/config.h         |   1 +
>  src/hw/tpm_drivers.c |   4 +
>  src/post.c           |   5 +
>  src/resume.c         |   2 +
>  src/tcgbios.c        | 480 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  src/tcgbios.h        | 326 ++++++++++++++++++++++++++++++++++
>  8 files changed, 827 insertions(+)
>
> diff --git a/src/Kconfig b/src/Kconfig
> index a863866..9e65449 100644
> --- a/src/Kconfig
> +++ b/src/Kconfig
> @@ -411,6 +411,13 @@ menu "BIOS interfaces"
>              modified by programs.  However, some old DOS high memory
>              managers may require the UMB region to be read-only.
> config TCGBIOS
>    select S3_RESUME
>    bool "TPM support and TCG BIOS extensions"
>    default y
>    help
>        Provide TPM support along with TCG BIOS extensions
>
> endmenu
>
> menu "BIOS Tables"
> diff --git a/src/boot.c b/src/boot.c
> index 133e206..f36f3d6 100644
> --- a/src/boot.c
> +++ b/src/boot.c
> @@ -19,6 +19,7 @@
>  #include "std/disk.h" // struct mbr_s
>  #include "string.h" // memset
>  #include "util.h" // irqtimer_calc
> +#include "tcgbios.h" // tcpa_*
>
> /****************************************************************
> @@ -475,6 +476,7 @@ interactive_bootmenu(void)
>  printf("Select boot device:\n\n");
>  wait_threads();
> tcpa_leave_bios();

This is an odd place for a tcpa call.  Shouldn't it go with the normal
_setup() and _prepboot() calls?
> --- a/src/hw/tpm_drivers.c
> +++ b/src/hw/tpm_drivers.c
> @@ -7,6 +7,8 @@
>  //
>  // This file may be distributed under the terms of the GNU LGPLv3 license.
>
> +#if CONFIG_TCGBIOS == 1

We try to avoid ifdefs in seabios.  This should go in the start of
exported functions as "if (!CONFIG_TCGBIOS) return;".

-Kevin