"Kevin O'Connor" <kevin@koconnor.net> 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 <stefanb@linux.vnet.ibm.com>
> >
> > 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 <stefanb@linux.vnet.ibm.com>
> > ---
> >  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
>