Re: [PATCH] x86: tpm: Remove a busy bit of the NVS area for supporting AMD's fTPM

From: Seunghun Han <kkamagui@gmail.com>
To: Matthew Garrett <mjg59@google.com>
Cc: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
	Matthew Garrett <mjg59@srcf.ucam.org>,
	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: Re: [PATCH] x86: tpm: Remove a busy bit of the NVS area for supporting AMD's fTPM
Date: Tue, 27 Aug 2019 17:23:33 +0900	[thread overview]
Message-ID: <CAHjaAcSFhQsDYL2iRwwhyvxh9mH4DhxZ__DNzhtk=iiZZ5JdbA@mail.gmail.com> (raw)
In-Reply-To: <CACdnJutomLNthYDzEc0wFBcBHK5iqnk0p-hkAkp57zQZ38oGPA@mail.gmail.com>

>
> On Mon, Aug 26, 2019 at 1:18 AM Seunghun Han <kkamagui@gmail.com> wrote:
> > To support AMD's fTPM, I removed the busy bit from the ACPI NVS area like
> > the reserved area so that AMD's fTPM regions could be assigned in it.
>
> drivers/acpi/nvs.c saves and restores the contents of NVS regions, and
> if other drivers use these regions without any awareness of this then
> things may break. I'm reluctant to say that just unilaterally marking
> these regions as available is a good thing, but it's clearly what's
> expected by AMD's implementation. One approach would be to have a
> callback into the nvs code to indicate that a certain region should be
> handed off to a driver, which would ensure that we can handle this on
> a case by case basis?

If the regions allocated in the NVS region need to be handled by a
driver, the callback mechanism is good for it. However, this case
doesn't need it because the regions allocated in NVS are just I/O
regions.

In my opinion, if the driver wants to handle the region in the NVS
while suspending or hibernating, it has to use register_pm_notifier()
function and handle the event. We already had the mechanism that could
ensure that the cases you worried about would be handled, so it seems
to me that removing the busy bit from the NVS region is fine.

Seunghun