diff mbox

[v8,1/5] tpm_tis: Improve reporting of IO errors

Message ID 1466557831-113440-2-git-send-email-eswierk@skyportsystems.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ed Swierk June 22, 2016, 1:10 a.m. UTC 
Mysterious TPM behavior can be difficult to track down through all the
layers of software. Add error messages for conditions that should
never happen. Also include the manufacturer ID along with other chip
data printed during init.

Signed-off-by: Ed Swierk <eswierk@skyportsystems.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
 drivers/char/tpm/tpm_tis.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Comments

Jason Gunthorpe June 24, 2016, 6:25 p.m. UTC | #1 
>  	expected = be32_to_cpu(*(__be32 *) (buf + 2));
>  	if (expected > count) {
> +		dev_err(chip->pdev, "Response too long (wanted %zd, got %d)\n",
> +			count, expected);

This all needs to be rebased on Jarkko's tree I guess, chip->pdev is
gone now.

http://git.infradead.org/users/jjs/linux-tpmdd.git/shortlog/refs/heads/master

Jarkko, did you miss a pull request for 4.7 or something? This is
4 month old stuff???

Jason

------------------------------------------------------------------------------
Attend Shape: An AT&T Tech Expo July 15-16. Meet us at AT&T Park in San
Francisco, CA to explore cutting-edge tech and listen to tech luminaries
present their vision of the future. This family event has something for
everyone, including kids. Get more information and register today.
http://sdm.link/attshape
Jarkko Sakkinen June 24, 2016, 8:21 p.m. UTC | #2 
Hi Jason,

On Fri, Jun 24, 2016 at 12:25:15PM -0600, Jason Gunthorpe wrote:
> >  	expected = be32_to_cpu(*(__be32 *) (buf + 2));
> >  	if (expected > count) {
> > +		dev_err(chip->pdev, "Response too long (wanted %zd, got %d)\n",
> > +			count, expected);
> 
> This all needs to be rebased on Jarkko's tree I guess, chip->pdev is
> gone now.
> 
> http://git.infradead.org/users/jjs/linux-tpmdd.git/shortlog/refs/heads/master
> 
> Jarkko, did you miss a pull request for 4.7 or something? This is
> 4 month old stuff???

Hmm... Do you mean by 4 month old stuff the stuff that is in mainline
and not in my master branch?

I'm not sure what happened with 4.7. I merged the changes for in about
4.6-rc5. There was one issue that I fixed that Stephen reported.

At the moment linux-next seems contain the stuff that I have in my
next.

> Jason

/Jarkko

------------------------------------------------------------------------------
Attend Shape: An AT&T Tech Expo July 15-16. Meet us at AT&T Park in San
Francisco, CA to explore cutting-edge tech and listen to tech luminaries
present their vision of the future. This family event has something for
everyone, including kids. Get more information and register today.
http://sdm.link/attshape
Jarkko Sakkinen June 24, 2016, 8:23 p.m. UTC | #3 
On Fri, Jun 24, 2016 at 11:21:31PM +0300, Jarkko Sakkinen wrote:
> Hi Jason,
> 
> On Fri, Jun 24, 2016 at 12:25:15PM -0600, Jason Gunthorpe wrote:
> > >  	expected = be32_to_cpu(*(__be32 *) (buf + 2));
> > >  	if (expected > count) {
> > > +		dev_err(chip->pdev, "Response too long (wanted %zd, got %d)\n",
> > > +			count, expected);
> > 
> > This all needs to be rebased on Jarkko's tree I guess, chip->pdev is
> > gone now.
> > 
> > http://git.infradead.org/users/jjs/linux-tpmdd.git/shortlog/refs/heads/master
> > 
> > Jarkko, did you miss a pull request for 4.7 or something? This is
> > 4 month old stuff???
> 
> Hmm... Do you mean by 4 month old stuff the stuff that is in mainline
> and not in my master branch?
> 
> I'm not sure what happened with 4.7. I merged the changes for in about
> 4.6-rc5. There was one issue that I fixed that Stephen reported.
> 
> At the moment linux-next seems contain the stuff that I have in my
> next.

Ed:

I was planning to applying these patches to my master next week and run
tests on them. If they do not apply it would be good if you could rebase
your series to apply to my master.

/Jarkko

------------------------------------------------------------------------------
Attend Shape: An AT&T Tech Expo July 15-16. Meet us at AT&T Park in San
Francisco, CA to explore cutting-edge tech and listen to tech luminaries
present their vision of the future. This family event has something for
everyone, including kids. Get more information and register today.
http://sdm.link/attshape
Jason Gunthorpe June 24, 2016, 8:26 p.m. UTC | #4 
On Fri, Jun 24, 2016 at 11:21:31PM +0300, Jarkko Sakkinen wrote:
> Hmm... Do you mean by 4 month old stuff the stuff that is in mainline
> and not in my master branch?

I mean the stuff that is in your branch but not in mainline.

$ git log --pretty=oneline jarkko/master ^v4.7-rc3 | wc -l
73

> I'm not sure what happened with 4.7. I merged the changes for in about
> 4.6-rc5. There was one issue that I fixed that Stephen reported.
> 
> At the moment linux-next seems contain the stuff that I have in my
> next.

linux-next is just pulling directly from your tree, you still have to
ensure that James gets and processes your pull request during the
merge window. If he dropped a pull request you should follow up and
ask why, if you never sent one then ... oops :)

Jason

------------------------------------------------------------------------------
Attend Shape: An AT&T Tech Expo July 15-16. Meet us at AT&T Park in San
Francisco, CA to explore cutting-edge tech and listen to tech luminaries
present their vision of the future. This family event has something for
everyone, including kids. Get more information and register today.
http://sdm.link/attshape
Jarkko Sakkinen June 25, 2016, 3:24 p.m. UTC | #5 
On Fri, Jun 24, 2016 at 02:26:15PM -0600, Jason Gunthorpe wrote:
> On Fri, Jun 24, 2016 at 11:21:31PM +0300, Jarkko Sakkinen wrote:
> > Hmm... Do you mean by 4 month old stuff the stuff that is in mainline
> > and not in my master branch?
> 
> I mean the stuff that is in your branch but not in mainline.
> 
> $ git log --pretty=oneline jarkko/master ^v4.7-rc3 | wc -l
> 73
> 
> > I'm not sure what happened with 4.7. I merged the changes for in about
> > 4.6-rc5. There was one issue that I fixed that Stephen reported.
> > 
> > At the moment linux-next seems contain the stuff that I have in my
> > next.
> 
> linux-next is just pulling directly from your tree, you still have to
> ensure that James gets and processes your pull request during the
> merge window. If he dropped a pull request you should follow up and
> ask why, if you never sent one then ... oops :)

For 4.6 I used pull request with a signed tag and everything went quite
well.

For 4.7 I did re-read the whole development process documentation but it
only speaks about pull requests and does not clearly state what you just
stated.

To summarize I screwed this one up but I guess the only big harm is that
vTPM support will skip to 4.8. I guess not big harm done?

My master is now rebased and this is what I get:

$ git log --oneline security/next...master | wc -l
67

I don't think that is too bad.

> Jason

/Jarkko

------------------------------------------------------------------------------
Attend Shape: An AT&T Tech Expo July 15-16. Meet us at AT&T Park in San
Francisco, CA to explore cutting-edge tech and listen to tech luminaries
present their vision of the future. This family event has something for
everyone, including kids. Get more information and register today.
http://sdm.link/attshape
Jarkko Sakkinen June 25, 2016, 3:47 p.m. UTC | #6 
On Sat, Jun 25, 2016 at 06:24:30PM +0300, Jarkko Sakkinen wrote:
> On Fri, Jun 24, 2016 at 02:26:15PM -0600, Jason Gunthorpe wrote:
> > On Fri, Jun 24, 2016 at 11:21:31PM +0300, Jarkko Sakkinen wrote:
> > > Hmm... Do you mean by 4 month old stuff the stuff that is in mainline
> > > and not in my master branch?
> > 
> > I mean the stuff that is in your branch but not in mainline.
> > 
> > $ git log --pretty=oneline jarkko/master ^v4.7-rc3 | wc -l
> > 73
> > 
> > > I'm not sure what happened with 4.7. I merged the changes for in about
> > > 4.6-rc5. There was one issue that I fixed that Stephen reported.
> > > 
> > > At the moment linux-next seems contain the stuff that I have in my
> > > next.
> > 
> > linux-next is just pulling directly from your tree, you still have to
> > ensure that James gets and processes your pull request during the
> > merge window. If he dropped a pull request you should follow up and
> > ask why, if you never sent one then ... oops :)
> 
> For 4.6 I used pull request with a signed tag and everything went quite
> well.
> 
> For 4.7 I did re-read the whole development process documentation but it
> only speaks about pull requests and does not clearly state what you just
> stated.
> 
> To summarize I screwed this one up but I guess the only big harm is that
> vTPM support will skip to 4.8. I guess not big harm done?
> 
> My master is now rebased and this is what I get:
> 
> $ git log --oneline security/next...master | wc -l
> 67
> 
> I don't think that is too bad.

