Re: [tpmdd-devel] [PATCH] TPM: Issue TPM_STARTUP at driver load if the TPM has not been started

Re: [tpmdd-devel] [PATCH] TPM: Issue TPM_STARTUP at driver load if the TPM has not been started

[Jonathan McCune]

Hi Jason,

On Sun, Sep 30, 2012 at 7:30 PM, Jason Gunthorpe
<jgunthorpe@obsidianresearch.com> wrote:
> The TPM will respond to TPM_GET_CAP with TPM_ERR_INVALID_POSTINIT if
> TPM_STARTUP has not been issued. This will result in the TPM driver
> failing to load and no way to recover. Detect this and automatically
> issue TPM_STARTUP.
>
> This is for embedded applications where the kernel is the first thing
> to touch the TPM.

I welcome such functionality.  Thanks for your efforts.

Regards,
-Jon

RE: [tpmdd-devel] [PATCH] TPM: Issue TPM_STARTUP at driver load if the TPM has not been started

[Peter.Huewe]

>I welcome such functionality.  Thanks for your efforts.
Me too ;)

Peter Huewe

Infineon Technologies AG 
CCS TI SWT SW ESW
Tel: 	+49 821 25851-86
Fax: 	+49 821 25851-40

Peter.Huewe@infineon.com

****VISIT US AT: www.infineon.com *****
Infineon Technologies AG 
Vorsitzender des Aufsichtsrats: Wolfgang Mayrhuber 
Vorstand: Peter Bauer (Vorsitzender), Dominik Asam, Arunjai Mittal, Dr. Reinhard Ploss 
Sitz der Gesellschaft: Neubiberg 

Registergericht: München HRB 126492 

Thanks,
Peter

RE: [tpmdd-devel] [PATCH] TPM: Issue TPM_STARTUP at driver load if the TPM has not been started

[Peter.Huewe]

Hi Kent,
>  Peter, you mentioned you have some embedded setups, would you be able
> to test?

I could probably test it on the beagleboard c4 with our Infineon SLB9635 TT1.2 I2C TPM, 
but I'm currently not sure if I have the suspend/resume thing currently running correctly.

Apart from that I also have some 'ancient' x86 systems without TPM bios integration here 
(so the BIOS doesn't even know anything about a tpm), there I could test our LPC based tpms (SLB9635 / SLB9655).

Unfortunately it'll take some time, due to other priorities.
Please tell me when the patch is ready for testing.

Thanks,
Peter

Re: [tpmdd-devel] [PATCH] TPM: Issue TPM_STARTUP at driver load if the TPM has not been started

[Jason Gunthorpe]

On Thu, Oct 04, 2012 at 12:41:15PM -0500, Kent Yoder wrote:

>   I'd rather see us just track the state and do the right thing
> here. If we don't get invalid postinit if we call tpm_startup during
> tpm_tis_init/tpm_tis_i2c_init, then set a flag we switch on here.

At least on my platform it is possible for tpm_tis_init to skip the
TPM_STARTUP path I added if kexec has been used, so it is not a
reliable indicator that the platform does/does not support the TPM.

Directly checking if the TPM has been started on resume seems fool
proof to me?

Jasson

Re: [tpmdd-devel] [PATCH] TPM: Issue TPM_STARTUP at driver load if the TPM has not been started

[Kent Yoder]

> Curiously the current code does call TPM_SaveState on suspend, but
> relies on the BIOS to do TPM_Startup(ST_STATE) on resume, why the
> asymmetry?

  This is based on the PC Client Implementation for BIOS spec in the
TCG. On suspend, the OS is responsible for the save state and then on
resume the BIOS should call startup.

> Anyhow, I think the thing would be something like this. I have no
> means to test TPM suspend, so I'll just post this as a note here. It
> will apply over v2 of my patch.
> 
> diff --git a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c
> index b13ad77..7a8136a 100644
> --- a/drivers/char/tpm/tpm.c
> +++ b/drivers/char/tpm/tpm.c
> @@ -1336,10 +1336,23 @@ EXPORT_SYMBOL_GPL(tpm_pm_suspend);
>  int tpm_pm_resume(struct device *dev)
>  {
>         struct tpm_chip *chip = dev_get_drvdata(dev);
> +       struct tpm_cmd_t tpm_cmd;
> 
>         if (chip == NULL)
>                 return -ENODEV;
> 
> +       tpm_cmd.header.in = tpm_getcap_header;
> +       tpm_cmd.params.getcap_in.cap = TPM_CAP_PROP;
> +       tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
> +       tpm_cmd.params.getcap_in.subcap = TPM_CAP_PROP_TIS_TIMEOUT;
> +       rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, 0);
> +       if (rc == TPM_ERR_INVALID_POSTINIT) {
> +               /* The BIOS did not restart the TPM, execute a startup
> +                  command. */
> +               dev_info(chip->dev, "Issuing TPM_STARTUP");
> +               tpm_startup(chip, TPM_ST_STATE);
> +       }
> +

  I'd rather see us just track the state and do the right thing here. If
we don't get invalid postinit if we call tpm_startup during
tpm_tis_init/tpm_tis_i2c_init, then set a flag we switch on here.

  Peter, you mentioned you have some embedded setups, would you be able
to test?

Thanks,
Kent

>         return 0;
>  }
>  EXPORT_SYMBOL_GPL(tpm_pm_resume);
> 
> Jason
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: [tpmdd-devel] [PATCH] TPM: Issue TPM_STARTUP at driver load if the TPM has not been started

