Re: [PATCH v2 1/5] tpm_tis: Fix check_locality for correct locality acquisition

[Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>]

On Mon, Oct 05, 2020 at 12:00:57PM -0700, James Bottomley wrote:
> On Mon, 2020-10-05 at 18:34 +0300, Jarkko Sakkinen wrote:
> > On Thu, Oct 01, 2020 at 11:09:21AM -0700, James Bottomley wrote:
> > > The TPM TIS specification says the TPM signals the acquisition of
> > > locality when the TMP_ACCESS_REQUEST_USE bit goes to one *and* the
> > > TPM_ACCESS_REQUEST_USE bit goes to zero.  Currently we only check
> > > the
> > 
> > Put a reference to the section.
> > 
> > I'm *guessing* that the spec is
> > 
> > https://trustedcomputinggroup.org/resource/pc-client-work-group-pc-client-specific-tpm-interface-specification-tis
> > 
> > Please have this and also location in this spec.
> 
> I can, but the TCG reorganizes its website every few months, so no URLs
> like that are permanent.

OK, that's good enough excuse :-( Let's then ignore this comment.
Just would had save trouble in future if that wasn't the case.

> > > former not the latter, so check both.  Adding the check on
> > > TPM_ACCESS_REQUEST_USE should fix the case where the locality is
> > > re-requested before the TPM has released it.  In this case the
> > > locality may get released briefly before it is reacquired, which
> > > causes all sorts of problems. However, with the added check,
> > > TPM_ACCESS_REQUEST_USE should remain 1 until the second request for
> > > the locality is granted.
> > 
> > The description is really good and understandable otherwise.
> > 
> > For me it is not obvious at all, why this is missing a fixes
> > tag?
> 
> It's been there ever since the initial commit:
> 
> commit 27084efee0c3dc0eb15b5ed750aa9f1adb3983c3
> Author: Leendert van Doorn <leendert@watson.ibm.com>
> Date:   Sat Apr 22 02:38:03 2006 -0700
> 
>     [PATCH] tpm: driver for next generation TPM chips

Then just "Cc: stable@vger.kernel.org" should do.

> > > Signed-off-by: James Bottomley <
> > > James.Bottomley@HansenPartnership.com>
> > > 
> > > ---
> > > 
> > > v2: added this patch
> > 
> > Use the cover letter for the changelog. I'm afraid that I might
> > miss these otherwise.
> 
> Submitting patches actually recommends doing this ... I think we want
> to keep to standard kernel process, but I can gather them in the cover
> letter as well.

Most of the patch sets that I encounter have the cover letter in the
changelog and usually it is great for getting overall image what is
happening.

In section 14 of "submitting patches" there is a remark that the area
just after the diffstat marker is a good place to store this kind of
information. I have not find any explicit instruction for patch sets,
i.e. I just trust the "majority vote".

> James

/Jarkko