Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752506AbdHHVuS (ORCPT ); Tue, 8 Aug 2017 17:50:18 -0400 Received: from mga09.intel.com ([134.134.136.24]:12886 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752434AbdHHVuQ (ORCPT ); Tue, 8 Aug 2017 17:50:16 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.41,345,1498546800"; d="scan'208";a="1203495565" Date: Wed, 9 Aug 2017 00:50:10 +0300 From: Jarkko Sakkinen To: Nayna Cc: Peter Huewe , tpmdd-devel@lists.sourceforge.net, tpmdd@selhorst.net, jgunthorpe@obsidianresearch.com, linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org, linux-ima-devel@lists.sourceforge.net, patrickc@us.ibm.com Subject: Re: [PATCH] tpm: improve tpm_tis send() performance by ignoring burstcount Message-ID: <20170808214638.4cbws5uogtihhtzg@linux.intel.com> References: <20170807114632.1339-1-nayna@linux.vnet.ibm.com> <5988786D.3080301@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5988786D.3080301@linux.vnet.ibm.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo User-Agent: NeoMutt/20170609 (1.8.3) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2480 Lines: 68 On Mon, Aug 07, 2017 at 07:55:49PM +0530, Nayna wrote: > > > On 08/07/2017 05:22 PM, Peter Huewe wrote: > > > > > > Am 7. August 2017 13:46:32 MESZ schrieb Nayna Jain : > > > The TPM burstcount status indicates the number of bytes that can > > > be sent to the TPM without causing bus wait states. Effectively, > > > it is the number of empty bytes in the command FIFO. Further, > > > some TPMs have a static burstcount, when the value remains zero > > > until the entire FIFO is empty. > > > > > > This patch ignores burstcount, permitting wait states, and thus > > > writes the command as fast as the TPM can accept the bytes. > > > The performance of a 34 byte extend on a TPM 1.2 improved from > > > 52 msec to 11 msec. > > > > > > Suggested-by: Ken Goldman in > > > conjunction with the TPM Device Driver work group. > > > Signed-off-by: Nayna Jain > > > Acked-by: Mimi Zohar > > > > Are you sure this is a good idea? > > On lpc systems this more or less stalls the bus, including keyboard/mouse (if connected via superio lpc). > > Thanks Peter for quick response. > > I actually meant to post this patch as RFC. Sorry, missed that. > It was meant to be a starting place for the discussion related to > burst_count. > > > > > On which systems have you tested this? > > Spi/Lpc? Architecture? > > Tested it with LPC on x86. > > > > > This might not be noticable for small transfers, but think about much larger transfers.... > > I did the following testing: > > * Ran a script with 1000 extends. This was to test continuous extends > which are generally in large numbers when IMA is enabled. > > * Ran a command to ask TPM to hash big size file like 1MB. This was to > test the long command. > > In both of the above cases, I didn't face any tpm specific errors. > > Is there any test-script or test-cases which I can try to test the > scenario(stalling the bus, including keyboard/mouse) with the patch ? > > Thanks & Regards, > - Nayna My stand here is that if you want to such patch included there should be outstanding evidence that: - Burst count could be always safely ignored. - There's no hardware platform including TPM that assumes that driver takes the burst count into account. My main concern is stability and only if the stability is not at risk we can consider this. There's no test script to take care all of this. /Jarkko