[Jason Gunthorpe]

On Mon, Oct 01, 2012 at 12:10:03PM -0500, Kent Yoder wrote:

> > I'm not familiar enough with how the power management flow works with
> > the TPM to do this. I don't think that can be the general case
> > because:
> > 
> > 3. If stType = TPM_ST_STATE
> >   a. If the TPM has no state to restore, the TPM MUST set the internal
> >      state such that it returns TPM_FAILEDSELFTEST to all subsequent
> >      commands.
> > 
> > So you need to know a save state exists in the TPM before attempting
> > the command?
> 
>  Presumably we'd have called TPM_SaveState on suspend. It might be
> possible to set a flag based on whether we needed to call startup at
> init time that tells the driver to call save/restore state during
> suspend/resume.

Curiously the current code does call TPM_SaveState on suspend, but
relies on the BIOS to do TPM_Startup(ST_STATE) on resume, why the
asymmetry?

Anyhow, I think the thing would be something like this. I have no
means to test TPM suspend, so I'll just post this as a note here. It
will apply over v2 of my patch.

diff --git a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c
index b13ad77..7a8136a 100644
--- a/drivers/char/tpm/tpm.c
+++ b/drivers/char/tpm/tpm.c
@@ -1336,10 +1336,23 @@ EXPORT_SYMBOL_GPL(tpm_pm_suspend);
 int tpm_pm_resume(struct device *dev)
 {
        struct tpm_chip *chip = dev_get_drvdata(dev);
+       struct tpm_cmd_t tpm_cmd;
 
        if (chip == NULL)
                return -ENODEV;
 
+       tpm_cmd.header.in = tpm_getcap_header;
+       tpm_cmd.params.getcap_in.cap = TPM_CAP_PROP;
+       tpm_cmd.params.getcap_in.subcap_size = cpu_to_be32(4);
+       tpm_cmd.params.getcap_in.subcap = TPM_CAP_PROP_TIS_TIMEOUT;
+       rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, 0);
+       if (rc == TPM_ERR_INVALID_POSTINIT) {
+               /* The BIOS did not restart the TPM, execute a startup
+                  command. */
+               dev_info(chip->dev, "Issuing TPM_STARTUP");
+               tpm_startup(chip, TPM_ST_STATE);
+       }
+
        return 0;
 }
 EXPORT_SYMBOL_GPL(tpm_pm_resume);

Jason

Re: [tpmdd-devel] [PATCH] TPM: Issue TPM_STARTUP at driver load if the TPM has not been started

