All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v4 0/5] Add support for TPM Physical Presence interface
@ 2018-06-21 11:55 Marc-André Lureau
  2018-06-21 11:55 ` [Qemu-devel] [PATCH v4 1/5] tpm: add a "ppi" boolean property Marc-André Lureau
                   ` (4 more replies)
  0 siblings, 5 replies; 19+ messages in thread
From: Marc-André Lureau @ 2018-06-21 11:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Eduardo Habkost, Marcel Apfelbaum, stefanb,
	Michael S. Tsirkin, Igor Mammedov, Richard Henderson,
	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.

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 (2):
  tpm: add a "ppi" boolean property
  tpm: add a fake ACPI memory clear interface

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      |  27 ++++
 include/hw/acpi/tpm.h |  30 ++++
 include/hw/compat.h   |  10 ++
 hw/i386/acpi-build.c  | 320 ++++++++++++++++++++++++++++++++++++++++++
 hw/tpm/tpm_crb.c      |  10 ++
 hw/tpm/tpm_ppi.c      |  57 ++++++++
 hw/tpm/tpm_tis.c      |  10 ++
 docs/specs/tpm.txt    |  20 +++
 hw/tpm/Makefile.objs  |   2 +-
 hw/tpm/trace-events   |   4 +
 10 files changed, 489 insertions(+), 1 deletion(-)
 create mode 100644 hw/tpm/tpm_ppi.h
 create mode 100644 hw/tpm/tpm_ppi.c

-- 
2.18.0.rc1

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH v4 1/5] tpm: add a "ppi" boolean property
  2018-06-21 11:55 [Qemu-devel] [PATCH v4 0/5] Add support for TPM Physical Presence interface Marc-André Lureau
@ 2018-06-21 11:55 ` Marc-André Lureau
  2018-06-21 12:11   ` Stefan Berger
  2018-06-21 11:55 ` [Qemu-devel] [PATCH v4 2/5] tpm: implement virtual memory device for TPM PPI Marc-André Lureau
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 19+ messages in thread
From: Marc-André Lureau @ 2018-06-21 11:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Eduardo Habkost, Marcel Apfelbaum, stefanb,
	Michael S. Tsirkin, Igor Mammedov, Richard Henderson,
	Marc-André Lureau

The following patches implement the TPM Physical Presence Interface,
and makes use of new memory region and fw_cfg entries. Enable it by
default on >2.12 machine type.

Signed-off-by: Marc-André Lureau <marcandre.lureau@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 563908b874..dac847548b 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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH v4 2/5] tpm: implement virtual memory device for TPM PPI
  2018-06-21 11:55 [Qemu-devel] [PATCH v4 0/5] Add support for TPM Physical Presence interface Marc-André Lureau
  2018-06-21 11:55 ` [Qemu-devel] [PATCH v4 1/5] tpm: add a "ppi" boolean property Marc-André Lureau
@ 2018-06-21 11:55 ` Marc-André Lureau
  2018-06-21 12:10   ` Stefan Berger
  2018-06-21 11:55 ` [Qemu-devel] [PATCH v4 3/5] acpi: add fw_cfg file for TPM and PPI virtual memory device Marc-André Lureau
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 19+ messages in thread
From: Marc-André Lureau @ 2018-06-21 11:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Eduardo Habkost, Marcel Apfelbaum, stefanb,
	Michael S. Tsirkin, Igor Mammedov, Richard Henderson,
	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 0xfffef000 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().

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

---

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      | 27 ++++++++++++++++++++
 include/hw/acpi/tpm.h |  6 +++++
 hw/tpm/tpm_crb.c      |  7 ++++++
 hw/tpm/tpm_ppi.c      | 57 +++++++++++++++++++++++++++++++++++++++++++
 hw/tpm/tpm_tis.c      |  7 ++++++
 hw/tpm/Makefile.objs  |  2 +-
 hw/tpm/trace-events   |  4 +++
 7 files changed, 109 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..ac7ad47238
--- /dev/null
+++ b/hw/tpm/tpm_ppi.h
@@ -0,0 +1,27 @@
+/*
+ * 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 mmio;
+
+    uint8_t ram[TPM_PPI_ADDR_SIZE];
+} TPMPPI;
+
+void tpm_ppi_init_io(TPMPPI *tpmppi, struct MemoryRegion *m,
+                     hwaddr addr, Object *obj);
+
+#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..37c095f00f 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,11 @@ 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_io(&s->ppi, get_system_memory(),
+                        TPM_PPI_ADDR_BASE, OBJECT(s));
+    }
+
     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..79bec186c7
--- /dev/null
+++ b/hw/tpm/tpm_ppi.c
@@ -0,0 +1,57 @@
+/*
+ * 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 "tpm_ppi.h"
+#include "trace.h"
+
+static uint64_t tpm_ppi_mmio_read(void *opaque, hwaddr addr,
+                                  unsigned size)
+{
+    TPMPPI *s = opaque;
+
+    trace_tpm_ppi_mmio_read(addr, size, s->ram[addr]);
+
+    return s->ram[addr];
+}
+
+static void tpm_ppi_mmio_write(void *opaque, hwaddr addr,
+                               uint64_t val, unsigned size)
+{
+    TPMPPI *s = opaque;
+
+    trace_tpm_ppi_mmio_write(addr, size, val);
+
+    s->ram[addr] = val;
+}
+
+static const MemoryRegionOps tpm_ppi_memory_ops = {
+    .read = tpm_ppi_mmio_read,
+    .write = tpm_ppi_mmio_write,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .valid = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    },
+};
+
+void tpm_ppi_init_io(TPMPPI *tpmppi, struct MemoryRegion *m,
+                     hwaddr addr, Object *obj)
+{
+    memory_region_init_io(&tpmppi->mmio, obj, &tpm_ppi_memory_ops,
+                          tpmppi, "tpm-ppi-mmio",
+                          TPM_PPI_ADDR_SIZE);
+
+    memory_region_add_subregion(m, addr, &tpmppi->mmio);
+}
diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index d9ddf9b723..e7f45a05b9 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,11 @@ 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_io(&s->ppi, isa_address_space(ISA_DEVICE(dev)),
+                        TPM_PPI_ADDR_BASE, OBJECT(s));
+    }
 }
 
 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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH v4 3/5] acpi: add fw_cfg file for TPM and PPI virtual memory device
  2018-06-21 11:55 [Qemu-devel] [PATCH v4 0/5] Add support for TPM Physical Presence interface Marc-André Lureau
  2018-06-21 11:55 ` [Qemu-devel] [PATCH v4 1/5] tpm: add a "ppi" boolean property Marc-André Lureau
  2018-06-21 11:55 ` [Qemu-devel] [PATCH v4 2/5] tpm: implement virtual memory device for TPM PPI Marc-André Lureau
@ 2018-06-21 11:55 ` Marc-André Lureau
  2018-06-21 11:55 ` [Qemu-devel] [PATCH v4 4/5] acpi: build TPM Physical Presence interface Marc-André Lureau
  2018-06-21 11:55 ` [Qemu-devel] [PATCH v4 5/5] tpm: add a fake ACPI memory clear interface Marc-André Lureau
  4 siblings, 0 replies; 19+ messages in thread
From: Marc-André Lureau @ 2018-06-21 11:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Eduardo Habkost, Marcel Apfelbaum, stefanb,
	Michael S. Tsirkin, Igor Mammedov, Richard Henderson,
	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>

---

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..d9320845ed 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 = cpu_to_le32(tpm_get_version(tpm_find())),
+            .tpmppi_version = cpu_to_le32(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

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH v4 4/5] acpi: build TPM Physical Presence interface
  2018-06-21 11:55 [Qemu-devel] [PATCH v4 0/5] Add support for TPM Physical Presence interface Marc-André Lureau
                   ` (2 preceding siblings ...)
  2018-06-21 11:55 ` [Qemu-devel] [PATCH v4 3/5] acpi: add fw_cfg file for TPM and PPI virtual memory device Marc-André Lureau
