Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190830095639.4562-1-kkamagui@gmail.com> <20190830095639.4562-3-kkamagui@gmail.com>
 <20190830124334.GA10004@ziepe.ca> <CAHjaAcQ0MrPCZUit7s0Rmqpwpp0w5jiYjNUNEEm2yc1AejZ3ng@mail.gmail.com>
 <BCA04D5D9A3B764C9B7405BBA4D4A3C035F1CC59@ALPMBAPA12.e2k.ad.ge.com>
 <CAHjaAcQu3jOSj0QV3u4GSgnhpkTmJTMqckY_cnuzeTY-HNUWcA@mail.gmail.com>
 <BCA04D5D9A3B764C9B7405BBA4D4A3C035F1CD06@ALPMBAPA12.e2k.ad.ge.com>
 <CAHjaAcRPg9-9MXiLH7AfJO6P1k25CSwJrSiuUwzFLwN5Ynr0DQ@mail.gmail.com> <3f3ce42707f09eded801ff8543be6aee6ef35cf8.camel@linux.intel.com>
In-Reply-To: <3f3ce42707f09eded801ff8543be6aee6ef35cf8.camel@linux.intel.com>
From:   Seunghun Han <kkamagui@gmail.com>
Date:   Wed, 4 Sep 2019 03:52:20 +0900
Message-ID: <CAHjaAcSftPfPNEHFco9Y609r+s=z8cy8Nnq-A368JgjQSEmJkg@mail.gmail.com>
Subject: Re: [PATCH 2/2] tpm: tpm_crb: enhance resource mapping mechanism for
 supporting AMD's fTPM
To:     Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>,
        Matthew Garrett <mjg59@srcf.ucam.org>,
        Matthew Garrett <mjg59@google.com>
Cc:     "Safford, David (GE Global Research, US)" <david.safford@ge.com>,
        Jason Gunthorpe <jgg@ziepe.ca>,
        Peter Huewe <peterhuewe@gmx.de>,
        "open list:TPM DEVICE DRIVER" <linux-integrity@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

>
> On Tue, 2019-09-03 at 18:56 +0900, Seunghun Han wrote:
> > Thank you for your notification. I am sorry. I missed it and
> > misunderstood Jarkko's idea. So, I would like to invite Matthew
> > Garrett to this thread and attach my opinion on that. The problem is
> > that command and response buffers are in ACPI NVS area. ACPI NVS area
> > is saved and restored by drivers/acpi/nvs.c during hibernation, so
> > command and response buffers in ACPI NVS are also handled by nvs.c
> > file. However, TPM CRB driver uses the buffers to control a TPM
> > device, therefore, something may break.
> >
> > I agree on that point. To remove uncertainty and find the solution,
> > I read the threads we discussed and did research about two points, 1)
> > the race condition and 2) the unexpected behavior of the TPM device.
> >
> > 1) The race condition concern comes from unknowing buffer access order
> > while hibernation.
> > If nvs.c and TPM CRB driver access the buffers concurrently, the race
> > condition occurs. Then, we can't know the contents of the buffers
> > deterministically, and it may occur the failure of TPM device.
> > However, hibernation_snapshot() function calls dpm_suspend() and
> > suspend_nvs_save() in order when the system enters into hibernation.
> > It also calls suspend_nvs_restore() and dpm_resume() in order when the
> > system exits from hibernation. So, no race condition occurs while
> > hibernation, and we always guarantee the contents of buffers as we
> > expect.
> >
> > 2) The unexpected behavior of the TPM device.
> > If nvs.c saves and restores the contents of the TPM CRB buffers while
> > hibernation, it may occur the unexpected behavior of the TPM device
> > because the buffers are used to control the TPM device. When the
> > system entered into hibernation, suspend_nvs_save() saved the command
> > and response buffers, and they had the last command and response data.
> > After exiting from hibernation, suspend_nvs_restore() restored the
> > last command and response data into the buffers and nothing happened.
> > I realized that they were just buffers. If we want to send a command
> > to the TPM device, we have to set the CRB_START_INVOKE bit to a
> > control_start register of a control area. The control area was not in
> > the ACPI NVS area, so it was not affected by nvs.c file. We can
> > guarantee the behavior of the TPM device.
> >
> > Because of these two reasons, I agreed on Jarkko's idea in
> > https://lkml.org/lkml/2019/8/29/962 . It seems that removing or
> > changing regions described in the ACPI table is not natural after
> > setup. In my view, saving and restoring buffers was OK like other NVS
> > areas were expected because the buffers were in ACPI NVS area.
> >
> > So, I made and sent this patch series. I would like to solve this
> > AMD's fTPM problem because I have been doing research on TPM and this
> > problem is critical for me (as you know fTPM doesn't work). If you
> > have any other concern or advice on the patch I made, please let me
> > know.
>
> Please take time to edit your responses. Nobody will read that properly
> because it is way too exhausting. A long prose only indicates unclear
> thoughts in the end. If you know what you are doing, you can put things
> into nutshell only in few senteces.
>
> /Jarkko
>

I'm sorry about that. I would like to invite Matthew Garrett and
discuss ACPI NVS and command/response buffer mapping again. So, I want
to summarize my test result and explain my opinion on that. I think
the data and result are important to make a decision clearly.
According to my test results, it seems that intersects between ACPI
NVS and command/response buffers will not make a problem.

Additionally, according to Dave's test results, this patch series can
cover not only an intersection with ACPI NVS area but also an
intersection with the reserved area. Here is the link,
https://lkml.org/lkml/2019/9/3/481 . Considering these results, my
patch series can solve AMD's fTPM problems.

Matthew,
what do you think about test results and this patch? In my view, if
the command/response buffers are in ACPI NVS area, saving and
restoring the buffers are ok and couldn't break anything.
I would like to get some feedback from you.

Seunghun