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
On Mon, Oct 01, 2012 at 09:17:28AM +0000, [email protected] 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
On Mon, Oct 01, 2012 at 10:15:36AM -0600, Jason Gunthorpe wrote:
> On Mon, Oct 01, 2012 at 09:17:28AM +0000, [email protected] 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
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
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
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
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
The TPM will respond to TPM_GET_CAP with TPM_ERR_INVALID_POSTINIT if
TPM_STARTUP has not been issued. Detect this and automatically
issue TPM_STARTUP.
This is for embedded applications where the kernel is the first thing
to touch the TPM.
Signed-off-by: Jason Gunthorpe <[email protected]>
---
drivers/char/tpm/tpm.c | 43 +++++++++++++++++++++++++++++++++++++++----
drivers/char/tpm/tpm.h | 6 ++++++
2 files changed, 45 insertions(+), 4 deletions(-)
Discussion on this got a bit sidetracked talking about
suspend/resume.. To be clear, this fixes a real, serious, problem on
normal embedded cases where the kernel refuses to attach the TPM driver
at all.
Key, are you happy with this as-is for the next merge window?
This version is rebased and retested to 3.7-rc6. Tested on Atmel
and Nuvoton LPC TPMs on PPC32.
diff --git a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c
index 93211df..c576718 100644
--- a/drivers/char/tpm/tpm.c
+++ b/drivers/char/tpm/tpm.c
@@ -468,7 +468,7 @@ static ssize_t transmit_cmd(struct tpm_chip *chip, struct tpm_cmd_t *cmd,
return -EFAULT;
err = be32_to_cpu(cmd->header.out.return_code);
- if (err != 0)
+ if (err != 0 && desc)
dev_err(chip->dev, "A TPM error (%d) occurred %s\n", err, desc);
return err;
@@ -485,6 +485,16 @@ static const struct tpm_input_header tpm_getcap_header = {
.ordinal = TPM_ORD_GET_CAP
};
+#define TPM_ORD_STARTUP cpu_to_be32(153)
+#define TPM_ST_CLEAR cpu_to_be16(1)
+#define TPM_ST_STATE cpu_to_be16(2)
+#define TPM_ST_DEACTIVATED cpu_to_be16(3)
+static const struct tpm_input_header tpm_startup_header = {
+ .tag = TPM_TAG_RQU_COMMAND,
+ .length = cpu_to_be32(12),
+ .ordinal = TPM_ORD_STARTUP
+};
+
ssize_t tpm_getcap(struct device *dev, __be32 subcap_id, cap_t *cap,
const char *desc)
{
@@ -528,6 +538,15 @@ void tpm_gen_interrupt(struct tpm_chip *chip)
}
EXPORT_SYMBOL_GPL(tpm_gen_interrupt);
+static int tpm_startup(struct tpm_chip *chip, __be16 startup_type)
+{
+ struct tpm_cmd_t start_cmd;
+ start_cmd.header.in = tpm_startup_header;
+ start_cmd.params.startup_in.startup_type = startup_type;
+ return transmit_cmd(chip, &start_cmd, TPM_INTERNAL_RESULT_SIZE,
+ "attempting to start the TPM");
+}
+
int tpm_get_timeouts(struct tpm_chip *chip)
{
struct tpm_cmd_t tpm_cmd;
@@ -541,11 +560,27 @@ int tpm_get_timeouts(struct tpm_chip *chip)
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);
- rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
- "attempting to determine the timeouts");
- if (rc)
+ if (rc == TPM_ERR_INVALID_POSTINIT) {
+ /* The TPM is not started, we are the first to talk to it.
+ Execute a startup command. */
+ dev_info(chip->dev, "Issuing TPM_STARTUP");
+ if (tpm_startup(chip, TPM_ST_CLEAR))
+ return rc;
+
+ 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) {
+ dev_err(chip->dev,
+ "A TPM error (%d) occurred attempting to determine the timeouts\n",
+ rc);
goto duration;
+ }
if (be32_to_cpu(tpm_cmd.header.out.return_code) != 0 ||
be32_to_cpu(tpm_cmd.header.out.length)
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 8ef7649..8971b12 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -47,6 +47,7 @@ enum tpm_addr {
#define TPM_WARN_DOING_SELFTEST 0x802
#define TPM_ERR_DEACTIVATED 0x6
#define TPM_ERR_DISABLED 0x7
+#define TPM_ERR_INVALID_POSTINIT 38
#define TPM_HEADER_SIZE 10
extern ssize_t tpm_show_pubek(struct device *, struct device_attribute *attr,
@@ -291,6 +292,10 @@ struct tpm_getrandom_in {
__be32 num_bytes;
}__attribute__((packed));
+struct tpm_startup_in {
+ __be16 startup_type;
+} __packed;
+
typedef union {
struct tpm_getcap_params_out getcap_out;
struct tpm_readpubek_params_out readpubek_out;
@@ -301,6 +306,7 @@ typedef union {
struct tpm_pcrextend_in pcrextend_in;
struct tpm_getrandom_in getrandom_in;
struct tpm_getrandom_out getrandom_out;
+ struct tpm_startup_in startup_in;
} tpm_cmd_params;
struct tpm_cmd_t {
--
1.7.4.1
Hi Jason, Hi Kent,
>
>Discussion on this got a bit sidetracked talking about
>suspend/resume.. To be clear, this fixes a real, serious, problem on
>normal embedded cases where the kernel refuses to attach the TPM driver
>at all.
>
>Key, are you happy with this as-is for the next merge window?
>
>This version is rebased and retested to 3.7-rc6. Tested on Atmel
>and Nuvoton LPC TPMs on PPC32.
Sorry I was tied up at work with other stuff, so I couldn't try out your initial patch.
Thanks for resubmitting!
I just gave the new version a run on my beagleboard with our Infineon SLB9635 TT 1.2 Soft I2C TPM
and it seems to work as expected. (Tested with and without previous startup).
Tested-by: Peter Huewe <[email protected]>
I just have some minor comments - please see below.
>+++ b/drivers/char/tpm/tpm.c
>+#define TPM_ORD_STARTUP cpu_to_be32(153)
>+#define TPM_ST_CLEAR cpu_to_be16(1)
>+#define TPM_ST_STATE cpu_to_be16(2)
>+#define TPM_ST_DEACTIVATED cpu_to_be16(3)
>+static const struct tpm_input_header tpm_startup_header = {
>+ .tag = TPM_TAG_RQU_COMMAND,
>+ .length = cpu_to_be32(12),
>+ .ordinal = TPM_ORD_STARTUP
>+};
>+
> ssize_t tpm_getcap(struct device *dev, __be32 subcap_id, cap_t *cap,
> const char *desc)
> {
Purely cosmetic question, but why did you define this before the tpm_getcap and not tpm_startup?
All the other definitions are made before they are used - so this should perhaps better be moved directly before tpm_startup.
(Maybe we should move out these definitions to a common location? Header?)
>@@ -541,11 +560,27 @@ int tpm_get_timeouts(struct tpm_chip *chip)
> 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);
Please use NULL instead of 0, otherwise sparse complains - so
- rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, 0);
+ rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, NULL);
>+ if (rc == TPM_ERR_INVALID_POSTINIT) {
> ...
>+ rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, 0);
Same here
please use NULL instead of 0, otherwise sparse complains - so
- rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, 0);
+ rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE, NULL);
>diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> extern ssize_t tpm_show_pubek(struct device *, struct device_attribute *attr,
>@@ -291,6 +292,10 @@ struct tpm_getrandom_in {
> __be32 num_bytes;
> }__attribute__((packed));
>
>+struct tpm_startup_in {
>+ __be16 startup_type;
>+} __packed;
All the other user
__attribute__((packed));
Care to change to be consistent?
Apart from these three minor topics also a
Reviewed-by: Peter Huewe <[email protected]>
Thanks,
Peter
On Wed, Nov 21, 2012 at 08:59:24AM +0000, [email protected] wrote:
> I just gave the new version a run on my beagleboard with our Infineon SLB9635 TT 1.2 Soft I2C TPM
> and it seems to work as expected. (Tested with and without previous startup).
>
> Tested-by: Peter Huewe <[email protected]>
Great, I'll send an updated version, thanks!
> >+++ b/drivers/char/tpm/tpm.c
> >+#define TPM_ORD_STARTUP cpu_to_be32(153)
> >+#define TPM_ST_CLEAR cpu_to_be16(1)
> >+#define TPM_ST_STATE cpu_to_be16(2)
> >+#define TPM_ST_DEACTIVATED cpu_to_be16(3)
> >+static const struct tpm_input_header tpm_startup_header = {
> >+ .tag = TPM_TAG_RQU_COMMAND,
> >+ .length = cpu_to_be32(12),
> >+ .ordinal = TPM_ORD_STARTUP
> >+};
> >+
> > ssize_t tpm_getcap(struct device *dev, __be32 subcap_id, cap_t *cap,
> > const char *desc)
> > {
> Purely cosmetic question, but why did you define this before the
> tpm_getcap and not tpm_startup? All the other definitions are made
> before they are used - so this should perhaps better be moved
> directly before tpm_startup. (Maybe we should move out these
> definitions to a common location? Header?)
Hmm, When I first read through this I thought all these definitions were
being grouped together, easy to move.
> >+struct tpm_startup_in {
> >+ __be16 startup_type;
> >+} __packed;
>
>
> All the other user
> __attribute__((packed));
> Care to change to be consistent?
I used to __packed to avoid a checkpatch warning, they should probably
all be changed?
Jason
Hi Jason,
>Great, I'll send an updated version, thanks!
;) You're welcome.
>> >+struct tpm_startup_in {
>> >+ __be16 startup_type;
>> >+} __packed;
>>
>>
>> All the other user
>> __attribute__((packed));
>> Care to change to be consistent?
> I used to __packed to avoid a checkpatch warning, they should probably
> all be changed?
Oh you're right - then sorry about my complaint.
Then I'd prefer submitting it as __packed and write another patch which change the remaining ones.
Are you creating the patch to change or should I?
Peter
The TPM will respond to TPM_GET_CAP with TPM_ERR_INVALID_POSTINIT if
TPM_STARTUP has not been issued. Detect this and automatically
issue TPM_STARTUP.
This is for embedded applications where the kernel is the first thing
to touch the TPM.
Signed-off-by: Jason Gunthorpe <[email protected]>
Tested-by: Peter Huewe <[email protected]>
Reviewed-by: Peter Huewe <[email protected]>
---
drivers/char/tpm/tpm.c | 44 ++++++++++++++++++++++++++++++++++++++++----
drivers/char/tpm/tpm.h | 6 ++++++
2 files changed, 46 insertions(+), 4 deletions(-)
v4 changes:
- Use NULL
- Put tpm_startup_header together with tpm_startup
- Add Peter's -by lines.
diff --git a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c
index 93211df..98d550e 100644
--- a/drivers/char/tpm/tpm.c
+++ b/drivers/char/tpm/tpm.c
@@ -468,7 +468,7 @@ static ssize_t transmit_cmd(struct tpm_chip *chip, struct tpm_cmd_t *cmd,
return -EFAULT;
err = be32_to_cpu(cmd->header.out.return_code);
- if (err != 0)
+ if (err != 0 && desc)
dev_err(chip->dev, "A TPM error (%d) occurred %s\n", err, desc);
return err;
@@ -528,6 +528,25 @@ void tpm_gen_interrupt(struct tpm_chip *chip)
}
EXPORT_SYMBOL_GPL(tpm_gen_interrupt);
+#define TPM_ORD_STARTUP cpu_to_be32(153)
+#define TPM_ST_CLEAR cpu_to_be16(1)
+#define TPM_ST_STATE cpu_to_be16(2)
+#define TPM_ST_DEACTIVATED cpu_to_be16(3)
+static const struct tpm_input_header tpm_startup_header = {
+ .tag = TPM_TAG_RQU_COMMAND,
+ .length = cpu_to_be32(12),
+ .ordinal = TPM_ORD_STARTUP
+};
+
+static int tpm_startup(struct tpm_chip *chip, __be16 startup_type)
+{
+ struct tpm_cmd_t start_cmd;
+ start_cmd.header.in = tpm_startup_header;
+ start_cmd.params.startup_in.startup_type = startup_type;
+ return transmit_cmd(chip, &start_cmd, TPM_INTERNAL_RESULT_SIZE,
+ "attempting to start the TPM");
+}
+
int tpm_get_timeouts(struct tpm_chip *chip)
{
struct tpm_cmd_t tpm_cmd;
@@ -541,11 +560,28 @@ int tpm_get_timeouts(struct tpm_chip *chip)
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, NULL);
- rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
- "attempting to determine the timeouts");
- if (rc)
+ if (rc == TPM_ERR_INVALID_POSTINIT) {
+ /* The TPM is not started, we are the first to talk to it.
+ Execute a startup command. */
+ dev_info(chip->dev, "Issuing TPM_STARTUP");
+ if (tpm_startup(chip, TPM_ST_CLEAR))
+ return rc;
+
+ 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,
+ NULL);
+ }
+ if (rc) {
+ dev_err(chip->dev,
+ "A TPM error (%d) occurred attempting to determine the timeouts\n",
+ rc);
goto duration;
+ }
if (be32_to_cpu(tpm_cmd.header.out.return_code) != 0 ||
be32_to_cpu(tpm_cmd.header.out.length)
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 8ef7649..8971b12 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -47,6 +47,7 @@ enum tpm_addr {
#define TPM_WARN_DOING_SELFTEST 0x802
#define TPM_ERR_DEACTIVATED 0x6
#define TPM_ERR_DISABLED 0x7
+#define TPM_ERR_INVALID_POSTINIT 38
#define TPM_HEADER_SIZE 10
extern ssize_t tpm_show_pubek(struct device *, struct device_attribute *attr,
@@ -291,6 +292,10 @@ struct tpm_getrandom_in {
__be32 num_bytes;
}__attribute__((packed));
+struct tpm_startup_in {
+ __be16 startup_type;
+} __packed;
+
typedef union {
struct tpm_getcap_params_out getcap_out;
struct tpm_readpubek_params_out readpubek_out;
@@ -301,6 +306,7 @@ typedef union {
struct tpm_pcrextend_in pcrextend_in;
struct tpm_getrandom_in getrandom_in;
struct tpm_getrandom_out getrandom_out;
+ struct tpm_startup_in startup_in;
} tpm_cmd_params;
struct tpm_cmd_t {
--
1.7.5.4
Hi Jason,
Thanks for the updated patch!
Sorry, I have one really minor remark left:
> + if (rc) {
> + dev_err(chip->dev,
> + "A TPM error (%d) occurred attempting to determine the timeouts\n",
rc is a ssize_t here and when compiling with C=1 I get
drivers/char/tpm/tpm.c:582:4: warning: format '%d' expects argument of type 'int', but argument 3 has type 'ssize_t' [-Wformat]
Care to change to
> + "A TPM error (%zd) occurred attempting to determine the timeouts\n",
Sorry that I didn't spot it earlier.
Thanks,
Peter
On Wed, Nov 21, 2012 at 09:17:54PM +0100, Peter H?we wrote:
> Care to change to
> > + "A TPM error (%zd) occurred attempting to determine the timeouts\n",
>
> Sorry that I didn't spot it earlier.
Right.. Probably like this in my tree because of:
http://permalink.gmane.org/gmane.linux.kernel/1397887
I should really get sparse setup here... Never enough hours.
Jason
The TPM will respond to TPM_GET_CAP with TPM_ERR_INVALID_POSTINIT if
TPM_STARTUP has not been issued. Detect this and automatically
issue TPM_STARTUP.
This is for embedded applications where the kernel is the first thing
to touch the TPM.
Signed-off-by: Jason Gunthorpe <[email protected]>
Tested-by: Peter Huewe <[email protected]>
Reviewed-by: Peter Huewe <[email protected]>
---
drivers/char/tpm/tpm.c | 44 ++++++++++++++++++++++++++++++++++++++++----
drivers/char/tpm/tpm.h | 6 ++++++
2 files changed, 46 insertions(+), 4 deletions(-)
v5 changes:
- Use %zd for printing ssize_t
diff --git a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c
index 93211df..433ad6b 100644
--- a/drivers/char/tpm/tpm.c
+++ b/drivers/char/tpm/tpm.c
@@ -468,7 +468,7 @@ static ssize_t transmit_cmd(struct tpm_chip *chip, struct tpm_cmd_t *cmd,
return -EFAULT;
err = be32_to_cpu(cmd->header.out.return_code);
- if (err != 0)
+ if (err != 0 && desc)
dev_err(chip->dev, "A TPM error (%d) occurred %s\n", err, desc);
return err;
@@ -528,6 +528,25 @@ void tpm_gen_interrupt(struct tpm_chip *chip)
}
EXPORT_SYMBOL_GPL(tpm_gen_interrupt);
+#define TPM_ORD_STARTUP cpu_to_be32(153)
+#define TPM_ST_CLEAR cpu_to_be16(1)
+#define TPM_ST_STATE cpu_to_be16(2)
+#define TPM_ST_DEACTIVATED cpu_to_be16(3)
+static const struct tpm_input_header tpm_startup_header = {
+ .tag = TPM_TAG_RQU_COMMAND,
+ .length = cpu_to_be32(12),
+ .ordinal = TPM_ORD_STARTUP
+};
+
+static int tpm_startup(struct tpm_chip *chip, __be16 startup_type)
+{
+ struct tpm_cmd_t start_cmd;
+ start_cmd.header.in = tpm_startup_header;
+ start_cmd.params.startup_in.startup_type = startup_type;
+ return transmit_cmd(chip, &start_cmd, TPM_INTERNAL_RESULT_SIZE,
+ "attempting to start the TPM");
+}
+
int tpm_get_timeouts(struct tpm_chip *chip)
{
struct tpm_cmd_t tpm_cmd;
@@ -541,11 +560,28 @@ int tpm_get_timeouts(struct tpm_chip *chip)
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, NULL);
- rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
- "attempting to determine the timeouts");
- if (rc)
+ if (rc == TPM_ERR_INVALID_POSTINIT) {
+ /* The TPM is not started, we are the first to talk to it.
+ Execute a startup command. */
+ dev_info(chip->dev, "Issuing TPM_STARTUP");
+ if (tpm_startup(chip, TPM_ST_CLEAR))
+ return rc;
+
+ 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,
+ NULL);
+ }
+ if (rc) {
+ dev_err(chip->dev,
+ "A TPM error (%zd) occurred attempting to determine the timeouts\n",
+ rc);
goto duration;
+ }
if (be32_to_cpu(tpm_cmd.header.out.return_code) != 0 ||
be32_to_cpu(tpm_cmd.header.out.length)
diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
index 8ef7649..8971b12 100644
--- a/drivers/char/tpm/tpm.h
+++ b/drivers/char/tpm/tpm.h
@@ -47,6 +47,7 @@ enum tpm_addr {
#define TPM_WARN_DOING_SELFTEST 0x802
#define TPM_ERR_DEACTIVATED 0x6
#define TPM_ERR_DISABLED 0x7
+#define TPM_ERR_INVALID_POSTINIT 38
#define TPM_HEADER_SIZE 10
extern ssize_t tpm_show_pubek(struct device *, struct device_attribute *attr,
@@ -291,6 +292,10 @@ struct tpm_getrandom_in {
__be32 num_bytes;
}__attribute__((packed));
+struct tpm_startup_in {
+ __be16 startup_type;
+} __packed;
+
typedef union {
struct tpm_getcap_params_out getcap_out;
struct tpm_readpubek_params_out readpubek_out;
@@ -301,6 +306,7 @@ typedef union {
struct tpm_pcrextend_in pcrextend_in;
struct tpm_getrandom_in getrandom_in;
struct tpm_getrandom_out getrandom_out;
+ struct tpm_startup_in startup_in;
} tpm_cmd_params;
struct tpm_cmd_t {
--
1.7.5.4
On Wed, Nov 21, 2012 at 01:54:33PM -0700, Jason Gunthorpe wrote:
> The TPM will respond to TPM_GET_CAP with TPM_ERR_INVALID_POSTINIT if
> TPM_STARTUP has not been issued. Detect this and automatically
> issue TPM_STARTUP.
>
> This is for embedded applications where the kernel is the first thing
> to touch the TPM.
>
> Signed-off-by: Jason Gunthorpe <[email protected]>
> Tested-by: Peter Huewe <[email protected]>
> Reviewed-by: Peter Huewe <[email protected]>
> ---
> drivers/char/tpm/tpm.c | 44 ++++++++++++++++++++++++++++++++++++++++----
> drivers/char/tpm/tpm.h | 6 ++++++
> 2 files changed, 46 insertions(+), 4 deletions(-)
>
> v5 changes:
> - Use %zd for printing ssize_t
Applied!
Thanks,
Kent
>
> diff --git a/drivers/char/tpm/tpm.c b/drivers/char/tpm/tpm.c
> index 93211df..433ad6b 100644
> --- a/drivers/char/tpm/tpm.c
> +++ b/drivers/char/tpm/tpm.c
> @@ -468,7 +468,7 @@ static ssize_t transmit_cmd(struct tpm_chip *chip, struct tpm_cmd_t *cmd,
> return -EFAULT;
>
> err = be32_to_cpu(cmd->header.out.return_code);
> - if (err != 0)
> + if (err != 0 && desc)
> dev_err(chip->dev, "A TPM error (%d) occurred %s\n", err, desc);
>
> return err;
> @@ -528,6 +528,25 @@ void tpm_gen_interrupt(struct tpm_chip *chip)
> }
> EXPORT_SYMBOL_GPL(tpm_gen_interrupt);
>
> +#define TPM_ORD_STARTUP cpu_to_be32(153)
> +#define TPM_ST_CLEAR cpu_to_be16(1)
> +#define TPM_ST_STATE cpu_to_be16(2)
> +#define TPM_ST_DEACTIVATED cpu_to_be16(3)
> +static const struct tpm_input_header tpm_startup_header = {
> + .tag = TPM_TAG_RQU_COMMAND,
> + .length = cpu_to_be32(12),
> + .ordinal = TPM_ORD_STARTUP
> +};
> +
> +static int tpm_startup(struct tpm_chip *chip, __be16 startup_type)
> +{
> + struct tpm_cmd_t start_cmd;
> + start_cmd.header.in = tpm_startup_header;
> + start_cmd.params.startup_in.startup_type = startup_type;
> + return transmit_cmd(chip, &start_cmd, TPM_INTERNAL_RESULT_SIZE,
> + "attempting to start the TPM");
> +}
> +
> int tpm_get_timeouts(struct tpm_chip *chip)
> {
> struct tpm_cmd_t tpm_cmd;
> @@ -541,11 +560,28 @@ int tpm_get_timeouts(struct tpm_chip *chip)
> 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, NULL);
>
> - rc = transmit_cmd(chip, &tpm_cmd, TPM_INTERNAL_RESULT_SIZE,
> - "attempting to determine the timeouts");
> - if (rc)
> + if (rc == TPM_ERR_INVALID_POSTINIT) {
> + /* The TPM is not started, we are the first to talk to it.
> + Execute a startup command. */
> + dev_info(chip->dev, "Issuing TPM_STARTUP");
> + if (tpm_startup(chip, TPM_ST_CLEAR))
> + return rc;
> +
> + 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,
> + NULL);
> + }
> + if (rc) {
> + dev_err(chip->dev,
> + "A TPM error (%zd) occurred attempting to determine the timeouts\n",
> + rc);
> goto duration;
> + }
>
> if (be32_to_cpu(tpm_cmd.header.out.return_code) != 0 ||
> be32_to_cpu(tpm_cmd.header.out.length)
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 8ef7649..8971b12 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -47,6 +47,7 @@ enum tpm_addr {
> #define TPM_WARN_DOING_SELFTEST 0x802
> #define TPM_ERR_DEACTIVATED 0x6
> #define TPM_ERR_DISABLED 0x7
> +#define TPM_ERR_INVALID_POSTINIT 38
>
> #define TPM_HEADER_SIZE 10
> extern ssize_t tpm_show_pubek(struct device *, struct device_attribute *attr,
> @@ -291,6 +292,10 @@ struct tpm_getrandom_in {
> __be32 num_bytes;
> }__attribute__((packed));
>
> +struct tpm_startup_in {
> + __be16 startup_type;
> +} __packed;
> +
> typedef union {
> struct tpm_getcap_params_out getcap_out;
> struct tpm_readpubek_params_out readpubek_out;
> @@ -301,6 +306,7 @@ typedef union {
> struct tpm_pcrextend_in pcrextend_in;
> struct tpm_getrandom_in getrandom_in;
> struct tpm_getrandom_out getrandom_out;
> + struct tpm_startup_in startup_in;
> } tpm_cmd_params;
>
> struct tpm_cmd_t {
> --
> 1.7.5.4
>