[SLOF] [PATCH v3 10/17] Add support for a TPM menu to control the state of the TPM

[Thomas Huth]

On 30/11/15 23:01, Stefan Berger wrote:
> From: Stefan Berger <stefanb at linux.vnet.ibm.com>
> 
> This patch provides an addtional menu that enables the user to control
> certain aspects of the TPM's state.
> 
> If a working TPM has been detected, the menu will look like this:
> 
> The TPM is enabled, active, does not have an owner but one can be installed.
> 
> To configure the TPM, choose one of the following actions:
> 
> d. Disable the TPM
> v. Deactivate the TPM
> p. Prevent installation of an owner
> 
> 
> Note: To fully use the TPM it must be enabled and activated.
> 
> Press escape to continue boot.
> 
> 
> This menu can be access by pressing the 't' key during boot. The menu will not
> be shown if no TPM is available.
> 
> Signed-off-by: Stefan Berger <stefanb at linux.vnet.ibm.com>
> ---
>  board-qemu/slof/OF.fs       |   3 +
>  board-qemu/slof/vtpm-sml.fs | 194 ++++++++++++++++++++++++++++++++++++++++++++
>  lib/libtpm/tcgbios.c        |  39 ++++++++-
>  lib/libtpm/tcgbios.h        |   9 ++
>  lib/libtpm/tpm.code         |  20 +++++
>  lib/libtpm/tpm.in           |   2 +
>  slof/fs/start-up.fs         |   9 ++
>  7 files changed, 273 insertions(+), 3 deletions(-)
...
> diff --git a/board-qemu/slof/vtpm-sml.fs b/board-qemu/slof/vtpm-sml.fs
> index 193b567..3e17c82 100644
> --- a/board-qemu/slof/vtpm-sml.fs
> +++ b/board-qemu/slof/vtpm-sml.fs
> @@ -120,6 +120,200 @@ log-base LOG-SIZE tpm-set-log-parameters
...
> +\ wait for keyboard input
> +: menu-key-get
> +    0 0 DO
> +       key? IF
> +           key
> +           UNLOOP EXIT
> +       THEN
> +       100 MS
> +    LOOP
> +    1b
> +;

Is that some kind of timeout that you've tried to do with the "100 MS"
and 1b at the end here? If so, that should have a proper comment. Then I
somewhat doubt that it is working as expected: Since you're looping with
"0 0" that means the loop is running almost forever - and since you're
delaying 256 ms inbetween (yes, we're hex), the loop will likely never
end while the user is sitting in front of the screen.
So if you really want to time-out here, I think you have to rework the
code. If you wanted to do an endless loop instead ... well then please
do not use "DO ... LOOP" and something like BEGIN ... WHILE ... REPEAT
or BEGIN ... UNTIL instead.

> +\ Send a code to the C-driver to change the state of the vTPM
> +: process-opcode ( verbose? opcode -- )
> +    tpm-process-opcode
> +    dup 0<> IF
> +        ." VTPM: Error code from tpm-process-opcode: " . cr
> +    ELSE
> +        drop
> +    THEN
> +;
> +
> +1  CONSTANT PPI_OP_ENABLE
> +2  CONSTANT PPI_OP_DISABLE
> +3  CONSTANT PPI_OP_ACTIVATE
> +4  CONSTANT PPI_OP_DEACTIVATE
> +5  CONSTANT PPI_OP_CLEAR
> +8  CONSTANT PPI_OP_SETOWNERINSTALL_TRUE
> +9  CONSTANT PPI_OP_SETOWNERINSTALL_FALSE
> +
> +\ if there's a vtpm available, display the menu
> +\ wait for keyboard input and have the C-driver
> +\ process opcodes we derive from the chosen menu
> +\ item
> +: vtpm-menu
> +    tpm-is-working IF
> +        \ vtpm-empty-keybuffer
> +        menu-show
> +        0 0  DO

Endless loop? Please use BEGIN ... REPEAT / UNTIL instead.

> +            CASE menu-key-get

I'd prefer "menu-key-get CASE" instead.

> +            [char] e OF  tpm-get-state                                  ( -- flags )
> +                         TPM_ST_ENABLED AND TPM_ST_ENABLED <> IF
> +                             0 PPI_OP_ENABLE     process-opcode
> +                             menu-show
> +                         THEN
> +                     ENDOF
> +            [char] d OF  tpm-get-state                                  ( -- flags )
> +                         TPM_ST_ENABLED AND TPM_ST_ENABLED = IF
> +                             0 PPI_OP_DISABLE    process-opcode
> +                             menu-show
> +                         THEN
> +                     ENDOF
> +            [char] a OF  tpm-get-state                                  ( -- flags )
> +                         TPM_ST_ACTIVE AND TPM_ST_ACTIVE <> IF
> +                             0 PPI_OP_ACTIVATE   process-opcode
> +                             tpm-get-state
> +                             TPM_ST_ACTIVE AND TPM_ST_ACTIVE = IF
> +                                 ." The system needs to reboot to activate the TPM."
> +                                 100 MS \ so the message shows
> +                                 reset-all
> +                             THEN
> +                         THEN
> +                     ENDOF
> +            [char] v OF  tpm-get-state                                  ( -- flags )
> +                         TPM_ST_ACTIVE AND TPM_ST_ACTIVE = IF
> +                             0 PPI_OP_DEACTIVATE process-opcode
> +                             menu-show
> +                         THEN
> +                     ENDOF
> +            [char] c OF  tpm-get-state                                  ( -- flags )
> +                         TPM_ST_OWNED AND TPM_ST_OWNED = IF
> +                             0 PPI_OP_CLEAR      process-opcode
> +                             menu-show
> +                         THEN
> +                     ENDOF
> +            [char] s OF  tpm-get-state
> +                         \ The TPM must be enabled and active to allow
> +                         \ owner installation mods
> +                         dup is-enabled-active? IF
> +                             TPM_ST_OWNERINSTALL AND TPM_ST_OWNERINSTALL <> IF
> +                                 0 PPI_OP_SETOWNERINSTALL_TRUE  process-opcode
> +                                 menu-show
> +                             THEN
> +                         THEN
> +                     ENDOF
> +            [char] p OF  tpm-get-state
> +                         \ The TPM must be enabled and active to allow
> +                         \ owner installation mods
> +                         dup is-enabled-active? IF
> +                             TPM_ST_OWNERINSTALL AND TPM_ST_OWNERINSTALL = IF
> +                                 0 PPI_OP_SETOWNERINSTALL_FALSE process-opcode
> +                                 menu-show
> +                             THEN
> +                         THEN
> +                     ENDOF
> +            1b OF UNLOOP EXIT ENDOF
> +            ENDCASE
> +        LOOP
> +    THEN
> +;
> +
>  : open  true ;
>  : close ;

 Thomas