[Kent Yoder]

On Mon, Oct 01, 2012 at 10:15:36AM -0600, Jason Gunthorpe wrote:
> On Mon, Oct 01, 2012 at 09:17:28AM +0000, Peter.Huewe@infineon.com wrote:
> > Hi Jason,
> > 
> > > The TPM will respond to TPM_GET_CAP with TPM_ERR_INVALID_POSTINIT if
> > > TPM_STARTUP has not been issued. This will result in the TPM driver
> > > failing to load and no way to recover. Detect this and automatically
> > > issue TPM_STARTUP.
> > 
> > > This is for embedded applications where the kernel is the first thing
> > > to touch the TPM.
> > 
> > Thanks for working on this.
> > I also thought about this scenario quite often.
> > 
> > Shouldn't we then also add a TpmStartup(ST_STATE) in case of a resume?
> >  rc=GetCapability()
> >  if(rc==INVALID_POSTINIT)
> >  	tpm_transmit ("TPM_STARTUP(ST_STATE)")...
> 
> I'm not familiar enough with how the power management flow works with
> the TPM to do this. I don't think that can be the general case
> because:
> 
> 3. If stType = TPM_ST_STATE
>   a. If the TPM has no state to restore, the TPM MUST set the internal
>      state such that it returns TPM_FAILEDSELFTEST to all subsequent
>      commands.
> 
> So you need to know a save state exists in the TPM before attempting
> the command?

 Presumably we'd have called TPM_SaveState on suspend. It might be
possible to set a flag based on whether we needed to call startup at
init time that tells the driver to call save/restore state during
suspend/resume.

Kent

> Would you agree that CLEAR is appropriate for an initial driver
> attach on probe?
> 
> Jason
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: [tpmdd-devel] [PATCH] TPM: Issue TPM_STARTUP at driver load if the TPM has not been started

[Jason Gunthorpe]

On Mon, Oct 01, 2012 at 09:17:28AM +0000, Peter.Huewe@infineon.com wrote:
> Hi Jason,
> 
> > The TPM will respond to TPM_GET_CAP with TPM_ERR_INVALID_POSTINIT if
> > TPM_STARTUP has not been issued. This will result in the TPM driver
> > failing to load and no way to recover. Detect this and automatically
> > issue TPM_STARTUP.
> 
> > This is for embedded applications where the kernel is the first thing
> > to touch the TPM.
> 
> Thanks for working on this.
> I also thought about this scenario quite often.
> 
> Shouldn't we then also add a TpmStartup(ST_STATE) in case of a resume?
>  rc=GetCapability()
>  if(rc==INVALID_POSTINIT)
>  	tpm_transmit ("TPM_STARTUP(ST_STATE)")...

I'm not familiar enough with how the power management flow works with
the TPM to do this. I don't think that can be the general case
because:

3. If stType = TPM_ST_STATE
  a. If the TPM has no state to restore, the TPM MUST set the internal
     state such that it returns TPM_FAILEDSELFTEST to all subsequent
     commands.

So you need to know a save state exists in the TPM before attempting
the command?

Would you agree that CLEAR is appropriate for an initial driver
attach on probe?

Jason

RE: [tpmdd-devel] [PATCH] TPM: Issue TPM_STARTUP at driver load if the TPM has not been started

[Peter.Huewe]

Hi Jason,

> The TPM will respond to TPM_GET_CAP with TPM_ERR_INVALID_POSTINIT if
> TPM_STARTUP has not been issued. This will result in the TPM driver
> failing to load and no way to recover. Detect this and automatically
> issue TPM_STARTUP.

> This is for embedded applications where the kernel is the first thing
> to touch the TPM.

Thanks for working on this.
I also thought about this scenario quite often.

Shouldn't we then also add a TpmStartup(ST_STATE) in case of a resume?
 rc=GetCapability()
 if(rc==INVALID_POSTINIT)
 	tpm_transmit ("TPM_STARTUP(ST_STATE)")...

Thanks,
Peter