tpmdd-devel Archive on lore.kernel.org
 help / color / Atom feed
From: Laurent Bigonville <bigon-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
To: Jerry Snitselaar
	<jsnitsel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
	Jarkko Sakkinen
	<jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
Cc: linux-integrity-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org
Subject: Re: tpm device not showing up in /dev anymore
Date: Mon, 23 Oct 2017 15:48:30 +0200
Message-ID: <e3d4e1b7-d90a-3547-cdb3-1aaedab7b932@debian.org> (raw)
In-Reply-To: <20171023134515.56siz3m6lhrhnovv@rhwork>

Le 23/10/17 à 15:45, Jerry Snitselaar a écrit :
> On Mon Oct 23 17, Jarkko Sakkinen wrote:
>> On Sat, Oct 21, 2017 at 10:53:55AM +0200, Laurent Bigonville wrote:
>>> Le 14/10/17 à 10:13, Jerry Snitselaar a écrit :
>>> > On Wed Sep 06 17, Jarkko Sakkinen wrote:
>>> > > On Fri, Sep 01, 2017 at 02:10:18PM +0200, Laurent Bigonville wrote:
>>> > > > Le 31/08/17 à 18:40, Jerry Snitselaar a écrit :
>>> > > > > On Thu Aug 31 17, Alexander.Steffen@infineon.com wrote:
>>> > > > > > > Le 29/08/17 à 18:35, Laurent Bigonville a écrit :
>>> > > > > > > > Le 29/08/17 à 18:00, Alexander.Steffen@infineon.com a 
>>> écrit :
>>> > > > > > > >>> An idea how to troubleshoot this?
>>> > > > > > > >> Can you run git bisect on the changes between 4.11 and
>>> > > > 4.12, so that
>>> > > > > > > >> we find the offending commit? It is probably sufficient
>>> > > > to limit the
>>> > > > > > > >> search to commits that touch something in 
>>> drivers/char/tpm.
>>> > > > > > > >
>>> > > > > > > > I'll try and keep you posted.
>>> > > > > > >
>>> > > > > > > OK I've been able to bisect the problem and the bad 
>>> commit is:
>>> > > > > > >
>>> > > > > > > e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3 is the first 
>>> bad commit
>>> > > > > > > commit e6aef069b6e97790cb127d5eeb86ae9ff0b7b0e3
>>> > > > > > > Author: Jerry Snitselaar <jsnitsel@redhat.com>
>>> > > > > > > Date:   Mon Mar 27 08:46:04 2017 -0700
>>> > > > > > >
>>> > > > > > >      tpm_tis: convert to using locality callbacks
>>> > > > > > >
>>> > > > > > >      This patch converts tpm_tis to use of the new tpm 
>>> class ops
>>> > > > > > >      request_locality, and relinquish_locality.
>>> > > > > > >
>>> > > > > > >      With the move to using the callbacks, 
>>> release_locality is
>>> > > > > > > changed so
>>> > > > > > >      that we now release the locality even if there is no
>>> > > > > > > request pending.
>>> > > > > > >
>>> > > > > > >      This required some changes to the tpm_tis_core_init
>>> > > > code path to
>>> > > > > > >      make sure locality is requested when needed:
>>> > > > > > >
>>> > > > > > >        - tpm2_probe code path will end up calling
>>> > > > > > > request/release through
>>> > > > > > >          callbacks, so request_locality prior to
>>> > > > tpm2_probe not needed.
>>> > > > > > >
>>> > > > > > >        - probe_itpm makes calls to tpm_tis_send_data 
>>> which no
>>> > > > > > > longer calls
>>> > > > > > >          request_locality, so add request_locality prior to
>>> > > > > > > tpm_tis_send_data
>>> > > > > > >          calls. Also drop release_locality call in middleof
>>> > > > > > > probe_itpm, and
>>> > > > > > >          keep locality until release_locality called at 
>>> end of
>>> > > > > > > probe_itpm.
>>> > > > > > >
>>> > > > > > >      Cc: Peter Huewe <peterhuewe@gmx.de>
>>> > > > > > >      Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
>>> > > > > > >      Cc: Jason Gunthorpe <jgunthorpe@obsidianresearch.com>
>>> > > > > > >      Cc: Marcel Selhorst <tpmdd@selhorst.net>
>>> > > > > > >      Signed-off-by: Jerry Snitselaar <jsnitsel@redhat.com>
>>> > > > > > >      Reviewed-by: Jarkko Sakkinen
>>> > > > <jarkko.sakkinen@linux.intel.com>
>>> > > > > > >      Tested-by: Jarkko Sakkinen 
>>> <jarkko.sakkinen@linux.intel.com>
>>> > > > > > >      Signed-off-by: Jarkko Sakkinen
>>> > > > <jarkko.sakkinen@linux.intel.com>
>>> > > > > > >
>>> > > > > > > :040000 040000 70234365da69959d47076ebb40c8d17f520c3e44
>>> > > > > > > 72f21b446e45ea1003de75902b0553deb99157fd M drivers
>>> > > > > > >
>>> > > > > >
>>> > > > > > I've looked again at the code in question, but could not find
>>> > > > > > anything that is obviously wrong there. Locality is now
>>> > > > > > requested/released at slightly different points in the 
>>> process than
>>> > > > > > before, but that's it. It does not seem to cause problems 
>>> with the
>>> > > > > > majority of TPMs, since you are the first to report any, so
>>> > > > maybe it
>>> > > > > > is a quirk that only affects this device.
>>> > > > > >
>>> > > > > > Perhaps Jerry can help, since this is his change?
>>> > > > > >
>>> > > > > > Alexander
>>> > > > >
>>> > > > > Getting some caffeine in me, and starting to take a look. 
>>> Adding
>>> > > > > Jarkko as well since this might involve the general locality 
>>> changes.
>>> > > > >
>>> > > > > Laurent, if I send you a patch with some debugging code 
>>> added, would
>>> > > > > you be able to run it on that system? I wasn't running into 
>>> issues
>>> > > > > on the system I had with a 1.2 device, but I no longer have 
>>> access
>>> > > > > to it. I'll see if I can find one in our labs and reproduce 
>>> it there.
>>> > > >
>>> > > > Yes I should be able to do that
>>> > >
>>> > > Any findings?
>>> > >
>>> > > /Jarkko
>>> >
>>> > Okay, finally getting back to this. Looking at the code it isn't 
>>> clear
>>> > to me
>>> > why the change is causing this. So while I stare at this some more
>>> > Laurent
>>> > could you reproduce it with this patch so I can see what the 
>>> status and
>>> > access registers look like? Does anyone else on here happen to have a
>>> > Sinosun
>>> > tpm device? The systems I have access to with TPM1.2 devices don't 
>>> have
>>> > this
>>> > issue.
>>> >
>>> > --8<--
>>> >
>>> > diff --git a/drivers/char/tpm/tpm_tis_core.c
>>> > b/drivers/char/tpm/tpm_tis_core.c
>>> > index fdde971bc810..7d60a7e4b50a 100644
>>> > --- a/drivers/char/tpm/tpm_tis_core.c
>>> > +++ b/drivers/char/tpm/tpm_tis_core.c
>>> > @@ -258,6 +258,7 @@ static int tpm_tis_send_data(struct tpm_chip 
>>> *chip,
>>> > const u8 *buf, size_t len)
>>> >     int rc, status, burstcnt;
>>> >     size_t count = 0;
>>> >     bool itpm = priv->flags & TPM_TIS_ITPM_WORKAROUND;
>>> > +    u8 access;
>>> >
>>> >     status = tpm_tis_status(chip);
>>> >     if ((status & TPM_STS_COMMAND_READY) == 0) {
>>> > @@ -292,6 +293,11 @@ static int tpm_tis_send_data(struct tpm_chip 
>>> *chip,
>>> > const u8 *buf, size_t len)
>>> >         }
>>> >         status = tpm_tis_status(chip);
>>> >         if (!itpm && (status & TPM_STS_DATA_EXPECT) == 0) {
>>> > +            rc = tpm_tis_read8(priv, TPM_ACCESS(priv->locality),
>>> > &access);
>>> > +            if (rc < 0)
>>> > +                dev_info(&chip->dev, "TPM_STS_DATA_EXPECT == 0: read
>>> > failure TPM_ACCESS(%d)\n", priv->locality);
>>> > +            else
>>> > +                dev_info(&chip->dev, "TPM_STS_DATA_EXPECT == 0:
>>> > locality: %d status: %x access: %x\n", priv->locality, status, 
>>> access);
>>> >             rc = -EIO;
>>> >             goto out_err;
>>> >         }
>>> > @@ -309,6 +315,11 @@ static int tpm_tis_send_data(struct tpm_chip 
>>> *chip,
>>> > const u8 *buf, size_t len)
>>> >     }
>>> >     status = tpm_tis_status(chip);
>>> >     if (!itpm && (status & TPM_STS_DATA_EXPECT) != 0) {
>>> > +        rc = tpm_tis_read8(priv, TPM_ACCESS(priv->locality), 
>>> &access);
>>> > +        if (rc < 0)
>>> > +            dev_info(&chip->dev, "TPM_STS_DATA_EXPECT != 0: read
>>> > failure TPM_ACCESS(%d)\n", priv->locality);
>>> > +        else
>>> > +            dev_info(&chip->dev, "TPM_STS_DATA_EXPECT != 0: 
>>> locality:
>>> > %d status: %x access: %x\n", priv->locality, status, access);
>>> >         rc = -EIO;
>>> >         goto out_err;
>>> >     }
>>>
>>> Please find here the dmesg output of the patched kernel
>>
>> At least 0xff is corrupted value in senseful way. CPU fills the read
>> with ones for example for unaligned bus read. See table 19 in PC client
>> spec. This can happen when you do unaligned read for example.
>>
>> Maybe TPM is unreachable i.e. powered off. Bit busy with stuff ATM but
>> would probably make sense to compare that 0x81 to table 18 in the same
>> spec.
>>
>> /Jarkko
>
> 0x81 is saying the access register status is valid, and the locality
> is not active. That first bit means A Dynamic OS has not been previously
> established on the platform. Normally we would see 0xa1, which would
> mean valid register status, and the locality is active.

FTR, the ownership has been claimed using tpm-tools on linux, but I 
configured windows 10 (dual-boot) on the machine to do its magic to 
recognize the tpm chip by entering the owner password manually there, 
not sure if that matters.

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
tpmdd-devel mailing list
tpmdd-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/tpmdd-devel

  reply index

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-28 11:28 Laurent Bigonville
     [not found] ` <f9526f55-df96-64fc-a4d6-877ce04e7156-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
2017-08-29 16:00   ` Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w
     [not found]     ` <dcad0104c46d4d5f88e642862bdb42c2-nFblLGNE8XKJSz+rYg/bSJowlv4uC7bZ@public.gmane.org>
2017-08-29 16:35       ` Laurent Bigonville
     [not found]         ` <47c4300b-8701-79a6-1c58-3a5853f4c5e3-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
2017-08-29 17:39           ` Peter Huewe
2017-08-29 18:55           ` Laurent Bigonville
     [not found]             ` <595efb25-8d87-f39d-037f-9c9a98462339-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
2017-08-31 12:10               ` Alexander.Steffen-d0qZbvYSIPpWk0Htik3J/w
     [not found]                 ` <857106e4bb864bb8a68b1381fffc8f50-nFblLGNE8XKJSz+rYg/bSJowlv4uC7bZ@public.gmane.org>
2017-08-31 16:40                   ` Jerry Snitselaar
2017-09-01 12:10                     ` Laurent Bigonville
     [not found]                       ` <0d9be244-ace0-030d-6ff9-c4e94c63b7e9-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
2017-09-06  4:05                         ` Jarkko Sakkinen
     [not found]                           ` <20170906040555.fqedhmo5277sd6fq-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2017-10-14  8:13                             ` Jerry Snitselaar
2017-10-21  8:53                               ` Laurent Bigonville
     [not found]                                 ` <b63b765d-2477-d9fe-9d80-c2ea7e582bce-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
2017-10-23 13:23                                   ` Jarkko Sakkinen
     [not found]                                     ` <20171023132346.jbqgokwv3ah2oqjo-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2017-10-23 13:45                                       ` Jerry Snitselaar
2017-10-23 13:48                                         ` Laurent Bigonville [this message]
2017-10-24 13:51                                         ` Jarkko Sakkinen
     [not found]                                           ` <20171024135123.uqail7olnespun4k-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2017-10-24 14:57                                             ` Jerry Snitselaar
2017-10-24 16:07                                               ` Jarkko Sakkinen
     [not found]                                                 ` <20171024160725.r6kj452jdzpkbb6o-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
2017-11-09  0:04                                                   ` Laurent Bigonville
     [not found]                                                     ` <8f4df9a9-c8cd-832f-4c3f-5305fabab7a8-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
2017-11-09 19:58                                                       ` Laurent Bigonville
     [not found]                                                         ` <0a6e4771-f871-b3ca-b5b0-26dbd9efa8b1-8fiUuRrzOP0dnm+yROfE0A@public.gmane.org>
2017-11-09 23:50                                                           ` Jerry Snitselaar
     [not found]                                                             ` <20171109235005.pmklfw6zkjirahfa-bi+AKbBUZKY6gyzm1THtWbp2dZbC/Bob@public.gmane.org>
2017-11-10  2:19                                                               ` Jerry Snitselaar
2017-10-25  8:04                                             ` Laurent Bigonville
2017-09-06  5:09 Jerry Snitselaar

Reply instructions:

You may reply publically 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=e3d4e1b7-d90a-3547-cdb3-1aaedab7b932@debian.org \
    --to=bigon-8fiuurrzop0dnm+yrofe0a@public.gmane.org \
    --cc=jarkko.sakkinen-VuQAYsv1563Yd54FQh9/CA@public.gmane.org \
    --cc=jsnitsel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=linux-integrity-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org \
    /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

tpmdd-devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/tpmdd-devel/0 tpmdd-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 tpmdd-devel tpmdd-devel/ https://lore.kernel.org/tpmdd-devel \
		tpmdd-devel@lists.sourceforge.net
	public-inbox-index tpmdd-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/net.sourceforge.lists.tpmdd-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git