From patchwork Sat Oct 14 08:13:18 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jerry Snitselaar X-Patchwork-Id: 10006279 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 1A05A60230 for ; Sat, 14 Oct 2017 08:13:27 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 084E0290AB for ; Sat, 14 Oct 2017 08:13:27 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id F11A42918D; Sat, 14 Oct 2017 08:13:26 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id EBFF2290AB for ; Sat, 14 Oct 2017 08:13:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752049AbdJNINY (ORCPT ); Sat, 14 Oct 2017 04:13:24 -0400 Received: from mx1.redhat.com ([209.132.183.28]:49174 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750970AbdJNINT (ORCPT ); Sat, 14 Oct 2017 04:13:19 -0400 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 085D97EBD5; Sat, 14 Oct 2017 08:13:19 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 085D97EBD5 Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx03.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=jsnitsel@redhat.com Received: from localhost (ovpn-116-159.phx2.redhat.com [10.3.116.159]) by smtp.corp.redhat.com (Postfix) with ESMTP id CD9FE60176; Sat, 14 Oct 2017 08:13:18 +0000 (UTC) Date: Sat, 14 Oct 2017 01:13:18 -0700 From: Jerry Snitselaar To: Jarkko Sakkinen Cc: Laurent Bigonville , Alexander.Steffen@infineon.com, tpmdd-devel@lists.sourceforge.net, linux-integrity@vger.kernel.org Subject: Re: [tpmdd-devel] tpm device not showing up in /dev anymore Message-ID: <20171014081318.busge2fhteusfjwx@rhwork> Reply-To: Jerry Snitselaar References: <47c4300b-8701-79a6-1c58-3a5853f4c5e3@debian.org> <595efb25-8d87-f39d-037f-9c9a98462339@debian.org> <857106e4bb864bb8a68b1381fffc8f50@MUCSE603.infineon.com> <20170831164015.3ajgwydgxtippwoz@rhwork> <0d9be244-ace0-030d-6ff9-c4e94c63b7e9@debian.org> <20170906040555.fqedhmo5277sd6fq@linux.intel.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20170906040555.fqedhmo5277sd6fq@linux.intel.com> User-Agent: NeoMutt/20170914 (1.9.0) X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Sat, 14 Oct 2017 08:13:19 +0000 (UTC) Sender: linux-integrity-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-integrity@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 >> > > > 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 >> > > >      Cc: Jarkko Sakkinen >> > > >      Cc: Jason Gunthorpe >> > > >      Cc: Marcel Selhorst >> > > >      Signed-off-by: Jerry Snitselaar >> > > >      Reviewed-by: Jarkko Sakkinen >> > > >      Tested-by: Jarkko Sakkinen >> > > >      Signed-off-by: Jarkko Sakkinen >> > > > >> > > > :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; }