On Wed Jan 17, 2024 at 8:44 AM UTC, Alexander Steffen wrote: > On 15.01.2024 02:15, Daniel P. Smith wrote: > > Commit 933bfc5ad213 introduced the use of a locality counter to control when > > locality request was actually sent to the TPM. This locality counter created a > > hard enforcement that the TPM had no active locality at the time of the driver > > initialization. The reality is that this may not always be the case coupled > > with the fact that the commit indiscriminately decremented the counter created > > the condition for integer underflow of the counter. The underflow was triggered > > by the first pair of request/relinquish calls made in tpm_tis_init_core and all > > subsequent calls to request/relinquished calls would have the counter flipping > > between the underflow value and 0. The result is that it appeared all calls to > > request/relinquish were successful, but they were not. The end result is that > > the locality that was active when the driver loaded would always remain active, > > to include after the driver shutdown. This creates a significant issue when > > using Intel TXT and Locality 2 is active at boot. After the GETSEC[SEXIT] > > instruction is called, the PCH will close access to Locality 2 MMIO address > > space, leaving the TPM locked in Locality 2 with no means to relinquish the > > locality until system reset. > > > > The commit seeks to address this situation through three changes. > > Could you split this up into multiple patches then, so that they can be > discussed separately? I have to agree with you ttly. Yeah also the text above is not exactly in the ballpark. I did not understand what I read. I had to read the code change instead to get an idea. A huge pile of text does not equal to stronger story. Like for any essay, scientific paper or a kernel message one should do also few edit rounds. The commit message is more important than the code change itself in bug fixes... There is trigger (TXT) and solution. A great commit message should have motivation and implementation parts and somewhat concise story where things lead to another. It should essentially make *any* reader who knows the basics of kernel code base convinced, not confused. This is at leat a good aim even tho sometimes unreachable. BR, Jarkko
