[Qemu-devel] [PATCH v6 0/4] Add support for TPM Physical Presence interface

[Qemu-devel] [PATCH v6 0/4] Add support for TPM Physical Presence interface

[Marc-André Lureau]

Hi,

The following patches implement the TPM Physical Presence Interface
that allows a user to set a command via ACPI (sysfs entry in Linux)
that, upon the next reboot, the firmware looks for and acts upon by
sending sequences of commands to the TPM.

A dedicated memory region is added to the TPM CRB & TIS devices, at
address/size 0xFED45000/0x400. A new "etc/tpm/config" fw_cfg entry
holds the location for that PPI region and some version details, to
allow for future flexibility.

With the associated edk2/ovmf firmware, the Windows HLK "PPI 1.3" test
now runs successfully.

It is based on previous work from Stefan Berger ("[PATCH v2 0/4]
Implement Physical Presence interface for TPM 1.2 and 2")

The edk2 support is merged upstream.

v6: (see changeset in patches)
 - replace mmio region with ram
 - ASL code improvements
 - endianess fix on 8bit fw-cfg fields

v5:
 - more code documentation (Marc-André)
 - use some explicit named variables to ease reading (Marc-André)
 - use fixed size fields/memory regions, remove PPI struct (Marc-André)
 - only add PPI ACPI methods if PPI is enabled (Marc-André)
 - document the qemu/firmware ACPI memory region (Stefan)
 - remove the dummy ACPI memory clear interface patch

v4:
 - add a "ppi" property, default to true, unless machine <= 2.12
 - pass PPI address to tpm_ppi_init_io()
 - renamed tpm_ppi struct name

Marc-André Lureau (1):
  tpm: add a "ppi" boolean property

Stefan Berger (3):
  tpm: implement virtual memory device for TPM PPI
  acpi: add fw_cfg file for TPM and PPI virtual memory device
  acpi: build TPM Physical Presence interface

 hw/tpm/tpm_ppi.h      |  25 +++
 include/hw/acpi/tpm.h |  17 ++
 include/hw/compat.h   |  10 +
 hw/i386/acpi-build.c  | 420 +++++++++++++++++++++++++++++++++++++++++-
 hw/tpm/tpm_crb.c      |  11 ++
 hw/tpm/tpm_ppi.c      |  33 ++++
 hw/tpm/tpm_tis.c      |  11 ++
 docs/specs/tpm.txt    |  99 ++++++++++
 hw/tpm/Makefile.objs  |   2 +-
 hw/tpm/trace-events   |   4 +
 10 files changed, 629 insertions(+), 3 deletions(-)
 create mode 100644 hw/tpm/tpm_ppi.h
 create mode 100644 hw/tpm/tpm_ppi.c

-- 
2.18.0.rc1

[Qemu-devel] [PATCH v6 1/4] tpm: add a "ppi" boolean property

[Marc-André Lureau]

The following patches implement the TPM Physical Presence Interface,
make use of a new memory region and a fw_cfg entry. Enable PPI by
default with >2.12 machine type, to avoid migration issues.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
 include/hw/compat.h | 10 ++++++++++
 hw/tpm/tpm_crb.c    |  3 +++
 hw/tpm/tpm_tis.c    |  3 +++
 3 files changed, 16 insertions(+)

diff --git a/include/hw/compat.h b/include/hw/compat.h
index 44d5964060..01758991a0 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -2,6 +2,16 @@
 #define HW_COMPAT_H
 
 #define HW_COMPAT_2_12 \
+    {\
+        .driver   = "tpm-crb",\
+        .property = "ppi",\
+        .value    = "false",\
+    },\
+    {\
+        .driver   = "tpm-tis",\
+        .property = "ppi",\
+        .value    = "false",\
+    },\
     {\
         .driver   = "migration",\
         .property = "decompress-error-check",\
diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
index a92dd50437..d5b0ac5920 100644
--- a/hw/tpm/tpm_crb.c
+++ b/hw/tpm/tpm_crb.c
@@ -41,6 +41,8 @@ typedef struct CRBState {
     MemoryRegion cmdmem;
 
     size_t be_buffer_size;
+
+    bool ppi_enabled;
 } CRBState;
 
 #define CRB(obj) OBJECT_CHECK(CRBState, (obj), TYPE_TPM_CRB)
@@ -221,6 +223,7 @@ static const VMStateDescription vmstate_tpm_crb = {
 
 static Property tpm_crb_properties[] = {
     DEFINE_PROP_TPMBE("tpmdev", CRBState, tpmbe),
+    DEFINE_PROP_BOOL("ppi", CRBState, ppi_enabled, true),
     DEFINE_PROP_END_OF_LIST(),
 };
 
diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index 12f5c9a759..d9ddf9b723 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -81,6 +81,8 @@ typedef struct TPMState {
     TPMVersion be_tpm_version;
 
     size_t be_buffer_size;
+
+    bool ppi_enabled;
 } TPMState;
 
 #define TPM(obj) OBJECT_CHECK(TPMState, (obj), TYPE_TPM_TIS)
@@ -950,6 +952,7 @@ static const VMStateDescription vmstate_tpm_tis = {
 static Property tpm_tis_properties[] = {
     DEFINE_PROP_UINT32("irq", TPMState, irq_num, TPM_TIS_IRQ),
     DEFINE_PROP_TPMBE("tpmdev", TPMState, be_driver),
+    DEFINE_PROP_BOOL("ppi", TPMState, ppi_enabled, true),
     DEFINE_PROP_END_OF_LIST(),
 };
 
-- 
2.18.0.rc1

[Qemu-devel] [PATCH v6 2/4] tpm: implement virtual memory device for TPM PPI

[Marc-André Lureau]

From: Stefan Berger <stefanb@linux.vnet.ibm.com>

Implement a virtual memory device for the TPM Physical Presence interface.
The memory is located at 0xFED45000 and used by ACPI to send messages to the
firmware (BIOS) and by the firmware to provide parameters for each one of
the supported codes.

This device should be used by all TPM interfaces on x86 and can be added
by calling tpm_ppi_init_io().

Note: bios_linker cannot be used to allocate the PPI memory region,
since the reserved memory should stay stable across reboots, and might
be needed before the ACPI tables are installed.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

---

v5 (Marc-André):
 - replace mmio region with ram

v4 (Marc-André):
 - pass TPM_PPI_ADDR_BASE as argument to tpm_ppi_init_io()
 - only enable PPI if property is set

v3 (Marc-André):
 - merge CRB support
 - use trace events instead of DEBUG printf
 - headers inclusion simplification

v2:
 - moved to byte access since an infrequently used device;
   this simplifies code
 - increase size of device to 0x400
 - move device to 0xfffef000 since SeaBIOS has some code at 0xffff0000:
   'On the emulators, the bios at 0xf0000 is also at 0xffff0000'
---
 hw/tpm/tpm_ppi.h      | 25 +++++++++++++++++++++++++
 include/hw/acpi/tpm.h |  6 ++++++
 hw/tpm/tpm_crb.c      |  8 ++++++++
 hw/tpm/tpm_ppi.c      | 33 +++++++++++++++++++++++++++++++++
 hw/tpm/tpm_tis.c      |  8 ++++++++
 hw/tpm/Makefile.objs  |  2 +-
 hw/tpm/trace-events   |  4 ++++
 7 files changed, 85 insertions(+), 1 deletion(-)
 create mode 100644 hw/tpm/tpm_ppi.h
 create mode 100644 hw/tpm/tpm_ppi.c

diff --git a/hw/tpm/tpm_ppi.h b/hw/tpm/tpm_ppi.h
new file mode 100644
index 0000000000..10e760fdd5
--- /dev/null
+++ b/hw/tpm/tpm_ppi.h
@@ -0,0 +1,25 @@
+/*
+ * TPM Physical Presence Interface
+ *
+ * Copyright (C) 2018 IBM Corporation
+ *
+ * Authors:
+ *  Stefan Berger    <stefanb@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#ifndef TPM_TPM_PPI_H
+#define TPM_TPM_PPI_H
+
+#include "hw/acpi/tpm.h"
+#include "exec/address-spaces.h"
+
+typedef struct TPMPPI {
+    MemoryRegion ram;
+} TPMPPI;
+
+bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
+                  hwaddr addr, Object *obj, Error **errp);
+
+#endif /* TPM_TPM_PPI_H */
diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
index 46ac4dc581..c082df7d1d 100644
--- a/include/hw/acpi/tpm.h
+++ b/include/hw/acpi/tpm.h
@@ -187,4 +187,10 @@ REG32(CRB_DATA_BUFFER, 0x80)
 #define TPM2_START_METHOD_MMIO      6
 #define TPM2_START_METHOD_CRB       7
 
+/*
+ * Physical Presence Interface
+ */
+#define TPM_PPI_ADDR_SIZE           0x400
+#define TPM_PPI_ADDR_BASE           0xFED45000
+
 #endif /* HW_ACPI_TPM_H */
diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
index d5b0ac5920..b243222fd6 100644
--- a/hw/tpm/tpm_crb.c
+++ b/hw/tpm/tpm_crb.c
@@ -29,6 +29,7 @@
 #include "sysemu/reset.h"
 #include "tpm_int.h"
 #include "tpm_util.h"
+#include "tpm_ppi.h"
 #include "trace.h"
 
 typedef struct CRBState {
@@ -43,6 +44,7 @@ typedef struct CRBState {
     size_t be_buffer_size;
 
     bool ppi_enabled;
+    TPMPPI ppi;
 } CRBState;
 
 #define CRB(obj) OBJECT_CHECK(CRBState, (obj), TYPE_TPM_CRB)
@@ -294,6 +296,12 @@ static void tpm_crb_realize(DeviceState *dev, Error **errp)
     memory_region_add_subregion(get_system_memory(),
         TPM_CRB_ADDR_BASE + sizeof(s->regs), &s->cmdmem);
 
+    if (s->ppi_enabled &&
+        !tpm_ppi_init(&s->ppi, get_system_memory(),
+                      TPM_PPI_ADDR_BASE, OBJECT(s), errp)) {
+        return;
+    }
+
     qemu_register_reset(tpm_crb_reset, dev);
 }
 
diff --git a/hw/tpm/tpm_ppi.c b/hw/tpm/tpm_ppi.c
new file mode 100644
index 0000000000..987daf099f
--- /dev/null
+++ b/hw/tpm/tpm_ppi.c
@@ -0,0 +1,33 @@
+/*
+ * tpm_ppi.c - TPM Physical Presence Interface
+ *
+ * Copyright (C) 2018 IBM Corporation
+ *
+ * Authors:
+ *  Stefan Berger <stefanb@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+
+#include "qapi/error.h"
+#include "tpm_ppi.h"
+
+bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
+                  hwaddr addr, Object *obj, Error **errp)
+{
+    Error *err = NULL;
+
+    memory_region_init_ram(&tpmppi->ram, obj, "tpm-ppi",
+                           TPM_PPI_ADDR_SIZE, &err);
+    if (err) {
+        error_propagate(errp, err);
+        return false;
+    }
+
+    memory_region_add_subregion(m, addr, &tpmppi->ram);
+    return true;
+}
diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index d9ddf9b723..70432ffe8b 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -31,6 +31,7 @@
 #include "sysemu/tpm_backend.h"
 #include "tpm_int.h"
 #include "tpm_util.h"
+#include "tpm_ppi.h"
 #include "trace.h"
 
 #define TPM_TIS_NUM_LOCALITIES      5     /* per spec */
@@ -83,6 +84,7 @@ typedef struct TPMState {
     size_t be_buffer_size;
 
     bool ppi_enabled;
+    TPMPPI ppi;
 } TPMState;
 
 #define TPM(obj) OBJECT_CHECK(TPMState, (obj), TYPE_TPM_TIS)
@@ -979,6 +981,12 @@ static void tpm_tis_realizefn(DeviceState *dev, Error **errp)
 
     memory_region_add_subregion(isa_address_space(ISA_DEVICE(dev)),
                                 TPM_TIS_ADDR_BASE, &s->mmio);
+
+    if (s->ppi_enabled &&
+        !tpm_ppi_init(&s->ppi, isa_address_space(ISA_DEVICE(dev)),
+                      TPM_PPI_ADDR_BASE, OBJECT(s), errp)) {
+        return;
+    }
 }
 
 static void tpm_tis_initfn(Object *obj)
diff --git a/hw/tpm/Makefile.objs b/hw/tpm/Makefile.objs
index 1dc9f8bf2c..eedd8b6858 100644
--- a/hw/tpm/Makefile.objs
+++ b/hw/tpm/Makefile.objs
@@ -1,4 +1,4 @@
-common-obj-y += tpm_util.o
+common-obj-y += tpm_util.o tpm_ppi.o
 common-obj-$(CONFIG_TPM_TIS) += tpm_tis.o
 common-obj-$(CONFIG_TPM_CRB) += tpm_crb.o
 common-obj-$(CONFIG_TPM_PASSTHROUGH) += tpm_passthrough.o
diff --git a/hw/tpm/trace-events b/hw/tpm/trace-events
index 25bee0cecf..81f9923401 100644
--- a/hw/tpm/trace-events
+++ b/hw/tpm/trace-events
@@ -8,6 +8,10 @@ tpm_crb_mmio_write(uint64_t addr, unsigned size, uint32_t val) "CRB write 0x" TA
 tpm_passthrough_handle_request(void *cmd) "processing command %p"
 tpm_passthrough_reset(void) "reset"
 
+# hw/tpm/tpm_ppi.c
+tpm_ppi_mmio_read(uint64_t addr, unsigned size, uint32_t val) "PPI read 0x" TARGET_FMT_plx " len:%u val: 0x%" PRIx32
+tpm_ppi_mmio_write(uint64_t addr, unsigned size, uint32_t val) "PPI write 0x" TARGET_FMT_plx " len:%u val: 0x%" PRIx32
+
 # hw/tpm/tpm_util.c
 tpm_util_get_buffer_size_hdr_len(uint32_t len, size_t expected) "tpm_resp->hdr.len = %u, expected = %zu"
 tpm_util_get_buffer_size_len(uint32_t len, size_t expected) "tpm_resp->len = %u, expected = %zu"
-- 
2.18.0.rc1

[Qemu-devel] [PATCH v6 3/4] acpi: add fw_cfg file for TPM and PPI virtual memory device

[Marc-André Lureau]

From: Stefan Berger <stefanb@linux.vnet.ibm.com>

To avoid having to hard code the base address of the PPI virtual
memory device we introduce a fw_cfg file etc/tpm/config that holds the
base address of the PPI device, the version of the PPI interface and
the version of the attached TPM.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
[ Marc-André: renamed to etc/tpm/config, made it static, document it ]
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

---

v5:
 - fix endianess issues on 8bit fields
v4:
 - add ACPI only if PPI is enabled
v3:
 - renamed to etc/tpm/config, made it static, document it
---
 include/hw/acpi/tpm.h |  3 +++
 hw/i386/acpi-build.c  | 19 +++++++++++++++++++
 docs/specs/tpm.txt    | 20 ++++++++++++++++++++
 3 files changed, 42 insertions(+)

diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
index c082df7d1d..f79d68a77a 100644
--- a/include/hw/acpi/tpm.h
+++ b/include/hw/acpi/tpm.h
@@ -193,4 +193,7 @@ REG32(CRB_DATA_BUFFER, 0x80)
 #define TPM_PPI_ADDR_SIZE           0x400
 #define TPM_PPI_ADDR_BASE           0xFED45000
 
+#define TPM_PPI_VERSION_NONE        0
+#define TPM_PPI_VERSION_1_30        1
+
 #endif /* HW_ACPI_TPM_H */
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 9bc6d97ea1..ebd64c4818 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -119,6 +119,12 @@ typedef struct AcpiBuildPciBusHotplugState {
     bool pcihp_bridge_en;
 } AcpiBuildPciBusHotplugState;
 
+typedef struct FWCfgTPMConfig {
+    uint32_t tpmppi_address;
+    uint8_t tpm_version;
+    uint8_t tpmppi_version;
+} QEMU_PACKED FWCfgTPMConfig;
+
 static void init_common_fadt_data(Object *o, AcpiFadtData *data)
 {
     uint32_t io = object_property_get_uint(o, ACPI_PM_PROP_PM_IO_BASE, NULL);
@@ -2873,6 +2879,8 @@ void acpi_setup(void)
     AcpiBuildTables tables;
     AcpiBuildState *build_state;
     Object *vmgenid_dev;
+    TPMIf *tpm;
+    static FWCfgTPMConfig tpm_config;
 
     if (!pcms->fw_cfg) {
         ACPI_BUILD_DPRINTF("No fw cfg. Bailing out.\n");
@@ -2907,6 +2915,17 @@ void acpi_setup(void)
     fw_cfg_add_file(pcms->fw_cfg, ACPI_BUILD_TPMLOG_FILE,
                     tables.tcpalog->data, acpi_data_len(tables.tcpalog));
 
+    tpm = tpm_find();
+    if (tpm && object_property_get_bool(OBJECT(tpm), "ppi", &error_abort)) {
+        tpm_config = (FWCfgTPMConfig) {
+            .tpmppi_address = cpu_to_le32(TPM_PPI_ADDR_BASE),
+            .tpm_version = tpm_get_version(tpm_find()),
+            .tpmppi_version = TPM_PPI_VERSION_NONE
+        };
+        fw_cfg_add_file(pcms->fw_cfg, "etc/tpm/config",
+                        &tpm_config, sizeof tpm_config);
+    }
+
     vmgenid_dev = find_vmgenid_dev();
     if (vmgenid_dev) {
         vmgenid_add_fw_cfg(VMGENID(vmgenid_dev), pcms->fw_cfg,
diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
index c230c4c93e..2ddb768084 100644
--- a/docs/specs/tpm.txt
+++ b/docs/specs/tpm.txt
@@ -20,6 +20,26 @@ QEMU files related to TPM TIS interface:
  - hw/tpm/tpm_tis.h
 
 
+= fw_cfg interface =
+
+The bios/firmware may use the "etc/tpm/config" fw_cfg entry for
+configuring the guest appropriately.
+
+The entry of 6 bytes has the following content, in little-endian:
+
+    #define TPM_VERSION_UNSPEC          0
+    #define TPM_VERSION_1_2             1
+    #define TPM_VERSION_2_0             2
+
+    #define TPM_PPI_VERSION_NONE        0
+    #define TPM_PPI_VERSION_1_30        1
+
+    struct FWCfgTPMConfig {
+        uint32_t tpmppi_address;         /* PPI memory location */
+        uint8_t tpm_version;             /* TPM version */
+        uint8_t tpmppi_version;          /* PPI version */
+    };
+
 = ACPI Interface =
 
 The TPM device is defined with ACPI ID "PNP0C31". QEMU builds a SSDT and passes
-- 
2.18.0.rc1

[Qemu-devel] [PATCH v6 4/4] acpi: build TPM Physical Presence interface

[Marc-André Lureau]

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>
[ Marc-André - ACPI code improvements and windows fixes ]
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

---

v7: (Marc-André)
 - use first spec version/section in code comments
 - use more variables for ASL code building
 - remove unnecessering ToInteger() calls

v6:
 - more code documentation (Marc-André)
 - use some explicit named variables to ease reading (Marc-André)
 - use fixed size fields/memory regions, remove PPI struct (Marc-André)
 - only add PPI ACPI methods if PPI is enabled (Marc-André)
 - document the qemu/firmware ACPI memory region (Stefan)

v5 (Marc-André):
 - /struct tpm_ppi/struct TPMPPIData

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 |   8 +
 hw/i386/acpi-build.c  | 403 +++++++++++++++++++++++++++++++++++++++++-
 docs/specs/tpm.txt    |  79 +++++++++
 3 files changed, 487 insertions(+), 3 deletions(-)

diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
index f79d68a77a..e0bd07862e 100644
--- a/include/hw/acpi/tpm.h
+++ b/include/hw/acpi/tpm.h
@@ -196,4 +196,12 @@ REG32(CRB_DATA_BUFFER, 0x80)
 #define TPM_PPI_VERSION_NONE        0
 #define TPM_PPI_VERSION_1_30        1
 
+/* whether function is blocked by BIOS settings; bits 0, 1, 2 */
+#define TPM_PPI_FUNC_NOT_IMPLEMENTED     (0 << 0)
+#define TPM_PPI_FUNC_BIOS_ONLY           (1 << 0)
+#define TPM_PPI_FUNC_BLOCKED             (2 << 0)
+#define TPM_PPI_FUNC_ALLOWED_USR_REQ     (3 << 0)
+#define TPM_PPI_FUNC_ALLOWED_USR_NOT_REQ (4 << 0)
+#define TPM_PPI_FUNC_MASK                (7 << 0)
+
 #endif /* HW_ACPI_TPM_H */
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index ebd64c4818..263677f3e4 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -43,6 +43,7 @@
 #include "hw/acpi/memory_hotplug.h"
 #include "sysemu/tpm.h"
 #include "hw/acpi/tpm.h"
+#include "hw/tpm/tpm_ppi.h"
 #include "hw/acpi/vmgenid.h"
 #include "sysemu/tpm_backend.h"
 #include "hw/timer/mc146818rtc_regs.h"
@@ -1789,6 +1790,396 @@ static Aml *build_q35_osc_method(void)
     return method;
 }
 
+static void
+build_tpm_ppi(TPMIf *tpm, Aml *dev)
+{
+    Aml *method, *field, *ifctx, *ifctx2, *ifctx3;
+    Aml *pak, *tpm2, *tpm3, *pprm, *pprq;
+    int i;
+
+    if (!object_property_get_bool(OBJECT(tpm), "ppi", &error_abort)) {
+        return;
+    }
+    /*
+     * TPP1 is for the flags that indicate which PPI operations
+     * are supported by the firmware. The firmware is expected to
+     * write these flags.
+     */
+    aml_append(dev,
+               aml_operation_region("TPP1", AML_SYSTEM_MEMORY,
+                                    aml_int(TPM_PPI_ADDR_BASE), 0x100));
+    field = aml_field("TPP1", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
+    for (i = 0; i < 0x100; i++) {
+        char *tmp = g_strdup_printf("FN%02X", i);
+        aml_append(field, aml_named_field(tmp, 8));
+        g_free(tmp);
+    }
+    aml_append(dev, field);
+
+    /*
+     * TPP2 is for the registers that ACPI code used to pass
+     * the PPI code and parameter (PPRQ, PPRM) to the firmware.
+     */
+    aml_append(dev,
+               aml_operation_region("TPP2", AML_SYSTEM_MEMORY,
+                                    aml_int(TPM_PPI_ADDR_BASE + 0x100),
+                                    0x5A));
+    field = aml_field("TPP2", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
+    aml_append(field, aml_named_field("PPIN", 8));
+    aml_append(field, aml_named_field("PPIP", 32));
+    aml_append(field, aml_named_field("PPRP", 32));
+    aml_append(field, aml_named_field("PPRQ", 32));
+    aml_append(field, aml_named_field("PPRM", 32));
+    aml_append(field, aml_named_field("LPPR", 32));
+    aml_append(dev, field);
+    pprq = aml_name("PPRQ");
+    pprm = aml_name("PPRM");
+
+    /*
+     * A function to return the value of DerefOf (FUNC [N]), by using
+     * accessing the fields individually instead. This is a workaround
+     * for what looks like a Windows ACPI bug in all versions so far
+     * (fwiw, DerefOf (FUNC [N]) works on Linux).
+     */
+    method = aml_method("TPFN", 1, AML_SERIALIZED);
+    {
+        for (i = 0; i < 0x100; i++) {
+            ifctx = aml_if(aml_equal(aml_int(i), aml_arg(0)));
+            {
+                aml_append(ifctx, aml_return(aml_name("FN%02X", i)));
+            }
+            aml_append(method, ifctx);
+        }
+        aml_append(method, aml_return(aml_int(0)));
+    }
+    aml_append(dev, method);
+
+    pak = aml_package(2);
+    aml_append(pak, aml_int(0));
+    aml_append(pak, aml_int(0));
+    aml_append(dev, aml_name_decl("TPM2", pak));
+    tpm2 = aml_name("TPM2");
+
+    pak = aml_package(3);
+    aml_append(pak, aml_int(0));
+    aml_append(pak, aml_int(0));
+    aml_append(pak, aml_int(0));
+    aml_append(dev, aml_name_decl("TPM3", pak));
+    tpm3 = aml_name("TPM3");
+
+    method = aml_method("_DSM", 4, AML_SERIALIZED);
+    {
+        uint8_t zerobyte[1] = { 0 };
+        Aml *function, *arguments, *rev, *op, *op_arg, *op_flags, *uuid;
+
+        uuid = aml_arg(0);
+        rev = aml_arg(1);
+        function = aml_arg(2);
+        ifctx = aml_if(
+            aml_equal(uuid,
+                      aml_touuid("3DDDFAA6-361B-4EB4-A424-8D10089D1653")));
+        {
+            /* standard DSM query function */
+            ifctx2 = aml_if(aml_equal(function, aml_int(0)));
+            {
+                uint8_t byte_list[2] = { 0xff, 0x01 };
+                aml_append(ifctx2, aml_return(aml_buffer(2, byte_list)));
+            }
+            aml_append(ifctx, ifctx2);
+
+            /*
+             * PPI 1.0: 2.1.1 Get Physical Presence Interface Version
+             *
+             * Arg 2 (Integer): Function Index = 1
+             * Arg 3 (Package): Arguments = Empty Package
+             * Returns: Type: String
+             */
+            ifctx2 = aml_if(aml_equal(function, aml_int(1)));
+            {
+                aml_append(ifctx2, aml_return(aml_string("1.3")));
+            }
+            aml_append(ifctx, ifctx2);
+
+            /*
+             * PPI 1.0: 2.1.3 Submit TPM Operation Request to Pre-OS Environment
+             *
+             * Arg 2 (Integer): Function Index = 2
+             * Arg 3 (Package): Arguments = Package: Type: Integer
+             *                              Operation Value of the Request
+             * Returns: Type: Integer
+             *          0: Success
+             *          1: Operation Value of the Request Not Supported
+             *          2: General Failure
+             */
+            ifctx2 = aml_if(aml_equal(function, aml_int(2)));
+            {
+                /* get opcode */
+                arguments = aml_arg(3);
+                op = aml_local(0);
+                op_flags = aml_local(1);
+                aml_append(ifctx2,
+                           aml_store(aml_derefof(aml_index(arguments,
+                                                           aml_int(0))), op));
+
+                /* get opcode flags */
+                aml_append(ifctx2,
+                           aml_store(aml_call1("TPFN", op), op_flags));
+
+                /* if func[opcode] & TPM_PPI_FUNC_NOT_IMPLEMENTED */
+                ifctx3 = aml_if(
+                    aml_equal(
+                        aml_and(op_flags, aml_int(TPM_PPI_FUNC_MASK), NULL),
+                        aml_int(TPM_PPI_FUNC_NOT_IMPLEMENTED)));
+                {
+                    /* 1: Operation Value of the Request Not Supported */
+                    aml_append(ifctx3, aml_return(aml_int(1)));
+                }
+                aml_append(ifctx2, ifctx3);
+
+                aml_append(ifctx2, aml_store(op, pprq));
+                aml_append(ifctx2, aml_store(aml_int(0), pprm));
+                /* 0: success */
+                aml_append(ifctx2, aml_return(aml_int(0)));
+            }
+            aml_append(ifctx, ifctx2);
+
+            /*
+             * PPI 1.0: 2.1.4 Get Pending TPM Operation Requested By the OS
+             *
+             * Arg 2 (Integer): Function Index = 3
+             * Arg 3 (Package): Arguments = Empty Package
+             * Returns: Type: Package of Integers
+             *          Integer 1: Function Return code
+             *                     0: Success
+             *                     1: General Failure
+             *          Integer 2: Pending operation requested by the OS
+             *                     0: None
+             *                    >0: Operation Value of the Pending Request
+             *          Integer 3: Optional argument to pending operation
+             *                     requested by the OS
+             *                     0: None
+             *                    >0: Argument Value of the Pending Request
+             */
+            ifctx2 = aml_if(aml_equal(function, aml_int(3)));
+            {
+                /*
+                 * Revision ID of 1, no integer parameter beyond
+                 * parameter two are expected
+                 */
+                ifctx3 = aml_if(aml_equal(rev, aml_int(1)));
+                {
+                    /* TPM2[1] = PPRQ */
+                    aml_append(ifctx3,
+                               aml_store(pprq, aml_index(tpm2, aml_int(1))));
+                    aml_append(ifctx3, aml_return(tpm2));
+                }
+                aml_append(ifctx2, ifctx3);
+
+                /*
+                 * A return value of {0, 23, 1} indicates that
+                 * operation 23 with argument 1 is pending.
+                 */
+                ifctx3 = aml_if(aml_equal(rev, aml_int(2)));
+                {
+                    /* TPM3[1] = PPRQ */
+                    aml_append(ifctx3,
+                               aml_store(pprq, aml_index(tpm3, aml_int(1))));
+                    /* TPM3[2] = PPRM */
+                    aml_append(ifctx3,
+                               aml_store(pprm, aml_index(tpm3, aml_int(2))));
+                    aml_append(ifctx3, aml_return(tpm3));
+                }
+                aml_append(ifctx2, ifctx3);
+            }
+            aml_append(ifctx, ifctx2);
+
+            /*
+             * PPI 1.0: 2.1.5 Get Platform-Specific Action to Transition to
+             *     Pre-OS Environment
+             *
+             * Arg 2 (Integer): Function Index = 4
+             * Arg 3 (Package): Arguments = Empty Package
+             * Returns: Type: Integer
+             *          0: None
+             *          1: Shutdown
+             *          2: Reboot
+             *          3: OS Vendor-specific
+             */
+            ifctx2 = aml_if(aml_equal(function, aml_int(4)));
+            {
+                /* reboot */
+                aml_append(ifctx2, aml_return(aml_int(2)));
+            }
+            aml_append(ifctx, ifctx2);
+
+            /*
+             * PPI 1.0: 2.1.6 Return TPM Operation Response to OS Environment
+             *
+             * Arg 2 (Integer): Function Index = 5
+             * Arg 3 (Package): Arguments = Empty Package
+             * Returns: Type: Package of Integer
+             *          Integer 1: Function Return code
+             *                     0: Success
+             *                     1: General Failure
+             *          Integer 2: Most recent operation request
+             *                     0: None
+             *                    >0: Operation Value of the most recent request
+             *          Integer 3: Response to the most recent operation request
+             *                     0: Success
+             *                     0x00000001..0x00000FFF: Corresponding TPM
+             *                                             error code
+             *                     0xFFFFFFF0: User Abort or timeout of dialog
+             *                     0xFFFFFFF1: firmware Failure
+             */
+            ifctx2 = aml_if(aml_equal(function, aml_int(5)));
+            {
+                /* TPM3[1] = LPPR */
+                aml_append(ifctx2,
+                           aml_store(aml_name("LPPR"),
+                                     aml_index(tpm3, aml_int(1))));
+                /* TPM3[2] = PPRP */
+                aml_append(ifctx2,
+                           aml_store(aml_name("PPRP"),
+                                     aml_index(tpm3, aml_int(2))));
+                aml_append(ifctx2, aml_return(tpm3));
+            }
+            aml_append(ifctx, ifctx2);
+
+            /*
+             * PPI 1.0: 2.1.7 Submit preferred user language
+             *
+             * Arg 2 (Integer): Function Index = 6
+             * Arg 3 (Package): Arguments = String Package
+             *                  Preferred language code
+             * Returns: Type: Integer
+             * Function Return Code
+             *          3: Not implemented
+             */
+            ifctx2 = aml_if(aml_equal(function, aml_int(6)));
+            {
+                /* 3 = not implemented */
+                aml_append(ifctx2, aml_return(aml_int(3)));
+            }
+            aml_append(ifctx, ifctx2);
+
+            /*
+             * PPI 1.1: 2.1.7 Submit TPM Operation Request to
+             *     Pre-OS Environment 2
+             *
+             * Arg 2 (Integer): Function Index = 7
+             * Arg 3 (Package): Arguments = Package: Type: Integer
+             *                  Integer 1: Operation Value of the Request
+             *                  Integer 2: Argument for Operation (optional)
+             * Returns: Type: Integer
+             *          0: Success
+             *          1: Not Implemented
+             *          2: General Failure
+             *          3: Operation blocked by current firmware settings
+             */
+            ifctx2 = aml_if(aml_equal(function, aml_int(7)));
+            {
+                /* get opcode */
+                arguments = aml_arg(3);
+                op = aml_local(0);
+                op_flags = aml_local(1);
+                aml_append(ifctx2, aml_store(aml_derefof(aml_index(arguments,
+                                                                   aml_int(0))),
+                                             op));
+
+                /* get opcode flags */
+                aml_append(ifctx2, aml_store(aml_call1("TPFN", op),
+                                             op_flags));
+                /* if func[opcode] & TPM_PPI_FUNC_NOT_IMPLEMENTED */
+                ifctx3 = aml_if(
+                    aml_equal(
+                        aml_and(op_flags, aml_int(TPM_PPI_FUNC_MASK), NULL),
+                        aml_int(TPM_PPI_FUNC_NOT_IMPLEMENTED)));
+                {
+                    /* 1: not implemented */
+                    aml_append(ifctx3, aml_return(aml_int(1)));
+                }
+                aml_append(ifctx2, ifctx3);
+
+                /* if func[opcode] & TPM_PPI_FUNC_BLOCKED */
+                ifctx3 = aml_if(
+                    aml_equal(
+                        aml_and(op_flags, aml_int(TPM_PPI_FUNC_MASK), NULL),
+                        aml_int(TPM_PPI_FUNC_BLOCKED)));
+                {
+                    /* 3: blocked by firmware */
+                    aml_append(ifctx3, aml_return(aml_int(3)));
+                }
+                aml_append(ifctx2, ifctx3);
+
+                /* revision to integer */
+                ifctx3 = aml_if(aml_equal(rev, aml_int(1)));
+                {
+                    /* revision 1 */
+                    /* PPRQ = op */
+                    aml_append(ifctx3, aml_store(op, pprq));
+                    /* no argument, PPRM = 0 */
+                    aml_append(ifctx3, aml_store(aml_int(0), pprm));
+                }
+                aml_append(ifctx2, ifctx3);
+
+                ifctx3 = aml_if(aml_equal(rev, aml_int(2)));
+                {
+                    /* revision 2 */
+                    /* PPRQ = op */
+                    arguments = aml_arg(3);
+                    op_arg = aml_derefof(aml_index(arguments, aml_int(1)));
+                    aml_append(ifctx3, aml_store(op, pprq));
+                    /* PPRM = arg3[1] */
+                    aml_append(ifctx3, aml_store(op_arg, pprm));
+                }
+                aml_append(ifctx2, ifctx3);
+                /* 0: success */
+                aml_append(ifctx2, aml_return(aml_int(0)));
+            }
+            aml_append(ifctx, ifctx2);
+
+            /*
+             * 8.1.8 Get User Confirmation Status for Operation
+             *
+             * Arg 2 (Integer): Function Index = 8
+             * Arg 3 (Package): Arguments = Package: Type: Integer
+             *                  Operation Value that may need user confirmation
+             * Returns: Type: Integer
+             *          0: Not implemented
+             *          1: Firmware only
+             *          2: Blocked for OS by firmware configuration
+             *          3: Allowed and physically present user required
+             *          4: Allowed and physically present user not required
+             */
+            ifctx2 = aml_if(aml_equal(function, aml_int(8)));
+            {
+                /* get opcode */
+                arguments = aml_arg(3);
+                op = aml_local(0);
+                op_flags = aml_local(1);
+                aml_append(ifctx2,
+                           aml_store(aml_derefof(aml_index(arguments,
+                                                           aml_int(0))),
+                                     op));
+
+                /* get opcode flags */
+                aml_append(ifctx2, aml_store(aml_call1("TPFN", op),
+                                             op_flags));
+                /* return confirmation status code */
+                aml_append(ifctx2,
+                           aml_return(
+                               aml_and(op_flags,
+                                       aml_int(TPM_PPI_FUNC_MASK), NULL)));
+            }
+            aml_append(ifctx, ifctx2);
+
+            aml_append(ifctx, aml_return(aml_buffer(1, zerobyte)));
+        }
+        aml_append(method, ifctx);
+    }
+    aml_append(dev, method);
+}
+
 static void
 build_dsdt(GArray *table_data, BIOSLinker *linker,
            AcpiPmInfo *pm, AcpiMiscInfo *misc,
@@ -1802,6 +2193,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
     uint32_t nr_mem = machine->ram_slots;
     int root_bus_limit = 0xFF;
     PCIBus *bus = NULL;
+    TPMIf *tpm = tpm_find();
     int i;
 
     dsdt = init_aml_allocator();
@@ -2139,7 +2531,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
             /* Scan all PCI buses. Generate tables to support hotplug. */
             build_append_pci_bus_devices(scope, bus, pm->pcihp_bridge_en);
 
-            if (TPM_IS_TIS(tpm_find())) {
+            if (TPM_IS_TIS(tpm)) {
                 dev = aml_device("ISA.TPM");
                 aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0C31")));
                 aml_append(dev, aml_name_decl("_STA", aml_int(0xF)));
@@ -2153,6 +2545,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
                  */
                 /* aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ)); */
                 aml_append(dev, aml_name_decl("_CRS", crs));
+
+                build_tpm_ppi(tpm, dev);
+
                 aml_append(scope, dev);
             }
 
@@ -2160,7 +2555,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
         }
     }
 
-    if (TPM_IS_CRB(tpm_find())) {
+    if (TPM_IS_CRB(tpm)) {
         dev = aml_device("TPM");
         aml_append(dev, aml_name_decl("_HID", aml_string("MSFT0101")));
         crs = aml_resource_template();
@@ -2172,6 +2567,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
         aml_append(method, aml_return(aml_int(0x0f)));
         aml_append(dev, method);
 
+        build_tpm_ppi(tpm, dev);
+
         aml_append(sb_scope, dev);
     }
 
@@ -2920,7 +3317,7 @@ void acpi_setup(void)
         tpm_config = (FWCfgTPMConfig) {
             .tpmppi_address = cpu_to_le32(TPM_PPI_ADDR_BASE),
             .tpm_version = tpm_get_version(tpm_find()),
-            .tpmppi_version = TPM_PPI_VERSION_NONE
+            .tpmppi_version = TPM_PPI_VERSION_1_30
         };
         fw_cfg_add_file(pcms->fw_cfg, "etc/tpm/config",
                         &tpm_config, sizeof tpm_config);
diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
index 2ddb768084..c27762c723 100644
--- a/docs/specs/tpm.txt
+++ b/docs/specs/tpm.txt
@@ -62,6 +62,85 @@ URL:
 
 https://trustedcomputinggroup.org/tcg-acpi-specification/
 
+== ACPI PPI Interface ==
+
+QEMU supports the Physical Presence Interface (PPI) for TPM 1.2 and TPM 2. This
+interface requires ACPI and firmware support. The specification can be found at
+the following URL:
+
+https://trustedcomputinggroup.org/resource/tcg-physical-presence-interface-specification/
+
+PPI enables a system administrator (root) to request a modification to the
+TPM upon reboot. The PPI specification defines the operation requests and the
+actions the firmware has to take. The system administrator passes the operation
+request number to the firmware through an ACPI interface which writes this
+number to a memory location that the firmware knows. Upon reboot, the firmware
+finds the number and sends commands to the the TPM. The firmware writes the TPM
+result code and the operation request number to a memory location that ACPI can
+read from and pass the result on to the administrator.
+
+The PPI specification defines a set of mandatory and optional operations for
+the firmware to implement. The ACPI interface also allows an administrator to
+list the supported operations. In QEMU the ACPI code is generated by QEMU, yet
+the firmware needs to implement support on a per-operations basis, and
+different firmwares may support a different subset. Therefore, QEMU introduces
+the virtual memory device for PPI where the firmware can indicate which
+operations it supports and ACPI can enable the ones that are supported and
+disable all others. This interface lies in main memory and has the following
+layout:
+
+ +----------+--------+--------+-------------------------------------------+
+ |  Field   | Length | Offset | Description                               |
+ +----------+--------+--------+-------------------------------------------+
+ | func     |  0x100 |  0x000 | Firmware sets values for each supported   |
+ |          |        |        | operation. See defined values below.      |
+ +----------+--------+--------+-------------------------------------------+
+ | ppin     |   0x1  |  0x100 | SMI interrupt to use. Set by firmware.    |
+ |          |        |        | Not supported.                            |
+ +----------+--------+--------+-------------------------------------------+
+ | ppip     |   0x4  |  0x101 | ACPI function index to pass to SMM code.  |
+ |          |        |        | Set by ACPI. Not supported.               |
+ +----------+--------+--------+-------------------------------------------+
+ | pprp     |   0x4  |  0x105 | Result of last executed operation. Set by |
+ |          |        |        | firmware. See function index 5 for values.|
+ +----------+--------+--------+-------------------------------------------+
+ | pprq     |   0x4  |  0x109 | Operation request number to execute. See  |
+ |          |        |        | 'Physical Presence Interface Operation    |
+ |          |        |        | Summary' tables in specs. Set by ACPI.    |
+ +----------+--------+--------+-------------------------------------------+
+ | pprm     |   0x4  |  0x10d | Operation request optional parameter.     |
+ |          |        |        | Values depend on operation. Set by ACPI.  |
+ +----------+--------+--------+-------------------------------------------+
+ | lppr     |   0x4  |  0x111 | Last executed operation request number.   |
+ |          |        |        | Copied from pprq field by firmware.       |
+ +----------+--------+--------+-------------------------------------------+
+ | fret     |   0x4  |  0x115 | Result code from SMM function.            |
+ |          |        |        | Not supported.                            |
+ +----------+--------+--------+-------------------------------------------+
+ | res1     |  0x40  |  0x119 | Reserved for future use                   |
+ +----------+--------+--------+-------------------------------------------+
+ | next_step|   0x1  |  0x159 | Operation to execute after reboot by      |
+ |          |        |        | firmware. Used by firmware.               |
+ +----------+--------+--------+-------------------------------------------+
+
+   The following values are supported for the 'func' field. They correspond
+   to the values used by ACPI function index 8.
+
+ +----------+-------------------------------------------------------------+
+ | value    | Description                                                 |
+ +----------+-------------------------------------------------------------+
+ | 0        | Operation is not implemented.                               |
+ +----------+-------------------------------------------------------------+
+ | 1        | Operation is only accessible through firmware.              |
+ +----------+-------------------------------------------------------------+
+ | 2        | Operation is blocked for OS by firmware configuration.      |
+ +----------+-------------------------------------------------------------+
+ | 3        | Operation is allowed and physically present user required.  |
+ +----------+-------------------------------------------------------------+
+ | 4        | Operation is allowed and physically present user is not     |
+ |          | required.                                                   |
+ +----------+-------------------------------------------------------------+
+
 
 QEMU files related to TPM ACPI tables:
  - hw/i386/acpi-build.c
-- 
2.18.0.rc1

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

[Igor Mammedov]

On Thu, 28 Jun 2018 19:26:57 +0200
Marc-André Lureau <marcandre.lureau@redhat.com> 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>
> [ Marc-André - ACPI code improvements and windows fixes ]
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> ---
> 
> v7: (Marc-André)
>  - use first spec version/section in code comments
>  - use more variables for ASL code building
>  - remove unnecessering ToInteger() calls
> 
> v6:
>  - more code documentation (Marc-André)
>  - use some explicit named variables to ease reading (Marc-André)
>  - use fixed size fields/memory regions, remove PPI struct (Marc-André)
>  - only add PPI ACPI methods if PPI is enabled (Marc-André)
>  - document the qemu/firmware ACPI memory region (Stefan)
> 
> v5 (Marc-André):
>  - /struct tpm_ppi/struct TPMPPIData
> 
> 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 |   8 +
>  hw/i386/acpi-build.c  | 403 +++++++++++++++++++++++++++++++++++++++++-
>  docs/specs/tpm.txt    |  79 +++++++++
>  3 files changed, 487 insertions(+), 3 deletions(-)
> 
> diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
> index f79d68a77a..e0bd07862e 100644
> --- a/include/hw/acpi/tpm.h
> +++ b/include/hw/acpi/tpm.h
> @@ -196,4 +196,12 @@ REG32(CRB_DATA_BUFFER, 0x80)
>  #define TPM_PPI_VERSION_NONE        0
>  #define TPM_PPI_VERSION_1_30        1
>  
> +/* whether function is blocked by BIOS settings; bits 0, 1, 2 */
> +#define TPM_PPI_FUNC_NOT_IMPLEMENTED     (0 << 0)
> +#define TPM_PPI_FUNC_BIOS_ONLY           (1 << 0)
> +#define TPM_PPI_FUNC_BLOCKED             (2 << 0)
> +#define TPM_PPI_FUNC_ALLOWED_USR_REQ     (3 << 0)
> +#define TPM_PPI_FUNC_ALLOWED_USR_NOT_REQ (4 << 0)
> +#define TPM_PPI_FUNC_MASK                (7 << 0)
> +
>  #endif /* HW_ACPI_TPM_H */
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index ebd64c4818..263677f3e4 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -43,6 +43,7 @@
>  #include "hw/acpi/memory_hotplug.h"
>  #include "sysemu/tpm.h"
>  #include "hw/acpi/tpm.h"
> +#include "hw/tpm/tpm_ppi.h"
>  #include "hw/acpi/vmgenid.h"
>  #include "sysemu/tpm_backend.h"
>  #include "hw/timer/mc146818rtc_regs.h"
> @@ -1789,6 +1790,396 @@ static Aml *build_q35_osc_method(void)
>      return method;
>  }
>  
> +static void
> +build_tpm_ppi(TPMIf *tpm, Aml *dev)
> +{
> +    Aml *method, *field, *ifctx, *ifctx2, *ifctx3;
> +    Aml *pak, *tpm2, *tpm3, *pprm, *pprq;
> +    int i;
> +
> +    if (!object_property_get_bool(OBJECT(tpm), "ppi", &error_abort)) {
> +        return;
> +    }
I'd prefer this being checked by caller, like:

@@ -2194,6 +2194,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
     int root_bus_limit = 0xFF;
     PCIBus *bus = NULL;
     TPMIf *tpm = tpm_find();
+    bool has_ppi = object_property_get_bool(OBJECT(tpm), "ppi", &error_abort);
     int i;
 
     dsdt = init_aml_allocator();
@@ -2545,8 +2546,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
                  */
                 /* aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ)); */
                 aml_append(dev, aml_name_decl("_CRS", crs));
-
-                build_tpm_ppi(tpm, dev);
+                if (has_ppi) {
+                    build_tpm_ppi(tpm, dev);
+                }
 
                 aml_append(scope, dev);
             }
@@ -2567,7 +2569,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
         aml_append(method, aml_return(aml_int(0x0f)));
         aml_append(dev, method);
 
-        build_tpm_ppi(tpm, dev);
+        if (has_ppi) {
+            build_tpm_ppi(tpm, dev);
+        }
 
         aml_append(sb_scope, dev);
     }

> +    /*
> +     * TPP1 is for the flags that indicate which PPI operations
> +     * are supported by the firmware. The firmware is expected to
> +     * write these flags.
> +     */
> +    aml_append(dev,
> +               aml_operation_region("TPP1", AML_SYSTEM_MEMORY,
> +                                    aml_int(TPM_PPI_ADDR_BASE), 0x100));
> +    field = aml_field("TPP1", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
> +    for (i = 0; i < 0x100; i++) {
> +        char *tmp = g_strdup_printf("FN%02X", i);
> +        aml_append(field, aml_named_field(tmp, 8));
> +        g_free(tmp);
> +    }
this and TPFN takes more than 4K.
Have you tried to declare only one filed of size TPP1, which makes
field Buffer type, which could be used with Index and would take only
several bytes to in AML. See ACPI 6.2a 19.6.61.2 Index with Buffers.
It also looks very portable (but we should test with win xp just to be safe)

> +    aml_append(dev, field);
> +
> +    /*
> +     * TPP2 is for the registers that ACPI code used to pass
> +     * the PPI code and parameter (PPRQ, PPRM) to the firmware.
> +     */
> +    aml_append(dev,
> +               aml_operation_region("TPP2", AML_SYSTEM_MEMORY,
> +                                    aml_int(TPM_PPI_ADDR_BASE + 0x100),
> +                                    0x5A));
> +    field = aml_field("TPP2", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
> +    aml_append(field, aml_named_field("PPIN", 8));
> +    aml_append(field, aml_named_field("PPIP", 32));
> +    aml_append(field, aml_named_field("PPRP", 32));
> +    aml_append(field, aml_named_field("PPRQ", 32));
> +    aml_append(field, aml_named_field("PPRM", 32));
> +    aml_append(field, aml_named_field("LPPR", 32));
> +    aml_append(dev, field);
> +    pprq = aml_name("PPRQ");
> +    pprm = aml_name("PPRM");
> +
> +    /*
> +     * A function to return the value of DerefOf (FUNC [N]), by using
> +     * accessing the fields individually instead. This is a workaround
> +     * for what looks like a Windows ACPI bug in all versions so far
> +     * (fwiw, DerefOf (FUNC [N]) works on Linux).
> +     */
> +    method = aml_method("TPFN", 1, AML_SERIALIZED);
> +    {
> +        for (i = 0; i < 0x100; i++) {
> +            ifctx = aml_if(aml_equal(aml_int(i), aml_arg(0)));
> +            {
> +                aml_append(ifctx, aml_return(aml_name("FN%02X", i)));
> +            }
> +            aml_append(method, ifctx);
> +        }
> +        aml_append(method, aml_return(aml_int(0)));
> +    }
> +    aml_append(dev, method);
> +
> +    pak = aml_package(2);
> +    aml_append(pak, aml_int(0));
> +    aml_append(pak, aml_int(0));
> +    aml_append(dev, aml_name_decl("TPM2", pak));
> +    tpm2 = aml_name("TPM2");
> +
> +    pak = aml_package(3);
> +    aml_append(pak, aml_int(0));
> +    aml_append(pak, aml_int(0));
> +    aml_append(pak, aml_int(0));
> +    aml_append(dev, aml_name_decl("TPM3", pak));
> +    tpm3 = aml_name("TPM3");
looks like above 2 packages  are used only by _DSM,
so you don't need to make them global, just use local variables within _DSM for it

> +
> +    method = aml_method("_DSM", 4, AML_SERIALIZED);
> +    {
> +        uint8_t zerobyte[1] = { 0 }; 
> +        Aml *function, *arguments, *rev, *op, *op_arg, *op_flags, *uuid;
> +
> +        uuid = aml_arg(0);
> +        rev = aml_arg(1);
> +        function = aml_arg(2);
> +        ifctx = aml_if(
> +            aml_equal(uuid,
> +                      aml_touuid("3DDDFAA6-361B-4EB4-A424-8D10089D1653")));
> +        {
it would be better to exit immediately if UUID doesn't match (less indentation and if contexts)
btw, one should return something if uuid doesn't match method,

if (LNotEqual(uuid, ToUUID(“893f00a6-660c-494e-bcfd-3043f4fb67c0”))) {
    return(Buffer(){0})
}

> +            /* standard DSM query function */
> +            ifctx2 = aml_if(aml_equal(function, aml_int(0)));
constants aml_int(0..3) are used multiple times, suggest to declare them
along with rev/function/..., like:

 Aml *one = aml_int(0);
 ...

and then use them through code.


> +            {
> +                uint8_t byte_list[2] = { 0xff, 0x01 };
> +                aml_append(ifctx2, aml_return(aml_buffer(2, byte_list)));
> +            }
> +            aml_append(ifctx, ifctx2);
> +
> +            /*
> +             * PPI 1.0: 2.1.1 Get Physical Presence Interface Version
> +             *
> +             * Arg 2 (Integer): Function Index = 1
> +             * Arg 3 (Package): Arguments = Empty Package
> +             * Returns: Type: String
> +             */
> +            ifctx2 = aml_if(aml_equal(function, aml_int(1)));
> +            {
> +                aml_append(ifctx2, aml_return(aml_string("1.3")));
> +            }
> +            aml_append(ifctx, ifctx2);
> +
> +            /*
> +             * PPI 1.0: 2.1.3 Submit TPM Operation Request to Pre-OS Environment
> +             *
> +             * Arg 2 (Integer): Function Index = 2
> +             * Arg 3 (Package): Arguments = Package: Type: Integer
> +             *                              Operation Value of the Request
> +             * Returns: Type: Integer
> +             *          0: Success
> +             *          1: Operation Value of the Request Not Supported
> +             *          2: General Failure
> +             */
> +            ifctx2 = aml_if(aml_equal(function, aml_int(2)));
> +            {
> +                /* get opcode */
> +                arguments = aml_arg(3);
> +                op = aml_local(0);
> +                op_flags = aml_local(1);
suggest to move these 3 close to function/rev/... as they are repeated several times
with the same meaning in several if contexts.

> +                aml_append(ifctx2,
> +                           aml_store(aml_derefof(aml_index(arguments,
> +                                                           aml_int(0))), op));
> +
> +                /* get opcode flags */
> +                aml_append(ifctx2,
> +                           aml_store(aml_call1("TPFN", op), op_flags));
> +
> +                /* if func[opcode] & TPM_PPI_FUNC_NOT_IMPLEMENTED */
> +                ifctx3 = aml_if(
> +                    aml_equal(
> +                        aml_and(op_flags, aml_int(TPM_PPI_FUNC_MASK), NULL),
> +                        aml_int(TPM_PPI_FUNC_NOT_IMPLEMENTED)));
same as for aml_int(0) TPM_PPI_FUNC_MASK/TPM_PPI_FUNC_NOT_IMPLEMENTED and other TPM_PPI_*
constants that are user as aml_int(), it might mkae code more readable with
shorter lines and shorter variable names.

> +                {
> +                    /* 1: Operation Value of the Request Not Supported */
> +                    aml_append(ifctx3, aml_return(aml_int(1)));
> +                }
> +                aml_append(ifctx2, ifctx3);
> +
> +                aml_append(ifctx2, aml_store(op, pprq));
> +                aml_append(ifctx2, aml_store(aml_int(0), pprm));
> +                /* 0: success */
> +                aml_append(ifctx2, aml_return(aml_int(0)));
> +            }
> +            aml_append(ifctx, ifctx2);
> +
> +            /*
> +             * PPI 1.0: 2.1.4 Get Pending TPM Operation Requested By the OS
> +             *
> +             * Arg 2 (Integer): Function Index = 3
> +             * Arg 3 (Package): Arguments = Empty Package
> +             * Returns: Type: Package of Integers
> +             *          Integer 1: Function Return code
> +             *                     0: Success
> +             *                     1: General Failure
> +             *          Integer 2: Pending operation requested by the OS
> +             *                     0: None
> +             *                    >0: Operation Value of the Pending Request
> +             *          Integer 3: Optional argument to pending operation
> +             *                     requested by the OS
> +             *                     0: None
> +             *                    >0: Argument Value of the Pending Request
> +             */
> +            ifctx2 = aml_if(aml_equal(function, aml_int(3)));
> +            {
> +                /*
> +                 * Revision ID of 1, no integer parameter beyond
> +                 * parameter two are expected
> +                 */
> +                ifctx3 = aml_if(aml_equal(rev, aml_int(1)));
> +                {
> +                    /* TPM2[1] = PPRQ */
> +                    aml_append(ifctx3,
> +                               aml_store(pprq, aml_index(tpm2, aml_int(1))));
> +                    aml_append(ifctx3, aml_return(tpm2));
> +                }
> +                aml_append(ifctx2, ifctx3);
> +
> +                /*
> +                 * A return value of {0, 23, 1} indicates that
> +                 * operation 23 with argument 1 is pending.
> +                 */
> +                ifctx3 = aml_if(aml_equal(rev, aml_int(2)));
> +                {
> +                    /* TPM3[1] = PPRQ */
> +                    aml_append(ifctx3,
> +                               aml_store(pprq, aml_index(tpm3, aml_int(1))));
> +                    /* TPM3[2] = PPRM */
> +                    aml_append(ifctx3,
> +                               aml_store(pprm, aml_index(tpm3, aml_int(2))));
> +                    aml_append(ifctx3, aml_return(tpm3));
> +                }
> +                aml_append(ifctx2, ifctx3);
> +            }
> +            aml_append(ifctx, ifctx2);
> +
> +            /*
> +             * PPI 1.0: 2.1.5 Get Platform-Specific Action to Transition to
> +             *     Pre-OS Environment
> +             *
> +             * Arg 2 (Integer): Function Index = 4
> +             * Arg 3 (Package): Arguments = Empty Package
> +             * Returns: Type: Integer
> +             *          0: None
> +             *          1: Shutdown
> +             *          2: Reboot
> +             *          3: OS Vendor-specific
> +             */
> +            ifctx2 = aml_if(aml_equal(function, aml_int(4)));
> +            {
> +                /* reboot */
> +                aml_append(ifctx2, aml_return(aml_int(2)));
> +            }
> +            aml_append(ifctx, ifctx2);
> +
> +            /*
> +             * PPI 1.0: 2.1.6 Return TPM Operation Response to OS Environment
> +             *
> +             * Arg 2 (Integer): Function Index = 5
> +             * Arg 3 (Package): Arguments = Empty Package
> +             * Returns: Type: Package of Integer
> +             *          Integer 1: Function Return code
> +             *                     0: Success
> +             *                     1: General Failure
> +             *          Integer 2: Most recent operation request
> +             *                     0: None
> +             *                    >0: Operation Value of the most recent request
> +             *          Integer 3: Response to the most recent operation request
> +             *                     0: Success
> +             *                     0x00000001..0x00000FFF: Corresponding TPM
> +             *                                             error code
> +             *                     0xFFFFFFF0: User Abort or timeout of dialog
> +             *                     0xFFFFFFF1: firmware Failure
> +             */
> +            ifctx2 = aml_if(aml_equal(function, aml_int(5)));
> +            {
> +                /* TPM3[1] = LPPR */
> +                aml_append(ifctx2,
> +                           aml_store(aml_name("LPPR"),
> +                                     aml_index(tpm3, aml_int(1))));
> +                /* TPM3[2] = PPRP */
> +                aml_append(ifctx2,
> +                           aml_store(aml_name("PPRP"),
> +                                     aml_index(tpm3, aml_int(2))));
> +                aml_append(ifctx2, aml_return(tpm3));
> +            }
> +            aml_append(ifctx, ifctx2);
> +
> +            /*
> +             * PPI 1.0: 2.1.7 Submit preferred user language
> +             *
> +             * Arg 2 (Integer): Function Index = 6
> +             * Arg 3 (Package): Arguments = String Package
> +             *                  Preferred language code
> +             * Returns: Type: Integer
> +             * Function Return Code
> +             *          3: Not implemented
> +             */
> +            ifctx2 = aml_if(aml_equal(function, aml_int(6)));
> +            {
> +                /* 3 = not implemented */
> +                aml_append(ifctx2, aml_return(aml_int(3)));
> +            }
> +            aml_append(ifctx, ifctx2);
> +
> +            /*
> +             * PPI 1.1: 2.1.7 Submit TPM Operation Request to
> +             *     Pre-OS Environment 2
> +             *
> +             * Arg 2 (Integer): Function Index = 7
> +             * Arg 3 (Package): Arguments = Package: Type: Integer
> +             *                  Integer 1: Operation Value of the Request
> +             *                  Integer 2: Argument for Operation (optional)
> +             * Returns: Type: Integer
> +             *          0: Success
> +             *          1: Not Implemented
> +             *          2: General Failure
> +             *          3: Operation blocked by current firmware settings
> +             */
> +            ifctx2 = aml_if(aml_equal(function, aml_int(7)));
> +            {
> +                /* get opcode */
> +                arguments = aml_arg(3);
> +                op = aml_local(0);
> +                op_flags = aml_local(1);
> +                aml_append(ifctx2, aml_store(aml_derefof(aml_index(arguments,
> +                                                                   aml_int(0))),
> +                                             op));
> +
> +                /* get opcode flags */
> +                aml_append(ifctx2, aml_store(aml_call1("TPFN", op),
> +                                             op_flags));
> +                /* if func[opcode] & TPM_PPI_FUNC_NOT_IMPLEMENTED */
> +                ifctx3 = aml_if(
> +                    aml_equal(
> +                        aml_and(op_flags, aml_int(TPM_PPI_FUNC_MASK), NULL),
> +                        aml_int(TPM_PPI_FUNC_NOT_IMPLEMENTED)));
> +                {
> +                    /* 1: not implemented */
> +                    aml_append(ifctx3, aml_return(aml_int(1)));
> +                }
> +                aml_append(ifctx2, ifctx3);
> +
> +                /* if func[opcode] & TPM_PPI_FUNC_BLOCKED */
> +                ifctx3 = aml_if(
> +                    aml_equal(
> +                        aml_and(op_flags, aml_int(TPM_PPI_FUNC_MASK), NULL),
> +                        aml_int(TPM_PPI_FUNC_BLOCKED)));
> +                {
> +                    /* 3: blocked by firmware */
> +                    aml_append(ifctx3, aml_return(aml_int(3)));
> +                }
> +                aml_append(ifctx2, ifctx3);
> +
> +                /* revision to integer */
> +                ifctx3 = aml_if(aml_equal(rev, aml_int(1)));
> +                {
> +                    /* revision 1 */
> +                    /* PPRQ = op */
> +                    aml_append(ifctx3, aml_store(op, pprq));
> +                    /* no argument, PPRM = 0 */
> +                    aml_append(ifctx3, aml_store(aml_int(0), pprm));
> +                }
> +                aml_append(ifctx2, ifctx3);
> +
> +                ifctx3 = aml_if(aml_equal(rev, aml_int(2)));
> +                {
> +                    /* revision 2 */
> +                    /* PPRQ = op */
> +                    arguments = aml_arg(3);
> +                    op_arg = aml_derefof(aml_index(arguments, aml_int(1)));
> +                    aml_append(ifctx3, aml_store(op, pprq));
> +                    /* PPRM = arg3[1] */
> +                    aml_append(ifctx3, aml_store(op_arg, pprm));
> +                }
> +                aml_append(ifctx2, ifctx3);
> +                /* 0: success */
> +                aml_append(ifctx2, aml_return(aml_int(0)));
> +            }
> +            aml_append(ifctx, ifctx2);
> +
> +            /*
> +             * 8.1.8 Get User Confirmation Status for Operation
> +             *
> +             * Arg 2 (Integer): Function Index = 8
> +             * Arg 3 (Package): Arguments = Package: Type: Integer
> +             *                  Operation Value that may need user confirmation
> +             * Returns: Type: Integer
> +             *          0: Not implemented
> +             *          1: Firmware only
> +             *          2: Blocked for OS by firmware configuration
> +             *          3: Allowed and physically present user required
> +             *          4: Allowed and physically present user not required
> +             */
> +            ifctx2 = aml_if(aml_equal(function, aml_int(8)));
> +            {
> +                /* get opcode */
> +                arguments = aml_arg(3);
> +                op = aml_local(0);
> +                op_flags = aml_local(1);
> +                aml_append(ifctx2,
> +                           aml_store(aml_derefof(aml_index(arguments,
> +                                                           aml_int(0))),
> +                                     op));
> +
> +                /* get opcode flags */
> +                aml_append(ifctx2, aml_store(aml_call1("TPFN", op),
> +                                             op_flags));
> +                /* return confirmation status code */
> +                aml_append(ifctx2,
> +                           aml_return(
> +                               aml_and(op_flags,
> +                                       aml_int(TPM_PPI_FUNC_MASK), NULL)));
> +            }
> +            aml_append(ifctx, ifctx2);
> +
> +            aml_append(ifctx, aml_return(aml_buffer(1, zerobyte)));
> +        }
> +        aml_append(method, ifctx);
> +    }
> +    aml_append(dev, method);
> +}
> +
>  static void
>  build_dsdt(GArray *table_data, BIOSLinker *linker,
>             AcpiPmInfo *pm, AcpiMiscInfo *misc,
> @@ -1802,6 +2193,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>      uint32_t nr_mem = machine->ram_slots;
>      int root_bus_limit = 0xFF;
>      PCIBus *bus = NULL;
> +    TPMIf *tpm = tpm_find();
>      int i;
>  
>      dsdt = init_aml_allocator();
> @@ -2139,7 +2531,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>              /* Scan all PCI buses. Generate tables to support hotplug. */
>              build_append_pci_bus_devices(scope, bus, pm->pcihp_bridge_en);
>  
> -            if (TPM_IS_TIS(tpm_find())) {
> +            if (TPM_IS_TIS(tpm)) {
>                  dev = aml_device("ISA.TPM");
>                  aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0C31")));
>                  aml_append(dev, aml_name_decl("_STA", aml_int(0xF)));
> @@ -2153,6 +2545,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>                   */
>                  /* aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ)); */
>                  aml_append(dev, aml_name_decl("_CRS", crs));
> +
> +                build_tpm_ppi(tpm, dev);
> +
>                  aml_append(scope, dev);
>              }
>  
> @@ -2160,7 +2555,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>          }
>      }
>  
> -    if (TPM_IS_CRB(tpm_find())) {
> +    if (TPM_IS_CRB(tpm)) {
>          dev = aml_device("TPM");
>          aml_append(dev, aml_name_decl("_HID", aml_string("MSFT0101")));
>          crs = aml_resource_template();
> @@ -2172,6 +2567,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>          aml_append(method, aml_return(aml_int(0x0f)));
>          aml_append(dev, method);
>  
> +        build_tpm_ppi(tpm, dev);
> +
>          aml_append(sb_scope, dev);
>      }
>  
> @@ -2920,7 +3317,7 @@ void acpi_setup(void)
>          tpm_config = (FWCfgTPMConfig) {
>              .tpmppi_address = cpu_to_le32(TPM_PPI_ADDR_BASE),
>              .tpm_version = tpm_get_version(tpm_find()),
> -            .tpmppi_version = TPM_PPI_VERSION_NONE
> +            .tpmppi_version = TPM_PPI_VERSION_1_30
>          };
>          fw_cfg_add_file(pcms->fw_cfg, "etc/tpm/config",
>                          &tpm_config, sizeof tpm_config);
> diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
> index 2ddb768084..c27762c723 100644
> --- a/docs/specs/tpm.txt
> +++ b/docs/specs/tpm.txt
> @@ -62,6 +62,85 @@ URL:
>  
>  https://trustedcomputinggroup.org/tcg-acpi-specification/
>  
> +== ACPI PPI Interface ==
> +
> +QEMU supports the Physical Presence Interface (PPI) for TPM 1.2 and TPM 2. This
> +interface requires ACPI and firmware support. The specification can be found at
> +the following URL:
> +
> +https://trustedcomputinggroup.org/resource/tcg-physical-presence-interface-specification/
> +
> +PPI enables a system administrator (root) to request a modification to the
> +TPM upon reboot. The PPI specification defines the operation requests and the
> +actions the firmware has to take. The system administrator passes the operation
> +request number to the firmware through an ACPI interface which writes this
> +number to a memory location that the firmware knows. Upon reboot, the firmware
> +finds the number and sends commands to the the TPM. The firmware writes the TPM
> +result code and the operation request number to a memory location that ACPI can
> +read from and pass the result on to the administrator.
> +
> +The PPI specification defines a set of mandatory and optional operations for
> +the firmware to implement. The ACPI interface also allows an administrator to
> +list the supported operations. In QEMU the ACPI code is generated by QEMU, yet
> +the firmware needs to implement support on a per-operations basis, and
> +different firmwares may support a different subset. Therefore, QEMU introduces
> +the virtual memory device for PPI where the firmware can indicate which
> +operations it supports and ACPI can enable the ones that are supported and
> +disable all others. This interface lies in main memory and has the following
> +layout:
> +
> + +----------+--------+--------+-------------------------------------------+
> + |  Field   | Length | Offset | Description                               |
> + +----------+--------+--------+-------------------------------------------+
> + | func     |  0x100 |  0x000 | Firmware sets values for each supported   |
> + |          |        |        | operation. See defined values below.      |
> + +----------+--------+--------+-------------------------------------------+
> + | ppin     |   0x1  |  0x100 | SMI interrupt to use. Set by firmware.    |
> + |          |        |        | Not supported.                            |
> + +----------+--------+--------+-------------------------------------------+
> + | ppip     |   0x4  |  0x101 | ACPI function index to pass to SMM code.  |
> + |          |        |        | Set by ACPI. Not supported.               |
> + +----------+--------+--------+-------------------------------------------+
> + | pprp     |   0x4  |  0x105 | Result of last executed operation. Set by |
> + |          |        |        | firmware. See function index 5 for values.|
> + +----------+--------+--------+-------------------------------------------+
> + | pprq     |   0x4  |  0x109 | Operation request number to execute. See  |
> + |          |        |        | 'Physical Presence Interface Operation    |
> + |          |        |        | Summary' tables in specs. Set by ACPI.    |
> + +----------+--------+--------+-------------------------------------------+
> + | pprm     |   0x4  |  0x10d | Operation request optional parameter.     |
> + |          |        |        | Values depend on operation. Set by ACPI.  |
> + +----------+--------+--------+-------------------------------------------+
> + | lppr     |   0x4  |  0x111 | Last executed operation request number.   |
> + |          |        |        | Copied from pprq field by firmware.       |
> + +----------+--------+--------+-------------------------------------------+
> + | fret     |   0x4  |  0x115 | Result code from SMM function.            |
> + |          |        |        | Not supported.                            |
> + +----------+--------+--------+-------------------------------------------+
> + | res1     |  0x40  |  0x119 | Reserved for future use                   |
> + +----------+--------+--------+-------------------------------------------+
> + | next_step|   0x1  |  0x159 | Operation to execute after reboot by      |
> + |          |        |        | firmware. Used by firmware.               |
> + +----------+--------+--------+-------------------------------------------+
> +
> +   The following values are supported for the 'func' field. They correspond
> +   to the values used by ACPI function index 8.
> +
> + +----------+-------------------------------------------------------------+
> + | value    | Description                                                 |
> + +----------+-------------------------------------------------------------+
> + | 0        | Operation is not implemented.                               |
> + +----------+-------------------------------------------------------------+
> + | 1        | Operation is only accessible through firmware.              |
> + +----------+-------------------------------------------------------------+
> + | 2        | Operation is blocked for OS by firmware configuration.      |
> + +----------+-------------------------------------------------------------+
> + | 3        | Operation is allowed and physically present user required.  |
> + +----------+-------------------------------------------------------------+
> + | 4        | Operation is allowed and physically present user is not     |
> + |          | required.                                                   |
> + +----------+-------------------------------------------------------------+
> +
>  
>  QEMU files related to TPM ACPI tables:
>   - hw/i386/acpi-build.c

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

[Marc-André Lureau]

HI

On Wed, Jul 4, 2018 at 5:39 PM, Igor Mammedov <imammedo@redhat.com> wrote:
> On Thu, 28 Jun 2018 19:26:57 +0200
> Marc-André Lureau <marcandre.lureau@redhat.com> 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>
>> [ Marc-André - ACPI code improvements and windows fixes ]
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> ---
>>
>> v7: (Marc-André)
>>  - use first spec version/section in code comments
>>  - use more variables for ASL code building
>>  - remove unnecessering ToInteger() calls
>>
>> v6:
>>  - more code documentation (Marc-André)
>>  - use some explicit named variables to ease reading (Marc-André)
>>  - use fixed size fields/memory regions, remove PPI struct (Marc-André)
>>  - only add PPI ACPI methods if PPI is enabled (Marc-André)
>>  - document the qemu/firmware ACPI memory region (Stefan)
>>
>> v5 (Marc-André):
>>  - /struct tpm_ppi/struct TPMPPIData
>>
>> 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 |   8 +
>>  hw/i386/acpi-build.c  | 403 +++++++++++++++++++++++++++++++++++++++++-
>>  docs/specs/tpm.txt    |  79 +++++++++
>>  3 files changed, 487 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
>> index f79d68a77a..e0bd07862e 100644
>> --- a/include/hw/acpi/tpm.h
>> +++ b/include/hw/acpi/tpm.h
>> @@ -196,4 +196,12 @@ REG32(CRB_DATA_BUFFER, 0x80)
>>  #define TPM_PPI_VERSION_NONE        0
>>  #define TPM_PPI_VERSION_1_30        1
>>
>> +/* whether function is blocked by BIOS settings; bits 0, 1, 2 */
>> +#define TPM_PPI_FUNC_NOT_IMPLEMENTED     (0 << 0)
>> +#define TPM_PPI_FUNC_BIOS_ONLY           (1 << 0)
>> +#define TPM_PPI_FUNC_BLOCKED             (2 << 0)
>> +#define TPM_PPI_FUNC_ALLOWED_USR_REQ     (3 << 0)
>> +#define TPM_PPI_FUNC_ALLOWED_USR_NOT_REQ (4 << 0)
>> +#define TPM_PPI_FUNC_MASK                (7 << 0)
>> +
>>  #endif /* HW_ACPI_TPM_H */
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index ebd64c4818..263677f3e4 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -43,6 +43,7 @@
>>  #include "hw/acpi/memory_hotplug.h"
>>  #include "sysemu/tpm.h"
>>  #include "hw/acpi/tpm.h"
>> +#include "hw/tpm/tpm_ppi.h"
>>  #include "hw/acpi/vmgenid.h"
>>  #include "sysemu/tpm_backend.h"
>>  #include "hw/timer/mc146818rtc_regs.h"
>> @@ -1789,6 +1790,396 @@ static Aml *build_q35_osc_method(void)
>>      return method;
>>  }
>>
>> +static void
>> +build_tpm_ppi(TPMIf *tpm, Aml *dev)
>> +{
>> +    Aml *method, *field, *ifctx, *ifctx2, *ifctx3;
>> +    Aml *pak, *tpm2, *tpm3, *pprm, *pprq;
>> +    int i;
>> +
>> +    if (!object_property_get_bool(OBJECT(tpm), "ppi", &error_abort)) {
>> +        return;
>> +    }
> I'd prefer this being checked by caller, like:
>
> @@ -2194,6 +2194,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>      int root_bus_limit = 0xFF;
>      PCIBus *bus = NULL;
>      TPMIf *tpm = tpm_find();
> +    bool has_ppi = object_property_get_bool(OBJECT(tpm), "ppi", &error_abort);

it will need an additional "tpm ? .. : false". I'd rather keep it away
from build_dsdt(), but your call.

>      int i;
>
>      dsdt = init_aml_allocator();
> @@ -2545,8 +2546,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>                   */
>                  /* aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ)); */
>                  aml_append(dev, aml_name_decl("_CRS", crs));
> -
> -                build_tpm_ppi(tpm, dev);
> +                if (has_ppi) {
> +                    build_tpm_ppi(tpm, dev);
> +                }
>
>                  aml_append(scope, dev);
>              }
> @@ -2567,7 +2569,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>          aml_append(method, aml_return(aml_int(0x0f)));
>          aml_append(dev, method);
>
> -        build_tpm_ppi(tpm, dev);
> +        if (has_ppi) {
> +            build_tpm_ppi(tpm, dev);
> +        }
>
>          aml_append(sb_scope, dev);
>      }
>
>> +    /*
>> +     * TPP1 is for the flags that indicate which PPI operations
>> +     * are supported by the firmware. The firmware is expected to
>> +     * write these flags.
>> +     */
>> +    aml_append(dev,
>> +               aml_operation_region("TPP1", AML_SYSTEM_MEMORY,
>> +                                    aml_int(TPM_PPI_ADDR_BASE), 0x100));
>> +    field = aml_field("TPP1", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
>> +    for (i = 0; i < 0x100; i++) {
>> +        char *tmp = g_strdup_printf("FN%02X", i);
>> +        aml_append(field, aml_named_field(tmp, 8));
>> +        g_free(tmp);
>> +    }
> this and TPFN takes more than 4K.
> Have you tried to declare only one filed of size TPP1, which makes
> field Buffer type, which could be used with Index and would take only
> several bytes to in AML. See ACPI 6.2a 19.6.61.2 Index with Buffers.
> It also looks very portable (but we should test with win xp just to be safe)

You mean like Stefan originally did? Yes, problem described in
http://www.osronline.com/showThread.CFM?link=288617.

>
>> +    aml_append(dev, field);
>> +
>> +    /*
>> +     * TPP2 is for the registers that ACPI code used to pass
>> +     * the PPI code and parameter (PPRQ, PPRM) to the firmware.
>> +     */
>> +    aml_append(dev,
>> +               aml_operation_region("TPP2", AML_SYSTEM_MEMORY,
>> +                                    aml_int(TPM_PPI_ADDR_BASE + 0x100),
>> +                                    0x5A));
>> +    field = aml_field("TPP2", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
>> +    aml_append(field, aml_named_field("PPIN", 8));
>> +    aml_append(field, aml_named_field("PPIP", 32));
>> +    aml_append(field, aml_named_field("PPRP", 32));
>> +    aml_append(field, aml_named_field("PPRQ", 32));
>> +    aml_append(field, aml_named_field("PPRM", 32));
>> +    aml_append(field, aml_named_field("LPPR", 32));
>> +    aml_append(dev, field);
>> +    pprq = aml_name("PPRQ");
>> +    pprm = aml_name("PPRM");
>> +
>> +    /*
>> +     * A function to return the value of DerefOf (FUNC [N]), by using
>> +     * accessing the fields individually instead. This is a workaround
>> +     * for what looks like a Windows ACPI bug in all versions so far
>> +     * (fwiw, DerefOf (FUNC [N]) works on Linux).
>> +     */
>> +    method = aml_method("TPFN", 1, AML_SERIALIZED);
>> +    {
>> +        for (i = 0; i < 0x100; i++) {
>> +            ifctx = aml_if(aml_equal(aml_int(i), aml_arg(0)));
>> +            {
>> +                aml_append(ifctx, aml_return(aml_name("FN%02X", i)));
>> +            }
>> +            aml_append(method, ifctx);
>> +        }
>> +        aml_append(method, aml_return(aml_int(0)));
>> +    }
>> +    aml_append(dev, method);
>> +
>> +    pak = aml_package(2);
>> +    aml_append(pak, aml_int(0));
>> +    aml_append(pak, aml_int(0));
>> +    aml_append(dev, aml_name_decl("TPM2", pak));
>> +    tpm2 = aml_name("TPM2");
>> +
>> +    pak = aml_package(3);
>> +    aml_append(pak, aml_int(0));
>> +    aml_append(pak, aml_int(0));
>> +    aml_append(pak, aml_int(0));
>> +    aml_append(dev, aml_name_decl("TPM3", pak));
>> +    tpm3 = aml_name("TPM3");
> looks like above 2 packages  are used only by _DSM,
> so you don't need to make them global, just use local variables within _DSM for it
>

That would also fail on Windows. Fwiw (this time I am sure) my up to
date t460p uses the same global variables trick, presumably to satisfy
Windows as well.

btw, not only it took me a while to find the shortcomings with Windows
ACPI, but testing any change in the ACPI code takes ~1.5h (with manual
intervention ever 10 minutes or so, not a lot of fun)

>> +
>> +    method = aml_method("_DSM", 4, AML_SERIALIZED);
>> +    {
>> +        uint8_t zerobyte[1] = { 0 };
>> +        Aml *function, *arguments, *rev, *op, *op_arg, *op_flags, *uuid;
>> +
>> +        uuid = aml_arg(0);
>> +        rev = aml_arg(1);
>> +        function = aml_arg(2);
>> +        ifctx = aml_if(
>> +            aml_equal(uuid,
>> +                      aml_touuid("3DDDFAA6-361B-4EB4-A424-8D10089D1653")));
>> +        {
> it would be better to exit immediately if UUID doesn't match (less indentation and if contexts)
> btw, one should return something if uuid doesn't match method,
>
> if (LNotEqual(uuid, ToUUID(“893f00a6-660c-494e-bcfd-3043f4fb67c0”))) {
>     return(Buffer(){0})
> }
>

Except that the next patch has another uuid to check (and it's not a
common ACPI style to return early it seems).

>> +            /* standard DSM query function */
>> +            ifctx2 = aml_if(aml_equal(function, aml_int(0)));
> constants aml_int(0..3) are used multiple times, suggest to declare them
> along with rev/function/..., like:
>
>  Aml *one = aml_int(0);
>  ...
>
> and then use them through code.

I don't see that as an improvement tbh, it makes the code
inconsistent. So you would like to have zero and one, but not others?
ok, at this point I don't think I care enough :).

>
>
>> +            {
>> +                uint8_t byte_list[2] = { 0xff, 0x01 };
>> +                aml_append(ifctx2, aml_return(aml_buffer(2, byte_list)));
>> +            }
>> +            aml_append(ifctx, ifctx2);
>> +
>> +            /*
>> +             * PPI 1.0: 2.1.1 Get Physical Presence Interface Version
>> +             *
>> +             * Arg 2 (Integer): Function Index = 1
>> +             * Arg 3 (Package): Arguments = Empty Package
>> +             * Returns: Type: String
>> +             */
>> +            ifctx2 = aml_if(aml_equal(function, aml_int(1)));
>> +            {
>> +                aml_append(ifctx2, aml_return(aml_string("1.3")));
>> +            }
>> +            aml_append(ifctx, ifctx2);
>> +
>> +            /*
>> +             * PPI 1.0: 2.1.3 Submit TPM Operation Request to Pre-OS Environment
>> +             *
>> +             * Arg 2 (Integer): Function Index = 2
>> +             * Arg 3 (Package): Arguments = Package: Type: Integer
>> +             *                              Operation Value of the Request
>> +             * Returns: Type: Integer
>> +             *          0: Success
>> +             *          1: Operation Value of the Request Not Supported
>> +             *          2: General Failure
>> +             */
>> +            ifctx2 = aml_if(aml_equal(function, aml_int(2)));
>> +            {
>> +                /* get opcode */
>> +                arguments = aml_arg(3);
>> +                op = aml_local(0);
>> +                op_flags = aml_local(1);
> suggest to move these 3 close to function/rev/... as they are repeated several times
> with the same meaning in several if contexts.

ok

>
>> +                aml_append(ifctx2,
>> +                           aml_store(aml_derefof(aml_index(arguments,
>> +                                                           aml_int(0))), op));
>> +
>> +                /* get opcode flags */
>> +                aml_append(ifctx2,
>> +                           aml_store(aml_call1("TPFN", op), op_flags));
>> +
>> +                /* if func[opcode] & TPM_PPI_FUNC_NOT_IMPLEMENTED */
>> +                ifctx3 = aml_if(
>> +                    aml_equal(
>> +                        aml_and(op_flags, aml_int(TPM_PPI_FUNC_MASK), NULL),
>> +                        aml_int(TPM_PPI_FUNC_NOT_IMPLEMENTED)));
> same as for aml_int(0) TPM_PPI_FUNC_MASK/TPM_PPI_FUNC_NOT_IMPLEMENTED and other TPM_PPI_*
> constants that are user as aml_int(), it might mkae code more readable with
> shorter lines and shorter variable names.

ok

>
>> +                {
>> +                    /* 1: Operation Value of the Request Not Supported */
>> +                    aml_append(ifctx3, aml_return(aml_int(1)));
>> +                }
>> +                aml_append(ifctx2, ifctx3);
>> +
>> +                aml_append(ifctx2, aml_store(op, pprq));
>> +                aml_append(ifctx2, aml_store(aml_int(0), pprm));
>> +                /* 0: success */
>> +                aml_append(ifctx2, aml_return(aml_int(0)));
>> +            }
>> +            aml_append(ifctx, ifctx2);
>> +
>> +            /*
>> +             * PPI 1.0: 2.1.4 Get Pending TPM Operation Requested By the OS
>> +             *
>> +             * Arg 2 (Integer): Function Index = 3
>> +             * Arg 3 (Package): Arguments = Empty Package
>> +             * Returns: Type: Package of Integers
>> +             *          Integer 1: Function Return code
>> +             *                     0: Success
>> +             *                     1: General Failure
>> +             *          Integer 2: Pending operation requested by the OS
>> +             *                     0: None
>> +             *                    >0: Operation Value of the Pending Request
>> +             *          Integer 3: Optional argument to pending operation
>> +             *                     requested by the OS
>> +             *                     0: None
>> +             *                    >0: Argument Value of the Pending Request
>> +             */
>> +            ifctx2 = aml_if(aml_equal(function, aml_int(3)));
>> +            {
>> +                /*
>> +                 * Revision ID of 1, no integer parameter beyond
>> +                 * parameter two are expected
>> +                 */
>> +                ifctx3 = aml_if(aml_equal(rev, aml_int(1)));
>> +                {
>> +                    /* TPM2[1] = PPRQ */
>> +                    aml_append(ifctx3,
>> +                               aml_store(pprq, aml_index(tpm2, aml_int(1))));
>> +                    aml_append(ifctx3, aml_return(tpm2));
>> +                }
>> +                aml_append(ifctx2, ifctx3);
>> +
>> +                /*
>> +                 * A return value of {0, 23, 1} indicates that
>> +                 * operation 23 with argument 1 is pending.
>> +                 */
>> +                ifctx3 = aml_if(aml_equal(rev, aml_int(2)));
>> +                {
>> +                    /* TPM3[1] = PPRQ */
>> +                    aml_append(ifctx3,
>> +                               aml_store(pprq, aml_index(tpm3, aml_int(1))));
>> +                    /* TPM3[2] = PPRM */
>> +                    aml_append(ifctx3,
>> +                               aml_store(pprm, aml_index(tpm3, aml_int(2))));
>> +                    aml_append(ifctx3, aml_return(tpm3));
>> +                }
>> +                aml_append(ifctx2, ifctx3);
>> +            }
>> +            aml_append(ifctx, ifctx2);
>> +
>> +            /*
>> +             * PPI 1.0: 2.1.5 Get Platform-Specific Action to Transition to
>> +             *     Pre-OS Environment
>> +             *
>> +             * Arg 2 (Integer): Function Index = 4
>> +             * Arg 3 (Package): Arguments = Empty Package
>> +             * Returns: Type: Integer
>> +             *          0: None
>> +             *          1: Shutdown
>> +             *          2: Reboot
>> +             *          3: OS Vendor-specific
>> +             */
>> +            ifctx2 = aml_if(aml_equal(function, aml_int(4)));
>> +            {
>> +                /* reboot */
>> +                aml_append(ifctx2, aml_return(aml_int(2)));
>> +            }
>> +            aml_append(ifctx, ifctx2);
>> +
>> +            /*
>> +             * PPI 1.0: 2.1.6 Return TPM Operation Response to OS Environment
>> +             *
>> +             * Arg 2 (Integer): Function Index = 5
>> +             * Arg 3 (Package): Arguments = Empty Package
>> +             * Returns: Type: Package of Integer
>> +             *          Integer 1: Function Return code
>> +             *                     0: Success
>> +             *                     1: General Failure
>> +             *          Integer 2: Most recent operation request
>> +             *                     0: None
>> +             *                    >0: Operation Value of the most recent request
>> +             *          Integer 3: Response to the most recent operation request
>> +             *                     0: Success
>> +             *                     0x00000001..0x00000FFF: Corresponding TPM
>> +             *                                             error code
>> +             *                     0xFFFFFFF0: User Abort or timeout of dialog
>> +             *                     0xFFFFFFF1: firmware Failure
>> +             */
>> +            ifctx2 = aml_if(aml_equal(function, aml_int(5)));
>> +            {
>> +                /* TPM3[1] = LPPR */
>> +                aml_append(ifctx2,
>> +                           aml_store(aml_name("LPPR"),
>> +                                     aml_index(tpm3, aml_int(1))));
>> +                /* TPM3[2] = PPRP */
>> +                aml_append(ifctx2,
>> +                           aml_store(aml_name("PPRP"),
>> +                                     aml_index(tpm3, aml_int(2))));
>> +                aml_append(ifctx2, aml_return(tpm3));
>> +            }
>> +            aml_append(ifctx, ifctx2);
>> +
>> +            /*
>> +             * PPI 1.0: 2.1.7 Submit preferred user language
>> +             *
>> +             * Arg 2 (Integer): Function Index = 6
>> +             * Arg 3 (Package): Arguments = String Package
>> +             *                  Preferred language code
>> +             * Returns: Type: Integer
>> +             * Function Return Code
>> +             *          3: Not implemented
>> +             */
>> +            ifctx2 = aml_if(aml_equal(function, aml_int(6)));
>> +            {
>> +                /* 3 = not implemented */
>> +                aml_append(ifctx2, aml_return(aml_int(3)));
>> +            }
>> +            aml_append(ifctx, ifctx2);
>> +
>> +            /*
>> +             * PPI 1.1: 2.1.7 Submit TPM Operation Request to
>> +             *     Pre-OS Environment 2
>> +             *
>> +             * Arg 2 (Integer): Function Index = 7
>> +             * Arg 3 (Package): Arguments = Package: Type: Integer
>> +             *                  Integer 1: Operation Value of the Request
>> +             *                  Integer 2: Argument for Operation (optional)
>> +             * Returns: Type: Integer
>> +             *          0: Success
>> +             *          1: Not Implemented
>> +             *          2: General Failure
>> +             *          3: Operation blocked by current firmware settings
>> +             */
>> +            ifctx2 = aml_if(aml_equal(function, aml_int(7)));
>> +            {
>> +                /* get opcode */
>> +                arguments = aml_arg(3);
>> +                op = aml_local(0);
>> +                op_flags = aml_local(1);
>> +                aml_append(ifctx2, aml_store(aml_derefof(aml_index(arguments,
>> +                                                                   aml_int(0))),
>> +                                             op));
>> +
>> +                /* get opcode flags */
>> +                aml_append(ifctx2, aml_store(aml_call1("TPFN", op),
>> +                                             op_flags));
>> +                /* if func[opcode] & TPM_PPI_FUNC_NOT_IMPLEMENTED */
>> +                ifctx3 = aml_if(
>> +                    aml_equal(
>> +                        aml_and(op_flags, aml_int(TPM_PPI_FUNC_MASK), NULL),
>> +                        aml_int(TPM_PPI_FUNC_NOT_IMPLEMENTED)));
>> +                {
>> +                    /* 1: not implemented */
>> +                    aml_append(ifctx3, aml_return(aml_int(1)));
>> +                }
>> +                aml_append(ifctx2, ifctx3);
>> +
>> +                /* if func[opcode] & TPM_PPI_FUNC_BLOCKED */
>> +                ifctx3 = aml_if(
>> +                    aml_equal(
>> +                        aml_and(op_flags, aml_int(TPM_PPI_FUNC_MASK), NULL),
>> +                        aml_int(TPM_PPI_FUNC_BLOCKED)));
>> +                {
>> +                    /* 3: blocked by firmware */
>> +                    aml_append(ifctx3, aml_return(aml_int(3)));
>> +                }
>> +                aml_append(ifctx2, ifctx3);
>> +
>> +                /* revision to integer */
>> +                ifctx3 = aml_if(aml_equal(rev, aml_int(1)));
>> +                {
>> +                    /* revision 1 */
>> +                    /* PPRQ = op */
>> +                    aml_append(ifctx3, aml_store(op, pprq));
>> +                    /* no argument, PPRM = 0 */
>> +                    aml_append(ifctx3, aml_store(aml_int(0), pprm));
>> +                }
>> +                aml_append(ifctx2, ifctx3);
>> +
>> +                ifctx3 = aml_if(aml_equal(rev, aml_int(2)));
>> +                {
>> +                    /* revision 2 */
>> +                    /* PPRQ = op */
>> +                    arguments = aml_arg(3);
>> +                    op_arg = aml_derefof(aml_index(arguments, aml_int(1)));
>> +                    aml_append(ifctx3, aml_store(op, pprq));
>> +                    /* PPRM = arg3[1] */
>> +                    aml_append(ifctx3, aml_store(op_arg, pprm));
>> +                }
>> +                aml_append(ifctx2, ifctx3);
>> +                /* 0: success */
>> +                aml_append(ifctx2, aml_return(aml_int(0)));
>> +            }
>> +            aml_append(ifctx, ifctx2);
>> +
>> +            /*
>> +             * 8.1.8 Get User Confirmation Status for Operation
>> +             *
>> +             * Arg 2 (Integer): Function Index = 8
>> +             * Arg 3 (Package): Arguments = Package: Type: Integer
>> +             *                  Operation Value that may need user confirmation
>> +             * Returns: Type: Integer
>> +             *          0: Not implemented
>> +             *          1: Firmware only
>> +             *          2: Blocked for OS by firmware configuration
>> +             *          3: Allowed and physically present user required
>> +             *          4: Allowed and physically present user not required
>> +             */
>> +            ifctx2 = aml_if(aml_equal(function, aml_int(8)));
>> +            {
>> +                /* get opcode */
>> +                arguments = aml_arg(3);
>> +                op = aml_local(0);
>> +                op_flags = aml_local(1);
>> +                aml_append(ifctx2,
>> +                           aml_store(aml_derefof(aml_index(arguments,
>> +                                                           aml_int(0))),
>> +                                     op));
>> +
>> +                /* get opcode flags */
>> +                aml_append(ifctx2, aml_store(aml_call1("TPFN", op),
>> +                                             op_flags));
>> +                /* return confirmation status code */
>> +                aml_append(ifctx2,
>> +                           aml_return(
>> +                               aml_and(op_flags,
>> +                                       aml_int(TPM_PPI_FUNC_MASK), NULL)));
>> +            }
>> +            aml_append(ifctx, ifctx2);
>> +
>> +            aml_append(ifctx, aml_return(aml_buffer(1, zerobyte)));
>> +        }
>> +        aml_append(method, ifctx);
>> +    }
>> +    aml_append(dev, method);
>> +}
>> +
>>  static void
>>  build_dsdt(GArray *table_data, BIOSLinker *linker,
>>             AcpiPmInfo *pm, AcpiMiscInfo *misc,
>> @@ -1802,6 +2193,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>>      uint32_t nr_mem = machine->ram_slots;
>>      int root_bus_limit = 0xFF;
>>      PCIBus *bus = NULL;
>> +    TPMIf *tpm = tpm_find();
>>      int i;
>>
>>      dsdt = init_aml_allocator();
>> @@ -2139,7 +2531,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>>              /* Scan all PCI buses. Generate tables to support hotplug. */
>>              build_append_pci_bus_devices(scope, bus, pm->pcihp_bridge_en);
>>
>> -            if (TPM_IS_TIS(tpm_find())) {
>> +            if (TPM_IS_TIS(tpm)) {
>>                  dev = aml_device("ISA.TPM");
>>                  aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0C31")));
>>                  aml_append(dev, aml_name_decl("_STA", aml_int(0xF)));
>> @@ -2153,6 +2545,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>>                   */
>>                  /* aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ)); */
>>                  aml_append(dev, aml_name_decl("_CRS", crs));
>> +
>> +                build_tpm_ppi(tpm, dev);
>> +
>>                  aml_append(scope, dev);
>>              }
>>
>> @@ -2160,7 +2555,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>>          }
>>      }
>>
>> -    if (TPM_IS_CRB(tpm_find())) {
>> +    if (TPM_IS_CRB(tpm)) {
>>          dev = aml_device("TPM");
>>          aml_append(dev, aml_name_decl("_HID", aml_string("MSFT0101")));
>>          crs = aml_resource_template();
>> @@ -2172,6 +2567,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>>          aml_append(method, aml_return(aml_int(0x0f)));
>>          aml_append(dev, method);
>>
>> +        build_tpm_ppi(tpm, dev);
>> +
>>          aml_append(sb_scope, dev);
>>      }
>>
>> @@ -2920,7 +3317,7 @@ void acpi_setup(void)
>>          tpm_config = (FWCfgTPMConfig) {
>>              .tpmppi_address = cpu_to_le32(TPM_PPI_ADDR_BASE),
>>              .tpm_version = tpm_get_version(tpm_find()),
>> -            .tpmppi_version = TPM_PPI_VERSION_NONE
>> +            .tpmppi_version = TPM_PPI_VERSION_1_30
>>          };
>>          fw_cfg_add_file(pcms->fw_cfg, "etc/tpm/config",
>>                          &tpm_config, sizeof tpm_config);
>> diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
>> index 2ddb768084..c27762c723 100644
>> --- a/docs/specs/tpm.txt
>> +++ b/docs/specs/tpm.txt
>> @@ -62,6 +62,85 @@ URL:
>>
>>  https://trustedcomputinggroup.org/tcg-acpi-specification/
>>
>> +== ACPI PPI Interface ==
>> +
>> +QEMU supports the Physical Presence Interface (PPI) for TPM 1.2 and TPM 2. This
>> +interface requires ACPI and firmware support. The specification can be found at
>> +the following URL:
>> +
>> +https://trustedcomputinggroup.org/resource/tcg-physical-presence-interface-specification/
>> +
>> +PPI enables a system administrator (root) to request a modification to the
>> +TPM upon reboot. The PPI specification defines the operation requests and the
>> +actions the firmware has to take. The system administrator passes the operation
>> +request number to the firmware through an ACPI interface which writes this
>> +number to a memory location that the firmware knows. Upon reboot, the firmware
>> +finds the number and sends commands to the the TPM. The firmware writes the TPM
>> +result code and the operation request number to a memory location that ACPI can
>> +read from and pass the result on to the administrator.
>> +
>> +The PPI specification defines a set of mandatory and optional operations for
>> +the firmware to implement. The ACPI interface also allows an administrator to
>> +list the supported operations. In QEMU the ACPI code is generated by QEMU, yet
>> +the firmware needs to implement support on a per-operations basis, and
>> +different firmwares may support a different subset. Therefore, QEMU introduces
>> +the virtual memory device for PPI where the firmware can indicate which
>> +operations it supports and ACPI can enable the ones that are supported and
>> +disable all others. This interface lies in main memory and has the following
>> +layout:
>> +
>> + +----------+--------+--------+-------------------------------------------+
>> + |  Field   | Length | Offset | Description                               |
>> + +----------+--------+--------+-------------------------------------------+
>> + | func     |  0x100 |  0x000 | Firmware sets values for each supported   |
>> + |          |        |        | operation. See defined values below.      |
>> + +----------+--------+--------+-------------------------------------------+
>> + | ppin     |   0x1  |  0x100 | SMI interrupt to use. Set by firmware.    |
>> + |          |        |        | Not supported.                            |
>> + +----------+--------+--------+-------------------------------------------+
>> + | ppip     |   0x4  |  0x101 | ACPI function index to pass to SMM code.  |
>> + |          |        |        | Set by ACPI. Not supported.               |
>> + +----------+--------+--------+-------------------------------------------+
>> + | pprp     |   0x4  |  0x105 | Result of last executed operation. Set by |
>> + |          |        |        | firmware. See function index 5 for values.|
>> + +----------+--------+--------+-------------------------------------------+
>> + | pprq     |   0x4  |  0x109 | Operation request number to execute. See  |
>> + |          |        |        | 'Physical Presence Interface Operation    |
>> + |          |        |        | Summary' tables in specs. Set by ACPI.    |
>> + +----------+--------+--------+-------------------------------------------+
>> + | pprm     |   0x4  |  0x10d | Operation request optional parameter.     |
>> + |          |        |        | Values depend on operation. Set by ACPI.  |
>> + +----------+--------+--------+-------------------------------------------+
>> + | lppr     |   0x4  |  0x111 | Last executed operation request number.   |
>> + |          |        |        | Copied from pprq field by firmware.       |
>> + +----------+--------+--------+-------------------------------------------+
>> + | fret     |   0x4  |  0x115 | Result code from SMM function.            |
>> + |          |        |        | Not supported.                            |
>> + +----------+--------+--------+-------------------------------------------+
>> + | res1     |  0x40  |  0x119 | Reserved for future use                   |
>> + +----------+--------+--------+-------------------------------------------+
>> + | next_step|   0x1  |  0x159 | Operation to execute after reboot by      |
>> + |          |        |        | firmware. Used by firmware.               |
>> + +----------+--------+--------+-------------------------------------------+
>> +
>> +   The following values are supported for the 'func' field. They correspond
>> +   to the values used by ACPI function index 8.
>> +
>> + +----------+-------------------------------------------------------------+
>> + | value    | Description                                                 |
>> + +----------+-------------------------------------------------------------+
>> + | 0        | Operation is not implemented.                               |
>> + +----------+-------------------------------------------------------------+
>> + | 1        | Operation is only accessible through firmware.              |
>> + +----------+-------------------------------------------------------------+
>> + | 2        | Operation is blocked for OS by firmware configuration.      |
>> + +----------+-------------------------------------------------------------+
>> + | 3        | Operation is allowed and physically present user required.  |
>> + +----------+-------------------------------------------------------------+
>> + | 4        | Operation is allowed and physically present user is not     |
>> + |          | required.                                                   |
>> + +----------+-------------------------------------------------------------+
>> +
>>
>>  QEMU files related to TPM ACPI tables:
>>   - hw/i386/acpi-build.c
>
>



-- 
Marc-André Lureau

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

[Igor Mammedov]

On Wed, 4 Jul 2018 18:00:41 +0200
Marc-André Lureau <marcandre.lureau@gmail.com> wrote:

> HI
> 
> On Wed, Jul 4, 2018 at 5:39 PM, Igor Mammedov <imammedo@redhat.com> wrote:
> > On Thu, 28 Jun 2018 19:26:57 +0200
> > Marc-André Lureau <marcandre.lureau@redhat.com> 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>
> >> [ Marc-André - ACPI code improvements and windows fixes ]
> >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> >>
> >> ---
> >>
> >> v7: (Marc-André)
> >>  - use first spec version/section in code comments
> >>  - use more variables for ASL code building
> >>  - remove unnecessering ToInteger() calls
> >>
> >> v6:
> >>  - more code documentation (Marc-André)
> >>  - use some explicit named variables to ease reading (Marc-André)
> >>  - use fixed size fields/memory regions, remove PPI struct (Marc-André)
> >>  - only add PPI ACPI methods if PPI is enabled (Marc-André)
> >>  - document the qemu/firmware ACPI memory region (Stefan)
> >>
> >> v5 (Marc-André):
> >>  - /struct tpm_ppi/struct TPMPPIData
> >>
> >> 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 |   8 +
> >>  hw/i386/acpi-build.c  | 403 +++++++++++++++++++++++++++++++++++++++++-
> >>  docs/specs/tpm.txt    |  79 +++++++++
> >>  3 files changed, 487 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
> >> index f79d68a77a..e0bd07862e 100644
> >> --- a/include/hw/acpi/tpm.h
> >> +++ b/include/hw/acpi/tpm.h
> >> @@ -196,4 +196,12 @@ REG32(CRB_DATA_BUFFER, 0x80)
> >>  #define TPM_PPI_VERSION_NONE        0
> >>  #define TPM_PPI_VERSION_1_30        1
> >>
> >> +/* whether function is blocked by BIOS settings; bits 0, 1, 2 */
> >> +#define TPM_PPI_FUNC_NOT_IMPLEMENTED     (0 << 0)
> >> +#define TPM_PPI_FUNC_BIOS_ONLY           (1 << 0)
> >> +#define TPM_PPI_FUNC_BLOCKED             (2 << 0)
> >> +#define TPM_PPI_FUNC_ALLOWED_USR_REQ     (3 << 0)
> >> +#define TPM_PPI_FUNC_ALLOWED_USR_NOT_REQ (4 << 0)
> >> +#define TPM_PPI_FUNC_MASK                (7 << 0)
> >> +
> >>  #endif /* HW_ACPI_TPM_H */
> >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> >> index ebd64c4818..263677f3e4 100644
> >> --- a/hw/i386/acpi-build.c
> >> +++ b/hw/i386/acpi-build.c
> >> @@ -43,6 +43,7 @@
> >>  #include "hw/acpi/memory_hotplug.h"
> >>  #include "sysemu/tpm.h"
> >>  #include "hw/acpi/tpm.h"
> >> +#include "hw/tpm/tpm_ppi.h"
> >>  #include "hw/acpi/vmgenid.h"
> >>  #include "sysemu/tpm_backend.h"
> >>  #include "hw/timer/mc146818rtc_regs.h"
> >> @@ -1789,6 +1790,396 @@ static Aml *build_q35_osc_method(void)
> >>      return method;
> >>  }
> >>
> >> +static void
> >> +build_tpm_ppi(TPMIf *tpm, Aml *dev)
> >> +{
> >> +    Aml *method, *field, *ifctx, *ifctx2, *ifctx3;
> >> +    Aml *pak, *tpm2, *tpm3, *pprm, *pprq;
> >> +    int i;
> >> +
> >> +    if (!object_property_get_bool(OBJECT(tpm), "ppi", &error_abort)) {
> >> +        return;
> >> +    }  
> > I'd prefer this being checked by caller, like:
> >
> > @@ -2194,6 +2194,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> >      int root_bus_limit = 0xFF;
> >      PCIBus *bus = NULL;
> >      TPMIf *tpm = tpm_find();
> > +    bool has_ppi = object_property_get_bool(OBJECT(tpm), "ppi", &error_abort);  
> 
> it will need an additional "tpm ? .. : false". I'd rather keep it away
> from build_dsdt(), but your call.
ok, keep it as it is now

> 
> >      int i;
> >
> >      dsdt = init_aml_allocator();
> > @@ -2545,8 +2546,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> >                   */
> >                  /* aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ)); */
> >                  aml_append(dev, aml_name_decl("_CRS", crs));
> > -
> > -                build_tpm_ppi(tpm, dev);
> > +                if (has_ppi) {
> > +                    build_tpm_ppi(tpm, dev);
> > +                }
> >
> >                  aml_append(scope, dev);
> >              }
> > @@ -2567,7 +2569,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> >          aml_append(method, aml_return(aml_int(0x0f)));
> >          aml_append(dev, method);
> >
> > -        build_tpm_ppi(tpm, dev);
> > +        if (has_ppi) {
> > +            build_tpm_ppi(tpm, dev);
> > +        }
> >
> >          aml_append(sb_scope, dev);
> >      }
> >  
> >> +    /*
> >> +     * TPP1 is for the flags that indicate which PPI operations
> >> +     * are supported by the firmware. The firmware is expected to
> >> +     * write these flags.
> >> +     */
> >> +    aml_append(dev,
> >> +               aml_operation_region("TPP1", AML_SYSTEM_MEMORY,
> >> +                                    aml_int(TPM_PPI_ADDR_BASE), 0x100));
> >> +    field = aml_field("TPP1", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
> >> +    for (i = 0; i < 0x100; i++) {
> >> +        char *tmp = g_strdup_printf("FN%02X", i);
> >> +        aml_append(field, aml_named_field(tmp, 8));
> >> +        g_free(tmp);
> >> +    }  
> > this and TPFN takes more than 4K.
> > Have you tried to declare only one filed of size TPP1, which makes
> > field Buffer type, which could be used with Index and would take only
> > several bytes to in AML. See ACPI 6.2a 19.6.61.2 Index with Buffers.
> > It also looks very portable (but we should test with win xp just to be safe)  
> 
> You mean like Stefan originally did? Yes, problem described in
> http://www.osronline.com/showThread.CFM?link=288617.

we have cpu hotplug that uses this approach, but it has IO based OpReg.

Looks like DerefOf in windows is broken wrt SYSTEM_MEMORY originated field buffer
(debugger shows a correct object returned from Index() but DerefOf reads
junk, sometimes it returns the last byte of buffer address).

but do not give up yet,
one can use dynamic operation region inside of method for indexing
so here goes dirty draft idea:
    TPFN(op)
        aml_append(method, aml_operation_region(
          "TPP1",
          AML_SYSTEM_MEMORY,
          aml_add(aml_int(TPM_PPI_ADDR_BASE), op, NULL),
          0x1));
        field = aml_field("TPP1", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);       
        aml_append(field, aml_named_field("TPPF", 8));                           
        aml_append(method, field);                                               
        aml_append(method, aml_return(aml_name("TPPF")));

result of addition probably should be stored in sanely named local var,
but idea stays the same (use address in OpRegion as index) and if 'op'
is user defined, we probably should have bounds check as well.

I don't like dynamic OpRegions but in general, but it savs us a lot of RAM
and much faster then walking 'if..else' chain multiple times.


> >  
> >> +    aml_append(dev, field);
> >> +
> >> +    /*
> >> +     * TPP2 is for the registers that ACPI code used to pass
> >> +     * the PPI code and parameter (PPRQ, PPRM) to the firmware.
> >> +     */
> >> +    aml_append(dev,
> >> +               aml_operation_region("TPP2", AML_SYSTEM_MEMORY,
> >> +                                    aml_int(TPM_PPI_ADDR_BASE + 0x100),
> >> +                                    0x5A));
> >> +    field = aml_field("TPP2", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
> >> +    aml_append(field, aml_named_field("PPIN", 8));
> >> +    aml_append(field, aml_named_field("PPIP", 32));
> >> +    aml_append(field, aml_named_field("PPRP", 32));
> >> +    aml_append(field, aml_named_field("PPRQ", 32));
> >> +    aml_append(field, aml_named_field("PPRM", 32));
> >> +    aml_append(field, aml_named_field("LPPR", 32));
> >> +    aml_append(dev, field);
> >> +    pprq = aml_name("PPRQ");
> >> +    pprm = aml_name("PPRM");
> >> +
> >> +    /*
> >> +     * A function to return the value of DerefOf (FUNC [N]), by using
> >> +     * accessing the fields individually instead. This is a workaround
> >> +     * for what looks like a Windows ACPI bug in all versions so far
> >> +     * (fwiw, DerefOf (FUNC [N]) works on Linux).
> >> +     */
> >> +    method = aml_method("TPFN", 1, AML_SERIALIZED);
> >> +    {
> >> +        for (i = 0; i < 0x100; i++) {
> >> +            ifctx = aml_if(aml_equal(aml_int(i), aml_arg(0)));
> >> +            {
> >> +                aml_append(ifctx, aml_return(aml_name("FN%02X", i)));
> >> +            }
> >> +            aml_append(method, ifctx);
> >> +        }
> >> +        aml_append(method, aml_return(aml_int(0)));
> >> +    }
> >> +    aml_append(dev, method);
> >> +
> >> +    pak = aml_package(2);
> >> +    aml_append(pak, aml_int(0));
> >> +    aml_append(pak, aml_int(0));
> >> +    aml_append(dev, aml_name_decl("TPM2", pak));
> >> +    tpm2 = aml_name("TPM2");
> >> +
> >> +    pak = aml_package(3);
> >> +    aml_append(pak, aml_int(0));
> >> +    aml_append(pak, aml_int(0));
> >> +    aml_append(pak, aml_int(0));
> >> +    aml_append(dev, aml_name_decl("TPM3", pak));
> >> +    tpm3 = aml_name("TPM3");  
> > looks like above 2 packages  are used only by _DSM,
> > so you don't need to make them global, just use local variables within _DSM for it
> >  
> 
> That would also fail on Windows. Fwiw (this time I am sure) my up to
> date t460p uses the same global variables trick, presumably to satisfy
> Windows as well.
ok, could you add a comment about it here so that we won't optimize it
out in future.

> btw, not only it took me a while to find the shortcomings with Windows
> ACPI, but testing any change in the ACPI code takes ~1.5h (with manual
> intervention ever 10 minutes or so, not a lot of fun)
don't tell me, I just killed a bunch of time first to setup debug
environment with broken serial for this series and then to
find a solution that would work (~15min per ACPI change)

> 
> >> +
> >> +    method = aml_method("_DSM", 4, AML_SERIALIZED);
> >> +    {
> >> +        uint8_t zerobyte[1] = { 0 };
> >> +        Aml *function, *arguments, *rev, *op, *op_arg, *op_flags, *uuid;
> >> +
> >> +        uuid = aml_arg(0);
> >> +        rev = aml_arg(1);
> >> +        function = aml_arg(2);
> >> +        ifctx = aml_if(
> >> +            aml_equal(uuid,
> >> +                      aml_touuid("3DDDFAA6-361B-4EB4-A424-8D10089D1653")));
> >> +        {  
> > it would be better to exit immediately if UUID doesn't match (less indentation and if contexts)
> > btw, one should return something if uuid doesn't match method,
> >
> > if (LNotEqual(uuid, ToUUID(“893f00a6-660c-494e-bcfd-3043f4fb67c0”))) {
> >     return(Buffer(){0})
> > }
> >  
> 
> Except that the next patch has another uuid to check (and it's not a
> common ACPI style to return early it seems).
ok, keep it as is

> 
> >> +            /* standard DSM query function */
> >> +            ifctx2 = aml_if(aml_equal(function, aml_int(0)));  
> > constants aml_int(0..3) are used multiple times, suggest to declare them
> > along with rev/function/..., like:
> >
> >  Aml *one = aml_int(0);
> >  ...
> >
> > and then use them through code.  
> 
> I don't see that as an improvement tbh, it makes the code
> inconsistent. So you would like to have zero and one, but not others?
it will be consistent with cpu/mem hotplug ACPI code and ASL
since 0 and 1 have their own magic names (Zero, One) in gramar

[...]

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

[Marc-André Lureau]

Hi

On Wed, Jul 11, 2018 at 6:25 PM, Igor Mammedov <imammedo@redhat.com> wrote:
> On Wed, 4 Jul 2018 18:00:41 +0200
> Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
>
>> HI
>>
>> On Wed, Jul 4, 2018 at 5:39 PM, Igor Mammedov <imammedo@redhat.com> wrote:
>> > On Thu, 28 Jun 2018 19:26:57 +0200
>> > Marc-André Lureau <marcandre.lureau@redhat.com> 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>
>> >> [ Marc-André - ACPI code improvements and windows fixes ]
>> >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> >>
>> >> ---
>> >>
>> >> v7: (Marc-André)
>> >>  - use first spec version/section in code comments
>> >>  - use more variables for ASL code building
>> >>  - remove unnecessering ToInteger() calls
>> >>
>> >> v6:
>> >>  - more code documentation (Marc-André)
>> >>  - use some explicit named variables to ease reading (Marc-André)
>> >>  - use fixed size fields/memory regions, remove PPI struct (Marc-André)
>> >>  - only add PPI ACPI methods if PPI is enabled (Marc-André)
>> >>  - document the qemu/firmware ACPI memory region (Stefan)
>> >>
>> >> v5 (Marc-André):
>> >>  - /struct tpm_ppi/struct TPMPPIData
>> >>
>> >> 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 |   8 +
>> >>  hw/i386/acpi-build.c  | 403 +++++++++++++++++++++++++++++++++++++++++-
>> >>  docs/specs/tpm.txt    |  79 +++++++++
>> >>  3 files changed, 487 insertions(+), 3 deletions(-)
>> >>
>> >> diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
>> >> index f79d68a77a..e0bd07862e 100644
>> >> --- a/include/hw/acpi/tpm.h
>> >> +++ b/include/hw/acpi/tpm.h
>> >> @@ -196,4 +196,12 @@ REG32(CRB_DATA_BUFFER, 0x80)
>> >>  #define TPM_PPI_VERSION_NONE        0
>> >>  #define TPM_PPI_VERSION_1_30        1
>> >>
>> >> +/* whether function is blocked by BIOS settings; bits 0, 1, 2 */
>> >> +#define TPM_PPI_FUNC_NOT_IMPLEMENTED     (0 << 0)
>> >> +#define TPM_PPI_FUNC_BIOS_ONLY           (1 << 0)
>> >> +#define TPM_PPI_FUNC_BLOCKED             (2 << 0)
>> >> +#define TPM_PPI_FUNC_ALLOWED_USR_REQ     (3 << 0)
>> >> +#define TPM_PPI_FUNC_ALLOWED_USR_NOT_REQ (4 << 0)
>> >> +#define TPM_PPI_FUNC_MASK                (7 << 0)
>> >> +
>> >>  #endif /* HW_ACPI_TPM_H */
>> >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> >> index ebd64c4818..263677f3e4 100644
>> >> --- a/hw/i386/acpi-build.c
>> >> +++ b/hw/i386/acpi-build.c
>> >> @@ -43,6 +43,7 @@
>> >>  #include "hw/acpi/memory_hotplug.h"
>> >>  #include "sysemu/tpm.h"
>> >>  #include "hw/acpi/tpm.h"
>> >> +#include "hw/tpm/tpm_ppi.h"
>> >>  #include "hw/acpi/vmgenid.h"
>> >>  #include "sysemu/tpm_backend.h"
>> >>  #include "hw/timer/mc146818rtc_regs.h"
>> >> @@ -1789,6 +1790,396 @@ static Aml *build_q35_osc_method(void)
>> >>      return method;
>> >>  }
>> >>
>> >> +static void
>> >> +build_tpm_ppi(TPMIf *tpm, Aml *dev)
>> >> +{
>> >> +    Aml *method, *field, *ifctx, *ifctx2, *ifctx3;
>> >> +    Aml *pak, *tpm2, *tpm3, *pprm, *pprq;
>> >> +    int i;
>> >> +
>> >> +    if (!object_property_get_bool(OBJECT(tpm), "ppi", &error_abort)) {
>> >> +        return;
>> >> +    }
>> > I'd prefer this being checked by caller, like:
>> >
>> > @@ -2194,6 +2194,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>> >      int root_bus_limit = 0xFF;
>> >      PCIBus *bus = NULL;
>> >      TPMIf *tpm = tpm_find();
>> > +    bool has_ppi = object_property_get_bool(OBJECT(tpm), "ppi", &error_abort);
>>
>> it will need an additional "tpm ? .. : false". I'd rather keep it away
>> from build_dsdt(), but your call.
> ok, keep it as it is now
>
>>
>> >      int i;
>> >
>> >      dsdt = init_aml_allocator();
>> > @@ -2545,8 +2546,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>> >                   */
>> >                  /* aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ)); */
>> >                  aml_append(dev, aml_name_decl("_CRS", crs));
>> > -
>> > -                build_tpm_ppi(tpm, dev);
>> > +                if (has_ppi) {
>> > +                    build_tpm_ppi(tpm, dev);
>> > +                }
>> >
>> >                  aml_append(scope, dev);
>> >              }
>> > @@ -2567,7 +2569,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>> >          aml_append(method, aml_return(aml_int(0x0f)));
>> >          aml_append(dev, method);
>> >
>> > -        build_tpm_ppi(tpm, dev);
>> > +        if (has_ppi) {
>> > +            build_tpm_ppi(tpm, dev);
>> > +        }
>> >
>> >          aml_append(sb_scope, dev);
>> >      }
>> >
>> >> +    /*
>> >> +     * TPP1 is for the flags that indicate which PPI operations
>> >> +     * are supported by the firmware. The firmware is expected to
>> >> +     * write these flags.
>> >> +     */
>> >> +    aml_append(dev,
>> >> +               aml_operation_region("TPP1", AML_SYSTEM_MEMORY,
>> >> +                                    aml_int(TPM_PPI_ADDR_BASE), 0x100));
>> >> +    field = aml_field("TPP1", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
>> >> +    for (i = 0; i < 0x100; i++) {
>> >> +        char *tmp = g_strdup_printf("FN%02X", i);
>> >> +        aml_append(field, aml_named_field(tmp, 8));
>> >> +        g_free(tmp);
>> >> +    }
>> > this and TPFN takes more than 4K.
>> > Have you tried to declare only one filed of size TPP1, which makes
>> > field Buffer type, which could be used with Index and would take only
>> > several bytes to in AML. See ACPI 6.2a 19.6.61.2 Index with Buffers.
>> > It also looks very portable (but we should test with win xp just to be safe)
>>
>> You mean like Stefan originally did? Yes, problem described in
>> http://www.osronline.com/showThread.CFM?link=288617.
>
> we have cpu hotplug that uses this approach, but it has IO based OpReg.
>
> Looks like DerefOf in windows is broken wrt SYSTEM_MEMORY originated field buffer
> (debugger shows a correct object returned from Index() but DerefOf reads
> junk, sometimes it returns the last byte of buffer address).
>
> but do not give up yet,
> one can use dynamic operation region inside of method for indexing
> so here goes dirty draft idea:
>     TPFN(op)
>         aml_append(method, aml_operation_region(
>           "TPP1",
>           AML_SYSTEM_MEMORY,
>           aml_add(aml_int(TPM_PPI_ADDR_BASE), op, NULL),
>           0x1));
>         field = aml_field("TPP1", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
>         aml_append(field, aml_named_field("TPPF", 8));
>         aml_append(method, field);
>         aml_append(method, aml_return(aml_name("TPPF")));
>
> result of addition probably should be stored in sanely named local var,
> but idea stays the same (use address in OpRegion as index) and if 'op'
> is user defined, we probably should have bounds check as well.
>
> I don't like dynamic OpRegions but in general, but it savs us a lot of RAM
> and much faster then walking 'if..else' chain multiple times.

Awesome! that works, thanks for the trick.

>
>
>> >
>> >> +    aml_append(dev, field);
>> >> +
>> >> +    /*
>> >> +     * TPP2 is for the registers that ACPI code used to pass
>> >> +     * the PPI code and parameter (PPRQ, PPRM) to the firmware.
>> >> +     */
>> >> +    aml_append(dev,
>> >> +               aml_operation_region("TPP2", AML_SYSTEM_MEMORY,
>> >> +                                    aml_int(TPM_PPI_ADDR_BASE + 0x100),
>> >> +                                    0x5A));
>> >> +    field = aml_field("TPP2", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
>> >> +    aml_append(field, aml_named_field("PPIN", 8));
>> >> +    aml_append(field, aml_named_field("PPIP", 32));
>> >> +    aml_append(field, aml_named_field("PPRP", 32));
>> >> +    aml_append(field, aml_named_field("PPRQ", 32));
>> >> +    aml_append(field, aml_named_field("PPRM", 32));
>> >> +    aml_append(field, aml_named_field("LPPR", 32));
>> >> +    aml_append(dev, field);
>> >> +    pprq = aml_name("PPRQ");
>> >> +    pprm = aml_name("PPRM");
>> >> +
>> >> +    /*
>> >> +     * A function to return the value of DerefOf (FUNC [N]), by using
>> >> +     * accessing the fields individually instead. This is a workaround
>> >> +     * for what looks like a Windows ACPI bug in all versions so far
>> >> +     * (fwiw, DerefOf (FUNC [N]) works on Linux).
>> >> +     */
>> >> +    method = aml_method("TPFN", 1, AML_SERIALIZED);
>> >> +    {
>> >> +        for (i = 0; i < 0x100; i++) {
>> >> +            ifctx = aml_if(aml_equal(aml_int(i), aml_arg(0)));
>> >> +            {
>> >> +                aml_append(ifctx, aml_return(aml_name("FN%02X", i)));
>> >> +            }
>> >> +            aml_append(method, ifctx);
>> >> +        }
>> >> +        aml_append(method, aml_return(aml_int(0)));
>> >> +    }
>> >> +    aml_append(dev, method);
>> >> +
>> >> +    pak = aml_package(2);
>> >> +    aml_append(pak, aml_int(0));
>> >> +    aml_append(pak, aml_int(0));
>> >> +    aml_append(dev, aml_name_decl("TPM2", pak));
>> >> +    tpm2 = aml_name("TPM2");
>> >> +
>> >> +    pak = aml_package(3);
>> >> +    aml_append(pak, aml_int(0));
>> >> +    aml_append(pak, aml_int(0));
>> >> +    aml_append(pak, aml_int(0));
>> >> +    aml_append(dev, aml_name_decl("TPM3", pak));
>> >> +    tpm3 = aml_name("TPM3");
>> > looks like above 2 packages  are used only by _DSM,
>> > so you don't need to make them global, just use local variables within _DSM for it
>> >
>>
>> That would also fail on Windows. Fwiw (this time I am sure) my up to
>> date t460p uses the same global variables trick, presumably to satisfy
>> Windows as well.
> ok, could you add a comment about it here so that we won't optimize it
> out in future.
>
>> btw, not only it took me a while to find the shortcomings with Windows
>> ACPI, but testing any change in the ACPI code takes ~1.5h (with manual
>> intervention ever 10 minutes or so, not a lot of fun)
> don't tell me, I just killed a bunch of time first to setup debug
> environment with broken serial for this series and then to
> find a solution that would work (~15min per ACPI change)
>
>>
>> >> +
>> >> +    method = aml_method("_DSM", 4, AML_SERIALIZED);
>> >> +    {
>> >> +        uint8_t zerobyte[1] = { 0 };
>> >> +        Aml *function, *arguments, *rev, *op, *op_arg, *op_flags, *uuid;
>> >> +
>> >> +        uuid = aml_arg(0);
>> >> +        rev = aml_arg(1);
>> >> +        function = aml_arg(2);
>> >> +        ifctx = aml_if(
>> >> +            aml_equal(uuid,
>> >> +                      aml_touuid("3DDDFAA6-361B-4EB4-A424-8D10089D1653")));
>> >> +        {
>> > it would be better to exit immediately if UUID doesn't match (less indentation and if contexts)
>> > btw, one should return something if uuid doesn't match method,
>> >
>> > if (LNotEqual(uuid, ToUUID(“893f00a6-660c-494e-bcfd-3043f4fb67c0”))) {
>> >     return(Buffer(){0})
>> > }
>> >
>>
>> Except that the next patch has another uuid to check (and it's not a
>> common ACPI style to return early it seems).
> ok, keep it as is
>
>>
>> >> +            /* standard DSM query function */
>> >> +            ifctx2 = aml_if(aml_equal(function, aml_int(0)));
>> > constants aml_int(0..3) are used multiple times, suggest to declare them
>> > along with rev/function/..., like:
>> >
>> >  Aml *one = aml_int(0);
>> >  ...
>> >
>> > and then use them through code.
>>
>> I don't see that as an improvement tbh, it makes the code
>> inconsistent. So you would like to have zero and one, but not others?
> it will be consistent with cpu/mem hotplug ACPI code and ASL
> since 0 and 1 have their own magic names (Zero, One) in gramar
>

Ok, changed.

thanks



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v6 0/4] Add support for TPM Physical Presence interface

[Stefan Berger]

On 06/28/2018 01:26 PM, Marc-André Lureau wrote:
> Hi,
>
> The following patches implement the TPM Physical Presence Interface
> that allows a user to set a command via ACPI (sysfs entry in Linux)
> that, upon the next reboot, the firmware looks for and acts upon by
> sending sequences of commands to the TPM.
>
> A dedicated memory region is added to the TPM CRB & TIS devices, at
> address/size 0xFED45000/0x400. A new "etc/tpm/config" fw_cfg entry
> holds the location for that PPI region and some version details, to
> allow for future flexibility.
>
> With the associated edk2/ovmf firmware, the Windows HLK "PPI 1.3" test
> now runs successfully.
>
> It is based on previous work from Stefan Berger ("[PATCH v2 0/4]
> Implement Physical Presence interface for TPM 1.2 and 2")
>
> The edk2 support is merged upstream.

The least I could do now is test this... So, I tested this now with the 
SeaBIOS support I have for this series. It's here:

https://github.com/stefanberger/seabios-tpm/tree/qemu-ppi.v6

It works fine with at least an attached TPM 1.2. I haven't tried TPM 2 
yet but would not expect complications from QEMU level. A operation 
request value put into Linux's PPI interface can be read back also after 
a VM suspend / resume operation. The list of supported operations is 
shown correctly (needs Linux extensions for TPM 2 operation values 
beyond a certain number iirc). The request operation is executed 
correctly and the response shows the last operation and its result. So 
it seems to work fine.


    Stefan

Re: [Qemu-devel] [PATCH v6 0/4] Add support for TPM Physical Presence interface

[Igor Mammedov]

On Fri, 29 Jun 2018 08:20:38 -0400
Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:

> On 06/28/2018 01:26 PM, Marc-André Lureau wrote:
> > Hi,
> >
> > The following patches implement the TPM Physical Presence Interface
> > that allows a user to set a command via ACPI (sysfs entry in Linux)
> > that, upon the next reboot, the firmware looks for and acts upon by
> > sending sequences of commands to the TPM.
> >
> > A dedicated memory region is added to the TPM CRB & TIS devices, at
> > address/size 0xFED45000/0x400. A new "etc/tpm/config" fw_cfg entry
> > holds the location for that PPI region and some version details, to
> > allow for future flexibility.
> >
> > With the associated edk2/ovmf firmware, the Windows HLK "PPI 1.3" test
> > now runs successfully.
> >
> > It is based on previous work from Stefan Berger ("[PATCH v2 0/4]
> > Implement Physical Presence interface for TPM 1.2 and 2")
> >
> > The edk2 support is merged upstream.  
> 
> The least I could do now is test this... So, I tested this now with the 
> SeaBIOS support I have for this series. It's here:
> 
> https://github.com/stefanberger/seabios-tpm/tree/qemu-ppi.v6
> 
> It works fine with at least an attached TPM 1.2. I haven't tried TPM 2 
> yet but would not expect complications from QEMU level. A operation 
> request value put into Linux's PPI interface can be read back also after 
> a VM suspend / resume operation. The list of supported operations is 
> shown correctly (needs Linux extensions for TPM 2 operation values 
> beyond a certain number iirc). The request operation is executed 
> correctly and the response shows the last operation and its result. So 
> it seems to work fine.
> 
> 
>     Stefan
> 

Are there any instructions how to test it?

Re: [Qemu-devel] [PATCH v6 0/4] Add support for TPM Physical Presence interface

[Stefan Berger]

On 07/02/2018 07:57 AM, Igor Mammedov wrote:
> On Fri, 29 Jun 2018 08:20:38 -0400
> Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
>
>> On 06/28/2018 01:26 PM, Marc-André Lureau wrote:
>>> Hi,
>>>
>>> The following patches implement the TPM Physical Presence Interface
>>> that allows a user to set a command via ACPI (sysfs entry in Linux)
>>> that, upon the next reboot, the firmware looks for and acts upon by
>>> sending sequences of commands to the TPM.
>>>
>>> A dedicated memory region is added to the TPM CRB & TIS devices, at
>>> address/size 0xFED45000/0x400. A new "etc/tpm/config" fw_cfg entry
>>> holds the location for that PPI region and some version details, to
>>> allow for future flexibility.
>>>
>>> With the associated edk2/ovmf firmware, the Windows HLK "PPI 1.3" test
>>> now runs successfully.
>>>
>>> It is based on previous work from Stefan Berger ("[PATCH v2 0/4]
>>> Implement Physical Presence interface for TPM 1.2 and 2")
>>>
>>> The edk2 support is merged upstream.
>> The least I could do now is test this... So, I tested this now with the
>> SeaBIOS support I have for this series. It's here:
>>
>> https://github.com/stefanberger/seabios-tpm/tree/qemu-ppi.v6
>>
>> It works fine with at least an attached TPM 1.2. I haven't tried TPM 2
>> yet but would not expect complications from QEMU level. A operation
>> request value put into Linux's PPI interface can be read back also after
>> a VM suspend / resume operation. The list of supported operations is
>> shown correctly (needs Linux extensions for TPM 2 operation values
>> beyond a certain number iirc). The request operation is executed
>> correctly and the response shows the last operation and its result. So
>> it seems to work fine.
>>
>>
>>      Stefan
>>
> Are there any instructions how to test it?
>
1) You need to get swtpm running on a machine.

Build libtpms from the 'TPM 2 preview' branch here:

https://github.com/stefanberger/libtpms/tree/tpm2-preview.rev146.v2

Instructions are here: https://github.com/stefanberger/libtpms/wiki


Build swtpm from the TPM 2 preview branch here:

https://github.com/stefanberger/swtpm/tree/tpm2-preview.v2

Instructions are here: https://github.com/stefanberger/swtpm/wiki


2) Compile and install my branch of SeaBIOS with the PPI support:

branch is here: https://github.com/stefanberger/seabios-tpm/tree/qemu-ppi.v6


3) Follow the QEMU instructions for running the swtpm or start one using 
latest libvirt + latest virt-manager:

 From QEMU tree: docs/specs/tpm.txt  (starting line 255; this will start 
a TPM 1.2)


4) Inside the VM install 'trousers' and tpm-tools packages for using the 
TPM 1.2.


5) Find the TPM devices ppi directory and cd into it:

# find /sys/devices | grep ppi$

# cd /sys/device/pnp0/00:04/tpm/tpm0/ppi    # this is the location on my 
installation


6) Display the tcg_operations, PPI version, pending request, last 
response from firmware

# cat tcg_operations

0 4: User not required

1 4: User not required

2 4: User not required

3 4: User not required

[...]

# cat version

1.3

# cat request

0

# cat response

0: No Recent Request


7) Take ownership of the TPM

# tcsd

# tpm_takeownership -y -z


8) Check the state of the TPM:

# cat ../enabled ../active ../own

1

1

1


9) Request PPI to give up ownership of the TPM and activate and enable 
the TPM again; this is operation request '14' per table 1 of PPI 1.30 
specification.

# echo 14 > request
# cat request
14
# reboot

10) cd into the ppi directory after the reboot and check the state of 
the TPM

# cd /sys/device/pnp0/00:04/tpm/tpm0/ppi    # this is the location on my 
installation
# response
14 0: Success

# cat ../enabled ../active ../owned
1
1
0

I hope this helps.

Regards,
    Stefan

Re: [Qemu-devel] [PATCH v6 0/4] Add support for TPM Physical Presence interface

[Igor Mammedov]

On Mon, 2 Jul 2018 10:54:13 -0400
Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:

> On 07/02/2018 07:57 AM, Igor Mammedov wrote:
> > On Fri, 29 Jun 2018 08:20:38 -0400
> > Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
> >  
> >> On 06/28/2018 01:26 PM, Marc-André Lureau wrote:  
> >>> Hi,
> >>>
> >>> The following patches implement the TPM Physical Presence Interface
> >>> that allows a user to set a command via ACPI (sysfs entry in Linux)
> >>> that, upon the next reboot, the firmware looks for and acts upon by
> >>> sending sequences of commands to the TPM.
> >>>
> >>> A dedicated memory region is added to the TPM CRB & TIS devices, at
> >>> address/size 0xFED45000/0x400. A new "etc/tpm/config" fw_cfg entry
> >>> holds the location for that PPI region and some version details, to
> >>> allow for future flexibility.
> >>>
> >>> With the associated edk2/ovmf firmware, the Windows HLK "PPI 1.3" test
> >>> now runs successfully.
> >>>
> >>> It is based on previous work from Stefan Berger ("[PATCH v2 0/4]
> >>> Implement Physical Presence interface for TPM 1.2 and 2")
> >>>
> >>> The edk2 support is merged upstream.  
> >> The least I could do now is test this... So, I tested this now with the
> >> SeaBIOS support I have for this series. It's here:
> >>
> >> https://github.com/stefanberger/seabios-tpm/tree/qemu-ppi.v6
> >>
> >> It works fine with at least an attached TPM 1.2. I haven't tried TPM 2
> >> yet but would not expect complications from QEMU level. A operation
> >> request value put into Linux's PPI interface can be read back also after
> >> a VM suspend / resume operation. The list of supported operations is
> >> shown correctly (needs Linux extensions for TPM 2 operation values
> >> beyond a certain number iirc). The request operation is executed
> >> correctly and the response shows the last operation and its result. So
> >> it seems to work fine.
> >>
> >>
> >>      Stefan
> >>  
> > Are there any instructions how to test it?
> >  
> 1) You need to get swtpm running on a machine.
> 
> Build libtpms from the 'TPM 2 preview' branch here:
> 
> https://github.com/stefanberger/libtpms/tree/tpm2-preview.rev146.v2
> 
> Instructions are here: https://github.com/stefanberger/libtpms/wiki
> 
> 
> Build swtpm from the TPM 2 preview branch here:
> 
> https://github.com/stefanberger/swtpm/tree/tpm2-preview.v2
> 
> Instructions are here: https://github.com/stefanberger/swtpm/wiki
> 
> 
> 2) Compile and install my branch of SeaBIOS with the PPI support:
> 
> branch is here: https://github.com/stefanberger/seabios-tpm/tree/qemu-ppi.v6
it looks like repo is gone, is it merged upstream?

Re: [Qemu-devel] [PATCH v6 0/4] Add support for TPM Physical Presence interface

[Stefan Berger]

On 07/09/2018 10:05 AM, Igor Mammedov wrote:
> On Mon, 2 Jul 2018 10:54:13 -0400
> Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
>
>> On 07/02/2018 07:57 AM, Igor Mammedov wrote:
>>> On Fri, 29 Jun 2018 08:20:38 -0400
>>> Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
>>>   
>>>> On 06/28/2018 01:26 PM, Marc-André Lureau wrote:
>>>>> Hi,
>>>>>
>>>>> The following patches implement the TPM Physical Presence Interface
>>>>> that allows a user to set a command via ACPI (sysfs entry in Linux)
>>>>> that, upon the next reboot, the firmware looks for and acts upon by
>>>>> sending sequences of commands to the TPM.
>>>>>
>>>>> A dedicated memory region is added to the TPM CRB & TIS devices, at
>>>>> address/size 0xFED45000/0x400. A new "etc/tpm/config" fw_cfg entry
>>>>> holds the location for that PPI region and some version details, to
>>>>> allow for future flexibility.
>>>>>
>>>>> With the associated edk2/ovmf firmware, the Windows HLK "PPI 1.3" test
>>>>> now runs successfully.
>>>>>
>>>>> It is based on previous work from Stefan Berger ("[PATCH v2 0/4]
>>>>> Implement Physical Presence interface for TPM 1.2 and 2")
>>>>>
>>>>> The edk2 support is merged upstream.
>>>> The least I could do now is test this... So, I tested this now with the
>>>> SeaBIOS support I have for this series. It's here:
>>>>
>>>> https://github.com/stefanberger/seabios-tpm/tree/qemu-ppi.v6
>>>>
>>>> It works fine with at least an attached TPM 1.2. I haven't tried TPM 2
>>>> yet but would not expect complications from QEMU level. A operation
>>>> request value put into Linux's PPI interface can be read back also after
>>>> a VM suspend / resume operation. The list of supported operations is
>>>> shown correctly (needs Linux extensions for TPM 2 operation values
>>>> beyond a certain number iirc). The request operation is executed
>>>> correctly and the response shows the last operation and its result. So
>>>> it seems to work fine.
>>>>
>>>>
>>>>       Stefan
>>>>   
>>> Are there any instructions how to test it?
>>>   
>> 1) You need to get swtpm running on a machine.
>>
>> Build libtpms from the 'TPM 2 preview' branch here:
>>
>> https://github.com/stefanberger/libtpms/tree/tpm2-preview.rev146.v2
>>
>> Instructions are here: https://github.com/stefanberger/libtpms/wiki
>>
>>
>> Build swtpm from the TPM 2 preview branch here:
>>
>> https://github.com/stefanberger/swtpm/tree/tpm2-preview.v2
>>
>> Instructions are here: https://github.com/stefanberger/swtpm/wiki
>>
>>
>> 2) Compile and install my branch of SeaBIOS with the PPI support:
>>
>> branch is here: https://github.com/stefanberger/seabios-tpm/tree/qemu-ppi.v6
> it looks like repo is gone, is it merged upstream?
>
>
It's the following repo with the branch qemu-ppi.v6.


https://github.com/stefanberger/seabios-tpm


    Stefan

Re: [Qemu-devel] [PATCH v6 0/4] Add support for TPM Physical Presence interface

[Igor Mammedov]

On Mon, 9 Jul 2018 10:07:46 -0400
Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:

> On 07/09/2018 10:05 AM, Igor Mammedov wrote:
> > On Mon, 2 Jul 2018 10:54:13 -0400
> > Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
> >  
> >> On 07/02/2018 07:57 AM, Igor Mammedov wrote:  
> >>> On Fri, 29 Jun 2018 08:20:38 -0400
> >>> Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
> >>>     
> >>>> On 06/28/2018 01:26 PM, Marc-André Lureau wrote:  
> >>>>> Hi,
> >>>>>
> >>>>> The following patches implement the TPM Physical Presence Interface
> >>>>> that allows a user to set a command via ACPI (sysfs entry in Linux)
> >>>>> that, upon the next reboot, the firmware looks for and acts upon by
> >>>>> sending sequences of commands to the TPM.
> >>>>>
> >>>>> A dedicated memory region is added to the TPM CRB & TIS devices, at
> >>>>> address/size 0xFED45000/0x400. A new "etc/tpm/config" fw_cfg entry
> >>>>> holds the location for that PPI region and some version details, to
> >>>>> allow for future flexibility.
> >>>>>
> >>>>> With the associated edk2/ovmf firmware, the Windows HLK "PPI 1.3" test
> >>>>> now runs successfully.
> >>>>>
> >>>>> It is based on previous work from Stefan Berger ("[PATCH v2 0/4]
> >>>>> Implement Physical Presence interface for TPM 1.2 and 2")
> >>>>>
> >>>>> The edk2 support is merged upstream.  
> >>>> The least I could do now is test this... So, I tested this now with the
> >>>> SeaBIOS support I have for this series. It's here:
> >>>>
> >>>> https://github.com/stefanberger/seabios-tpm/tree/qemu-ppi.v6
> >>>>
> >>>> It works fine with at least an attached TPM 1.2. I haven't tried TPM 2
> >>>> yet but would not expect complications from QEMU level. A operation
> >>>> request value put into Linux's PPI interface can be read back also after
> >>>> a VM suspend / resume operation. The list of supported operations is
> >>>> shown correctly (needs Linux extensions for TPM 2 operation values
> >>>> beyond a certain number iirc). The request operation is executed
> >>>> correctly and the response shows the last operation and its result. So
> >>>> it seems to work fine.
> >>>>
> >>>>
> >>>>       Stefan
> >>>>     
> >>> Are there any instructions how to test it?
> >>>     
> >> 1) You need to get swtpm running on a machine.
> >>
> >> Build libtpms from the 'TPM 2 preview' branch here:
> >>
> >> https://github.com/stefanberger/libtpms/tree/tpm2-preview.rev146.v2
> >>
> >> Instructions are here: https://github.com/stefanberger/libtpms/wiki
> >>
> >>
> >> Build swtpm from the TPM 2 preview branch here:
> >>
> >> https://github.com/stefanberger/swtpm/tree/tpm2-preview.v2
> >>
> >> Instructions are here: https://github.com/stefanberger/swtpm/wiki
> >>
> >>
> >> 2) Compile and install my branch of SeaBIOS with the PPI support:
> >>
> >> branch is here: https://github.com/stefanberger/seabios-tpm/tree/qemu-ppi.v6  
> > it looks like repo is gone, is it merged upstream?
> >
> >  
> It's the following repo with the branch qemu-ppi.v6.
> 
> 
> https://github.com/stefanberger/seabios-tpm
Weird, I'm getting page not found.
(tried just your account without seabios-tpm, I see on full repos list qemu-tpm but nothing with ppi)

> 
>     Stefan
> 

Re: [Qemu-devel] [PATCH v6 0/4] Add support for TPM Physical Presence interface

[Stefan Berger]

On 07/09/2018 11:35 AM, Igor Mammedov wrote:
> On Mon, 9 Jul 2018 10:07:46 -0400
> Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
>
>> On 07/09/2018 10:05 AM, Igor Mammedov wrote:
>>> On Mon, 2 Jul 2018 10:54:13 -0400
>>> Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
>>>   
>>>> On 07/02/2018 07:57 AM, Igor Mammedov wrote:
>>>>> On Fri, 29 Jun 2018 08:20:38 -0400
>>>>> Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
>>>>>      
>>>>>> On 06/28/2018 01:26 PM, Marc-André Lureau wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> The following patches implement the TPM Physical Presence Interface
>>>>>>> that allows a user to set a command via ACPI (sysfs entry in Linux)
>>>>>>> that, upon the next reboot, the firmware looks for and acts upon by
>>>>>>> sending sequences of commands to the TPM.
>>>>>>>
>>>>>>> A dedicated memory region is added to the TPM CRB & TIS devices, at
>>>>>>> address/size 0xFED45000/0x400. A new "etc/tpm/config" fw_cfg entry
>>>>>>> holds the location for that PPI region and some version details, to
>>>>>>> allow for future flexibility.
>>>>>>>
>>>>>>> With the associated edk2/ovmf firmware, the Windows HLK "PPI 1.3" test
>>>>>>> now runs successfully.
>>>>>>>
>>>>>>> It is based on previous work from Stefan Berger ("[PATCH v2 0/4]
>>>>>>> Implement Physical Presence interface for TPM 1.2 and 2")
>>>>>>>
>>>>>>> The edk2 support is merged upstream.
>>>>>> The least I could do now is test this... So, I tested this now with the
>>>>>> SeaBIOS support I have for this series. It's here:
>>>>>>
>>>>>> https://github.com/stefanberger/seabios-tpm/tree/qemu-ppi.v6
>>>>>>
>>>>>> It works fine with at least an attached TPM 1.2. I haven't tried TPM 2
>>>>>> yet but would not expect complications from QEMU level. A operation
>>>>>> request value put into Linux's PPI interface can be read back also after
>>>>>> a VM suspend / resume operation. The list of supported operations is
>>>>>> shown correctly (needs Linux extensions for TPM 2 operation values
>>>>>> beyond a certain number iirc). The request operation is executed
>>>>>> correctly and the response shows the last operation and its result. So
>>>>>> it seems to work fine.
>>>>>>
>>>>>>
>>>>>>        Stefan
>>>>>>      
>>>>> Are there any instructions how to test it?
>>>>>      
>>>> 1) You need to get swtpm running on a machine.
>>>>
>>>> Build libtpms from the 'TPM 2 preview' branch here:
>>>>
>>>> https://github.com/stefanberger/libtpms/tree/tpm2-preview.rev146.v2
>>>>
>>>> Instructions are here: https://github.com/stefanberger/libtpms/wiki
>>>>
>>>>
>>>> Build swtpm from the TPM 2 preview branch here:
>>>>
>>>> https://github.com/stefanberger/swtpm/tree/tpm2-preview.v2
>>>>
>>>> Instructions are here: https://github.com/stefanberger/swtpm/wiki
>>>>
>>>>
>>>> 2) Compile and install my branch of SeaBIOS with the PPI support:
>>>>
>>>> branch is here: https://github.com/stefanberger/seabios-tpm/tree/qemu-ppi.v6
>>> it looks like repo is gone, is it merged upstream?
>>>
>>>   
>> It's the following repo with the branch qemu-ppi.v6.
>>
>>
>> https://github.com/stefanberger/seabios-tpm
> Weird, I'm getting page not found.
> (tried just your account without seabios-tpm, I see on full repos list qemu-tpm but nothing with ppi)

Forgot, it was a private repo. Now it's public.