Re: [PATCH 05/13] x86: Add early TPM1.2/TPM2.0 interface support for Secure Launch

From: "Daniel P. Smith" <dpsmith@apertussolutions.com>
To: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
	Ross Philipson <ross.philipson@oracle.com>
Cc: linux-kernel@vger.kernel.org, x86@kernel.org,
	iommu@lists.linux-foundation.org,
	linux-integrity@vger.kernel.org, linux-doc@vger.kernel.org,
	tglx@linutronix.de, mingo@redhat.com, bp@alien8.de,
	hpa@zytor.com, luto@amacapital.net,
	trenchboot-devel@googlegroups.com
Subject: Re: [PATCH 05/13] x86: Add early TPM1.2/TPM2.0 interface support for Secure Launch
Date: Tue, 29 Sep 2020 19:47:52 -0400	[thread overview]
Message-ID: <bf1d8df9-ec79-2cc6-534f-ce1f0a58f123@apertussolutions.com> (raw)
In-Reply-To: <20200925054313.GB165011@linux.intel.com>

On 9/25/20 1:43 AM, Jarkko Sakkinen wrote:
> On Thu, Sep 24, 2020 at 10:58:33AM -0400, Ross Philipson wrote:
>> From: "Daniel P. Smith" <dpsmith@apertussolutions.com>
>>
>> This commit introduces an abstraction for TPM1.2 and TPM2.0 devices
>> above the TPM hardware interface.
>>
>> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
>> Signed-off-by: Ross Philipson <ross.philipson@oracle.com>
> 
> This is way, way too PoC. I wonder why there is no RFC tag.

An RFC was sent back in March and we incorporated the feedback we
received at that time.

> Please also read section 2 of
> 
> https://www.kernel.org/doc/html/v5.8/process/submitting-patches.html
> 
> You should leverage existing TPM code in a way or another. Refine it so
> that it scales for your purpose and then compile it into your thing
> (just include the necesary C-files with relative paths).

We explained during the RFC phase that we took a fair bit of time and a
very hard look to see if we could #include sections out the TPM driver
but as it is today none of the TPM driver's c files can be included
outside of the mainline kernel. If you look at the early boot stub for
the compressed kernel you will see that we are interacting with the TPM
as the first thing upon leaving the assembly world and entering C. Since
we weren't going to be able to get the mainline TPM driver plucked down,
we could either 1.) borrow an implementation from a colleague that
provides the minimum command strings hard coded in C macros to send
measurements to the TPM or 2.) reuse the TPM implementation we wrote for
TrenchBoot's AMD Secure Loader (LZ). The former is not well supported
and the latter will be getting maintenance under TB. While this is not
preferred, we had to weigh this versus trying to convince you and the
other TPM driver maintainers on a significant refactoring of the TPM
driver. It was elected for the reuse of a clean implementation that can
be replaced later if/when the TPM driver was refactored. When we
explained this during the RFC and it was not rejected, therefore we
carried it forward into this submission.

> How it is now is never going to fly.

We would gladly work with you and the other TPM maintainers on a
refactoring of the TPM driver to separate core logic into standalone
files that both the driver and the compressed kernel can share.

> /Jarkko
> 