@ 2018-06-21 11:55 ` Marc-André Lureau
  2018-06-21 20:11   ` Stefan Berger
  2018-06-21 11:55 ` [Qemu-devel] [PATCH v4 5/5] tpm: add a fake ACPI memory clear interface Marc-André Lureau
  4 siblings, 1 reply; 19+ messages in thread
From: Marc-André Lureau @ 2018-06-21 11:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Eduardo Habkost, Marcel Apfelbaum, stefanb,
	Michael S. Tsirkin, Igor Mammedov, Richard Henderson

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>

---

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 |  21 +++
 hw/i386/acpi-build.c  | 294 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 314 insertions(+), 1 deletion(-)

diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
index f79d68a77a..430605a8e5 100644
--- a/include/hw/acpi/tpm.h
+++ b/include/hw/acpi/tpm.h
@@ -196,4 +196,25 @@ REG32(CRB_DATA_BUFFER, 0x80)
 #define TPM_PPI_VERSION_NONE        0
 #define TPM_PPI_VERSION_1_30        1
 
+struct TPMPPIData {
+    uint8_t  func[256];      /* 0x000: per TPM function implementation flags;
+                                       set by BIOS */
+/* 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)
+    uint8_t ppin;            /* 0x100 : set by BIOS */
+    uint32_t ppip;           /* 0x101 : set by ACPI; not used */
+    uint32_t pprp;           /* 0x105 : response from TPM; set by BIOS */
+    uint32_t pprq;           /* 0x109 : opcode; set by ACPI */
+    uint32_t pprm;           /* 0x10d : parameter for opcode; set by ACPI */
+    uint32_t lppr;           /* 0x111 : last opcode; set by BIOS */
+    uint32_t fret;           /* 0x115 : set by ACPI; not used */
+    uint8_t res1[0x40];      /* 0x119 : reserved for future use */
+    uint8_t next_step;       /* 0x159 : next step after reboot; set by BIOS */
+} QEMU_PACKED;
+
 #endif /* HW_ACPI_TPM_H */
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index d9320845ed..4cb3ac9000 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,292 @@ static Aml *build_q35_osc_method(void)
     return method;
 }
 
+static void
+build_tpm_ppi(Aml *dev)
+{
+    Aml *method, *name, *field, *ifctx, *ifctx2, *ifctx3, *pak;
+    struct TPMPPIData *tpm_ppi = NULL;
+    int i;
+
+    /*
+     * 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),
+                                    sizeof(tpm_ppi->func)));
+    field = aml_field("TPP1", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
+    for (i = 0; i < sizeof(tpm_ppi->func); i++) {
+        char *tmp = g_strdup_printf("FN%02X", i);
+        aml_append(field, aml_named_field(tmp, BITS_PER_BYTE));
+        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 +
+                                            offsetof(struct TPMPPIData, ppin)),
+                                    sizeof(struct TPMPPIData) -
+                                        sizeof(tpm_ppi->func)));
+    field = aml_field("TPP2", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
+    aml_append(field, aml_named_field("PPIN",
+               sizeof(uint8_t) * BITS_PER_BYTE));
+    aml_append(field, aml_named_field("PPIP",
+               sizeof(uint32_t) * BITS_PER_BYTE));
+    aml_append(field, aml_named_field("PPRP",
+               sizeof(uint32_t) * BITS_PER_BYTE));
+    aml_append(field, aml_named_field("PPRQ",
+               sizeof(uint32_t) * BITS_PER_BYTE));
+    aml_append(field, aml_named_field("PPRM",
+               sizeof(uint32_t) * BITS_PER_BYTE));
+    aml_append(field, aml_named_field("LPPR",
+               sizeof(uint32_t) * BITS_PER_BYTE));
+    aml_append(dev, field);
+
+    method = aml_method("TPFN", 1, AML_SERIALIZED);
+    {
+        for (i = 0; i < sizeof(tpm_ppi->func); 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));
+    name = aml_name_decl("TPM2", pak);
+    aml_append(dev, name);
+
+    pak = aml_package(3);
+    aml_append(pak, aml_int(0));
+    aml_append(pak, aml_int(0));
+    aml_append(pak, aml_int(0));
+    name = aml_name_decl("TPM3", pak);
+    aml_append(dev, name);
+
+    method = aml_method("_DSM", 4, AML_SERIALIZED);
+    {
+        uint8_t zerobyte[1] = { 0 };
+
+        ifctx = aml_if(
+            aml_equal(aml_arg(0),
+                      aml_touuid("3DDDFAA6-361B-4EB4-A424-8D10089D1653")));
+        {
+            aml_append(ifctx,
+                       aml_store(aml_to_integer(aml_arg(2)), aml_local(0)));
+
+            /* standard DSM query function */
+            ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(0)));
+            {
+                uint8_t byte_list[2] = { 0xff, 0x01 };
+                aml_append(ifctx2, aml_return(aml_buffer(2, byte_list)));
+            }
+            aml_append(ifctx, ifctx2);
+
+            /* interface version: 1.3 */
+            ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(1)));
+            {
+                aml_append(ifctx2, aml_return(aml_string("1.3")));
+            }
+            aml_append(ifctx, ifctx2);
+
+            /* submit TPM operation */
+            ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(2)));
+            {
+                /* get opcode */
+                aml_append(ifctx2,
+                           aml_store(aml_derefof(aml_index(aml_arg(3),
+                                                           aml_int(0))),
+                                     aml_local(0)));
+                /* get opcode flags */
+                aml_append(ifctx2,
+                           aml_store(aml_call1("TPFN", aml_local(0)),
+                                     aml_local(1)));
+                ifctx3 = aml_if(
+                    aml_equal(
+                        aml_and(aml_local(1), 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);
+                aml_append(ifctx2, aml_store(aml_local(0), aml_name("PPRQ")));
+                aml_append(ifctx2, aml_store(aml_int(0), aml_name("PPRM")));
+                /* 0: success */
+                aml_append(ifctx2, aml_return(aml_int(0)));
+            }
+            aml_append(ifctx, ifctx2);
+
+            /* get pending TPM operation */
+            ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(3)));
+            {
+                /* revision to integer */
+                aml_append(ifctx2,
+                           aml_store(
+                               aml_to_integer(aml_arg(1)),
+                               aml_local(1)));
+                ifctx3 = aml_if(aml_equal(aml_local(1), aml_int(1)));
+                {
+                    aml_append(ifctx3,
+                               aml_store(
+                                   aml_name("PPRQ"),
+                                   aml_index(aml_name("TPM2"), aml_int(1))));
+                    aml_append(ifctx3, aml_return(aml_name("TPM2")));
+                }
+                aml_append(ifctx2, ifctx3);
+
+                ifctx3 = aml_if(aml_equal(aml_local(1), aml_int(2)));
+                {
+                    aml_append(ifctx3,
+                               aml_store(
+                                   aml_name("PPRQ"),
+                                   aml_index(aml_name("TPM3"), aml_int(1))));
+                    aml_append(ifctx3,
+                               aml_store(
+                                   aml_name("PPRM"),
+                                   aml_index(aml_name("TPM3"), aml_int(2))));
+                    aml_append(ifctx3, aml_return(aml_name("TPM3")));
+                }
+                aml_append(ifctx2, ifctx3);
+            }
+            aml_append(ifctx, ifctx2);
+
+            /* get platform-specific action to transition to pre-OS env. */
+            ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(4)));
+            {
+                /* reboot */
+                aml_append(ifctx2, aml_return(aml_int(2)));
+            }
+            aml_append(ifctx, ifctx2);
+
+            /* get TPM operation response */
+            ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(5)));
+            {
+                aml_append(ifctx2,
+                           aml_store(
+                               aml_name("LPPR"),
+                               aml_index(aml_name("TPM3"), aml_int(1))));
+                aml_append(ifctx2,
+                           aml_store(
+                               aml_name("PPRP"),
+                               aml_index(aml_name("TPM3"), aml_int(2))));
+                aml_append(ifctx2, aml_return(aml_name("TPM3")));
+            }
+            aml_append(ifctx, ifctx2);
+
+            /* submit preferred user language */
+            ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(6)));
+            {
+                /* 3 = not implemented */
+                aml_append(ifctx2, aml_return(aml_int(3)));
+            }
+            aml_append(ifctx, ifctx2);
+
+            /* submit TPM operation v2 */
+            ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(7)));
+            {
+                /* get opcode */
+                aml_append(ifctx2,
+                           aml_store(aml_derefof(aml_index(aml_arg(3),
+                                                           aml_int(0))),
+                                     aml_local(0)));
+                /* get opcode flags */
+                aml_append(ifctx2,
+                           aml_store(aml_call1("TPFN", aml_local(0)),
+                                     aml_local(1)));
+                ifctx3 = aml_if(
+                    aml_equal(
+                        aml_and(aml_local(1), 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);
+
+                ifctx3 = aml_if(
+                    aml_equal(
+                        aml_and(aml_local(1), 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 */
+                aml_append(ifctx2,
+                           aml_store(
+                               aml_to_integer(aml_arg(1)),
+                               aml_local(1)));
+
+                ifctx3 = aml_if(aml_equal(aml_local(1), aml_int(1)));
+                {
+                    /* revision 1 */
+                    aml_append(ifctx3, aml_store(aml_local(0),
+                                                 aml_name("PPRQ")));
+                    aml_append(ifctx3, aml_store(aml_int(0),
+                                                 aml_name("PPRM")));
+                }
+                aml_append(ifctx2, ifctx3);
+
+                ifctx3 = aml_if(aml_equal(aml_local(1), aml_int(2)));
+                {
+                    /* revision 2 */
+                    aml_append(ifctx3,
+                               aml_store(aml_local(0), aml_name("PPRQ")));
+                    aml_append(ifctx3,
+                               aml_store(
+                                   aml_derefof(aml_index(aml_arg(3),
+                                                         aml_int(1))),
+                                   aml_name("PPRM")));
+                }
+                aml_append(ifctx2, ifctx3);
+                /* 0: success */
+                aml_append(ifctx2, aml_return(aml_int(0)));
+            }
+            aml_append(ifctx, ifctx2);
+
+            /* get user confirmation status for operation */
+            ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(8)));
+            {
+                /* get opcode */
+                aml_append(ifctx2,
+                           aml_store(aml_derefof(aml_index(aml_arg(3),
+                                                           aml_int(0))),
+                                     aml_local(0)));
+                /* get opcode flags */
+                aml_append(ifctx2,
+                           aml_store(aml_call1("TPFN", aml_local(0)),
+                                     aml_local(1)));
+                /* return confirmation status code */
+                aml_append(ifctx2,
+                           aml_return(
+                               aml_and(aml_local(1),
+                                       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,
@@ -2153,6 +2440,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(dev);
+
                 aml_append(scope, dev);
             }
 
@@ -2172,6 +2462,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
         aml_append(method, aml_return(aml_int(0x0f)));
         aml_append(dev, method);
 
+        build_tpm_ppi(dev);
+
         aml_append(sb_scope, dev);
     }
 
@@ -2920,7 +3212,7 @@ void acpi_setup(void)
         tpm_config = (FWCfgTPMConfig) {
             .tpmppi_address = cpu_to_le32(TPM_PPI_ADDR_BASE),
             .tpm_version = cpu_to_le32(tpm_get_version(tpm_find())),
-            .tpmppi_version = cpu_to_le32(TPM_PPI_VERSION_NONE)
+            .tpmppi_version = cpu_to_le32(TPM_PPI_VERSION_1_30)
         };
         fw_cfg_add_file(pcms->fw_cfg, "etc/tpm/config",
                         &tpm_config, sizeof tpm_config);
-- 
2.18.0.rc1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [Qemu-devel] [PATCH v4 5/5] tpm: add a fake ACPI memory clear interface
  2018-06-21 11:55 [Qemu-devel] [PATCH v4 0/5] Add support for TPM Physical Presence interface Marc-André Lureau
                   ` (3 preceding siblings ...)
  2018-06-21 11:55 ` [Qemu-devel] [PATCH v4 4/5] acpi: build TPM Physical Presence interface Marc-André Lureau
@ 2018-06-21 11:55 ` Marc-André Lureau
  4 siblings, 0 replies; 19+ messages in thread
From: Marc-André Lureau @ 2018-06-21 11:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: Paolo Bonzini, Eduardo Habkost, Marcel Apfelbaum, stefanb,
	Michael S. Tsirkin, Igor Mammedov, Richard Henderson,
	Marc-André Lureau

This allows to pass the last failing test from the Windows HLK TPM 2.0
TCG PPI 1.3 tests.

