Re: [Qemu-devel] [PATCH v3 3/4] acpi: build TPM Physical Presence interface

[Stefan Berger <stefanb@linux.vnet.ibm.com>]

On 06/20/2018 10:35 AM, Marc-André Lureau wrote:
> Hi
>
> On Wed, Jun 20, 2018 at 4:08 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> On Tue, May 15, 2018 at 02:14:32PM +0200, Marc-André Lureau wrote:
>>> From: Stefan Berger <stefanb@linux.vnet.ibm.com>
>>>
>>> The TPM Physical Presence interface consists of an ACPI part, a shared
>>> memory part, and code in the firmware. Users can send messages to the
>>> firmware by writing a code into the shared memory through invoking the
>>> ACPI code. When a reboot happens, the firmware looks for the code and
>>> acts on it by sending sequences of commands to the TPM.
>>>
>>> This patch adds the ACPI code. It is similar to the one in EDK2 but doesn't
>>> assume that SMIs are necessary to use. It uses a similar datastructure for
>>> the shared memory as EDK2 does so that EDK2 and SeaBIOS could both make use
>>> of it. I extended the shared memory data structure with an array of 256
>>> bytes, one for each code that could be implemented. The array contains
>>> flags describing the individual codes. This decouples the ACPI implementation
>>> from the firmware implementation.
>>>
>>> The underlying TCG specification is accessible from the following page.
>>>
>>> https://trustedcomputinggroup.org/tcg-physical-presence-interface-specification/
>>>
>>> This patch implements version 1.30.
>>>
>>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>>>
>>> ---
>>>
>>> v4 (Marc-André):
>>>   - replace 'DerefOf (FUNC [N])' with a function, to fix Windows ACPI
>>>      handling.
>>>   - replace 'return Package (..) {} ' with scoped variables, to fix
>>>     Windows ACPI handling.
>>>
>>> v3:
>>>   - add support for PPI to CRB
>>>   - split up OperationRegion TPPI into two parts, one containing
>>>     the registers (TPP1) and the other one the flags (TPP2); switched
>>>     the order of the flags versus registers in the code
>>>   - adapted ACPI code to small changes to the array of flags where
>>>     previous flag 0 was removed and now shifting right wasn't always
>>>     necessary anymore
>>>
>>> v2:
>>>   - get rid of FAIL variable; function 5 was using it and always
>>>     returns 0; the value is related to the ACPI function call not
>>>     a possible failure of the TPM function call.
>>>   - extend shared memory data structure with per-opcode entries
>>>     holding flags and use those flags to determine what to return
>>>     to caller
>>>   - implement interface version 1.3
>>> ---
>>>   include/hw/acpi/tpm.h |  21 +++
>>>   hw/i386/acpi-build.c  | 294 +++++++++++++++++++++++++++++++++++++++++-
>>>   2 files changed, 314 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
>>> index f79d68a77a..fc53f08827 100644
>>> --- a/include/hw/acpi/tpm.h
>>> +++ b/include/hw/acpi/tpm.h
>>> @@ -196,4 +196,25 @@ REG32(CRB_DATA_BUFFER, 0x80)
>>>   #define TPM_PPI_VERSION_NONE        0
>>>   #define TPM_PPI_VERSION_1_30        1
>>>
>>> +struct tpm_ppi {
>> The name violate the coding style.
> That's easy to change. Stefan could do it on commit if the rest of the
> patch is unchanged.

Call it TPMPPIData?