Re: [PATCH] Fix Atmel TPM crash caused by too frequent queries

From: Hao Wu <hao.wu@rubrik.com>
To: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>,
	Nayna Jain <nayna@linux.vnet.ibm.com>,
	peterhuewe@gmx.de, jgg@ziepe.ca, arnd@arndb.de,
	gregkh@linuxfoundation.org, Hamza Attak <hamza@hpe.com>,
	why2jjj.linux@gmail.com, zohar@linux.vnet.ibm.com,
	linux-integrity@vger.kernel.org,
	Paul Menzel <pmenzel@molgen.mpg.de>,
	Ken Goldman <kgold@linux.ibm.com>,
	Seungyeop Han <seungyeop.han@rubrik.com>,
	Shrihari Kalkar <shrihari.kalkar@rubrik.com>,
	Anish Jhaveri <anish.jhaveri@rubrik.com>
Subject: Re: [PATCH] Fix Atmel TPM crash caused by too frequent queries
Date: Tue, 29 Sep 2020 20:41:27 -0700	[thread overview]
Message-ID: <5B6A7DBB-C06B-467E-9BA4-C0F9E241F5A6@rubrik.com> (raw)
In-Reply-To: <20200930021136.GD808399@linux.intel.com>

> On Sep 29, 2020, at 7:11 PM, Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote:
> 
> On Mon, Sep 28, 2020 at 01:27:14PM -0700, Hao Wu wrote:
>> 
>> 
>>> On Sep 28, 2020, at 12:47 PM, Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> wrote:
>>> 
>>> On Mon, Sep 28, 2020 at 10:49:56AM -0700, Hao Wu wrote:
>>>> Hi Jarkko,
>>>> 
>>>> https://github.com/torvalds/linux/commit/9f3fc7bcddcb51234e23494531f93ab60475e1c3 
>>>> Is the one introducing the issue since 4.14. Then the other three commits
>>>> changed the relevant code a bit. Probably you can check the timestamp / release version
>>>> on each commit to understand the relationship.
>>>> 
>>>> I think the original patch commit message can help you understand the root cause.
>>>> Attaching the commit here for your convenience.
>>>> 
>>>> Thanks
>>>> Hao
>>> 
>>> Please, again, when you respond quote properly instead of putting your
>>> response on top. Thank you.
>>> 
>>> Yes, I know the issue and it is already documented also in the James'
>>> earlier patch that did a similar change. I.e. for some reason some TPM's
>>> (or the bus itself) do not like poking it too often.
>> Yes, probably. Although the issue James’s patch fixes has the same error code,
>> it is about a different issue which is similar.
> 
> OK, great.
> 
>>> So: what if you revert on using msleep(TPM_TIMEOUT) in
>>> wait_for_tpm_stat(), i.e. revert to the behaviour before the
>>> aformentioned commit?
>> I believe that should resolve the issue as well
> 
> I'd return to the old code that works instead of doing something new
> along the lines. James?
> 
I would not use msleep back which is actually wrong way to do. 
We don’t know the actual time it sleeps on different system in the future.
Currently, my measurement over msleep(TPM_TIMEOUT) , i.e. msleep(5) 
sleeps 15ms. Maybe we should use tpm_msleep to precisely do the sleep.

I will test out James’ patch and your proposal this week and get you back anyway. 

> Anyway, thanks a lot for coming with this. I think we are making at
> least some progress sorting this out.
> 
> Also want to underline that my comments about quoting emails did not
> have anything to do that I would not appreciate this feedback. It is
> just a "protocol thing".
No worries. I am not familiar with the rules here. Thank you for corrections.

> 
>> Thanks
>> Hao
> 
> /Jarkko

Hao