Re: [PATCH 2/4] ovmf: add QemuTpm.h header

[Laszlo Ersek <lersek@redhat.com>]

On 05/15/18 14:30, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Add some common macros and type definitions corresponding to the QEMU
> TPM interface.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  OvmfPkg/Include/IndustryStandard/QemuTpm.h | 67 ++++++++++++++++++++++
>  1 file changed, 67 insertions(+)
>  create mode 100644 OvmfPkg/Include/IndustryStandard/QemuTpm.h
> 
> diff --git a/OvmfPkg/Include/IndustryStandard/QemuTpm.h b/OvmfPkg/Include/IndustryStandard/QemuTpm.h
> new file mode 100644
> index 000000000000..054cf79374b5
> --- /dev/null
> +++ b/OvmfPkg/Include/IndustryStandard/QemuTpm.h
> @@ -0,0 +1,67 @@
> +/** @file
> +  Macro and type definitions corresponding to the QEMU TPM interface.
> +
> +  Refer to "docs/specs/tpm.txt" in the QEMU source directory.
> +
> +  Copyright (C) 2018, Red Hat, Inc.
> +  Copyright (c) 2018, IBM Corporation. All rights reserved.<BR>
> +
> +  This program and the accompanying materials are licensed and made available
> +  under the terms and conditions of the BSD License which accompanies this
> +  distribution. The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
> +  WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +**/
> +
> +#ifndef __QEMU_TPM_H__
> +#define __QEMU_TPM_H__
> +
> +#include <Base.h>
> +
> +/* whether function is blocked by BIOS settings; bits 0, 1, 2 */
> +#define QEMU_TPM_PPI_FUNC_NOT_IMPLEMENTED     (0 << 0)
> +#define QEMU_TPM_PPI_FUNC_BIOS_ONLY           (1 << 0)
> +#define QEMU_TPM_PPI_FUNC_BLOCKED             (2 << 0)
> +#define QEMU_TPM_PPI_FUNC_ALLOWED_USR_REQ     (3 << 0)
> +#define QEMU_TPM_PPI_FUNC_ALLOWED_USR_NOT_REQ (4 << 0)
> +#define QEMU_TPM_PPI_FUNC_MASK                (7 << 0)
> +
> +//
> +// The following structure is shared between firmware and ACPI.
> +//
> +#pragma pack (1)
> +typedef struct {
> +  UINT8 Func[256];           /* func */
> +  UINT8 In;                  /* ppin */
> +  UINT32 Ip;                 /* ppip */
> +  UINT32 Response;           /* pprp */
> +  UINT32 Request;            /* pprq */
> +  UINT32 RequestParameter;   /* pprm */
> +  UINT32 LastRequest;        /* lppr */
> +  UINT32 FRet;               /* fret */
> +  UINT8 Res1[0x40];          /* res1 */
> +  UINT8 NextStep;            /* next_step */
> +} QEMU_TPM_PPI;
> +#pragma pack ()
> +
> +//
> +// The following structure is for the fw_cfg etc/tpm/config file.
> +//
> +#pragma pack (1)
> +typedef struct {
> +  UINT32 PpiAddress;
> +  UINT8 TpmVersion;
> +  UINT8 PpiVersion;
> +} QEMU_FWCFG_TPM_CONFIG;
> +#pragma pack ()
> +
> +#define QEMU_TPM_VERSION_UNSPEC    0
> +#define QEMU_TPM_VERSION_1_2       1
> +#define QEMU_TPM_VERSION_2         2
> +
> +#define QEMU_TPM_PPI_VERSION_NONE  0
> +#define QEMU_TPM_PPI_VERSION_1_30  1
> +
> +#endif
> 

(1) Please update the subject line as discussed earlier; for example:

OvmfPkg/IndustryStandard: add QemuTpm.h header

(2) Please convert the file to CRLF.

(3) Please use the "// ..." comment style near the fields of QEMU_TPM_PPI.

(4) Please align the member identifiers in each of QEMU_TPM_PPI and
QEMU_FWCFG_TPM_CONFIG -- in practice this means inserting another space
char after each "UINT8" type name.

With those changes:

Acked-by: Laszlo Ersek <lersek@redhat.com>

Thanks!
Laszlo