From: <Peter.Huewe@infineon.com>
To: <jgunthorpe@obsidianresearch.com>, <key@linux.vnet.ibm.com>
Cc: <linux-kernel@vger.kernel.org>, <tpmdd-devel@lists.sourceforge.net>
Subject: RE: [PATCH resend] TPM: Issue TPM_STARTUP at driver load if the TPM has not been started
Date: Wed, 21 Nov 2012 08:59:24 +0000 [thread overview]
Message-ID: <74A44E99E3274B4CB570415926B37D440F7675@MUCSE501.eu.infineon.com> (raw)
In-Reply-To: <20121121071015.GD19837@obsidianresearch.com>
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 <peter.huewe@infineon.com>
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 <peter.huewe@infineon.com>
Thanks,
Peter
next prev parent reply other threads:[~2012-11-21 8:59 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-09-30 23:30 [PATCH] TPM: Issue TPM_STARTUP at driver load if the TPM has not been started Jason Gunthorpe
2012-10-01 9:17 ` [tpmdd-devel] " Peter.Huewe
2012-10-01 16:15 ` Jason Gunthorpe
2012-10-01 17:10 ` Kent Yoder
2012-10-01 17:39 ` Jason Gunthorpe
2012-10-04 17:41 ` Kent Yoder
2012-10-04 18:02 ` Jason Gunthorpe
2012-10-08 7:09 ` Peter.Huewe
2012-11-21 7:10 ` [PATCH resend] " Jason Gunthorpe
2012-11-21 8:59 ` Peter.Huewe [this message]
2012-11-21 17:29 ` Jason Gunthorpe
2012-11-21 17:37 ` Peter.Huewe
2012-11-21 18:37 ` [PATCH v4] " Jason Gunthorpe
2012-11-21 20:17 ` Peter Hüwe
2012-11-21 20:12 ` Jason Gunthorpe
2012-11-21 20:54 ` [PATCH v5] " Jason Gunthorpe
2012-11-26 20:08 ` Kent Yoder
2012-10-01 15:14 ` [PATCH] " Kent Yoder
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=74A44E99E3274B4CB570415926B37D440F7675@MUCSE501.eu.infineon.com \
--to=peter.huewe@infineon.com \
--cc=jgunthorpe@obsidianresearch.com \
--cc=key@linux.vnet.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=tpmdd-devel@lists.sourceforge.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).