The interface is described in the "TCG Platform Reset Attack
Mitigation Specification", chapter 6 "ACPI _DSM Function". Whether or
not we should have a real implementation remains an open question to me.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/i386/acpi-build.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 4cb3ac9000..fafc61c9dd 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -2072,6 +2072,15 @@ build_tpm_ppi(Aml *dev)
             aml_append(ifctx, aml_return(aml_buffer(1, zerobyte)));
         }
         aml_append(method, ifctx);
+
+       /* dummy MOR Memory Clear for the sake of WLK PPI test */
+        ifctx = aml_if(
+            aml_equal(aml_arg(0),
+                      aml_touuid("376054ED-CC13-4675-901C-4756D7F2D45D")));
+        {
+            aml_append(ifctx, aml_return(aml_int(0)));
+        }
+        aml_append(method, ifctx);
     }
     aml_append(dev, method);
 }
-- 
2.18.0.rc1

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v4 2/5] tpm: implement virtual memory device for TPM PPI
  2018-06-21 11:55 ` [Qemu-devel] [PATCH v4 2/5] tpm: implement virtual memory device for TPM PPI Marc-André Lureau
@ 2018-06-21 12:10   ` Stefan Berger
  2018-06-21 12:24     ` Marc-André Lureau
  0 siblings, 1 reply; 19+ messages in thread
From: Stefan Berger @ 2018-06-21 12:10 UTC (permalink / raw)
  To: Marc-André Lureau, qemu-devel
  Cc: Paolo Bonzini, Eduardo Habkost, Marcel Apfelbaum,
	Michael S. Tsirkin, Igor Mammedov, Richard Henderson

On 06/21/2018 07:55 AM, Marc-André Lureau wrote:
> From: Stefan Berger <stefanb@linux.vnet.ibm.com>
>
> Implement a virtual memory device for the TPM Physical Presence interface.
> The memory is located at 0xfffef000 and used by ACPI to send messages to the

It should say 0xFED45000.

    Stefan

> 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().
>
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> ---
>
> 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      | 27 ++++++++++++++++++++
>   include/hw/acpi/tpm.h |  6 +++++
>   hw/tpm/tpm_crb.c      |  7 ++++++
>   hw/tpm/tpm_ppi.c      | 57 +++++++++++++++++++++++++++++++++++++++++++
>   hw/tpm/tpm_tis.c      |  7 ++++++
>   hw/tpm/Makefile.objs  |  2 +-
>   hw/tpm/trace-events   |  4 +++
>   7 files changed, 109 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..ac7ad47238
> --- /dev/null
> +++ b/hw/tpm/tpm_ppi.h
> @@ -0,0 +1,27 @@
> +/*
> + * 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 mmio;
> +
> +    uint8_t ram[TPM_PPI_ADDR_SIZE];
> +} TPMPPI;
> +
> +void tpm_ppi_init_io(TPMPPI *tpmppi, struct MemoryRegion *m,
> +                     hwaddr addr, Object *obj);
> +
> +#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..37c095f00f 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,11 @@ 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_io(&s->ppi, get_system_memory(),
> +                        TPM_PPI_ADDR_BASE, OBJECT(s));
> +    }
> +
>       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..79bec186c7
> --- /dev/null
> +++ b/hw/tpm/tpm_ppi.c
> @@ -0,0 +1,57 @@
> +/*
> + * 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 "tpm_ppi.h"
> +#include "trace.h"
> +
> +static uint64_t tpm_ppi_mmio_read(void *opaque, hwaddr addr,
> +                                  unsigned size)
> +{
> +    TPMPPI *s = opaque;
> +
> +    trace_tpm_ppi_mmio_read(addr, size, s->ram[addr]);
> +
> +    return s->ram[addr];
> +}
> +
> +static void tpm_ppi_mmio_write(void *opaque, hwaddr addr,
> +                               uint64_t val, unsigned size)
> +{
> +    TPMPPI *s = opaque;
> +
> +    trace_tpm_ppi_mmio_write(addr, size, val);
> +
> +    s->ram[addr] = val;
> +}
> +
> +static const MemoryRegionOps tpm_ppi_memory_ops = {
> +    .read = tpm_ppi_mmio_read,
> +    .write = tpm_ppi_mmio_write,
> +    .endianness = DEVICE_NATIVE_ENDIAN,
> +    .valid = {
> +        .min_access_size = 1,
> +        .max_access_size = 1,
> +    },
> +};
> +
> +void tpm_ppi_init_io(TPMPPI *tpmppi, struct MemoryRegion *m,
> +                     hwaddr addr, Object *obj)
> +{
> +    memory_region_init_io(&tpmppi->mmio, obj, &tpm_ppi_memory_ops,
> +                          tpmppi, "tpm-ppi-mmio",
> +                          TPM_PPI_ADDR_SIZE);
> +
> +    memory_region_add_subregion(m, addr, &tpmppi->mmio);
> +}
> diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
> index d9ddf9b723..e7f45a05b9 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,11 @@ 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_io(&s->ppi, isa_address_space(ISA_DEVICE(dev)),
> +                        TPM_PPI_ADDR_BASE, OBJECT(s));
> +    }
>   }
>
>   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"

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v4 1/5] tpm: add a "ppi" boolean property
  2018-06-21 11:55 ` [Qemu-devel] [PATCH v4 1/5] tpm: add a "ppi" boolean property Marc-André Lureau
@ 2018-06-21 12:11   ` Stefan Berger
  2018-06-21 12:19     ` Stefan Berger
  2018-06-28 11:21     ` Marc-André Lureau
  0 siblings, 2 replies; 19+ messages in thread
From: Stefan Berger @ 2018-06-21 12:11 UTC (permalink / raw)
  To: Marc-André Lureau, qemu-devel
  Cc: Eduardo Habkost, Michael S. Tsirkin, Igor Mammedov,
	Paolo Bonzini, Richard Henderson

On 06/21/2018 07:55 AM, Marc-André Lureau wrote:
> The following patches implement the TPM Physical Presence Interface,
> and makes use of new memory region and fw_cfg entries. Enable it by
> default on >2.12 machine type.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@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 563908b874..dac847548b 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(),
>   };

Wouldn't we have to bump up the version of teh VMStateDescription?

    Stefan
>   
> 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(),
>   };
>   

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v4 1/5] tpm: add a "ppi" boolean property
  2018-06-21 12:11   ` Stefan Berger
@ 2018-06-21 12:19     ` Stefan Berger
  2018-06-28 11:21     ` Marc-André Lureau
  1 sibling, 0 replies; 19+ messages in thread
From: Stefan Berger @ 2018-06-21 12:19 UTC (permalink / raw)
  To: Marc-André Lureau, qemu-devel
  Cc: Eduardo Habkost, Michael S. Tsirkin, Igor Mammedov,
	Paolo Bonzini, Richard Henderson

On 06/21/2018 08:11 AM, Stefan Berger wrote:
> On 06/21/2018 07:55 AM, Marc-André Lureau wrote:
>> The following patches implement the TPM Physical Presence Interface,
>> and makes use of new memory region and fw_cfg entries. Enable it by
>> default on >2.12 machine type.
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@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 563908b874..dac847548b 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(),
>>   };
>
> Wouldn't we have to bump up the version of teh VMStateDescription?

Ah, command line option...

    Stefan


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v4 2/5] tpm: implement virtual memory device for TPM PPI
  2018-06-21 12:10   ` Stefan Berger
@ 2018-06-21 12:24     ` Marc-André Lureau
  0 siblings, 0 replies; 19+ messages in thread
From: Marc-André Lureau @ 2018-06-21 12:24 UTC (permalink / raw)
  To: Stefan Berger
  Cc: qemu-devel, Paolo Bonzini, Eduardo Habkost, Marcel Apfelbaum,
	Michael S. Tsirkin, Igor Mammedov, Richard Henderson

On Thu, Jun 21, 2018 at 2:10 PM, Stefan Berger
<stefanb@linux.vnet.ibm.com> wrote:
> On 06/21/2018 07:55 AM, Marc-André Lureau wrote:
>>
>> From: Stefan Berger <stefanb@linux.vnet.ibm.com>
>>
>> Implement a virtual memory device for the TPM Physical Presence interface.
>> The memory is located at 0xfffef000 and used by ACPI to send messages to
>> the
>
>
> It should say 0xFED45000.

right, I forgot to update the commit message

