Re: [PATCH] tpm: tpm_crb: Add an AMD fTPM support feature

From: Seunghun Han <kkamagui@gmail.com>
To: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Cc: Peter Huewe <peterhuewe@gmx.de>,
	Thomas Gleixner <tglx@linutronix.de>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	linux-integrity@vger.kernel.org,
	Matthew Garrett <mjg59@google.com>
Subject: Re: [PATCH] tpm: tpm_crb: Add an AMD fTPM support feature
Date: Mon, 26 Aug 2019 18:12:47 +0900	[thread overview]
Message-ID: <CAHjaAcQ5ArtrGzKsPRasFfTmdE59cYWLVbOT4pMHhgJZ_fXUcQ@mail.gmail.com> (raw)
In-Reply-To: <20190826055903.5um5pfweoszibem3@linux.intel.com>

>
> On Mon, Aug 26, 2019 at 02:40:19AM +0900, Seunghun Han wrote:
> > I'm Seunghun Han and work at the Affiliated Institute of ETRI. I got an AMD
> > system which had a Ryzen Threadripper 1950X and MSI mainboard, and I had
> > a problem with AMD's fTPM. My machine showed an error message below, and
> > the fTPM didn't work because of it.
> >
> > [    5.732084] tpm_crb MSFT0101:00: can't request region for resource
> >                [mem 0x79b4f000-0x79b4ffff]
> > [    5.732089] tpm_crb: probe of MSFT0101:00 failed with error -16
> >
> > When I saw the iomem areas and found two TPM CRB regions were in the ACPI
> > NVS area.  The iomem regions are below.
> >
> > 79a39000-79b6afff : ACPI Non-volatile Storage
> >   79b4b000-79b4bfff : MSFT0101:00
> >   79b4f000-79b4ffff : MSFT0101:00
> >
> > After analyzing this issue, I found out that a busy bit was set to the ACPI
> > NVS area, and the current Linux kernel allowed nothing to be assigned in
> > it. I also found that the kernel couldn't calculate the sizes of command
> > and response buffers correctly when the TPM regions were two or more.
> >
> > To support AMD's fTPM, I removed the busy bit from the ACPI NVS area
> > so that AMD's fTPM regions could be assigned in it. I also fixed the bug
> > that did not calculate the sizes of command and response buffer correctly.
> >
> > Signed-off-by: Seunghun Han <kkamagui@gmail.com>
>
> You need to split this into multiple patches e.g. if you think you've
> fixed a bug, please write a patch with just the bug fix and nothing
> else.
>
> For further information, read the section three of
>
> https://www.kernel.org/doc/html/latest/process/submitting-patches.html
>
> I'd also recommend to check out the earlier discussion on ACPI NVS:
>
> https://lore.kernel.org/linux-integrity/BCA04D5D9A3B764C9B7405BBA4D4A3C035EF7BC7@ALPMBAPA12.e2k.ad.ge.com/
>
> /Jarkko

Thank you for your advice. I have made two separated patches on your advice.
Please check these patches, https://lkml.org/lkml/2019/8/26/125 and
https://lkml.org/lkml/2019/8/26/163.

In my opinion, the last link you gave me had two problems with AMD's
fTPM. One problem was the ACPI NVS area was set to the busy area, and
TPM regions of the ACPI table were in it. Therefore, TPM CRB driver
couldn't allocate command and response buffers in it because ACPI NVS
area was busy. The other problem was buffer size calculation bugs.
Because of it, TPM CRB driver requested larger than the size ACPI
table described. So, TPM CRB driver also couldn't map command and
response buffers even though the reserved area was not busy.

It seems that my separated patches could handle those two problems and
enable AMD's fTPM in any case.

Seunghun