[PATCH 2/2] tpm: tpm_crb: enhance resource mapping mechanism for supporting AMD's fTPM

From: "Safford, David (GE Global Research, US)" <david.safford@ge.com>
To: Seunghun Han <kkamagui@gmail.com>
Cc: Jason Gunthorpe <jgg@ziepe.ca>,
	Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
	Peter Huewe <peterhuewe@gmx.de>,
	"open list:TPM DEVICE DRIVER" <linux-integrity@vger.kernel.org>,
	"Linux Kernel Mailing List" <linux-kernel@vger.kernel.org>
Subject: [PATCH 2/2] tpm: tpm_crb: enhance resource mapping mechanism for supporting AMD's fTPM
Date: Tue, 3 Sep 2019 12:26:00 +0000	[thread overview]
Message-ID: <BCA04D5D9A3B764C9B7405BBA4D4A3C035F1CF04@ALPMBAPA12.e2k.ad.ge.com> (raw)
In-Reply-To: <CAHjaAcRPg9-9MXiLH7AfJO6P1k25CSwJrSiuUwzFLwN5Ynr0DQ@mail.gmail.com>

> > > > First, you only force the remap if the overlap is with NVS, but I
> > > > have systems where the overlap is with other reserved regions. You
> > > > should force the remap regardless, but if it is NVS, grab the space back
> > > > from NVS.
> > >
> > > I didn't know about that. I just found the case from your thread
> > > that AMD system assigned TPM region into the reserved area. However,
> > > as I know, the reserved area has no busy bit so that TPM CRB driver
> > > could assign buffer resources in it. Right? In my view, if you
> > > patched your TPM driver with my patch series, then it could work.
> > > Would you explain what happened in TPM CRB driver and reserved area?
> >
> > Good question. I'll try it out this weekend.
> 
> Thank you for your help.

I tried your patch out on my systems with a "reserved" but not "NVS"
region conflict, and you are correct - the region is not busy, and
the driver is able to map the buffers with your patch.

> First of all, I misunderstood your message.
> I have to tell you about the buffer size exactly. The command and response
> buffer sizes in ACPI table were 0x1000 and this was 4K, not 1K. The sizes in
> the control register were 0x4000 and this was 16K (large buffer size), not 4K.
> I have been using the TPM for my research and the typical cases like creating
> public/private keys, encrypting/decrypting data, sealing/unsealing a secrete,
> and getting random numbers are not over 4K buffer. So, as you know, I think
> the 4K buffer can handle the most cases and the current implementation of
> crb_fixup_cmd_size() works well. If you concern the specific case that uses
> over 4K buffer, please let me know.

I have read postings of some systems where ACPI says 1K, but in all of my cases
that I can test,  you are correct that ACPI is saying 4K instead of the device's 16K.
I tried really hard, but couldn't send any valid requests over 4K, (I believe that's
actually the max by the spec), and therefore never saw any failures on my
systems. I think the driver behavior is wrong for those other cases, but perhaps
this should wait until someone can get access and do the testing.

So I'm happy with your patches, other than what is decided for the nvs driver
conflict. I'm testing them on some production systems, and have seen no other
issues.

dave