>
>    Stefan
>
>
>> 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().
>>
>> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> ---
>>
>> 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      | 27 ++++++++++++++++++++
>>   include/hw/acpi/tpm.h |  6 +++++
>>   hw/tpm/tpm_crb.c      |  7 ++++++
>>   hw/tpm/tpm_ppi.c      | 57 +++++++++++++++++++++++++++++++++++++++++++
>>   hw/tpm/tpm_tis.c      |  7 ++++++
>>   hw/tpm/Makefile.objs  |  2 +-
>>   hw/tpm/trace-events   |  4 +++
>>   7 files changed, 109 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..ac7ad47238
>> --- /dev/null
>> +++ b/hw/tpm/tpm_ppi.h
>> @@ -0,0 +1,27 @@
>> +/*
>> + * 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 mmio;
>> +
>> +    uint8_t ram[TPM_PPI_ADDR_SIZE];
>> +} TPMPPI;
>> +
>> +void tpm_ppi_init_io(TPMPPI *tpmppi, struct MemoryRegion *m,
>> +                     hwaddr addr, Object *obj);
>> +
>> +#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..37c095f00f 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,11 @@ 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_io(&s->ppi, get_system_memory(),
>> +                        TPM_PPI_ADDR_BASE, OBJECT(s));
>> +    }
>> +
>>       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..79bec186c7
>> --- /dev/null
>> +++ b/hw/tpm/tpm_ppi.c
>> @@ -0,0 +1,57 @@
>> +/*
>> + * 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 "tpm_ppi.h"
>> +#include "trace.h"
>> +
>> +static uint64_t tpm_ppi_mmio_read(void *opaque, hwaddr addr,
>> +                                  unsigned size)
>> +{
>> +    TPMPPI *s = opaque;
>> +
>> +    trace_tpm_ppi_mmio_read(addr, size, s->ram[addr]);
>> +
>> +    return s->ram[addr];
>> +}
>> +
>> +static void tpm_ppi_mmio_write(void *opaque, hwaddr addr,
>> +                               uint64_t val, unsigned size)
>> +{
>> +    TPMPPI *s = opaque;
>> +
>> +    trace_tpm_ppi_mmio_write(addr, size, val);
>> +
>> +    s->ram[addr] = val;
>> +}
>> +
>> +static const MemoryRegionOps tpm_ppi_memory_ops = {
>> +    .read = tpm_ppi_mmio_read,
>> +    .write = tpm_ppi_mmio_write,
>> +    .endianness = DEVICE_NATIVE_ENDIAN,
>> +    .valid = {
>> +        .min_access_size = 1,
>> +        .max_access_size = 1,
>> +    },
>> +};
>> +
>> +void tpm_ppi_init_io(TPMPPI *tpmppi, struct MemoryRegion *m,
>> +                     hwaddr addr, Object *obj)
>> +{
>> +    memory_region_init_io(&tpmppi->mmio, obj, &tpm_ppi_memory_ops,
>> +                          tpmppi, "tpm-ppi-mmio",
>> +                          TPM_PPI_ADDR_SIZE);
>> +
>> +    memory_region_add_subregion(m, addr, &tpmppi->mmio);
>> +}
>> diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
>> index d9ddf9b723..e7f45a05b9 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,11 @@ 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_io(&s->ppi, isa_address_space(ISA_DEVICE(dev)),
>> +                        TPM_PPI_ADDR_BASE, OBJECT(s));
>> +    }
>>   }
>>
>>   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"
>
>
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v4 4/5] acpi: build TPM Physical Presence interface
  2018-06-21 11:55 ` [Qemu-devel] [PATCH v4 4/5] acpi: build TPM Physical Presence interface Marc-André Lureau
@ 2018-06-21 20:11   ` Stefan Berger
  2018-06-22  9:03     ` Igor Mammedov
  0 siblings, 1 reply; 19+ messages in thread
From: Stefan Berger @ 2018-06-21 20:11 UTC (permalink / raw)
  To: Marc-André Lureau, qemu-devel
  Cc: Paolo Bonzini, Eduardo Habkost, Marcel Apfelbaum,
	Michael S. Tsirkin, Igor Mammedov, Richard Henderson

On 06/21/2018 07:55 AM, Marc-André Lureau wrote:
> From: Stefan Berger <stefanb@linux.vnet.ibm.com>
>
> The TPM Physical Presence interface consists of an ACPI part, a shared
> memory part, and code in the firmware. Users can send messages to the
> firmware by writing a code into the shared memory through invoking the
> ACPI code. When a reboot happens, the firmware looks for the code and
> acts on it by sending sequences of commands to the TPM.
>
> This patch adds the ACPI code. It is similar to the one in EDK2 but doesn't
> assume that SMIs are necessary to use. It uses a similar datastructure for
> the shared memory as EDK2 does so that EDK2 and SeaBIOS could both make use
> of it. I extended the shared memory data structure with an array of 256
> bytes, one for each code that could be implemented. The array contains
> flags describing the individual codes. This decouples the ACPI implementation
> from the firmware implementation.
>
> The underlying TCG specification is accessible from the following page.
>
> https://trustedcomputinggroup.org/tcg-physical-presence-interface-specification/
>
> This patch implements version 1.30.
>
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
>
> ---
>
> 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 |  21 +++
>   hw/i386/acpi-build.c  | 294 +++++++++++++++++++++++++++++++++++++++++-
>   2 files changed, 314 insertions(+), 1 deletion(-)
>
> diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
> index f79d68a77a..430605a8e5 100644
> --- a/include/hw/acpi/tpm.h
> +++ b/include/hw/acpi/tpm.h
> @@ -196,4 +196,25 @@ REG32(CRB_DATA_BUFFER, 0x80)
>   #define TPM_PPI_VERSION_NONE        0
>   #define TPM_PPI_VERSION_1_30        1
>
> +struct TPMPPIData {
> +    uint8_t  func[256];      /* 0x000: per TPM function implementation flags;
> +                                       set by BIOS */
> +/* 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)
> +    uint8_t ppin;            /* 0x100 : set by BIOS */
> +    uint32_t ppip;           /* 0x101 : set by ACPI; not used */
> +    uint32_t pprp;           /* 0x105 : response from TPM; set by BIOS */
> +    uint32_t pprq;           /* 0x109 : opcode; set by ACPI */
> +    uint32_t pprm;           /* 0x10d : parameter for opcode; set by ACPI */
> +    uint32_t lppr;           /* 0x111 : last opcode; set by BIOS */
> +    uint32_t fret;           /* 0x115 : set by ACPI; not used */
> +    uint8_t res1[0x40];      /* 0x119 : reserved for future use */
> +    uint8_t next_step;       /* 0x159 : next step after reboot; set by BIOS */
> +} QEMU_PACKED;
> +
>   #endif /* HW_ACPI_TPM_H */

Here's a description of this interface. The SMM related fields, ppin, 
ppip and fret could be
renamed to reserved fields since we are not supporting SMM.

diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
index c230c4c93e..17d811f633 100644
--- a/docs/specs/tpm.txt
+++ b/docs/specs/tpm.txt
@@ -42,6 +42,73 @@ 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:
+
+    struct TPMPPIData {
+        uint8_t  func[256];      /* 0x000 */
+    /* 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)
+        uint8_t ppin;            /* 0x100 */
+        uint32_t ppip;           /* 0x101 */
+        uint32_t pprp;           /* 0x105 */
+        uint32_t pprq;           /* 0x109 */
+        uint32_t pprm;           /* 0x10d */
+        uint32_t lppr;           /* 0x111 */
+        uint32_t fret;           /* 0x115 */
+        uint8_t res1[0x40];      /* 0x119 */
+        uint8_t next_step;       /* 0x159 */
+    } QEMU_PACKED;
+
+For each code the firmware suppports, the firmware needs to set the 
appropriate
+flags in the func array. The number of the operation serves as the 
index for the
+array.
+
+The operation request's number is written into the pprq field. Any optional
+additional parameters needed by an operation request must be written into
+the pprm field.
+
+The firmware indicates the last executed command in the lppr field and
+writes the result into the pprp field.
+
+For SMM support, the field ppin describes the software SMI interrupt to 
use.
+This field needs to be written by the firmware. The ppip field is used
+to pass the ACPI function number to the SMM code. This field needs to be
+written by ACPI. The fret field holds the result of the SMM operation and
+needs to be set by SMM code.
+
+Some operations require the firmware to reboot the machine before it can
+send more commands to the TPM. For this, the firmware can use the next_step
+field to remember what operation to execute after the reboot.

  QEMU files related to TPM ACPI tables:
   - hw/i386/acpi-build.c


> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index d9320845ed..4cb3ac9000 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,292 @@ static Aml *build_q35_osc_method(void)
>       return method;
>   }
>
> +static void
> +build_tpm_ppi(Aml *dev)
> +{
> +    Aml *method, *name, *field, *ifctx, *ifctx2, *ifctx3, *pak;
> +    struct TPMPPIData *tpm_ppi = NULL;
> +    int i;
> +
> +    /*
> +     * 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),
> +                                    sizeof(tpm_ppi->func)));
> +    field = aml_field("TPP1", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
> +    for (i = 0; i < sizeof(tpm_ppi->func); i++) {
> +        char *tmp = g_strdup_printf("FN%02X", i);
> +        aml_append(field, aml_named_field(tmp, BITS_PER_BYTE));
> +        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 +
> +                                            offsetof(struct TPMPPIData, ppin)),
> +                                    sizeof(struct TPMPPIData) -
> +                                        sizeof(tpm_ppi->func)));
> +    field = aml_field("TPP2", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
> +    aml_append(field, aml_named_field("PPIN",
> +               sizeof(uint8_t) * BITS_PER_BYTE));
> +    aml_append(field, aml_named_field("PPIP",
> +               sizeof(uint32_t) * BITS_PER_BYTE));
> +    aml_append(field, aml_named_field("PPRP",
> +               sizeof(uint32_t) * BITS_PER_BYTE));
> +    aml_append(field, aml_named_field("PPRQ",
> +               sizeof(uint32_t) * BITS_PER_BYTE));
> +    aml_append(field, aml_named_field("PPRM",
> +               sizeof(uint32_t) * BITS_PER_BYTE));
> +    aml_append(field, aml_named_field("LPPR",
> +               sizeof(uint32_t) * BITS_PER_BYTE));
> +    aml_append(dev, field);
> +
> +    method = aml_method("TPFN", 1, AML_SERIALIZED);
> +    {
> +        for (i = 0; i < sizeof(tpm_ppi->func); 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));
> +    name = aml_name_decl("TPM2", pak);
> +    aml_append(dev, name);
> +
> +    pak = aml_package(3);
> +    aml_append(pak, aml_int(0));
> +    aml_append(pak, aml_int(0));
> +    aml_append(pak, aml_int(0));
> +    name = aml_name_decl("TPM3", pak);
> +    aml_append(dev, name);
> +
> +    method = aml_method("_DSM", 4, AML_SERIALIZED);
> +    {
> +        uint8_t zerobyte[1] = { 0 };
> +
> +        ifctx = aml_if(
> +            aml_equal(aml_arg(0),
> +                      aml_touuid("3DDDFAA6-361B-4EB4-A424-8D10089D1653")));
> +        {
> +            aml_append(ifctx,
> +                       aml_store(aml_to_integer(aml_arg(2)), aml_local(0)));
> +
> +            /* standard DSM query function */
> +            ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(0)));
> +            {
> +                uint8_t byte_list[2] = { 0xff, 0x01 };
> +                aml_append(ifctx2, aml_return(aml_buffer(2, byte_list)));
> +            }
> +            aml_append(ifctx, ifctx2);
> +
> +            /* interface version: 1.3 */
> +            ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(1)));
> +            {
> +                aml_append(ifctx2, aml_return(aml_string("1.3")));
> +            }
> +            aml_append(ifctx, ifctx2);
> +
> +            /* submit TPM operation */
> +            ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(2)));
> +            {
> +                /* get opcode */
> +                aml_append(ifctx2,
> +                           aml_store(aml_derefof(aml_index(aml_arg(3),
> +                                                           aml_int(0))),
> +                                     aml_local(0)));
> +                /* get opcode flags */
> +                aml_append(ifctx2,
> +                           aml_store(aml_call1("TPFN", aml_local(0)),
> +                                     aml_local(1)));
> +                ifctx3 = aml_if(
> +                    aml_equal(
> +                        aml_and(aml_local(1), 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);
> +                aml_append(ifctx2, aml_store(aml_local(0), aml_name("PPRQ")));
> +                aml_append(ifctx2, aml_store(aml_int(0), aml_name("PPRM")));
> +                /* 0: success */
> +                aml_append(ifctx2, aml_return(aml_int(0)));
> +            }
> +            aml_append(ifctx, ifctx2);
> +
> +            /* get pending TPM operation */
> +            ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(3)));
> +            {
> +                /* revision to integer */
> +                aml_append(ifctx2,
> +                           aml_store(
> +                               aml_to_integer(aml_arg(1)),
> +                               aml_local(1)));
> +                ifctx3 = aml_if(aml_equal(aml_local(1), aml_int(1)));
> +                {
> +                    aml_append(ifctx3,
> +                               aml_store(
> +                                   aml_name("PPRQ"),
> +                                   aml_index(aml_name("TPM2"), aml_int(1))));
> +                    aml_append(ifctx3, aml_return(aml_name("TPM2")));
> +                }
> +                aml_append(ifctx2, ifctx3);
> +
> +                ifctx3 = aml_if(aml_equal(aml_local(1), aml_int(2)));
> +                {
> +                    aml_append(ifctx3,
> +                               aml_store(
> +                                   aml_name("PPRQ"),
> +                                   aml_index(aml_name("TPM3"), aml_int(1))));
> +                    aml_append(ifctx3,
> +                               aml_store(
> +                                   aml_name("PPRM"),
> +                                   aml_index(aml_name("TPM3"), aml_int(2))));
> +                    aml_append(ifctx3, aml_return(aml_name("TPM3")));
> +                }
> +                aml_append(ifctx2, ifctx3);
> +            }
> +            aml_append(ifctx, ifctx2);
> +
> +            /* get platform-specific action to transition to pre-OS env. */
> +            ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(4)));
> +            {
> +                /* reboot */
> +                aml_append(ifctx2, aml_return(aml_int(2)));
> +            }
> +            aml_append(ifctx, ifctx2);
> +
> +            /* get TPM operation response */
> +            ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(5)));
> +            {
> +                aml_append(ifctx2,
> +                           aml_store(
> +                               aml_name("LPPR"),
> +                               aml_index(aml_name("TPM3"), aml_int(1))));
> +                aml_append(ifctx2,
> +                           aml_store(
> +                               aml_name("PPRP"),
> +                               aml_index(aml_name("TPM3"), aml_int(2))));
> +                aml_append(ifctx2, aml_return(aml_name("TPM3")));
> +            }
> +            aml_append(ifctx, ifctx2);
> +
> +            /* submit preferred user language */
> +            ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(6)));
> +            {
> +                /* 3 = not implemented */
> +                aml_append(ifctx2, aml_return(aml_int(3)));
> +            }
> +            aml_append(ifctx, ifctx2);
> +
> +            /* submit TPM operation v2 */
> +            ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(7)));
> +            {
> +                /* get opcode */
> +                aml_append(ifctx2,
> +                           aml_store(aml_derefof(aml_index(aml_arg(3),
> +                                                           aml_int(0))),
> +                                     aml_local(0)));
> +                /* get opcode flags */
> +                aml_append(ifctx2,
> +                           aml_store(aml_call1("TPFN", aml_local(0)),
> +                                     aml_local(1)));
> +                ifctx3 = aml_if(
> +                    aml_equal(
> +                        aml_and(aml_local(1), 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);
> +
> +                ifctx3 = aml_if(
> +                    aml_equal(
> +                        aml_and(aml_local(1), 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 */
> +                aml_append(ifctx2,
> +                           aml_store(
> +                               aml_to_integer(aml_arg(1)),
> +                               aml_local(1)));
> +
> +                ifctx3 = aml_if(aml_equal(aml_local(1), aml_int(1)));
> +                {
> +                    /* revision 1 */
> +                    aml_append(ifctx3, aml_store(aml_local(0),
> +                                                 aml_name("PPRQ")));
> +                    aml_append(ifctx3, aml_store(aml_int(0),
> +                                                 aml_name("PPRM")));
> +                }
> +                aml_append(ifctx2, ifctx3);
> +
> +                ifctx3 = aml_if(aml_equal(aml_local(1), aml_int(2)));
> +                {
> +                    /* revision 2 */
> +                    aml_append(ifctx3,
> +                               aml_store(aml_local(0), aml_name("PPRQ")));
> +                    aml_append(ifctx3,
> +                               aml_store(
> +                                   aml_derefof(aml_index(aml_arg(3),
> +                                                         aml_int(1))),
> +                                   aml_name("PPRM")));
> +                }
> +                aml_append(ifctx2, ifctx3);
> +                /* 0: success */
> +                aml_append(ifctx2, aml_return(aml_int(0)));
> +            }
> +            aml_append(ifctx, ifctx2);
> +
> +            /* get user confirmation status for operation */
> +            ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(8)));
> +            {
> +                /* get opcode */
> +                aml_append(ifctx2,
> +                           aml_store(aml_derefof(aml_index(aml_arg(3),
> +                                                           aml_int(0))),
> +                                     aml_local(0)));
> +                /* get opcode flags */
> +                aml_append(ifctx2,
> +                           aml_store(aml_call1("TPFN", aml_local(0)),
> +                                     aml_local(1)));
> +                /* return confirmation status code */
> +                aml_append(ifctx2,
> +                           aml_return(
> +                               aml_and(aml_local(1),
> +                                       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,
> @@ -2153,6 +2440,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(dev);
> +
>                   aml_append(scope, dev);
>               }
>
> @@ -2172,6 +2462,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>           aml_append(method, aml_return(aml_int(0x0f)));
>           aml_append(dev, method);
>
> +        build_tpm_ppi(dev);
> +
>           aml_append(sb_scope, dev);
>       }
>
> @@ -2920,7 +3212,7 @@ void acpi_setup(void)
>           tpm_config = (FWCfgTPMConfig) {
>               .tpmppi_address = cpu_to_le32(TPM_PPI_ADDR_BASE),
>               .tpm_version = cpu_to_le32(tpm_get_version(tpm_find())),
> -            .tpmppi_version = cpu_to_le32(TPM_PPI_VERSION_NONE)
> +            .tpmppi_version = cpu_to_le32(TPM_PPI_VERSION_1_30)
>           };
>           fw_cfg_add_file(pcms->fw_cfg, "etc/tpm/config",
>                           &tpm_config, sizeof tpm_config);

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v4 4/5] acpi: build TPM Physical Presence interface
  2018-06-21 20:11   ` Stefan Berger
@ 2018-06-22  9:03     ` Igor Mammedov
  2018-06-22 13:26       ` Stefan Berger
  0 siblings, 1 reply; 19+ messages in thread
From: Igor Mammedov @ 2018-06-22  9:03 UTC (permalink / raw)
  To: Stefan Berger
  Cc: Marc-André Lureau, qemu-devel, Eduardo Habkost,
	Michael S. Tsirkin, Paolo Bonzini, Richard Henderson

On Thu, 21 Jun 2018 16:11:16 -0400
Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:

> On 06/21/2018 07:55 AM, Marc-André Lureau wrote:
> > From: Stefan Berger <stefanb@linux.vnet.ibm.com>
> >
> > The TPM Physical Presence interface consists of an ACPI part, a shared
> > memory part, and code in the firmware. Users can send messages to the
> > firmware by writing a code into the shared memory through invoking the
> > ACPI code. When a reboot happens, the firmware looks for the code and
> > acts on it by sending sequences of commands to the TPM.
> >
> > This patch adds the ACPI code. It is similar to the one in EDK2 but doesn't
> > assume that SMIs are necessary to use. It uses a similar datastructure for
> > the shared memory as EDK2 does so that EDK2 and SeaBIOS could both make use
> > of it. I extended the shared memory data structure with an array of 256
> > bytes, one for each code that could be implemented. The array contains
> > flags describing the individual codes. This decouples the ACPI implementation
> > from the firmware implementation.
> >
> > The underlying TCG specification is accessible from the following page.
> >
> > https://trustedcomputinggroup.org/tcg-physical-presence-interface-specification/
> >
> > This patch implements version 1.30.
> >
> > Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> >
> > ---
> >
> > 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 |  21 +++
> >   hw/i386/acpi-build.c  | 294 +++++++++++++++++++++++++++++++++++++++++-
> >   2 files changed, 314 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
> > index f79d68a77a..430605a8e5 100644
> > --- a/include/hw/acpi/tpm.h
> > +++ b/include/hw/acpi/tpm.h
> > @@ -196,4 +196,25 @@ REG32(CRB_DATA_BUFFER, 0x80)
> >   #define TPM_PPI_VERSION_NONE        0
> >   #define TPM_PPI_VERSION_1_30        1
> >
> > +struct TPMPPIData {
> > +    uint8_t  func[256];      /* 0x000: per TPM function implementation flags;
> > +                                       set by BIOS */
> > +/* 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)
> > +    uint8_t ppin;            /* 0x100 : set by BIOS */
> > +    uint32_t ppip;           /* 0x101 : set by ACPI; not used */
> > +    uint32_t pprp;           /* 0x105 : response from TPM; set by BIOS */
> > +    uint32_t pprq;           /* 0x109 : opcode; set by ACPI */
> > +    uint32_t pprm;           /* 0x10d : parameter for opcode; set by ACPI */
> > +    uint32_t lppr;           /* 0x111 : last opcode; set by BIOS */
> > +    uint32_t fret;           /* 0x115 : set by ACPI; not used */
> > +    uint8_t res1[0x40];      /* 0x119 : reserved for future use */
> > +    uint8_t next_step;       /* 0x159 : next step after reboot; set by BIOS */
> > +} QEMU_PACKED;
> > +
> >   #endif /* HW_ACPI_TPM_H */  
> 
> Here's a description of this interface. The SMM related fields, ppin, 
> ppip and fret could be
> renamed to reserved fields since we are not supporting SMM.
> 
> diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
> index c230c4c93e..17d811f633 100644
> --- a/docs/specs/tpm.txt
> +++ b/docs/specs/tpm.txt
> @@ -42,6 +42,73 @@ 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:
I'd prefer a table format to describe layout, like in
docs/specs/acpi_nvdimm.txt or docs/specs/acpi_mem_hotplug.txt


> +
> +    struct TPMPPIData {
> +        uint8_t  func[256];      /* 0x000 */
> +    /* 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)
> +        uint8_t ppin;            /* 0x100 */
> +        uint32_t ppip;           /* 0x101 */
> +        uint32_t pprp;           /* 0x105 */
> +        uint32_t pprq;           /* 0x109 */
> +        uint32_t pprm;           /* 0x10d */
> +        uint32_t lppr;           /* 0x111 */
> +        uint32_t fret;           /* 0x115 */
> +        uint8_t res1[0x40];      /* 0x119 */
> +        uint8_t next_step;       /* 0x159 */
> +    } QEMU_PACKED;
> +
> +For each code the firmware suppports, the firmware needs to set the 
> appropriate
> +flags in the func array. The number of the operation serves as the 
> index for the
> +array.
> +
> +The operation request's number is written into the pprq field. Any optional
> +additional parameters needed by an operation request must be written into
> +the pprm field.
> +
> +The firmware indicates the last executed command in the lppr field and
> +writes the result into the pprp field.
> +
> +For SMM support, the field ppin describes the software SMI interrupt to 
> use.
> +This field needs to be written by the firmware. The ppip field is used
> +to pass the ACPI function number to the SMM code. This field needs to be
> +written by ACPI. The fret field holds the result of the SMM operation and
> +needs to be set by SMM code.
> +
> +Some operations require the firmware to reboot the machine before it can
> +send more commands to the TPM. For this, the firmware can use the next_step
> +field to remember what operation to execute after the reboot.
> 
>   QEMU files related to TPM ACPI tables:
>    - hw/i386/acpi-build.c
> 
> 
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index d9320845ed..4cb3ac9000 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,292 @@ static Aml *build_q35_osc_method(void)
> >       return method;
> >   }
> >
> > +static void
> > +build_tpm_ppi(Aml *dev)
> > +{
> > +    Aml *method, *name, *field, *ifctx, *ifctx2, *ifctx3, *pak;
> > +    struct TPMPPIData *tpm_ppi = NULL;
> > +    int i;
> > +
> > +    /*
> > +     * 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),
> > +                                    sizeof(tpm_ppi->func)));
> > +    field = aml_field("TPP1", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
> > +    for (i = 0; i < sizeof(tpm_ppi->func); i++) {
> > +        char *tmp = g_strdup_printf("FN%02X", i);
> > +        aml_append(field, aml_named_field(tmp, BITS_PER_BYTE));
> > +        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 +
> > +                                            offsetof(struct TPMPPIData, ppin)),
> > +                                    sizeof(struct TPMPPIData) -
> > +                                        sizeof(tpm_ppi->func)));
> > +    field = aml_field("TPP2", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
> > +    aml_append(field, aml_named_field("PPIN",
> > +               sizeof(uint8_t) * BITS_PER_BYTE));
> > +    aml_append(field, aml_named_field("PPIP",
> > +               sizeof(uint32_t) * BITS_PER_BYTE));
> > +    aml_append(field, aml_named_field("PPRP",
> > +               sizeof(uint32_t) * BITS_PER_BYTE));
> > +    aml_append(field, aml_named_field("PPRQ",
> > +               sizeof(uint32_t) * BITS_PER_BYTE));
> > +    aml_append(field, aml_named_field("PPRM",
> > +               sizeof(uint32_t) * BITS_PER_BYTE));
> > +    aml_append(field, aml_named_field("LPPR",
> > +               sizeof(uint32_t) * BITS_PER_BYTE));
> > +    aml_append(dev, field);
> > +
> > +    method = aml_method("TPFN", 1, AML_SERIALIZED);
> > +    {
> > +        for (i = 0; i < sizeof(tpm_ppi->func); 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));
> > +    name = aml_name_decl("TPM2", pak);
> > +    aml_append(dev, name);
> > +
> > +    pak = aml_package(3);
> > +    aml_append(pak, aml_int(0));
> > +    aml_append(pak, aml_int(0));
> > +    aml_append(pak, aml_int(0));
> > +    name = aml_name_decl("TPM3", pak);
> > +    aml_append(dev, name);
> > +
> > +    method = aml_method("_DSM", 4, AML_SERIALIZED);
> > +    {
> > +        uint8_t zerobyte[1] = { 0 };
> > +
> > +        ifctx = aml_if(
> > +            aml_equal(aml_arg(0),
> > +                      aml_touuid("3DDDFAA6-361B-4EB4-A424-8D10089D1653")));
> > +        {
> > +            aml_append(ifctx,
> > +                       aml_store(aml_to_integer(aml_arg(2)), aml_local(0)));
> > +
> > +            /* standard DSM query function */
> > +            ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(0)));
> > +            {
> > +                uint8_t byte_list[2] = { 0xff, 0x01 };
> > +                aml_append(ifctx2, aml_return(aml_buffer(2, byte_list)));
> > +            }
> > +            aml_append(ifctx, ifctx2);
> > +
> > +            /* interface version: 1.3 */
> > +            ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(1)));
> > +            {
> > +                aml_append(ifctx2, aml_return(aml_string("1.3")));
> > +            }
> > +            aml_append(ifctx, ifctx2);
> > +
> > +            /* submit TPM operation */
> > +            ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(2)));
> > +            {
> > +                /* get opcode */
> > +                aml_append(ifctx2,
> > +                           aml_store(aml_derefof(aml_index(aml_arg(3),
> > +                                                           aml_int(0))),
> > +                                     aml_local(0)));
> > +                /* get opcode flags */
> > +                aml_append(ifctx2,
> > +                           aml_store(aml_call1("TPFN", aml_local(0)),
> > +                                     aml_local(1)));
> > +                ifctx3 = aml_if(
> > +                    aml_equal(
> > +                        aml_and(aml_local(1), 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);
> > +                aml_append(ifctx2, aml_store(aml_local(0), aml_name("PPRQ")));
> > +                aml_append(ifctx2, aml_store(aml_int(0), aml_name("PPRM")));
> > +                /* 0: success */
> > +                aml_append(ifctx2, aml_return(aml_int(0)));
> > +            }
> > +            aml_append(ifctx, ifctx2);
> > +
> > +            /* get pending TPM operation */
> > +            ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(3)));
> > +            {
> > +                /* revision to integer */
> > +                aml_append(ifctx2,
> > +                           aml_store(
> > +                               aml_to_integer(aml_arg(1)),
> > +                               aml_local(1)));
> > +                ifctx3 = aml_if(aml_equal(aml_local(1), aml_int(1)));
> > +                {
> > +                    aml_append(ifctx3,
> > +                               aml_store(
> > +                                   aml_name("PPRQ"),
> > +                                   aml_index(aml_name("TPM2"), aml_int(1))));
> > +                    aml_append(ifctx3, aml_return(aml_name("TPM2")));
> > +                }
> > +                aml_append(ifctx2, ifctx3);
> > +
> > +                ifctx3 = aml_if(aml_equal(aml_local(1), aml_int(2)));
> > +                {
> > +                    aml_append(ifctx3,
> > +                               aml_store(
> > +                                   aml_name("PPRQ"),
> > +                                   aml_index(aml_name("TPM3"), aml_int(1))));
> > +                    aml_append(ifctx3,
> > +                               aml_store(
> > +                                   aml_name("PPRM"),
> > +                                   aml_index(aml_name("TPM3"), aml_int(2))));
> > +                    aml_append(ifctx3, aml_return(aml_name("TPM3")));
> > +                }
> > +                aml_append(ifctx2, ifctx3);
> > +            }
> > +            aml_append(ifctx, ifctx2);
> > +
> > +            /* get platform-specific action to transition to pre-OS env. */
> > +            ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(4)));
> > +            {
> > +                /* reboot */
> > +                aml_append(ifctx2, aml_return(aml_int(2)));
> > +            }
> > +            aml_append(ifctx, ifctx2);
> > +
> > +            /* get TPM operation response */
> > +            ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(5)));
> > +            {
> > +                aml_append(ifctx2,
> > +                           aml_store(
> > +                               aml_name("LPPR"),
> > +                               aml_index(aml_name("TPM3"), aml_int(1))));
> > +                aml_append(ifctx2,
> > +                           aml_store(
> > +                               aml_name("PPRP"),
> > +                               aml_index(aml_name("TPM3"), aml_int(2))));
> > +                aml_append(ifctx2, aml_return(aml_name("TPM3")));
> > +            }
> > +            aml_append(ifctx, ifctx2);
> > +
> > +            /* submit preferred user language */
> > +            ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(6)));
> > +            {
> > +                /* 3 = not implemented */
> > +                aml_append(ifctx2, aml_return(aml_int(3)));
> > +            }
> > +            aml_append(ifctx, ifctx2);
> > +
> > +            /* submit TPM operation v2 */
> > +            ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(7)));
> > +            {
> > +                /* get opcode */
> > +                aml_append(ifctx2,
> > +                           aml_store(aml_derefof(aml_index(aml_arg(3),
> > +                                                           aml_int(0))),
> > +                                     aml_local(0)));
> > +                /* get opcode flags */
> > +                aml_append(ifctx2,
> > +                           aml_store(aml_call1("TPFN", aml_local(0)),
> > +                                     aml_local(1)));
> > +                ifctx3 = aml_if(
> > +                    aml_equal(
> > +                        aml_and(aml_local(1), 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);
> > +
> > +                ifctx3 = aml_if(
> > +                    aml_equal(
> > +                        aml_and(aml_local(1), 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 */
> > +                aml_append(ifctx2,
> > +                           aml_store(
> > +                               aml_to_integer(aml_arg(1)),
> > +                               aml_local(1)));
> > +
> > +                ifctx3 = aml_if(aml_equal(aml_local(1), aml_int(1)));
> > +                {
> > +                    /* revision 1 */
> > +                    aml_append(ifctx3, aml_store(aml_local(0),
> > +                                                 aml_name("PPRQ")));
> > +                    aml_append(ifctx3, aml_store(aml_int(0),
> > +                                                 aml_name("PPRM")));
> > +                }
> > +                aml_append(ifctx2, ifctx3);
> > +
> > +                ifctx3 = aml_if(aml_equal(aml_local(1), aml_int(2)));
> > +                {
> > +                    /* revision 2 */
> > +                    aml_append(ifctx3,
> > +                               aml_store(aml_local(0), aml_name("PPRQ")));
> > +                    aml_append(ifctx3,
> > +                               aml_store(
> > +                                   aml_derefof(aml_index(aml_arg(3),
> > +                                                         aml_int(1))),
> > +                                   aml_name("PPRM")));
> > +                }
> > +                aml_append(ifctx2, ifctx3);
> > +                /* 0: success */
> > +                aml_append(ifctx2, aml_return(aml_int(0)));
> > +            }
> > +            aml_append(ifctx, ifctx2);
> > +
> > +            /* get user confirmation status for operation */
> > +            ifctx2 = aml_if(aml_equal(aml_local(0), aml_int(8)));
> > +            {
> > +                /* get opcode */
> > +                aml_append(ifctx2,
> > +                           aml_store(aml_derefof(aml_index(aml_arg(3),
> > +                                                           aml_int(0))),
> > +                                     aml_local(0)));
> > +                /* get opcode flags */
> > +                aml_append(ifctx2,
> > +                           aml_store(aml_call1("TPFN", aml_local(0)),
> > +                                     aml_local(1)));
> > +                /* return confirmation status code */
> > +                aml_append(ifctx2,
> > +                           aml_return(
> > +                               aml_and(aml_local(1),
> > +                                       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,
> > @@ -2153,6 +2440,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(dev);
> > +
> >                   aml_append(scope, dev);
> >               }
> >
> > @@ -2172,6 +2462,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> >           aml_append(method, aml_return(aml_int(0x0f)));
> >           aml_append(dev, method);
> >
> > +        build_tpm_ppi(dev);
> > +
> >           aml_append(sb_scope, dev);
> >       }
> >
> > @@ -2920,7 +3212,7 @@ void acpi_setup(void)
> >           tpm_config = (FWCfgTPMConfig) {
> >               .tpmppi_address = cpu_to_le32(TPM_PPI_ADDR_BASE),
> >               .tpm_version = cpu_to_le32(tpm_get_version(tpm_find())),
> > -            .tpmppi_version = cpu_to_le32(TPM_PPI_VERSION_NONE)
> > +            .tpmppi_version = cpu_to_le32(TPM_PPI_VERSION_1_30)
> >           };
> >           fw_cfg_add_file(pcms->fw_cfg, "etc/tpm/config",
> >                           &tpm_config, sizeof tpm_config);  
> 
> 
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v4 4/5] acpi: build TPM Physical Presence interface
  2018-06-22  9:03     ` Igor Mammedov
@ 2018-06-22 13:26       ` Stefan Berger
  2018-06-22 13:56         ` Igor Mammedov
  0 siblings, 1 reply; 19+ messages in thread
From: Stefan Berger @ 2018-06-22 13:26 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: Eduardo Habkost, Michael S. Tsirkin, qemu-devel, Paolo Bonzini,
	Marc-André Lureau, Richard Henderson

On 06/22/2018 05:03 AM, Igor Mammedov wrote:
> I'd prefer a table format to describe layout, like in
> docs/specs/acpi_nvdimm.txt or docs/specs/acpi_mem_hotplug.txt

diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
index c230c4c93e..5bf4e892a0 100644
--- a/docs/specs/tpm.txt
+++ b/docs/specs/tpm.txt
@@ -42,6 +42,76 @@ 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.                                 |
+ +----------+--------+--------+-------------------------------------------+
+   | pprq     |   0x4  |  0x109 | Operation request number to execute. 
Set  |
+   |          |        |        | by 
ACPI.                                  |
+ +----------+--------+--------+-------------------------------------------+
+   | pprm     |   0x4  |  0x10d | Operation request optional parameter. 
Set |
+   |          |        |        | by 
ACPI.                                  |
+ +----------+--------+--------+-------------------------------------------+
+   | lppr     |   0x4  |  0x111 | Last execute request number. Set 
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.
+
+    #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)
+

  QEMU files related to TPM ACPI tables:
   - hw/i386/acpi-build.c


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v4 4/5] acpi: build TPM Physical Presence interface
  2018-06-22 13:26       ` Stefan Berger
@ 2018-06-22 13:56         ` Igor Mammedov
  2018-06-22 14:23           ` Stefan Berger
  0 siblings, 1 reply; 19+ messages in thread
From: Igor Mammedov @ 2018-06-22 13:56 UTC (permalink / raw)
  To: Stefan Berger
  Cc: Eduardo Habkost, Michael S. Tsirkin, qemu-devel, Paolo Bonzini,
	Marc-André Lureau, Richard Henderson

On Fri, 22 Jun 2018 09:26:45 -0400
Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:

> On 06/22/2018 05:03 AM, Igor Mammedov wrote:
> > I'd prefer a table format to describe layout, like in
> > docs/specs/acpi_nvdimm.txt or docs/specs/acpi_mem_hotplug.txt  
> 
> diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
> index c230c4c93e..5bf4e892a0 100644
> --- a/docs/specs/tpm.txt
> +++ b/docs/specs/tpm.txt
> @@ -42,6 +42,76 @@ 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
what about usecase when it's mapped in isa address space?
  tpm_tis_realizefn() ->
       tpm_ppi_init_io(&s->ppi, isa_address_space(ISA_DEVICE(dev))

> +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.                                 |
> + +----------+--------+--------+-------------------------------------------+
> +   | pprq     |   0x4  |  0x109 | Operation request number to execute. 
> Set  |
> +   |          |        |        | by 
> ACPI.                                  |
> + +----------+--------+--------+-------------------------------------------+
> +   | pprm     |   0x4  |  0x10d | Operation request optional parameter. 
> Set |
> +   |          |        |        | by 
> ACPI.                                  |
> + +----------+--------+--------+-------------------------------------------+
> +   | lppr     |   0x4  |  0x111 | Last execute request number. Set 
> 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.               |
> + +----------+--------+--------+-------------------------------------------+
if there is relevant match/description in PPI spec for above fields
it would be better to put reference to it in description fields and
as field name that could be easily grepped for in PPI spec,
like we we do with ACPI primitives:

  "ACPI 1.0b: 16.2.5.1 Namespace Modifier Objects Encoding: DefScope"

so reader could easily match spec with parts he/she is interested in.

> +
> +   The following values are supported for the 'func' field. They correspond
> +   to the values used by ACPI function index 8.
> +
> +    #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)
> +

looks good to me
PS:
maybe drop #define part and make a table from it too so reader
won't have to do shift on his own, like

  0x0  | function is not implemented
  0x1  | ...
  ...


>   QEMU files related to TPM ACPI tables:
>    - hw/i386/acpi-build.c
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v4 4/5] acpi: build TPM Physical Presence interface
  2018-06-22 13:56         ` Igor Mammedov
@ 2018-06-22 14:23           ` Stefan Berger
  2018-06-25  9:31             ` Igor Mammedov
  0 siblings, 1 reply; 19+ messages in thread
From: Stefan Berger @ 2018-06-22 14:23 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: Eduardo Habkost, Michael S. Tsirkin, qemu-devel, Paolo Bonzini,
	Marc-André Lureau, Richard Henderson

On 06/22/2018 09:56 AM, Igor Mammedov wrote:
> On Fri, 22 Jun 2018 09:26:45 -0400
> Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
>
>> On 06/22/2018 05:03 AM, Igor Mammedov wrote:
>>> I'd prefer a table format to describe layout, like in
>>> docs/specs/acpi_nvdimm.txt or docs/specs/acpi_mem_hotplug.txt
>> diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
>> index c230c4c93e..5bf4e892a0 100644
>> --- a/docs/specs/tpm.txt
>> +++ b/docs/specs/tpm.txt
>> @@ -42,6 +42,76 @@ 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
> what about usecase when it's mapped in isa address space?
>    tpm_tis_realizefn() ->
>         tpm_ppi_init_io(&s->ppi, isa_address_space(ISA_DEVICE(dev))

The use case for PPI is 'administrator wants to reconfigure the TPM on 
next reboot.' I am not sure what else to add?!

>
>> +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.                                 |
>> + +----------+--------+--------+-------------------------------------------+
>> +   | pprq     |   0x4  |  0x109 | Operation request number to execute.
>> Set  |
>> +   |          |        |        | by
>> ACPI.                                  |
>> + +----------+--------+--------+-------------------------------------------+
>> +   | pprm     |   0x4  |  0x10d | Operation request optional parameter.
>> Set |
>> +   |          |        |        | by
>> ACPI.                                  |
>> + +----------+--------+--------+-------------------------------------------+
>> +   | lppr     |   0x4  |  0x111 | Last execute request number. Set
>> 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.               |
>> + +----------+--------+--------+-------------------------------------------+
> if there is relevant match/description in PPI spec for above fields
> it would be better to put reference to it in description fields and
> as field name that could be easily grepped for in PPI spec,
> like we we do with ACPI primitives:
>
>    "ACPI 1.0b: 16.2.5.1 Namespace Modifier Objects Encoding: DefScope"
>
> so reader could easily match spec with parts he/she is interested in.

The ACPI spec does not go to the level of the implementation let alone 
the necessary fields. The names of the fields stem from edk2.


>
>> +
>> +   The following values are supported for the 'func' field. They correspond
>> +   to the values used by ACPI function index 8.
>> +
>> +    #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)
>> +
> looks good to me
> PS:
> maybe drop #define part and make a table from it too so reader
> won't have to do shift on his own, like
>
>    0x0  | function is not implemented
>    0x1  | ...
>    ...
>
>
>>    QEMU files related to TPM ACPI tables:
>>     - hw/i386/acpi-build.c
>>


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v4 4/5] acpi: build TPM Physical Presence interface
  2018-06-22 14:23           ` Stefan Berger
@ 2018-06-25  9:31             ` Igor Mammedov
  2018-06-25 13:57               ` Stefan Berger
  2018-06-25 14:23               ` Stefan Berger
  0 siblings, 2 replies; 19+ messages in thread
From: Igor Mammedov @ 2018-06-25  9:31 UTC (permalink / raw)
  To: Stefan Berger
  Cc: Eduardo Habkost, Michael S. Tsirkin, qemu-devel, Paolo Bonzini,
	Marc-André Lureau, Richard Henderson

On Fri, 22 Jun 2018 10:23:05 -0400
Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:

> On 06/22/2018 09:56 AM, Igor Mammedov wrote:
> > On Fri, 22 Jun 2018 09:26:45 -0400
> > Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
> >  
> >> On 06/22/2018 05:03 AM, Igor Mammedov wrote:  
> >>> I'd prefer a table format to describe layout, like in
> >>> docs/specs/acpi_nvdimm.txt or docs/specs/acpi_mem_hotplug.txt  
> >> diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
> >> index c230c4c93e..5bf4e892a0 100644
> >> --- a/docs/specs/tpm.txt
> >> +++ b/docs/specs/tpm.txt
> >> @@ -42,6 +42,76 @@ 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  
> > what about usecase when it's mapped in isa address space?
> >    tpm_tis_realizefn() ->
> >         tpm_ppi_init_io(&s->ppi, isa_address_space(ISA_DEVICE(dev))  
> 
> The use case for PPI is 'administrator wants to reconfigure the TPM on 
> next reboot.' I am not sure what else to add?!
> 
> >  
> >> +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.                                 |
> >> + +----------+--------+--------+-------------------------------------------+
> >> +   | pprq     |   0x4  |  0x109 | Operation request number to execute.
> >> Set  |
> >> +   |          |        |        | by
> >> ACPI.                                  |
> >> + +----------+--------+--------+-------------------------------------------+
> >> +   | pprm     |   0x4  |  0x10d | Operation request optional parameter.
> >> Set |
> >> +   |          |        |        | by
> >> ACPI.                                  |
> >> + +----------+--------+--------+-------------------------------------------+
> >> +   | lppr     |   0x4  |  0x111 | Last execute request number. Set
> >> 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.               |
> >> + +----------+--------+--------+-------------------------------------------+  
> > if there is relevant match/description in PPI spec for above fields
> > it would be better to put reference to it in description fields and
> > as field name that could be easily grepped for in PPI spec,
> > like we we do with ACPI primitives:
> >
> >    "ACPI 1.0b: 16.2.5.1 Namespace Modifier Objects Encoding: DefScope"
> >
> > so reader could easily match spec with parts he/she is interested in.  
> 
> The ACPI spec does not go to the level of the implementation let alone 
> the necessary fields. The names of the fields stem from edk2.

my understanding was that values returned in above fields (or some of them)
are standardized in spec since they are an interface defined by functions
implemented in _DSM method.

It would be excessive to describe them here, but sending reader to
a relevant part/term of TPM spec would be helpful.
(that's what we do with ACPI code, so it would be nice to it in this case
as well if possible)
 
> 
> >  
> >> +
> >> +   The following values are supported for the 'func' field. They correspond
> >> +   to the values used by ACPI function index 8.
> >> +
> >> +    #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)
> >> +  
> > looks good to me
> > PS:
> > maybe drop #define part and make a table from it too so reader
> > won't have to do shift on his own, like
> >
> >    0x0  | function is not implemented
> >    0x1  | ...
> >    ...
> >
> >  
> >>    QEMU files related to TPM ACPI tables:
> >>     - hw/i386/acpi-build.c
> >>  
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v4 4/5] acpi: build TPM Physical Presence interface
  2018-06-25  9:31             ` Igor Mammedov
@ 2018-06-25 13:57               ` Stefan Berger
  2018-06-25 14:23               ` Stefan Berger
  1 sibling, 0 replies; 19+ messages in thread
From: Stefan Berger @ 2018-06-25 13:57 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: Eduardo Habkost, Michael S. Tsirkin, qemu-devel, Paolo Bonzini,
	Marc-André Lureau, Richard Henderson

On 06/25/2018 05:31 AM, Igor Mammedov wrote:
> On Fri, 22 Jun 2018 10:23:05 -0400
> Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
>
>> On 06/22/2018 09:56 AM, Igor Mammedov wrote:
>>> On Fri, 22 Jun 2018 09:26:45 -0400
>>> Stefan Berger <stefanb@linux.vnet.ibm.com> wrote:
>>>   
>>>> On 06/22/2018 05:03 AM, Igor Mammedov wrote:
>>>>> I'd prefer a table format to describe layout, like in
>>>>> docs/specs/acpi_nvdimm.txt or docs/specs/acpi_mem_hotplug.txt
>>>> diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
>>>> index c230c4c93e..5bf4e892a0 100644
>>>> --- a/docs/specs/tpm.txt
>>>> +++ b/docs/specs/tpm.txt
>>>> @@ -42,6 +42,76 @@ 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
>>> what about usecase when it's mapped in isa address space?
>>>     tpm_tis_realizefn() ->
>>>          tpm_ppi_init_io(&s->ppi, isa_address_space(ISA_DEVICE(dev))
>> The use case for PPI is 'administrator wants to reconfigure the TPM on
>> next reboot.' I am not sure what else to add?!
>>
>>>   
>>>> +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.                                 |
>>>> + +----------+--------+--------+-------------------------------------------+
>>>> +   | pprq     |   0x4  |  0x109 | Operation request number to execute.
>>>> Set  |
>>>> +   |          |        |        | by
>>>> ACPI.                                  |
>>>> + +----------+--------+--------+-------------------------------------------+
>>>> +   | pprm     |   0x4  |  0x10d | Operation request optional parameter.
>>>> Set |
>>>> +   |          |        |        | by
>>>> ACPI.                                  |
>>>> + +----------+--------+--------+-------------------------------------------+
>>>> +   | lppr     |   0x4  |  0x111 | Last execute request number. Set
>>>> 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.               |
>>>> + +----------+--------+--------+-------------------------------------------+
>>> if there is relevant match/description in PPI spec for above fields
>>> it would be better to put reference to it in description fields and
>>> as field name that could be easily grepped for in PPI spec,
>>> like we we do with ACPI primitives:
>>>
>>>     "ACPI 1.0b: 16.2.5.1 Namespace Modifier Objects Encoding: DefScope"
>>>
>>> so reader could easily match spec with parts he/she is interested in.
>> The ACPI spec does not go to the level of the implementation let alone
>> the necessary fields. The names of the fields stem from edk2.
> my understanding was that values returned in above fields (or some of them)
> are standardized in spec since they are an interface defined by functions
> implemented in _DSM method.
>
> It would be excessive to describe them here, but sending reader to
> a relevant part/term of TPM spec would be helpful.
> (that's what we do with ACPI code, so it would be nice to it in this case
> as well if possible)

Will revise.
>   
>>>   
>>>> +
>>>> +   The following values are supported for the 'func' field. They correspond
>>>> +   to the values used by ACPI function index 8.
>>>> +
>>>> +    #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)
>>>> +
>>> looks good to me
>>> PS:
>>> maybe drop #define part and make a table from it too so reader
>>> won't have to do shift on his own, like
>>>
>>>     0x0  | function is not implemented
>>>     0x1  | ...
>>>     ...
>>>
>>>   
>>>>     QEMU files related to TPM ACPI tables:
>>>>      - hw/i386/acpi-build.c
>>>>   



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v4 4/5] acpi: build TPM Physical Presence interface
  2018-06-25  9:31             ` Igor Mammedov
  2018-06-25 13:57               ` Stefan Berger
@ 2018-06-25 14:23               ` Stefan Berger
  1 sibling, 0 replies; 19+ messages in thread
From: Stefan Berger @ 2018-06-25 14:23 UTC (permalink / raw)
  To: Igor Mammedov
  Cc: Eduardo Habkost, Michael S. Tsirkin, qemu-devel, Paolo Bonzini,
	Marc-André Lureau, Richard Henderson

diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
index c230c4c93e..90f96c53cb 100644
--- a/docs/specs/tpm.txt
+++ b/docs/specs/tpm.txt
@@ -42,6 +42,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


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [Qemu-devel] [PATCH v4 1/5] tpm: add a "ppi" boolean property
  2018-06-21 12:11   ` Stefan Berger
  2018-06-21 12:19     ` Stefan Berger
@ 2018-06-28 11:21     ` Marc-André Lureau
  1 sibling, 0 replies; 19+ messages in thread
From: Marc-André Lureau @ 2018-06-28 11:21 UTC (permalink / raw)
  To: Stefan Berger
  Cc: QEMU, Igor Mammedov, Richard Henderson, Paolo Bonzini,
	Eduardo Habkost, Michael S. Tsirkin

Hi

On Thu, Jun 21, 2018 at 2:11 PM, Stefan Berger
<stefanb@linux.vnet.ibm.com> wrote:
> On 06/21/2018 07:55 AM, Marc-André Lureau wrote:
>>
>> The following patches implement the TPM Physical Presence Interface,
>> and makes use of new memory region and fw_cfg entries. Enable it by
>> default on >2.12 machine type.
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@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 563908b874..dac847548b 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(),
>>   };
>
>
> Wouldn't we have to bump up the version of teh VMStateDescription?

A device property is not saved with vmstate afaik.

>
>    Stefan
>
>>   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(),
>>   };
>>
>
>
>
>



-- 
Marc-André Lureau

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2018-06-28 11:21 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-21 11:55 [Qemu-devel] [PATCH v4 0/5] Add support for TPM Physical Presence interface Marc-André Lureau
2018-06-21 11:55 ` [Qemu-devel] [PATCH v4 1/5] tpm: add a "ppi" boolean property Marc-André Lureau
2018-06-21 12:11   ` Stefan Berger
2018-06-21 12:19     ` Stefan Berger
2018-06-28 11:21     ` Marc-André Lureau
2018-06-21 11:55 ` [Qemu-devel] [PATCH v4 2/5] tpm: implement virtual memory device for TPM PPI Marc-André Lureau
2018-06-21 12:10   ` Stefan Berger
2018-06-21 12:24     ` Marc-André Lureau
2018-06-21 11:55 ` [Qemu-devel] [PATCH v4 3/5] acpi: add fw_cfg file for TPM and PPI virtual memory device Marc-André Lureau
2018-06-21 11:55 ` [Qemu-devel] [PATCH v4 4/5] acpi: build TPM Physical Presence interface Marc-André Lureau
2018-06-21 20:11   ` Stefan Berger
2018-06-22  9:03     ` Igor Mammedov
2018-06-22 13:26       ` Stefan Berger
2018-06-22 13:56         ` Igor Mammedov
2018-06-22 14:23           ` Stefan Berger
2018-06-25  9:31             ` Igor Mammedov
2018-06-25 13:57               ` Stefan Berger
2018-06-25 14:23               ` Stefan Berger
2018-06-21 11:55 ` [Qemu-devel] [PATCH v4 5/5] tpm: add a fake ACPI memory clear interface Marc-André Lureau

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.