From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751764AbcFUU7E (ORCPT ); Tue, 21 Jun 2016 16:59:04 -0400 Received: from mga01.intel.com ([192.55.52.88]:28749 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751536AbcFUU7B (ORCPT ); Tue, 21 Jun 2016 16:59:01 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.26,506,1459839600"; d="scan'208";a="1006938790" Date: Tue, 21 Jun 2016 23:55:53 +0300 From: Jarkko Sakkinen To: Ed Swierk Cc: tpmdd-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org, linux-security-module@vger.kernel.org, jgunthorpe@obsidianresearch.com, stefanb@us.ibm.com Subject: Re: [PATCH v7 5/5] tpm_tis: Increase ST19NP18 TPM command duration to avoid chip lockup Message-ID: <20160621205553.GD8218@intel.com> References: <1465610107-87762-1-git-send-email-eswierk@skyportsystems.com> <1466474042-110773-1-git-send-email-eswierk@skyportsystems.com> <1466474042-110773-6-git-send-email-eswierk@skyportsystems.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1466474042-110773-6-git-send-email-eswierk@skyportsystems.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Jun 20, 2016 at 06:54:02PM -0700, Ed Swierk wrote: > The STMicro ST19NP18-TPM sometimes takes much longer to execute > commands than it reports in its capabilities. For example, command 186 > (TPM_FlushSpecific) has been observed to take 14560 msec to complete, > far longer than the 3000 msec limit for "short" commands reported by > the chip. The behavior has also been seen with command 101 > (TPM_GetCapability). > > Worse, when the tpm_tis driver attempts to cancel the current command > (by writing commandReady = 1 to TPM_STS_x), the chip locks up > completely, returning all-1s from all memory-mapped register > reads. The lockup can be cleared only by resetting the system. > > The occurrence of this excessive command duration depends on the > sequence of commands preceding it. One sequence is creating at least 2 > new keys via TPM_CreateWrapKey, then letting the TPM idle for at least > 30 seconds, then loading a key via TPM_LoadKey2. The next > TPM_FlushSpecific occasionally takes tens of seconds to > complete. Another sequence is creating many keys in a row without > pause. The TPM_CreateWrapKey operation gets much slower after the > first few iterations, as one would expect when the pool of precomputed > keys is exhausted. Then after a 35-second pause, the same TPM_LoadKey2 > followed by TPM_FlushSpecific sequence triggers the behavior. > > Our working theory is that this older TPM sometimes pauses to > precompute keys, which modern chips implement as a background > process. Without access to the chip's implementation details it's > impossible to know whether any commands are immune to being blocked by > this process. So it seems safest to ignore the chip's reported command > durations, and use a value much higher than any observed duration, > like 180 sec (which is the duration this chip reports for "long" > commands). > > Signed-off-by: Ed Swierk I think this fine but I would like to hear other opinions on this. Stefan? /Jarkko > --- > drivers/char/tpm/tpm_tis.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/drivers/char/tpm/tpm_tis.c b/drivers/char/tpm/tpm_tis.c > index caf7278..8355b45 100644 > --- a/drivers/char/tpm/tpm_tis.c > +++ b/drivers/char/tpm/tpm_tis.c > @@ -485,6 +485,12 @@ static void tpm_tis_update_timeouts(struct tpm_chip *chip) > chip->vendor.timeout_d = msecs_to_jiffies(TIS_SHORT_TIMEOUT); > chip->vendor.timeout_adjusted = true; > break; > + case 0x0000104a: /* STMicro ST19NP18-TPM */ > + chip->vendor.duration[TPM_SHORT] = 180 * HZ; > + chip->vendor.duration[TPM_MEDIUM] = 180 * HZ; > + chip->vendor.duration[TPM_LONG] = 180 * HZ; > + chip->vendor.duration_adjusted = true; > + break; > } > } > > -- > 1.9.1 >