On Tue, Jan 22, 2019 at 10:46:24AM -0500, Stefan Berger wrote:
> Implement a TPM 2.0 menu item that allows a user to toggle the activation
> of PCR banks of the TPM 2.0. After successful activation we shut down the
> TPM 2.0 and reset the machine.
Thanks. In general, it looks fine to me. Just for my understanding,
could you give a high level overview of when/why a user would use this
menu and what functionality they would miss if this menu were not
available?
I also have a few comments - see below.
[...]
> --- a/src/tcgbios.c
> +++ b/src/tcgbios.c
[...]
> @@ -1780,6 +1926,13 @@ tpm20_process_cfg(tpm_ppi_code msgCode, int verbose)
> if (!ret)
> ret = tpm20_clear();
> break;
> +
> + case TPM_PPI_OP_SET_PCR_BANKS:
> + ret = tpm20_set_pcrbanks(pprm);
> + if (!ret)
> + ret = tpm_simple_cmd(0, TPM2_CC_Shutdown,
> + 2, TPM2_SU_CLEAR,
> TPM_DURATION_TYPE_SHORT);
> + break;
> }
>
> if (ret)
[...]
> static void
> tpm20_menu(void)
> {
> int scan_code;
> tpm_ppi_code msgCode;
> + u32 pprm;
> + u8 active_banks;
> + int do_reset = 0;
>
> for (;;) {
> printf("1. Clear TPM\n");
> + printf("2. Change active PCR banks\n");
>
> printf("\nIf no change is desired or if this menu was reached by "
> "mistake, press ESC to\n"
> @@ -1988,11 +2204,20 @@ tpm20_menu(void)
> case 2:
> msgCode = TPM_PPI_OP_CLEAR;
> break;
> + case 3:
> + if (tpm20_menu_change_active_pcrbanks(&active_banks) < 0)
> + continue;
> + msgCode = TPM_PPI_OP_SET_PCR_BANKS;
> + pprm = active_banks;
> + do_reset = 1;
> + break;
> default:
> continue;
> }
>
> - tpm20_process_cfg(msgCode, 0);
> + tpm20_process_cfg(msgCode, pprm, 0);
> + if (do_reset)
> + reset();
> }
FWIW, I find the above confusing. Could
tpm20_menu_change_active_pcrbanks() just directly invoke
tpm20_set_pcrbanks() et al when needed?
> --- a/src/util.h
> +++ b/src/util.h
> @@ -38,6 +38,8 @@ struct usbdevice_s;
> int bootprio_find_usb(struct usbdevice_s *usbdev, int lun);
> int get_keystroke(int msec);
>
> +#define SCANCODE_A 0x1e
The convention for util.h is to only have forward declarations for
variables and functions - no macros, typedefs, or struct definitions.
Also, you might want take a look at possibly using ascii_to_keycode().
-Kevin
_______________________________________________
SeaBIOS mailing list -- seabios@seabios.org
To unsubscribe send an email to seabios-le...@seabios.org
