All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jarkko Sakkinen <jarkko@kernel.org>
To: Hao Wu <hao.wu@rubrik.com>
Cc: Shrihari Kalkar <shrihari.kalkar@rubrik.com>,
	Seungyeop Han <seungyeop.han@rubrik.com>,
	Anish Jhaveri <anish.jhaveri@rubrik.com>,
	peterhuewe@gmx.de, jgg@ziepe.ca, linux-integrity@vger.kernel.org,
	Paul Menzel <pmenzel@molgen.mpg.de>,
	Ken Goldman <kgold@linux.ibm.com>,
	zohar@linux.vnet.ibm.com, why2jjj.linux@gmail.com,
	Hamza Attak <hamza@hpe.com>,
	gregkh@linuxfoundation.org, arnd@arndb.de,
	Nayna <nayna@linux.vnet.ibm.com>,
	James.Bottomley@hansenpartnership.com
Subject: Re: [PATCH v2] tpm: fix ATMEL TPM crash caused by too frequent queries
Date: Thu, 8 Jul 2021 00:10:53 +0300	[thread overview]
Message-ID: <20210707211053.obfqcfq42cqlamns@kernel.org> (raw)
In-Reply-To: <712FD1C1-941C-4ABA-866D-6EA6AFA0FE9F@rubrik.com>

On Wed, Jul 07, 2021 at 11:28:35AM -0700, Hao Wu wrote:
> > On Jul 7, 2021, at 2:24 AM, Jarkko Sakkinen <jarkko@kernel.org> wrote:
> > 
> > On Tue, Jul 06, 2021 at 09:31:35PM -0700, Hao Wu wrote:
> >> Since kernel 4.14, there was a commit (9f3fc7bcddcb)

BTW, please remove this. You have a fixes tag.

> >> fixed the TPM sleep logic from msleep to usleep_range,
> >> so that the TPM sleeps exactly with TPM_TIMEOUT (=5ms) afterward.
> >> Before that fix, msleep(5) actually sleeps for around 15ms.
> > 
> > What is TPM sleep logic?
> It is about the commit metnioned in the description
> `tpm: replace msleep() with usleep_range() in TPM 1.2/2.0 generic drivers`
> https://github.com/torvalds/linux/commit/9f3fc7bcddcb51234e23494531f93ab60475e1c3

What you should do is to explain in simple terms the unwanted behaviour
that you are observing, and also, *when* you observe it. E.g. does this
happen when you use /dev/tpm0, or is it visible already in klog at boot
time. And also: does it occur for anything you put to /dev/tpm0, or is
the bug triggering for some particular TPM commands.

You also need to have information what kind of Atmel chip triggers the
bug. I'd presume that you have access to a machine with such chip?

When you get all that figured out, you should explain how you change
the existing behaviour, and why it makes sense. E.g. if you fixup
timeouts, please just tell how'd you end up choosing the values that
you picked.

E.g. the rationale for that could come from testing and finding the "sweet
spot", or perhaps the reason could be that old values worked, new ones
don't.

Especially in bug fixes the reasoning is *at least* as important as the
the code change itself because I need to know what is going on.

/Jarkko

  reply	other threads:[~2021-07-07 21:10 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-20 23:18 [PATCH] Fix Atmel TPM crash caused by too frequent queries Hao Wu
2021-06-23 13:35 ` Jarkko Sakkinen
2021-06-24  5:49   ` Hao Wu
2021-06-29 20:06     ` Jarkko Sakkinen
2021-06-30  4:27       ` Hao Wu
2021-06-24  5:33 ` Hao Wu
2021-06-29 20:07   ` Jarkko Sakkinen
2021-06-30  4:22   ` [PATCH] tpm: fix ATMEL " Hao Wu
2021-07-02  6:35     ` Jarkko Sakkinen
2021-07-02  7:12       ` Greg KH
2021-07-02  7:33       ` Hao Wu
2021-07-02  7:35         ` Hao Wu
2021-07-02  7:45         ` Jarkko Sakkinen
2021-07-02  7:59           ` Hao Wu
2021-07-02  8:42             ` Jarkko Sakkinen
2021-07-02 11:57               ` Jarkko Sakkinen
2021-07-02 19:16                 ` Hao Wu
2021-07-05  5:19                   ` Jarkko Sakkinen
2021-07-05  5:29                     ` Hao Wu
2021-07-04  0:07     ` Hao Wu
2021-07-05  7:15       ` Jarkko Sakkinen
2021-07-05 23:09         ` Hao Wu
2021-07-06 12:34           ` Mimi Zohar
2021-07-07  4:18             ` Hao Wu
2021-07-07  4:34               ` Hao Wu
2021-07-07  4:31     ` [PATCH v2] " Hao Wu
2021-07-07  9:24       ` Jarkko Sakkinen
2021-07-07 18:28         ` Hao Wu
2021-07-07 21:10           ` Jarkko Sakkinen [this message]
2021-07-09  4:43             ` Hao Wu
2021-07-09  4:40     ` [PATCH v2] tpm: fix Atmel " Hao Wu
2021-07-09 17:47       ` Jarkko Sakkinen
2021-07-09 19:23         ` Hao Wu
2021-07-11  7:37           ` Hao Wu
2021-07-16  5:30             ` Hao Wu
2021-07-11  7:51       ` [PATCH v3] " Hao Wu
2021-07-27  2:46         ` Jarkko Sakkinen
2021-07-27  3:40           ` Hao Wu
2021-08-14 22:25         ` [PATCH v4] " Hao Wu
2021-08-26  5:38           ` Hao Wu
2021-08-26 16:24             ` Jarkko Sakkinen
2021-08-27  0:35               ` Hao Wu
2021-09-04 21:14                 ` Hao Wu
2021-09-04 23:15                   ` Hao Wu
2021-09-05  3:51           ` [PATCH v5] " Hao Wu
2021-09-07 17:43             ` Jarkko Sakkinen
2021-09-08  8:33               ` Hao Wu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210707211053.obfqcfq42cqlamns@kernel.org \
    --to=jarkko@kernel.org \
    --cc=James.Bottomley@hansenpartnership.com \
    --cc=anish.jhaveri@rubrik.com \
    --cc=arnd@arndb.de \
    --cc=gregkh@linuxfoundation.org \
    --cc=hamza@hpe.com \
    --cc=hao.wu@rubrik.com \
    --cc=jgg@ziepe.ca \
    --cc=kgold@linux.ibm.com \
    --cc=linux-integrity@vger.kernel.org \
    --cc=nayna@linux.vnet.ibm.com \
    --cc=peterhuewe@gmx.de \
    --cc=pmenzel@molgen.mpg.de \
    --cc=seungyeop.han@rubrik.com \
    --cc=shrihari.kalkar@rubrik.com \
    --cc=why2jjj.linux@gmail.com \
    --cc=zohar@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.