From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel De Graaf Subject: Re: [PATCH] vTPM: Fix Atmel timeout bug. Date: Thu, 06 Nov 2014 17:01:41 -0500 Message-ID: <545BEFC5.3010803@tycho.nsa.gov> References: <1414674330-2779-1-git-send-email-emilcondrea@gmail.com> <545235EE.4040000@citrix.com> <1415096148.11486.11.camel@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1415096148.11486.11.camel@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell , Emil Condrea Cc: Andrew Cooper , xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org On 11/04/2014 05:15 AM, Ian Campbell wrote: > On Thu, 2014-10-30 at 15:48 +0200, Emil Condrea wrote: >> Of course we can use max, but I thought that it might be useful to >> have a prink to inform the user that the timeout was adjusted. >> In init_tpm_tis the default timeouts are set using: >> /* Set default timeouts */ tpm->timeout_a = >> MILLISECS(TIS_SHORT_TIMEOUT);//750*1000000UL tpm->timeout_b = >> MILLISECS(TIS_LONG_TIMEOUT);//2000*1000000UL tpm->timeout_c = >> MILLISECS(TIS_SHORT_TIMEOUT); tpm->timeout_d = >> MILLISECS(TIS_SHORT_TIMEOUT); >> >> >> >> But in kernel fix they are set as 750*1000 instead of 750*1000000UL : >> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/char/tpm/tpm_tis.c#n381 >> So if we want to integrate kernel changes I think we should use >> MICROSECS(TIS_SHORT_TIMEOUT) which is 750000 >> Also in kernel the default timeouts are initialized using >> msecs_to_jiffies which is different from MILLISECS >> macro.: https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/char/tpm/tpm_tis.c#n548 >> Is there a certain reason for not using msecs_to_jiffies ? > > jiffies are a Linux specific concept which mini-os doesn't share. > > Daniel, do you have any opinion on this patch? > > It seems like the Linux fix is made only for the specifically broken > platform. That seems to make sense to me since presumably other systems > report short timeouts which they can indeed cope with. It's only Atmel > which brokenly reports something it cannot handle. > > Ian. I agree that an adjustment is needed when values are too short. Adjusting in all cases is not quite as nice as only fixing the broken TPMs, but it is a lot simpler. It also doesn't seem harmful to have the timeouts be too large in the driver: a properly functioning TPM will not time out its requests in any case, so the user won't notice normally, and the default short timeout is 0.75 seconds - very few people will complain if they have to wait that long to get a timeout instead of what their TPM actually uses. -- Daniel De Graaf National Security Agency