My repositories are ready for next pull request. The master has been
rebased to James' tree and merged to next.

I won't add any new commits expect critical bug fixes for 4.8 release
content.

/Jarkko

------------------------------------------------------------------------------
Attend Shape: An AT&T Tech Expo July 15-16. Meet us at AT&T Park in San
Francisco, CA to explore cutting-edge tech and listen to tech luminaries
present their vision of the future. This family event has something for
everyone, including kids. Get more information and register today.
http://sdm.link/attshape
Jason Gunthorpe June 27, 2016, 5:55 p.m. UTC | #7 
On Sat, Jun 25, 2016 at 06:47:45PM +0300, Jarkko Sakkinen wrote:

> My repositories are ready for next pull request. The master has been
> rebased to James' tree and merged to next.

This seems fine..

Generally you shouldn't rebase to create pull requests, but this
seemed needed..

Organize your git tree so that it is always pullable and just use
merges/resets/etc in the -next branch

There is no reason to hold off on more stuff for 4.8, if another batch
is ready before the merge window then send it, James already took the
stuff you sent.

Jason

------------------------------------------------------------------------------
Attend Shape: An AT&T Tech Expo July 15-16. Meet us at AT&T Park in San
Francisco, CA to explore cutting-edge tech and listen to tech luminaries
present their vision of the future. This family event has something for
everyone, including kids. Get more information and register today.
http://sdm.link/attshape
diff mbox

