Re: [SeaBIOS] [PATCH v2 4/6] Rework the assertion of physical presence

[Stefan Berger]

"Kevin O'Connor"  wrote on 01/07/2016 11:21:02 AM:
> On Thu, Jan 07, 2016 at 07:55:39AM -0500, Stefan Berger wrote:
> > From: Stefan Berger
> >
> > Rework the assertion of physical presence by calling
>
> assert_physical_presence
> > in tpm_setup. This call will assert physical presence if SW assertion

is
> > possible or by checking whether HW physical presence is enabled.
> > The TPM menu will only be shown if physical presence is asserted or HW
> > physical presence is enabled after this call.
> > Signed-off-by: Stefan Berger
> >
> > src/boot.c    |  2 +-
> >  src/tcgbios.c | 33 +++++++++++++++++----------------
> >  src/tcgbios.h |  1 +
> >  3 files changed, 19 insertions(+), 17 deletions(-)
> >
> > diff --git a/src/boot.c b/src/boot.c
> > index a251eb4..27b85d5 100644
> > --- a/src/boot.c
> > +++ b/src/boot.c
> > @@ -499,7 +499,7 @@ interactive_bootmenu(void)
> >          scan_code = get_keystroke(1000);
> >          if (scan_code == 1 && !irqtimer_check(esc_accepted_time))
> >              continue;
> >    if (tpm_is_working() && scan_code == 20 /* t */) {
> >    if (tpm_can_show_menu() && scan_code == 20 /* t */) {
> >        printf("\n");
> >        tpm_menu();
> >    }
> >
> > diff --git a/src/tcgbios.c b/src/tcgbios.c
> > index 7a81d00..d14468e 100644
> > --- a/src/tcgbios.c
> > +++ b/src/tcgbios.c
> > @@ -60,6 +60,8 @@ struct {
> >      u8 *          log_area_last_entry;
> >  } tpm_state VARLOW;
> >
> > +static int TPM_has_physical_presence;
> >
> > static struct tcpa_descriptor_rev2 *
> >  find_tcpa_by_rsdp(struct rsdp_descriptor *rsdp)
> >  {
> > @@ -164,6 +166,12 @@ tpm_is_working(void)
> >      return CONFIG_TCGBIOS && TPM_working;
> >  }
> >
> > +int
> > +tpm_can_show_menu(void)
> > +{
> > return tpm_is_working() && TPM_has_physical_presence;
> >
> > +}
> >
> > /*
> > Send a TPM command with the given ordinal. Append the given buffer
> > containing all data in network byte order to the command (this is
> >
> > @@ -477,6 +485,11 @@ tpm_startup(void)
> >      if (ret)
> >          goto err_exit;
> > /* assertion of physical presence is only possible after startup

*/
> > ret = assert_physical_presence();
> > if (!ret)
> >    TPM_has_physical_presence = 1;
> > ret = determine_timeouts();
> >    if (ret)
> >        return -1;
> >
> > @@ -521,6 +534,10 @@ tpm_setup(void)
> >      if (ret)
> >          return;
> > ret = assert_physical_presence();
> > if (!ret)
> >    TPM_has_physical_presence = 1;
> > tpm_smbios_measure();
> >    tpm_add_action(2, "Start Option ROM Scan");
> >
> > }
>
> This calls assert_physical_presence() twice during setup.  I'm
> guessing the first was a copy-and-paste error and only the one in
> tpm_setup() is desired?

Right, copy-paste error. Only the first is necessary.
> [...]
> > --- a/src/tcgbios.h
> > +++ b/src/tcgbios.h
> > @@ -14,6 +14,7 @@ void tpm_add_cdrom(u32 bootdrv, const u8 *addr,
>
> u32 length);
> > void tpm_add_cdrom_catalog(const u8 *addr, u32 length);
> >  void tpm_option_rom(const void *addr, u32 len);
> >  int tpm_is_working(void);
> > +int tpm_can_show_menu(void);
>
> Now that tpm_is_working() is no longer used, it should be marked as
> static and not exported.

Good catch.

Stefan
> -Kevin