On Sun, Nov 29, 2015 at 11:49:43PM -0500, Stefan Berger wrote:
> From: Stefan Berger <stef...@linux.vnet.ibm.com>
> 
> This patch adds an new menu entry to the main menu. This menu item enables
> the user to enter a TPM control menu which allows control of those aspects
> of the TPM's state that can only be controlled while in the firmware
> and while physical presence can be asserted.
[...]
> --- a/src/std/tcg.h
> +++ b/src/std/tcg.h
[...]
> @@ -325,4 +348,22 @@ struct tpm_res_sha1complete {
>      u8     hash[20];
>  } PACKED;
>  
> +#define TPM_STATE_ENABLED 1
> +#define TPM_STATE_ACTIVE 2
> +#define TPM_STATE_OWNED 4
> +#define TPM_STATE_OWNERINSTALL 8
> +
> +/*
> + * physical presence interface
> + */
> +
> +#define TPM_PPI_OP_NOOP 0
> +#define TPM_PPI_OP_ENABLE 1
> +#define TPM_PPI_OP_DISABLE 2
> +#define TPM_PPI_OP_ACTIVATE 3
> +#define TPM_PPI_OP_DEACTIVATE 4
> +#define TPM_PPI_OP_CLEAR 5
> +#define TPM_PPI_OP_SET_OWNERINSTALL_TRUE 8
> +#define TPM_PPI_OP_SET_OWNERINSTALL_FALSE 9

Are the above definitions part of the standard, or internal to the
implementation?  If the latter, they should go into tcgbios.[ch]

> --- a/src/tcgbios.c
> +++ b/src/tcgbios.c
> @@ -23,6 +23,8 @@
>  #include "string.h" // checksum
>  #include "tcgbios.h"// tpm_*, prototypes
>  #include "util.h" // printf, get_keystroke
> +#include "malloc.h" // malloc_*
> +#include "stacks.h" // wait_threads

Doesn't look like malloc_x() is used in this code.

[...]
> +typedef struct {
> +    u8  op;
> +} tpm_bios_cfg;

What is the purpose of this struct?

> +extern void reset_vector(void) __noreturn;

The code should use reset() (defined in stacks.h).  Directly calling
reset_vector() in 32bit mode isn't strictly correct.

Looks good to me.
-Kevin

_______________________________________________
SeaBIOS mailing list
SeaBIOS@seabios.org
http://www.seabios.org/mailman/listinfo/seabios

Reply via email to