Patch

diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c
index 65f7eec..088fa86 100644
--- a/drivers/char/tpm/tpm_tis.c
+++ b/drivers/char/tpm/tpm_tis.c
@@ -299,6 +299,8 @@  static int tpm_tis_recv(struct tpm_chip *chip, u8 *buf, size_t count)
 
 	expected = be32_to_cpu(*(__be32 *) (buf + 2));
 	if (expected > count) {
+		dev_err(chip->pdev, "Response too long (wanted %zd, got %d)\n",
+			count, expected);
 		size = -EIO;
 		goto out;
 	}
@@ -366,6 +368,8 @@  static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
 				  &chip->vendor.int_queue, false);
 		status = tpm_tis_status(chip);
 		if (!itpm && (status & TPM_STS_DATA_EXPECT) == 0) {
+			dev_err(chip->pdev, "Chip not accepting %zd bytes\n",
+				len - count);
 			rc = -EIO;
 			goto out_err;
 		}
@@ -378,6 +382,7 @@  static int tpm_tis_send_data(struct tpm_chip *chip, u8 *buf, size_t len)
 			  &chip->vendor.int_queue, false);
 	status = tpm_tis_status(chip);
 	if ((status & TPM_STS_DATA_EXPECT) != 0) {
+		dev_err(chip->pdev, "Chip not accepting last byte\n");
 		rc = -EIO;
 		goto out_err;
 	}
@@ -689,8 +694,9 @@  static int tpm_tis_init(struct device *dev, struct tpm_info *tpm_info,
 	vendor = ioread32(chip->vendor.iobase + TPM_DID_VID(0));
 	chip->vendor.manufacturer_id = vendor;
 
-	dev_info(dev, "%s TPM (device-id 0x%X, rev-id %d)\n",
+	dev_info(dev, "%s TPM (manufacturer-id 0x%X, device-id 0x%X, rev-id %d)\n",
 		 (chip->flags & TPM_CHIP_FLAG_TPM2) ? "2.0" : "1.2",
+		 chip->vendor.manufacturer_id,
 		 vendor >> 16, ioread8(chip->vendor.iobase + TPM_RID(0)));
 
 	if (!itpm) {