[Qemu-devel] [PATCH v12 0/6] Add support for TPM Physical Presence interface

[Qemu-devel] [PATCH v12 0/6] Add support for TPM Physical Presence interface

[Marc-André Lureau]

Hi,

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

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

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

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

The edk2 support is merged upstream.

v12:
- mark dirty memory regions when zeroing

v11:
- fix migration issue with bad RAM size, round it up to host-page size

v10:
- fix 3.1 pc machines patch
- describe PPI memory size in doc
- change "Memory overwrite variable" location to offset 0x15a

Marc-André Lureau (3):
  hw/i386: add pc-i440fx-3.1 & pc-q35-3.1
  tpm: add a "ppi" boolean property
  tpm: add ACPI memory clear interface

Stefan Berger (3):
  tpm: allocate/map buffer for TPM Physical Presence interface
  acpi: expose TPM/PPI configuration parameters to firmware via fw_cfg
  acpi: build TPM Physical Presence interface

 hw/tpm/tpm_ppi.h      |  28 +++
 include/hw/acpi/tpm.h |  17 ++
 include/hw/compat.h   |  11 +-
 include/hw/i386/pc.h  |   5 +-
 hw/i386/acpi-build.c  | 456 +++++++++++++++++++++++++++++++++++++++++-
 hw/i386/pc_piix.c     |  15 +-
 hw/i386/pc_q35.c      |  13 +-
 hw/tpm/tpm_crb.c      |  12 ++
 hw/tpm/tpm_ppi.c      |  54 +++++
 hw/tpm/tpm_tis.c      |  12 ++
 docs/specs/tpm.txt    | 105 ++++++++++
 hw/tpm/Makefile.objs  |   1 +
 hw/tpm/trace-events   |   3 +
 13 files changed, 723 insertions(+), 9 deletions(-)
 create mode 100644 hw/tpm/tpm_ppi.h
 create mode 100644 hw/tpm/tpm_ppi.c

-- 
2.19.0.rc1

[Qemu-devel] [PATCH v12 1/6] hw/i386: add pc-i440fx-3.1 & pc-q35-3.1

[Marc-André Lureau]

The following patch is going to add compatiblity parameters.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/hw/i386/pc.h |  5 ++++-
 hw/i386/pc_piix.c    | 15 ++++++++++++---
 hw/i386/pc_q35.c     | 13 +++++++++++--
 3 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
index 6894f37df1..09b0365a8e 100644
--- a/include/hw/i386/pc.h
+++ b/include/hw/i386/pc.h
@@ -294,7 +294,10 @@ int e820_add_entry(uint64_t, uint64_t, uint32_t);
 int e820_get_num_entries(void);
 bool e820_get_entry(int, uint32_t, uint64_t *, uint64_t *);
 
-#define PC_COMPAT_2_12 \
+#define PC_COMPAT_3_0 \
+    HW_COMPAT_3_0
+
+#define PC_COMPAT_2_12                          \
     HW_COMPAT_2_12 \
     {\
         .driver   = TYPE_X86_CPU,\
diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index dc09466b3e..7092d6d13f 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -428,21 +428,30 @@ static void pc_i440fx_machine_options(MachineClass *m)
     machine_class_allow_dynamic_sysbus_dev(m, TYPE_RAMFB_DEVICE);
 }
 
-static void pc_i440fx_3_0_machine_options(MachineClass *m)
+static void pc_i440fx_3_1_machine_options(MachineClass *m)
 {
     pc_i440fx_machine_options(m);
     m->alias = "pc";
     m->is_default = 1;
 }
 
+DEFINE_I440FX_MACHINE(v3_1, "pc-i440fx-3.1", NULL,
+                      pc_i440fx_3_1_machine_options);
+
+static void pc_i440fx_3_0_machine_options(MachineClass *m)
+{
+    pc_i440fx_3_1_machine_options(m);
+    m->is_default = 0;
+    m->alias = NULL;
+    SET_MACHINE_COMPAT(m, PC_COMPAT_3_0);
+}
+
 DEFINE_I440FX_MACHINE(v3_0, "pc-i440fx-3.0", NULL,
                       pc_i440fx_3_0_machine_options);
 
 static void pc_i440fx_2_12_machine_options(MachineClass *m)
 {
     pc_i440fx_3_0_machine_options(m);
-    m->is_default = 0;
-    m->alias = NULL;
     SET_MACHINE_COMPAT(m, PC_COMPAT_2_12);
 }
 
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 532241e3f8..4702bb13c4 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -311,19 +311,28 @@ static void pc_q35_machine_options(MachineClass *m)
     m->max_cpus = 288;
 }
 
-static void pc_q35_3_0_machine_options(MachineClass *m)
+static void pc_q35_3_1_machine_options(MachineClass *m)
 {
     pc_q35_machine_options(m);
     m->alias = "q35";
 }
 
+DEFINE_Q35_MACHINE(v3_1, "pc-q35-3.1", NULL,
+                   pc_q35_3_1_machine_options);
+
+static void pc_q35_3_0_machine_options(MachineClass *m)
+{
+    pc_q35_3_1_machine_options(m);
+    m->alias = NULL;
+    SET_MACHINE_COMPAT(m, PC_COMPAT_3_0);
+}
+
 DEFINE_Q35_MACHINE(v3_0, "pc-q35-3.0", NULL,
                     pc_q35_3_0_machine_options);
 
 static void pc_q35_2_12_machine_options(MachineClass *m)
 {
     pc_q35_3_0_machine_options(m);
-    m->alias = NULL;
     SET_MACHINE_COMPAT(m, PC_COMPAT_2_12);
 }
 
-- 
2.19.0.rc1

[Qemu-devel] [PATCH v12 2/6] tpm: add a "ppi" boolean property

[Marc-André Lureau]

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

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

diff --git a/include/hw/compat.h b/include/hw/compat.h
index 6f4d5fc647..8098a62744 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -2,7 +2,16 @@
 #define HW_COMPAT_H
 
 #define HW_COMPAT_3_0 \
-    /* empty */
+    {\
+        .driver   = "tpm-crb",\
+        .property = "ppi",\
+        .value    = "false",\
+    },\
+    {\
+        .driver   = "tpm-tis",\
+        .property = "ppi",\
+        .value    = "false",\
+    },
 
 #define HW_COMPAT_2_12 \
     {\
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.19.0.rc1

[Qemu-devel] [PATCH v12 3/6] tpm: allocate/map buffer for TPM Physical Presence interface

[Marc-André Lureau]

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

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

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

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

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/tpm/tpm_ppi.h      | 26 ++++++++++++++++++++++++++
 include/hw/acpi/tpm.h |  6 ++++++
 hw/tpm/tpm_crb.c      |  8 ++++++++
 hw/tpm/tpm_ppi.c      | 32 ++++++++++++++++++++++++++++++++
 hw/tpm/tpm_tis.c      |  8 ++++++++
 hw/tpm/Makefile.objs  |  1 +
 6 files changed, 81 insertions(+)
 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..c2ab2ed300
--- /dev/null
+++ b/hw/tpm/tpm_ppi.h
@@ -0,0 +1,26 @@
+/*
+ * TPM Physical Presence Interface
+ *
+ * Copyright (C) 2018 IBM Corporation
+ *
+ * Authors:
+ *  Stefan Berger    <stefanb@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#ifndef TPM_TPM_PPI_H
+#define TPM_TPM_PPI_H
+
+#include "hw/acpi/tpm.h"
+#include "exec/address-spaces.h"
+
+typedef struct TPMPPI {
+    MemoryRegion ram;
+    uint8_t *buf;
+} TPMPPI;
+
+bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
+                  hwaddr addr, Object *obj, Error **errp);
+
+#endif /* TPM_TPM_PPI_H */
diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
index 3580ffd50c..b8796df916 100644
--- a/include/hw/acpi/tpm.h
+++ b/include/hw/acpi/tpm.h
@@ -188,4 +188,10 @@ REG32(CRB_DATA_BUFFER, 0x80)
 #define TPM2_START_METHOD_MMIO      6
 #define TPM2_START_METHOD_CRB       7
 
+/*
+ * Physical Presence Interface
+ */
+#define TPM_PPI_ADDR_SIZE           0x400
+#define TPM_PPI_ADDR_BASE           0xFED45000
+
 #endif /* HW_ACPI_TPM_H */
diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
index d5b0ac5920..b243222fd6 100644
--- a/hw/tpm/tpm_crb.c
+++ b/hw/tpm/tpm_crb.c
@@ -29,6 +29,7 @@
 #include "sysemu/reset.h"
 #include "tpm_int.h"
 #include "tpm_util.h"
+#include "tpm_ppi.h"
 #include "trace.h"
 
 typedef struct CRBState {
@@ -43,6 +44,7 @@ typedef struct CRBState {
     size_t be_buffer_size;
 
     bool ppi_enabled;
+    TPMPPI ppi;
 } CRBState;
 
 #define CRB(obj) OBJECT_CHECK(CRBState, (obj), TYPE_TPM_CRB)
@@ -294,6 +296,12 @@ static void tpm_crb_realize(DeviceState *dev, Error **errp)
     memory_region_add_subregion(get_system_memory(),
         TPM_CRB_ADDR_BASE + sizeof(s->regs), &s->cmdmem);
 
+    if (s->ppi_enabled &&
+        !tpm_ppi_init(&s->ppi, get_system_memory(),
+                      TPM_PPI_ADDR_BASE, OBJECT(s), errp)) {
+        return;
+    }
+
     qemu_register_reset(tpm_crb_reset, dev);
 }
 
diff --git a/hw/tpm/tpm_ppi.c b/hw/tpm/tpm_ppi.c
new file mode 100644
index 0000000000..f2f07f895e
--- /dev/null
+++ b/hw/tpm/tpm_ppi.c
@@ -0,0 +1,32 @@
+/*
+ * tpm_ppi.c - TPM Physical Presence Interface
+ *
+ * Copyright (C) 2018 IBM Corporation
+ *
+ * Authors:
+ *  Stefan Berger <stefanb@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+
+#include "qapi/error.h"
+#include "cpu.h"
+#include "sysemu/memory_mapping.h"
+#include "migration/vmstate.h"
+#include "tpm_ppi.h"
+
+bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
+                  hwaddr addr, Object *obj, Error **errp)
+{
+    tpmppi->buf = g_malloc0(HOST_PAGE_ALIGN(TPM_PPI_ADDR_SIZE));
+    memory_region_init_ram_device_ptr(&tpmppi->ram, obj, "tpm-ppi",
+                                      TPM_PPI_ADDR_SIZE, tpmppi->buf);
+    vmstate_register_ram(&tpmppi->ram, DEVICE(obj));
+
+    memory_region_add_subregion(m, addr, &tpmppi->ram);
+    return true;
+}
diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index d9ddf9b723..70432ffe8b 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -31,6 +31,7 @@
 #include "sysemu/tpm_backend.h"
 #include "tpm_int.h"
 #include "tpm_util.h"
+#include "tpm_ppi.h"
 #include "trace.h"
 
 #define TPM_TIS_NUM_LOCALITIES      5     /* per spec */
@@ -83,6 +84,7 @@ typedef struct TPMState {
     size_t be_buffer_size;
 
     bool ppi_enabled;
+    TPMPPI ppi;
 } TPMState;
 
 #define TPM(obj) OBJECT_CHECK(TPMState, (obj), TYPE_TPM_TIS)
@@ -979,6 +981,12 @@ static void tpm_tis_realizefn(DeviceState *dev, Error **errp)
 
     memory_region_add_subregion(isa_address_space(ISA_DEVICE(dev)),
                                 TPM_TIS_ADDR_BASE, &s->mmio);
+
+    if (s->ppi_enabled &&
+        !tpm_ppi_init(&s->ppi, isa_address_space(ISA_DEVICE(dev)),
+                      TPM_PPI_ADDR_BASE, OBJECT(s), errp)) {
+        return;
+    }
 }
 
 static void tpm_tis_initfn(Object *obj)
diff --git a/hw/tpm/Makefile.objs b/hw/tpm/Makefile.objs
index 1dc9f8bf2c..700c878622 100644
--- a/hw/tpm/Makefile.objs
+++ b/hw/tpm/Makefile.objs
@@ -1,4 +1,5 @@
 common-obj-y += tpm_util.o
+obj-y += 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
-- 
2.19.0.rc1

Re: [Qemu-devel] [PATCH v12 3/6] tpm: allocate/map buffer for TPM Physical Presence interface

[Philippe Mathieu-Daudé]

Hi Stefan, Marc-André,

On 9/10/18 10:32 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 0xFED45000 and used by ACPI to send messages to the
> firmware (BIOS) and by the firmware to provide parameters for each one of
> the supported codes.
> 
> This interface should be used by all TPM devices on x86 and can be
> added by calling tpm_ppi_init_io().
> 
> Note: bios_linker cannot be used to allocate the PPI memory region,
> since the reserved memory should stay stable across reboots, and might
> be needed before the ACPI tables are installed.
> 
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  hw/tpm/tpm_ppi.h      | 26 ++++++++++++++++++++++++++
>  include/hw/acpi/tpm.h |  6 ++++++
>  hw/tpm/tpm_crb.c      |  8 ++++++++
>  hw/tpm/tpm_ppi.c      | 32 ++++++++++++++++++++++++++++++++
>  hw/tpm/tpm_tis.c      |  8 ++++++++
>  hw/tpm/Makefile.objs  |  1 +
>  6 files changed, 81 insertions(+)
>  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..c2ab2ed300
> --- /dev/null
> +++ b/hw/tpm/tpm_ppi.h
> @@ -0,0 +1,26 @@
> +/*
> + * TPM Physical Presence Interface
> + *
> + * Copyright (C) 2018 IBM Corporation
> + *
> + * Authors:
> + *  Stefan Berger    <stefanb@us.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + */
> +#ifndef TPM_TPM_PPI_H
> +#define TPM_TPM_PPI_H
> +
> +#include "hw/acpi/tpm.h"
> +#include "exec/address-spaces.h"
> +
> +typedef struct TPMPPI {
> +    MemoryRegion ram;
> +    uint8_t *buf;
> +} TPMPPI;
> +

Can you add a description for this public function?

> +bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
> +                  hwaddr addr, Object *obj, Error **errp);
> +
> +#endif /* TPM_TPM_PPI_H */
> diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
> index 3580ffd50c..b8796df916 100644
> --- a/include/hw/acpi/tpm.h
> +++ b/include/hw/acpi/tpm.h
> @@ -188,4 +188,10 @@ REG32(CRB_DATA_BUFFER, 0x80)
>  #define TPM2_START_METHOD_MMIO      6
>  #define TPM2_START_METHOD_CRB       7
>  
> +/*
> + * Physical Presence Interface
> + */
> +#define TPM_PPI_ADDR_SIZE           0x400
> +#define TPM_PPI_ADDR_BASE           0xFED45000
> +
>  #endif /* HW_ACPI_TPM_H */
> diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> index d5b0ac5920..b243222fd6 100644
> --- a/hw/tpm/tpm_crb.c
> +++ b/hw/tpm/tpm_crb.c
> @@ -29,6 +29,7 @@
>  #include "sysemu/reset.h"
>  #include "tpm_int.h"
>  #include "tpm_util.h"
> +#include "tpm_ppi.h"
>  #include "trace.h"
>  
>  typedef struct CRBState {
> @@ -43,6 +44,7 @@ typedef struct CRBState {
>      size_t be_buffer_size;
>  
>      bool ppi_enabled;
> +    TPMPPI ppi;
>  } CRBState;
>  
>  #define CRB(obj) OBJECT_CHECK(CRBState, (obj), TYPE_TPM_CRB)
> @@ -294,6 +296,12 @@ static void tpm_crb_realize(DeviceState *dev, Error **errp)
>      memory_region_add_subregion(get_system_memory(),
>          TPM_CRB_ADDR_BASE + sizeof(s->regs), &s->cmdmem);
>  
> +    if (s->ppi_enabled &&
> +        !tpm_ppi_init(&s->ppi, get_system_memory(),
> +                      TPM_PPI_ADDR_BASE, OBJECT(s), errp)) {
> +        return;
> +    }
> +
>      qemu_register_reset(tpm_crb_reset, dev);
>  }
>  
> diff --git a/hw/tpm/tpm_ppi.c b/hw/tpm/tpm_ppi.c
> new file mode 100644
> index 0000000000..f2f07f895e
> --- /dev/null
> +++ b/hw/tpm/tpm_ppi.c
> @@ -0,0 +1,32 @@
> +/*
> + * tpm_ppi.c - TPM Physical Presence Interface
> + *
> + * Copyright (C) 2018 IBM Corporation
> + *
> + * Authors:
> + *  Stefan Berger <stefanb@us.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "qemu/osdep.h"
> +
> +#include "qapi/error.h"
> +#include "cpu.h"
> +#include "sysemu/memory_mapping.h"
> +#include "migration/vmstate.h"
> +#include "tpm_ppi.h"
> +
> +bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
> +                  hwaddr addr, Object *obj, Error **errp)
> +{
> +    tpmppi->buf = g_malloc0(HOST_PAGE_ALIGN(TPM_PPI_ADDR_SIZE));
> +    memory_region_init_ram_device_ptr(&tpmppi->ram, obj, "tpm-ppi",
> +                                      TPM_PPI_ADDR_SIZE, tpmppi->buf);
> +    vmstate_register_ram(&tpmppi->ram, DEVICE(obj));
> +
> +    memory_region_add_subregion(m, addr, &tpmppi->ram);
> +    return true;
> +}
> diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
> index d9ddf9b723..70432ffe8b 100644
> --- a/hw/tpm/tpm_tis.c
> +++ b/hw/tpm/tpm_tis.c
> @@ -31,6 +31,7 @@
>  #include "sysemu/tpm_backend.h"
>  #include "tpm_int.h"
>  #include "tpm_util.h"
> +#include "tpm_ppi.h"
>  #include "trace.h"
>  
>  #define TPM_TIS_NUM_LOCALITIES      5     /* per spec */
> @@ -83,6 +84,7 @@ typedef struct TPMState {
>      size_t be_buffer_size;
>  
>      bool ppi_enabled;
> +    TPMPPI ppi;
>  } TPMState;
>  
>  #define TPM(obj) OBJECT_CHECK(TPMState, (obj), TYPE_TPM_TIS)
> @@ -979,6 +981,12 @@ static void tpm_tis_realizefn(DeviceState *dev, Error **errp)
>  
>      memory_region_add_subregion(isa_address_space(ISA_DEVICE(dev)),
>                                  TPM_TIS_ADDR_BASE, &s->mmio);
> +
> +    if (s->ppi_enabled &&
> +        !tpm_ppi_init(&s->ppi, isa_address_space(ISA_DEVICE(dev)),
> +                      TPM_PPI_ADDR_BASE, OBJECT(s), errp)) {
> +        return;
> +    }
>  }
>  

Shouldn't you implement the unrealize() function, destroying the PPI buffer?

Patch looks good otherwise.

>  static void tpm_tis_initfn(Object *obj)
> diff --git a/hw/tpm/Makefile.objs b/hw/tpm/Makefile.objs
> index 1dc9f8bf2c..700c878622 100644
> --- a/hw/tpm/Makefile.objs
> +++ b/hw/tpm/Makefile.objs
> @@ -1,4 +1,5 @@
>  common-obj-y += tpm_util.o
> +obj-y += 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
> 

Re: [Qemu-devel] [PATCH v12 3/6] tpm: allocate/map buffer for TPM Physical Presence interface

[Marc-André Lureau]

Hi

On Wed, Dec 12, 2018 at 8:13 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> Hi Stefan, Marc-André,
>
> On 9/10/18 10:32 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 0xFED45000 and used by ACPI to send messages to the
> > firmware (BIOS) and by the firmware to provide parameters for each one of
> > the supported codes.
> >
> > This interface should be used by all TPM devices on x86 and can be
> > added by calling tpm_ppi_init_io().
> >
> > Note: bios_linker cannot be used to allocate the PPI memory region,
> > since the reserved memory should stay stable across reboots, and might
> > be needed before the ACPI tables are installed.
> >
> > Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  hw/tpm/tpm_ppi.h      | 26 ++++++++++++++++++++++++++
> >  include/hw/acpi/tpm.h |  6 ++++++
> >  hw/tpm/tpm_crb.c      |  8 ++++++++
> >  hw/tpm/tpm_ppi.c      | 32 ++++++++++++++++++++++++++++++++
> >  hw/tpm/tpm_tis.c      |  8 ++++++++
> >  hw/tpm/Makefile.objs  |  1 +
> >  6 files changed, 81 insertions(+)
> >  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..c2ab2ed300
> > --- /dev/null
> > +++ b/hw/tpm/tpm_ppi.h
> > @@ -0,0 +1,26 @@
> > +/*
> > + * TPM Physical Presence Interface
> > + *
> > + * Copyright (C) 2018 IBM Corporation
> > + *
> > + * Authors:
> > + *  Stefan Berger    <stefanb@us.ibm.com>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> > + * See the COPYING file in the top-level directory.
> > + */
> > +#ifndef TPM_TPM_PPI_H
> > +#define TPM_TPM_PPI_H
> > +
> > +#include "hw/acpi/tpm.h"
> > +#include "exec/address-spaces.h"
> > +
> > +typedef struct TPMPPI {
> > +    MemoryRegion ram;
> > +    uint8_t *buf;
> > +} TPMPPI;
> > +
>
> Can you add a description for this public function?

ok

>
> > +bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
> > +                  hwaddr addr, Object *obj, Error **errp);
> > +
> > +#endif /* TPM_TPM_PPI_H */
> > diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
> > index 3580ffd50c..b8796df916 100644
> > --- a/include/hw/acpi/tpm.h
> > +++ b/include/hw/acpi/tpm.h
> > @@ -188,4 +188,10 @@ REG32(CRB_DATA_BUFFER, 0x80)
> >  #define TPM2_START_METHOD_MMIO      6
> >  #define TPM2_START_METHOD_CRB       7
> >
> > +/*
> > + * Physical Presence Interface
> > + */
> > +#define TPM_PPI_ADDR_SIZE           0x400
> > +#define TPM_PPI_ADDR_BASE           0xFED45000
> > +
> >  #endif /* HW_ACPI_TPM_H */
> > diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> > index d5b0ac5920..b243222fd6 100644
> > --- a/hw/tpm/tpm_crb.c
> > +++ b/hw/tpm/tpm_crb.c
> > @@ -29,6 +29,7 @@
> >  #include "sysemu/reset.h"
> >  #include "tpm_int.h"
> >  #include "tpm_util.h"
> > +#include "tpm_ppi.h"
> >  #include "trace.h"
> >
> >  typedef struct CRBState {
> > @@ -43,6 +44,7 @@ typedef struct CRBState {
> >      size_t be_buffer_size;
> >
> >      bool ppi_enabled;
> > +    TPMPPI ppi;
> >  } CRBState;
> >
> >  #define CRB(obj) OBJECT_CHECK(CRBState, (obj), TYPE_TPM_CRB)
> > @@ -294,6 +296,12 @@ static void tpm_crb_realize(DeviceState *dev, Error **errp)
> >      memory_region_add_subregion(get_system_memory(),
> >          TPM_CRB_ADDR_BASE + sizeof(s->regs), &s->cmdmem);
> >
> > +    if (s->ppi_enabled &&
> > +        !tpm_ppi_init(&s->ppi, get_system_memory(),
> > +                      TPM_PPI_ADDR_BASE, OBJECT(s), errp)) {
> > +        return;
> > +    }
> > +
> >      qemu_register_reset(tpm_crb_reset, dev);
> >  }
> >
> > diff --git a/hw/tpm/tpm_ppi.c b/hw/tpm/tpm_ppi.c
> > new file mode 100644
> > index 0000000000..f2f07f895e
> > --- /dev/null
> > +++ b/hw/tpm/tpm_ppi.c
> > @@ -0,0 +1,32 @@
> > +/*
> > + * tpm_ppi.c - TPM Physical Presence Interface
> > + *
> > + * Copyright (C) 2018 IBM Corporation
> > + *
> > + * Authors:
> > + *  Stefan Berger <stefanb@us.ibm.com>
> > + *
> > + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> > + * See the COPYING file in the top-level directory.
> > + *
> > + */
> > +
> > +#include "qemu/osdep.h"
> > +
> > +#include "qapi/error.h"
> > +#include "cpu.h"
> > +#include "sysemu/memory_mapping.h"
> > +#include "migration/vmstate.h"
> > +#include "tpm_ppi.h"
> > +
> > +bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
> > +                  hwaddr addr, Object *obj, Error **errp)
> > +{
> > +    tpmppi->buf = g_malloc0(HOST_PAGE_ALIGN(TPM_PPI_ADDR_SIZE));
> > +    memory_region_init_ram_device_ptr(&tpmppi->ram, obj, "tpm-ppi",
> > +                                      TPM_PPI_ADDR_SIZE, tpmppi->buf);
> > +    vmstate_register_ram(&tpmppi->ram, DEVICE(obj));
> > +
> > +    memory_region_add_subregion(m, addr, &tpmppi->ram);
> > +    return true;
> > +}
> > diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
> > index d9ddf9b723..70432ffe8b 100644
> > --- a/hw/tpm/tpm_tis.c
> > +++ b/hw/tpm/tpm_tis.c
> > @@ -31,6 +31,7 @@
> >  #include "sysemu/tpm_backend.h"
> >  #include "tpm_int.h"
> >  #include "tpm_util.h"
> > +#include "tpm_ppi.h"
> >  #include "trace.h"
> >
> >  #define TPM_TIS_NUM_LOCALITIES      5     /* per spec */
> > @@ -83,6 +84,7 @@ typedef struct TPMState {
> >      size_t be_buffer_size;
> >
> >      bool ppi_enabled;
> > +    TPMPPI ppi;
> >  } TPMState;
> >
> >  #define TPM(obj) OBJECT_CHECK(TPMState, (obj), TYPE_TPM_TIS)
> > @@ -979,6 +981,12 @@ static void tpm_tis_realizefn(DeviceState *dev, Error **errp)
> >
> >      memory_region_add_subregion(isa_address_space(ISA_DEVICE(dev)),
> >                                  TPM_TIS_ADDR_BASE, &s->mmio);
> > +
> > +    if (s->ppi_enabled &&
> > +        !tpm_ppi_init(&s->ppi, isa_address_space(ISA_DEVICE(dev)),
> > +                      TPM_PPI_ADDR_BASE, OBJECT(s), errp)) {
> > +        return;
> > +    }
> >  }
> >
>
> Shouldn't you implement the unrealize() function, destroying the PPI buffer?

I don't think it's necessary since the TPM and PPI are not hotpluggable.

> Patch looks good otherwise.
>
> >  static void tpm_tis_initfn(Object *obj)
> > diff --git a/hw/tpm/Makefile.objs b/hw/tpm/Makefile.objs
> > index 1dc9f8bf2c..700c878622 100644
> > --- a/hw/tpm/Makefile.objs
> > +++ b/hw/tpm/Makefile.objs
> > @@ -1,4 +1,5 @@
> >  common-obj-y += tpm_util.o
> > +obj-y += 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
> >
>


-- 
Marc-André Lureau

[Qemu-devel] [PATCH v12 4/6] acpi: expose TPM/PPI configuration parameters to firmware via fw_cfg

[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>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
---
 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 b8796df916..a6109a97fc 100644
--- a/include/hw/acpi/tpm.h
+++ b/include/hw/acpi/tpm.h
@@ -194,4 +194,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 e1ee8ae9e0..c24f68df02 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);
@@ -2877,6 +2883,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");
@@ -2911,6 +2919,17 @@ void acpi_setup(void)
     fw_cfg_add_file(pcms->fw_cfg, ACPI_BUILD_TPMLOG_FILE,
                     tables.tcpalog->data, acpi_data_len(tables.tcpalog));
 
+    tpm = tpm_find();
+    if (tpm && object_property_get_bool(OBJECT(tpm), "ppi", &error_abort)) {
+        tpm_config = (FWCfgTPMConfig) {
+            .tpmppi_address = cpu_to_le32(TPM_PPI_ADDR_BASE),
+            .tpm_version = tpm_get_version(tpm_find()),
+            .tpmppi_version = TPM_PPI_VERSION_NONE
+        };
+        fw_cfg_add_file(pcms->fw_cfg, "etc/tpm/config",
+                        &tpm_config, sizeof tpm_config);
+    }
+
     vmgenid_dev = find_vmgenid_dev();
     if (vmgenid_dev) {
         vmgenid_add_fw_cfg(VMGENID(vmgenid_dev), pcms->fw_cfg,
diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
index 0e9bbebe1d..a5bdd5f26e 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 read 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.19.0.rc1

Re: [Qemu-devel] [PATCH v12 4/6] acpi: expose TPM/PPI configuration parameters to firmware via fw_cfg

[Philippe Mathieu-Daudé]

Cc'ing Laszlo

On 9/10/18 10:32 AM, Marc-André Lureau wrote:
> 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>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  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 b8796df916..a6109a97fc 100644
> --- a/include/hw/acpi/tpm.h
> +++ b/include/hw/acpi/tpm.h
> @@ -194,4 +194,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 e1ee8ae9e0..c24f68df02 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 {

FwCfgTPMConfig (lowercase 'w')

> +    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);
> @@ -2877,6 +2883,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");
> @@ -2911,6 +2919,17 @@ void acpi_setup(void)
>      fw_cfg_add_file(pcms->fw_cfg, ACPI_BUILD_TPMLOG_FILE,
>                      tables.tcpalog->data, acpi_data_len(tables.tcpalog));
>  
> +    tpm = tpm_find();
> +    if (tpm && object_property_get_bool(OBJECT(tpm), "ppi", &error_abort)) {
> +        tpm_config = (FWCfgTPMConfig) {
> +            .tpmppi_address = cpu_to_le32(TPM_PPI_ADDR_BASE),
> +            .tpm_version = tpm_get_version(tpm_find()),

               .tpm_version = tpm_get_version(tpm),

> +            .tpmppi_version = TPM_PPI_VERSION_NONE
> +        };
> +        fw_cfg_add_file(pcms->fw_cfg, "etc/tpm/config",
> +                        &tpm_config, sizeof tpm_config);
> +    }
> +
>      vmgenid_dev = find_vmgenid_dev();
>      if (vmgenid_dev) {
>          vmgenid_add_fw_cfg(VMGENID(vmgenid_dev), pcms->fw_cfg,
> diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
> index 0e9bbebe1d..a5bdd5f26e 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 read 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
> 

Re: [Qemu-devel] [PATCH v12 4/6] acpi: expose TPM/PPI configuration parameters to firmware via fw_cfg

[Laszlo Ersek]

On 12/12/18 17:04, Philippe Mathieu-Daudé wrote:
> Cc'ing Laszlo

Thanks.

As noted in the blurb, the edk2 side has been upstream for a while (it
was applied after an extremely long and thorough design discussion). As
long as the QEMU side conforms to the agreed-upon interface, I'm neutral
on the QEMU code.

Of course, this doesn't mean that *you* shouldn't review it! :)

Thanks,
Laszlo

[Qemu-devel] [PATCH v12 5/6] acpi: build TPM Physical Presence interface

[Marc-André Lureau]

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

The TPM Physical Presence interface consists of an ACPI part, a shared
memory part, and code in the firmware. Users can send messages to the
firmware by writing a code into the shared memory through invoking the
ACPI code. When a reboot happens, the firmware looks for the code and
acts on it by sending sequences of commands to the TPM.

This patch adds the ACPI code. It is similar to the one in EDK2 but doesn't
assume that SMIs are necessary to use. It uses a similar datastructure for
the shared memory as EDK2 does so that EDK2 and SeaBIOS could both make use
of it. I extended the shared memory data structure with an array of 256
bytes, one for each code that could be implemented. The array contains
flags describing the individual codes. This decouples the ACPI implementation
from the firmware implementation.

The underlying TCG specification is accessible from the following page.

https://trustedcomputinggroup.org/tcg-physical-presence-interface-specification/

This patch implements version 1.30.

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
[ Marc-André - ACPI code improvements and windows fixes ]
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
---
 include/hw/acpi/tpm.h |   8 +
 hw/i386/acpi-build.c  | 393 +++++++++++++++++++++++++++++++++++++++++-
 docs/specs/tpm.txt    |  83 +++++++++
 3 files changed, 481 insertions(+), 3 deletions(-)

diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
index a6109a97fc..ecccb96933 100644
--- a/include/hw/acpi/tpm.h
+++ b/include/hw/acpi/tpm.h
@@ -197,4 +197,12 @@ REG32(CRB_DATA_BUFFER, 0x80)
 #define TPM_PPI_VERSION_NONE        0
 #define TPM_PPI_VERSION_1_30        1
 
+/* whether function is blocked by BIOS settings; bits 0, 1, 2 */
+#define TPM_PPI_FUNC_NOT_IMPLEMENTED     (0 << 0)
+#define TPM_PPI_FUNC_BIOS_ONLY           (1 << 0)
+#define TPM_PPI_FUNC_BLOCKED             (2 << 0)
+#define TPM_PPI_FUNC_ALLOWED_USR_REQ     (3 << 0)
+#define TPM_PPI_FUNC_ALLOWED_USR_NOT_REQ (4 << 0)
+#define TPM_PPI_FUNC_MASK                (7 << 0)
+
 #endif /* HW_ACPI_TPM_H */
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index c24f68df02..c5e9a6e11d 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,386 @@ static Aml *build_q35_osc_method(void)
     return method;
 }
 
+static void
+build_tpm_ppi(TPMIf *tpm, Aml *dev)
+{
+    Aml *method, *field, *ifctx, *ifctx2, *ifctx3, *func_mask,
+        *not_implemented, *pak, *tpm2, *tpm3, *pprm, *pprq, *zero, *one;
+
+    if (!object_property_get_bool(OBJECT(tpm), "ppi", &error_abort)) {
+        return;
+    }
+
+    zero = aml_int(0);
+    one = aml_int(1);
+    func_mask = aml_int(TPM_PPI_FUNC_MASK);
+    not_implemented = aml_int(TPM_PPI_FUNC_NOT_IMPLEMENTED);
+
+    /*
+     * TPP2 is for the registers that ACPI code used to pass
+     * the PPI code and parameter (PPRQ, PPRM) to the firmware.
+     */
+    aml_append(dev,
+               aml_operation_region("TPP2", AML_SYSTEM_MEMORY,
+                                    aml_int(TPM_PPI_ADDR_BASE + 0x100),
+                                    0x5A));
+    field = aml_field("TPP2", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
+    aml_append(field, aml_named_field("PPIN", 8));
+    aml_append(field, aml_named_field("PPIP", 32));
+    aml_append(field, aml_named_field("PPRP", 32));
+    aml_append(field, aml_named_field("PPRQ", 32));
+    aml_append(field, aml_named_field("PPRM", 32));
+    aml_append(field, aml_named_field("LPPR", 32));
+    aml_append(dev, field);
+    pprq = aml_name("PPRQ");
+    pprm = aml_name("PPRM");
+
+    /*
+     * DerefOf in Windows is broken with SYSTEM_MEMORY.  Use a dynamic
+     * operation region inside of a method for getting FUNC[op].
+     */
+    method = aml_method("TPFN", 1, AML_SERIALIZED);
+    {
+        Aml *op = aml_arg(0);
+        ifctx = aml_if(aml_lgreater_equal(op, aml_int(0x100)));
+        {
+            aml_append(ifctx, aml_return(zero));
+        }
+        aml_append(method, ifctx);
+
+        aml_append(method,
+            aml_operation_region("TPP1", AML_SYSTEM_MEMORY,
+                aml_add(aml_int(TPM_PPI_ADDR_BASE), op, NULL), 0x1));
+        field = aml_field("TPP1", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
+        aml_append(field, aml_named_field("TPPF", 8));
+        aml_append(method, field);
+        aml_append(method, aml_return(aml_name("TPPF")));
+    }
+    aml_append(dev, method);
+
+    /*
+     * Use global TPM2 & TPM3 variables to workaround Windows ACPI bug
+     * when returning packages.
+     */
+    pak = aml_package(2);
+    aml_append(pak, zero);
+    aml_append(pak, zero);
+    aml_append(dev, aml_name_decl("TPM2", pak));
+    tpm2 = aml_name("TPM2");
+
+    pak = aml_package(3);
+    aml_append(pak, zero);
+    aml_append(pak, zero);
+    aml_append(pak, zero);
+    aml_append(dev, aml_name_decl("TPM3", pak));
+    tpm3 = aml_name("TPM3");
+
+    method = aml_method("_DSM", 4, AML_SERIALIZED);
+    {
+        uint8_t zerobyte[1] = { 0 };
+        Aml *function, *arguments, *rev, *op, *op_arg, *op_flags, *uuid;
+
+        uuid = aml_arg(0);
+        rev = aml_arg(1);
+        function = aml_arg(2);
+        arguments = aml_arg(3);
+        op = aml_local(0);
+        op_flags = aml_local(1);
+
+        ifctx = aml_if(
+            aml_equal(uuid,
+                      aml_touuid("3DDDFAA6-361B-4EB4-A424-8D10089D1653")));
+        {
+            /* standard DSM query function */
+            ifctx2 = aml_if(aml_equal(function, zero));
+            {
+                uint8_t byte_list[2] = { 0xff, 0x01 };
+                aml_append(ifctx2, aml_return(aml_buffer(2, byte_list)));
+            }
+            aml_append(ifctx, ifctx2);
+
+            /*
+             * PPI 1.0: 2.1.1 Get Physical Presence Interface Version
+             *
+             * Arg 2 (Integer): Function Index = 1
+             * Arg 3 (Package): Arguments = Empty Package
+             * Returns: Type: String
+             */
+            ifctx2 = aml_if(aml_equal(function, one));
+            {
+                aml_append(ifctx2, aml_return(aml_string("1.3")));
+            }
+            aml_append(ifctx, ifctx2);
+
+            /*
+             * PPI 1.0: 2.1.3 Submit TPM Operation Request to Pre-OS Environment
+             *
+             * Arg 2 (Integer): Function Index = 2
+             * Arg 3 (Package): Arguments = Package: Type: Integer
+             *                              Operation Value of the Request
+             * Returns: Type: Integer
+             *          0: Success
+             *          1: Operation Value of the Request Not Supported
+             *          2: General Failure
+             */
+            ifctx2 = aml_if(aml_equal(function, aml_int(2)));
+            {
+                /* get opcode */
+                aml_append(ifctx2,
+                           aml_store(aml_derefof(aml_index(arguments,
+                                                           zero)), op));
+
+                /* get opcode flags */
+                aml_append(ifctx2,
+                           aml_store(aml_call1("TPFN", op), op_flags));
+
+                /* if func[opcode] & TPM_PPI_FUNC_NOT_IMPLEMENTED */
+                ifctx3 = aml_if(
+                    aml_equal(
+                        aml_and(op_flags, func_mask, NULL),
+                        not_implemented));
+                {
+                    /* 1: Operation Value of the Request Not Supported */
+                    aml_append(ifctx3, aml_return(one));
+                }
+                aml_append(ifctx2, ifctx3);
+
+                aml_append(ifctx2, aml_store(op, pprq));
+                aml_append(ifctx2, aml_store(zero, pprm));
+                /* 0: success */
+                aml_append(ifctx2, aml_return(zero));
+            }
+            aml_append(ifctx, ifctx2);
+
+            /*
+             * PPI 1.0: 2.1.4 Get Pending TPM Operation Requested By the OS
+             *
+             * Arg 2 (Integer): Function Index = 3
+             * Arg 3 (Package): Arguments = Empty Package
+             * Returns: Type: Package of Integers
+             *          Integer 1: Function Return code
+             *                     0: Success
+             *                     1: General Failure
+             *          Integer 2: Pending operation requested by the OS
+             *                     0: None
+             *                    >0: Operation Value of the Pending Request
+             *          Integer 3: Optional argument to pending operation
+             *                     requested by the OS
+             *                     0: None
+             *                    >0: Argument Value of the Pending Request
+             */
+            ifctx2 = aml_if(aml_equal(function, aml_int(3)));
+            {
+                /*
+                 * Revision ID of 1, no integer parameter beyond
+                 * parameter two are expected
+                 */
+                ifctx3 = aml_if(aml_equal(rev, one));
+                {
+                    /* TPM2[1] = PPRQ */
+                    aml_append(ifctx3,
+                               aml_store(pprq, aml_index(tpm2, one)));
+                    aml_append(ifctx3, aml_return(tpm2));
+                }
+                aml_append(ifctx2, ifctx3);
+
+                /*
+                 * A return value of {0, 23, 1} indicates that
+                 * operation 23 with argument 1 is pending.
+                 */
+                ifctx3 = aml_if(aml_equal(rev, aml_int(2)));
+                {
+                    /* TPM3[1] = PPRQ */
+                    aml_append(ifctx3,
+                               aml_store(pprq, aml_index(tpm3, one)));
+                    /* TPM3[2] = PPRM */
+                    aml_append(ifctx3,
+                               aml_store(pprm, aml_index(tpm3, aml_int(2))));
+                    aml_append(ifctx3, aml_return(tpm3));
+                }
+                aml_append(ifctx2, ifctx3);
+            }
+            aml_append(ifctx, ifctx2);
+
+            /*
+             * PPI 1.0: 2.1.5 Get Platform-Specific Action to Transition to
+             *     Pre-OS Environment
+             *
+             * Arg 2 (Integer): Function Index = 4
+             * Arg 3 (Package): Arguments = Empty Package
+             * Returns: Type: Integer
+             *          0: None
+             *          1: Shutdown
+             *          2: Reboot
+             *          3: OS Vendor-specific
+             */
+            ifctx2 = aml_if(aml_equal(function, aml_int(4)));
+            {
+                /* reboot */
+                aml_append(ifctx2, aml_return(aml_int(2)));
+            }
+            aml_append(ifctx, ifctx2);
+
+            /*
+             * PPI 1.0: 2.1.6 Return TPM Operation Response to OS Environment
+             *
+             * Arg 2 (Integer): Function Index = 5
+             * Arg 3 (Package): Arguments = Empty Package
+             * Returns: Type: Package of Integer
+             *          Integer 1: Function Return code
+             *                     0: Success
+             *                     1: General Failure
+             *          Integer 2: Most recent operation request
+             *                     0: None
+             *                    >0: Operation Value of the most recent request
+             *          Integer 3: Response to the most recent operation request
+             *                     0: Success
+             *                     0x00000001..0x00000FFF: Corresponding TPM
+             *                                             error code
+             *                     0xFFFFFFF0: User Abort or timeout of dialog
+             *                     0xFFFFFFF1: firmware Failure
+             */
+            ifctx2 = aml_if(aml_equal(function, aml_int(5)));
+            {
+                /* TPM3[1] = LPPR */
+                aml_append(ifctx2,
+                           aml_store(aml_name("LPPR"),
+                                     aml_index(tpm3, one)));
+                /* TPM3[2] = PPRP */
+                aml_append(ifctx2,
+                           aml_store(aml_name("PPRP"),
+                                     aml_index(tpm3, aml_int(2))));
+                aml_append(ifctx2, aml_return(tpm3));
+            }
+            aml_append(ifctx, ifctx2);
+
+            /*
+             * PPI 1.0: 2.1.7 Submit preferred user language
+             *
+             * Arg 2 (Integer): Function Index = 6
+             * Arg 3 (Package): Arguments = String Package
+             *                  Preferred language code
+             * Returns: Type: Integer
+             * Function Return Code
+             *          3: Not implemented
+             */
+            ifctx2 = aml_if(aml_equal(function, aml_int(6)));
+            {
+                /* 3 = not implemented */
+                aml_append(ifctx2, aml_return(aml_int(3)));
+            }
+            aml_append(ifctx, ifctx2);
+
+            /*
+             * PPI 1.1: 2.1.7 Submit TPM Operation Request to
+             *     Pre-OS Environment 2
+             *
+             * Arg 2 (Integer): Function Index = 7
+             * Arg 3 (Package): Arguments = Package: Type: Integer
+             *                  Integer 1: Operation Value of the Request
+             *                  Integer 2: Argument for Operation (optional)
+             * Returns: Type: Integer
+             *          0: Success
+             *          1: Not Implemented
+             *          2: General Failure
+             *          3: Operation blocked by current firmware settings
+             */
+            ifctx2 = aml_if(aml_equal(function, aml_int(7)));
+            {
+                /* get opcode */
+                aml_append(ifctx2, aml_store(aml_derefof(aml_index(arguments,
+                                                                   zero)),
+                                             op));
+
+                /* get opcode flags */
+                aml_append(ifctx2, aml_store(aml_call1("TPFN", op),
+                                             op_flags));
+                /* if func[opcode] & TPM_PPI_FUNC_NOT_IMPLEMENTED */
+                ifctx3 = aml_if(
+                    aml_equal(
+                        aml_and(op_flags, func_mask, NULL),
+                        not_implemented));
+                {
+                    /* 1: not implemented */
+                    aml_append(ifctx3, aml_return(one));
+                }
+                aml_append(ifctx2, ifctx3);
+
+                /* if func[opcode] & TPM_PPI_FUNC_BLOCKED */
+                ifctx3 = aml_if(
+                    aml_equal(
+                        aml_and(op_flags, func_mask, NULL),
+                        aml_int(TPM_PPI_FUNC_BLOCKED)));
+                {
+                    /* 3: blocked by firmware */
+                    aml_append(ifctx3, aml_return(aml_int(3)));
+                }
+                aml_append(ifctx2, ifctx3);
+
+                /* revision to integer */
+                ifctx3 = aml_if(aml_equal(rev, one));
+                {
+                    /* revision 1 */
+                    /* PPRQ = op */
+                    aml_append(ifctx3, aml_store(op, pprq));
+                    /* no argument, PPRM = 0 */
+                    aml_append(ifctx3, aml_store(zero, pprm));
+                }
+                aml_append(ifctx2, ifctx3);
+
+                ifctx3 = aml_if(aml_equal(rev, aml_int(2)));
+                {
+                    /* revision 2 */
+                    /* PPRQ = op */
+                    op_arg = aml_derefof(aml_index(arguments, one));
+                    aml_append(ifctx3, aml_store(op, pprq));
+                    /* PPRM = arg3[1] */
+                    aml_append(ifctx3, aml_store(op_arg, pprm));
+                }
+                aml_append(ifctx2, ifctx3);
+                /* 0: success */
+                aml_append(ifctx2, aml_return(zero));
+            }
+            aml_append(ifctx, ifctx2);
+
+            /*
+             * PPI 1.1: 2.1.8 Get User Confirmation Status for Operation
+             *
+             * Arg 2 (Integer): Function Index = 8
+             * Arg 3 (Package): Arguments = Package: Type: Integer
+             *                  Operation Value that may need user confirmation
+             * Returns: Type: Integer
+             *          0: Not implemented
+             *          1: Firmware only
+             *          2: Blocked for OS by firmware configuration
+             *          3: Allowed and physically present user required
+             *          4: Allowed and physically present user not required
+             */
+            ifctx2 = aml_if(aml_equal(function, aml_int(8)));
+            {
+                /* get opcode */
+                aml_append(ifctx2,
+                           aml_store(aml_derefof(aml_index(arguments,
+                                                           zero)),
+                                     op));
+
+                /* get opcode flags */
+                aml_append(ifctx2, aml_store(aml_call1("TPFN", op),
+                                             op_flags));
+                /* return confirmation status code */
+                aml_append(ifctx2,
+                           aml_return(
+                               aml_and(op_flags, func_mask, NULL)));
+            }
+            aml_append(ifctx, ifctx2);
+
+            aml_append(ifctx, aml_return(aml_buffer(1, zerobyte)));
+        }
+        aml_append(method, ifctx);
+    }
+    aml_append(dev, method);
+}
+
 static void
 build_dsdt(GArray *table_data, BIOSLinker *linker,
            AcpiPmInfo *pm, AcpiMiscInfo *misc,
@@ -1802,6 +2183,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
     uint32_t nr_mem = machine->ram_slots;
     int root_bus_limit = 0xFF;
     PCIBus *bus = NULL;
+    TPMIf *tpm = tpm_find();
     int i;
 
     dsdt = init_aml_allocator();
@@ -2139,7 +2521,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
             /* Scan all PCI buses. Generate tables to support hotplug. */
             build_append_pci_bus_devices(scope, bus, pm->pcihp_bridge_en);
 
-            if (TPM_IS_TIS(tpm_find())) {
+            if (TPM_IS_TIS(tpm)) {
                 dev = aml_device("ISA.TPM");
                 aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0C31")));
                 aml_append(dev, aml_name_decl("_STA", aml_int(0xF)));
@@ -2153,6 +2535,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
                  */
                 /* aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ)); */
                 aml_append(dev, aml_name_decl("_CRS", crs));
+
+                build_tpm_ppi(tpm, dev);
+
                 aml_append(scope, dev);
             }
 
@@ -2160,7 +2545,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
         }
     }
 
-    if (TPM_IS_CRB(tpm_find())) {
+    if (TPM_IS_CRB(tpm)) {
         dev = aml_device("TPM");
         aml_append(dev, aml_name_decl("_HID", aml_string("MSFT0101")));
         crs = aml_resource_template();
@@ -2172,6 +2557,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
         aml_append(method, aml_return(aml_int(0x0f)));
         aml_append(dev, method);
 
+        build_tpm_ppi(tpm, dev);
+
         aml_append(sb_scope, dev);
     }
 
@@ -2924,7 +3311,7 @@ void acpi_setup(void)
         tpm_config = (FWCfgTPMConfig) {
             .tpmppi_address = cpu_to_le32(TPM_PPI_ADDR_BASE),
             .tpm_version = tpm_get_version(tpm_find()),
-            .tpmppi_version = TPM_PPI_VERSION_NONE
+            .tpmppi_version = TPM_PPI_VERSION_1_30
         };
         fw_cfg_add_file(pcms->fw_cfg, "etc/tpm/config",
                         &tpm_config, sizeof tpm_config);
diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
index a5bdd5f26e..332c2ae597 100644
--- a/docs/specs/tpm.txt
+++ b/docs/specs/tpm.txt
@@ -62,6 +62,89 @@ 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.                                                   |
+ +----------+-------------------------------------------------------------+
+
+The location of the table is given by the fw_cfg tpmppi_address field.
+The PPI memory region size is 0x400 (TPM_PPI_ADDR_SIZE) to leave
+enough room for future updates.
+
 
 QEMU files related to TPM ACPI tables:
  - hw/i386/acpi-build.c
-- 
2.19.0.rc1

Re: [Qemu-devel] [PATCH v12 5/6] acpi: build TPM Physical Presence interface

[Philippe Mathieu-Daudé]

On 9/10/18 10:32 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>
> [ Marc-André - ACPI code improvements and windows fixes ]
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
> Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
>  include/hw/acpi/tpm.h |   8 +
>  hw/i386/acpi-build.c  | 393 +++++++++++++++++++++++++++++++++++++++++-

The spec doesnt' look X86 specific, I'd add this code in "hw/acpi/tpm.c"

>  docs/specs/tpm.txt    |  83 +++++++++
>  3 files changed, 481 insertions(+), 3 deletions(-)
> 
> diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
> index a6109a97fc..ecccb96933 100644
> --- a/include/hw/acpi/tpm.h
> +++ b/include/hw/acpi/tpm.h
> @@ -197,4 +197,12 @@ REG32(CRB_DATA_BUFFER, 0x80)
>  #define TPM_PPI_VERSION_NONE        0
>  #define TPM_PPI_VERSION_1_30        1
>  
> +/* whether function is blocked by BIOS settings; bits 0, 1, 2 */
> +#define TPM_PPI_FUNC_NOT_IMPLEMENTED     (0 << 0)
> +#define TPM_PPI_FUNC_BIOS_ONLY           (1 << 0)
> +#define TPM_PPI_FUNC_BLOCKED             (2 << 0)
> +#define TPM_PPI_FUNC_ALLOWED_USR_REQ     (3 << 0)
> +#define TPM_PPI_FUNC_ALLOWED_USR_NOT_REQ (4 << 0)
> +#define TPM_PPI_FUNC_MASK                (7 << 0)
> +
>  #endif /* HW_ACPI_TPM_H */
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index c24f68df02..c5e9a6e11d 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,386 @@ static Aml *build_q35_osc_method(void)
>      return method;
>  }
>  
> +static void
> +build_tpm_ppi(TPMIf *tpm, Aml *dev)
> +{
> +    Aml *method, *field, *ifctx, *ifctx2, *ifctx3, *func_mask,
> +        *not_implemented, *pak, *tpm2, *tpm3, *pprm, *pprq, *zero, *one;
> +
> +    if (!object_property_get_bool(OBJECT(tpm), "ppi", &error_abort)) {
> +        return;
> +    }
> +
> +    zero = aml_int(0);
> +    one = aml_int(1);
> +    func_mask = aml_int(TPM_PPI_FUNC_MASK);
> +    not_implemented = aml_int(TPM_PPI_FUNC_NOT_IMPLEMENTED);
> +
> +    /*
> +     * TPP2 is for the registers that ACPI code used to pass
> +     * the PPI code and parameter (PPRQ, PPRM) to the firmware.
> +     */
> +    aml_append(dev,
> +               aml_operation_region("TPP2", AML_SYSTEM_MEMORY,
> +                                    aml_int(TPM_PPI_ADDR_BASE + 0x100),
> +                                    0x5A));
> +    field = aml_field("TPP2", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
> +    aml_append(field, aml_named_field("PPIN", 8));
> +    aml_append(field, aml_named_field("PPIP", 32));
> +    aml_append(field, aml_named_field("PPRP", 32));
> +    aml_append(field, aml_named_field("PPRQ", 32));
> +    aml_append(field, aml_named_field("PPRM", 32));
> +    aml_append(field, aml_named_field("LPPR", 32));
> +    aml_append(dev, field);
> +    pprq = aml_name("PPRQ");
> +    pprm = aml_name("PPRM");
> +
> +    /*
> +     * DerefOf in Windows is broken with SYSTEM_MEMORY.  Use a dynamic
> +     * operation region inside of a method for getting FUNC[op].
> +     */
> +    method = aml_method("TPFN", 1, AML_SERIALIZED);
> +    {
> +        Aml *op = aml_arg(0);
> +        ifctx = aml_if(aml_lgreater_equal(op, aml_int(0x100)));
> +        {
> +            aml_append(ifctx, aml_return(zero));
> +        }
> +        aml_append(method, ifctx);
> +
> +        aml_append(method,
> +            aml_operation_region("TPP1", AML_SYSTEM_MEMORY,
> +                aml_add(aml_int(TPM_PPI_ADDR_BASE), op, NULL), 0x1));
> +        field = aml_field("TPP1", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
> +        aml_append(field, aml_named_field("TPPF", 8));
> +        aml_append(method, field);
> +        aml_append(method, aml_return(aml_name("TPPF")));
> +    }
> +    aml_append(dev, method);
> +
> +    /*
> +     * Use global TPM2 & TPM3 variables to workaround Windows ACPI bug
> +     * when returning packages.
> +     */
> +    pak = aml_package(2);
> +    aml_append(pak, zero);
> +    aml_append(pak, zero);
> +    aml_append(dev, aml_name_decl("TPM2", pak));
> +    tpm2 = aml_name("TPM2");
> +
> +    pak = aml_package(3);
> +    aml_append(pak, zero);
> +    aml_append(pak, zero);
> +    aml_append(pak, zero);
> +    aml_append(dev, aml_name_decl("TPM3", pak));
> +    tpm3 = aml_name("TPM3");
> +
> +    method = aml_method("_DSM", 4, AML_SERIALIZED);
> +    {
> +        uint8_t zerobyte[1] = { 0 };
> +        Aml *function, *arguments, *rev, *op, *op_arg, *op_flags, *uuid;
> +
> +        uuid = aml_arg(0);
> +        rev = aml_arg(1);
> +        function = aml_arg(2);
> +        arguments = aml_arg(3);
> +        op = aml_local(0);
> +        op_flags = aml_local(1);
> +
> +        ifctx = aml_if(
> +            aml_equal(uuid,
> +                      aml_touuid("3DDDFAA6-361B-4EB4-A424-8D10089D1653")));
> +        {
> +            /* standard DSM query function */
> +            ifctx2 = aml_if(aml_equal(function, zero));
> +            {
> +                uint8_t byte_list[2] = { 0xff, 0x01 };
> +                aml_append(ifctx2, aml_return(aml_buffer(2, byte_list)));
> +            }
> +            aml_append(ifctx, ifctx2);
> +
> +            /*
> +             * PPI 1.0: 2.1.1 Get Physical Presence Interface Version
> +             *
> +             * Arg 2 (Integer): Function Index = 1
> +             * Arg 3 (Package): Arguments = Empty Package
> +             * Returns: Type: String
> +             */
> +            ifctx2 = aml_if(aml_equal(function, one));
> +            {
> +                aml_append(ifctx2, aml_return(aml_string("1.3")));
> +            }
> +            aml_append(ifctx, ifctx2);
> +
> +            /*
> +             * PPI 1.0: 2.1.3 Submit TPM Operation Request to Pre-OS Environment
> +             *
> +             * Arg 2 (Integer): Function Index = 2
> +             * Arg 3 (Package): Arguments = Package: Type: Integer
> +             *                              Operation Value of the Request
> +             * Returns: Type: Integer
> +             *          0: Success
> +             *          1: Operation Value of the Request Not Supported
> +             *          2: General Failure
> +             */
> +            ifctx2 = aml_if(aml_equal(function, aml_int(2)));
> +            {
> +                /* get opcode */
> +                aml_append(ifctx2,
> +                           aml_store(aml_derefof(aml_index(arguments,
> +                                                           zero)), op));
> +
> +                /* get opcode flags */
> +                aml_append(ifctx2,
> +                           aml_store(aml_call1("TPFN", op), op_flags));
> +
> +                /* if func[opcode] & TPM_PPI_FUNC_NOT_IMPLEMENTED */
> +                ifctx3 = aml_if(
> +                    aml_equal(
> +                        aml_and(op_flags, func_mask, NULL),
> +                        not_implemented));
> +                {
> +                    /* 1: Operation Value of the Request Not Supported */
> +                    aml_append(ifctx3, aml_return(one));
> +                }
> +                aml_append(ifctx2, ifctx3);
> +
> +                aml_append(ifctx2, aml_store(op, pprq));
> +                aml_append(ifctx2, aml_store(zero, pprm));
> +                /* 0: success */
> +                aml_append(ifctx2, aml_return(zero));
> +            }
> +            aml_append(ifctx, ifctx2);
> +
> +            /*
> +             * PPI 1.0: 2.1.4 Get Pending TPM Operation Requested By the OS
> +             *
> +             * Arg 2 (Integer): Function Index = 3
> +             * Arg 3 (Package): Arguments = Empty Package
> +             * Returns: Type: Package of Integers
> +             *          Integer 1: Function Return code
> +             *                     0: Success
> +             *                     1: General Failure
> +             *          Integer 2: Pending operation requested by the OS
> +             *                     0: None
> +             *                    >0: Operation Value of the Pending Request
> +             *          Integer 3: Optional argument to pending operation
> +             *                     requested by the OS
> +             *                     0: None
> +             *                    >0: Argument Value of the Pending Request
> +             */
> +            ifctx2 = aml_if(aml_equal(function, aml_int(3)));
> +            {
> +                /*
> +                 * Revision ID of 1, no integer parameter beyond
> +                 * parameter two are expected
> +                 */
> +                ifctx3 = aml_if(aml_equal(rev, one));
> +                {
> +                    /* TPM2[1] = PPRQ */
> +                    aml_append(ifctx3,
> +                               aml_store(pprq, aml_index(tpm2, one)));
> +                    aml_append(ifctx3, aml_return(tpm2));
> +                }
> +                aml_append(ifctx2, ifctx3);
> +
> +                /*
> +                 * A return value of {0, 23, 1} indicates that
> +                 * operation 23 with argument 1 is pending.
> +                 */
> +                ifctx3 = aml_if(aml_equal(rev, aml_int(2)));
> +                {
> +                    /* TPM3[1] = PPRQ */
> +                    aml_append(ifctx3,
> +                               aml_store(pprq, aml_index(tpm3, one)));
> +                    /* TPM3[2] = PPRM */
> +                    aml_append(ifctx3,
> +                               aml_store(pprm, aml_index(tpm3, aml_int(2))));
> +                    aml_append(ifctx3, aml_return(tpm3));
> +                }
> +                aml_append(ifctx2, ifctx3);
> +            }
> +            aml_append(ifctx, ifctx2);
> +
> +            /*
> +             * PPI 1.0: 2.1.5 Get Platform-Specific Action to Transition to
> +             *     Pre-OS Environment
> +             *
> +             * Arg 2 (Integer): Function Index = 4
> +             * Arg 3 (Package): Arguments = Empty Package
> +             * Returns: Type: Integer
> +             *          0: None
> +             *          1: Shutdown
> +             *          2: Reboot
> +             *          3: OS Vendor-specific
> +             */
> +            ifctx2 = aml_if(aml_equal(function, aml_int(4)));
> +            {
> +                /* reboot */
> +                aml_append(ifctx2, aml_return(aml_int(2)));
> +            }
> +            aml_append(ifctx, ifctx2);
> +
> +            /*
> +             * PPI 1.0: 2.1.6 Return TPM Operation Response to OS Environment
> +             *
> +             * Arg 2 (Integer): Function Index = 5
> +             * Arg 3 (Package): Arguments = Empty Package
> +             * Returns: Type: Package of Integer
> +             *          Integer 1: Function Return code
> +             *                     0: Success
> +             *                     1: General Failure
> +             *          Integer 2: Most recent operation request
> +             *                     0: None
> +             *                    >0: Operation Value of the most recent request
> +             *          Integer 3: Response to the most recent operation request
> +             *                     0: Success
> +             *                     0x00000001..0x00000FFF: Corresponding TPM
> +             *                                             error code
> +             *                     0xFFFFFFF0: User Abort or timeout of dialog
> +             *                     0xFFFFFFF1: firmware Failure
> +             */
> +            ifctx2 = aml_if(aml_equal(function, aml_int(5)));
> +            {
> +                /* TPM3[1] = LPPR */
> +                aml_append(ifctx2,
> +                           aml_store(aml_name("LPPR"),
> +                                     aml_index(tpm3, one)));
> +                /* TPM3[2] = PPRP */
> +                aml_append(ifctx2,
> +                           aml_store(aml_name("PPRP"),
> +                                     aml_index(tpm3, aml_int(2))));
> +                aml_append(ifctx2, aml_return(tpm3));
> +            }
> +            aml_append(ifctx, ifctx2);
> +
> +            /*
> +             * PPI 1.0: 2.1.7 Submit preferred user language
> +             *
> +             * Arg 2 (Integer): Function Index = 6
> +             * Arg 3 (Package): Arguments = String Package
> +             *                  Preferred language code
> +             * Returns: Type: Integer
> +             * Function Return Code
> +             *          3: Not implemented
> +             */
> +            ifctx2 = aml_if(aml_equal(function, aml_int(6)));
> +            {
> +                /* 3 = not implemented */
> +                aml_append(ifctx2, aml_return(aml_int(3)));
> +            }
> +            aml_append(ifctx, ifctx2);
> +
> +            /*
> +             * PPI 1.1: 2.1.7 Submit TPM Operation Request to
> +             *     Pre-OS Environment 2
> +             *
> +             * Arg 2 (Integer): Function Index = 7
> +             * Arg 3 (Package): Arguments = Package: Type: Integer
> +             *                  Integer 1: Operation Value of the Request
> +             *                  Integer 2: Argument for Operation (optional)
> +             * Returns: Type: Integer
> +             *          0: Success
> +             *          1: Not Implemented
> +             *          2: General Failure
> +             *          3: Operation blocked by current firmware settings
> +             */
> +            ifctx2 = aml_if(aml_equal(function, aml_int(7)));
> +            {
> +                /* get opcode */
> +                aml_append(ifctx2, aml_store(aml_derefof(aml_index(arguments,
> +                                                                   zero)),
> +                                             op));
> +
> +                /* get opcode flags */
> +                aml_append(ifctx2, aml_store(aml_call1("TPFN", op),
> +                                             op_flags));
> +                /* if func[opcode] & TPM_PPI_FUNC_NOT_IMPLEMENTED */
> +                ifctx3 = aml_if(
> +                    aml_equal(
> +                        aml_and(op_flags, func_mask, NULL),
> +                        not_implemented));
> +                {
> +                    /* 1: not implemented */
> +                    aml_append(ifctx3, aml_return(one));
> +                }
> +                aml_append(ifctx2, ifctx3);
> +
> +                /* if func[opcode] & TPM_PPI_FUNC_BLOCKED */
> +                ifctx3 = aml_if(
> +                    aml_equal(
> +                        aml_and(op_flags, func_mask, NULL),
> +                        aml_int(TPM_PPI_FUNC_BLOCKED)));
> +                {
> +                    /* 3: blocked by firmware */
> +                    aml_append(ifctx3, aml_return(aml_int(3)));
> +                }
> +                aml_append(ifctx2, ifctx3);
> +
> +                /* revision to integer */
> +                ifctx3 = aml_if(aml_equal(rev, one));
> +                {
> +                    /* revision 1 */
> +                    /* PPRQ = op */
> +                    aml_append(ifctx3, aml_store(op, pprq));
> +                    /* no argument, PPRM = 0 */
> +                    aml_append(ifctx3, aml_store(zero, pprm));
> +                }
> +                aml_append(ifctx2, ifctx3);
> +
> +                ifctx3 = aml_if(aml_equal(rev, aml_int(2)));
> +                {
> +                    /* revision 2 */
> +                    /* PPRQ = op */
> +                    op_arg = aml_derefof(aml_index(arguments, one));
> +                    aml_append(ifctx3, aml_store(op, pprq));
> +                    /* PPRM = arg3[1] */
> +                    aml_append(ifctx3, aml_store(op_arg, pprm));
> +                }
> +                aml_append(ifctx2, ifctx3);
> +                /* 0: success */
> +                aml_append(ifctx2, aml_return(zero));
> +            }
> +            aml_append(ifctx, ifctx2);
> +
> +            /*
> +             * PPI 1.1: 2.1.8 Get User Confirmation Status for Operation
> +             *
> +             * Arg 2 (Integer): Function Index = 8
> +             * Arg 3 (Package): Arguments = Package: Type: Integer
> +             *                  Operation Value that may need user confirmation
> +             * Returns: Type: Integer
> +             *          0: Not implemented
> +             *          1: Firmware only
> +             *          2: Blocked for OS by firmware configuration
> +             *          3: Allowed and physically present user required
> +             *          4: Allowed and physically present user not required
> +             */
> +            ifctx2 = aml_if(aml_equal(function, aml_int(8)));
> +            {
> +                /* get opcode */
> +                aml_append(ifctx2,
> +                           aml_store(aml_derefof(aml_index(arguments,
> +                                                           zero)),
> +                                     op));
> +
> +                /* get opcode flags */
> +                aml_append(ifctx2, aml_store(aml_call1("TPFN", op),
> +                                             op_flags));
> +                /* return confirmation status code */
> +                aml_append(ifctx2,
> +                           aml_return(
> +                               aml_and(op_flags, func_mask, NULL)));
> +            }
> +            aml_append(ifctx, ifctx2);
> +
> +            aml_append(ifctx, aml_return(aml_buffer(1, zerobyte)));
> +        }
> +        aml_append(method, ifctx);
> +    }
> +    aml_append(dev, method);
> +}
> +
>  static void
>  build_dsdt(GArray *table_data, BIOSLinker *linker,
>             AcpiPmInfo *pm, AcpiMiscInfo *misc,
> @@ -1802,6 +2183,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>      uint32_t nr_mem = machine->ram_slots;
>      int root_bus_limit = 0xFF;
>      PCIBus *bus = NULL;
> +    TPMIf *tpm = tpm_find();
>      int i;
>  
>      dsdt = init_aml_allocator();
> @@ -2139,7 +2521,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>              /* Scan all PCI buses. Generate tables to support hotplug. */
>              build_append_pci_bus_devices(scope, bus, pm->pcihp_bridge_en);
>  
> -            if (TPM_IS_TIS(tpm_find())) {
> +            if (TPM_IS_TIS(tpm)) {
>                  dev = aml_device("ISA.TPM");
>                  aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0C31")));
>                  aml_append(dev, aml_name_decl("_STA", aml_int(0xF)));
> @@ -2153,6 +2535,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>                   */
>                  /* aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ)); */
>                  aml_append(dev, aml_name_decl("_CRS", crs));
> +
> +                build_tpm_ppi(tpm, dev);
> +
>                  aml_append(scope, dev);
>              }
>  
> @@ -2160,7 +2545,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>          }
>      }
>  
> -    if (TPM_IS_CRB(tpm_find())) {
> +    if (TPM_IS_CRB(tpm)) {
>          dev = aml_device("TPM");
>          aml_append(dev, aml_name_decl("_HID", aml_string("MSFT0101")));
>          crs = aml_resource_template();
> @@ -2172,6 +2557,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
>          aml_append(method, aml_return(aml_int(0x0f)));
>          aml_append(dev, method);
>  
> +        build_tpm_ppi(tpm, dev);
> +
>          aml_append(sb_scope, dev);
>      }
>  
> @@ -2924,7 +3311,7 @@ void acpi_setup(void)
>          tpm_config = (FWCfgTPMConfig) {
>              .tpmppi_address = cpu_to_le32(TPM_PPI_ADDR_BASE),
>              .tpm_version = tpm_get_version(tpm_find()),
> -            .tpmppi_version = TPM_PPI_VERSION_NONE
> +            .tpmppi_version = TPM_PPI_VERSION_1_30
>          };
>          fw_cfg_add_file(pcms->fw_cfg, "etc/tpm/config",
>                          &tpm_config, sizeof tpm_config);
> diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
> index a5bdd5f26e..332c2ae597 100644
> --- a/docs/specs/tpm.txt
> +++ b/docs/specs/tpm.txt
> @@ -62,6 +62,89 @@ 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.                                                   |
> + +----------+-------------------------------------------------------------+
> +
> +The location of the table is given by the fw_cfg tpmppi_address field.
> +The PPI memory region size is 0x400 (TPM_PPI_ADDR_SIZE) to leave
> +enough room for future updates.
> +
>  
>  QEMU files related to TPM ACPI tables:
>   - hw/i386/acpi-build.c
> 

Re: [Qemu-devel] [PATCH v12 5/6] acpi: build TPM Physical Presence interface

[Marc-André Lureau]

On Wed, Dec 12, 2018 at 8:16 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> On 9/10/18 10:32 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>
> > [ Marc-André - ACPI code improvements and windows fixes ]
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Acked-by: Michael S. Tsirkin <mst@redhat.com>
> > Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >  include/hw/acpi/tpm.h |   8 +
> >  hw/i386/acpi-build.c  | 393 +++++++++++++++++++++++++++++++++++++++++-
>
> The spec doesnt' look X86 specific, I'd add this code in "hw/acpi/tpm.c"

ok

>
> >  docs/specs/tpm.txt    |  83 +++++++++
> >  3 files changed, 481 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/hw/acpi/tpm.h b/include/hw/acpi/tpm.h
> > index a6109a97fc..ecccb96933 100644
> > --- a/include/hw/acpi/tpm.h
> > +++ b/include/hw/acpi/tpm.h
> > @@ -197,4 +197,12 @@ REG32(CRB_DATA_BUFFER, 0x80)
> >  #define TPM_PPI_VERSION_NONE        0
> >  #define TPM_PPI_VERSION_1_30        1
> >
> > +/* whether function is blocked by BIOS settings; bits 0, 1, 2 */
> > +#define TPM_PPI_FUNC_NOT_IMPLEMENTED     (0 << 0)
> > +#define TPM_PPI_FUNC_BIOS_ONLY           (1 << 0)
> > +#define TPM_PPI_FUNC_BLOCKED             (2 << 0)
> > +#define TPM_PPI_FUNC_ALLOWED_USR_REQ     (3 << 0)
> > +#define TPM_PPI_FUNC_ALLOWED_USR_NOT_REQ (4 << 0)
> > +#define TPM_PPI_FUNC_MASK                (7 << 0)
> > +
> >  #endif /* HW_ACPI_TPM_H */
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index c24f68df02..c5e9a6e11d 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,386 @@ static Aml *build_q35_osc_method(void)
> >      return method;
> >  }
> >
> > +static void
> > +build_tpm_ppi(TPMIf *tpm, Aml *dev)
> > +{
> > +    Aml *method, *field, *ifctx, *ifctx2, *ifctx3, *func_mask,
> > +        *not_implemented, *pak, *tpm2, *tpm3, *pprm, *pprq, *zero, *one;
> > +
> > +    if (!object_property_get_bool(OBJECT(tpm), "ppi", &error_abort)) {
> > +        return;
> > +    }
> > +
> > +    zero = aml_int(0);
> > +    one = aml_int(1);
> > +    func_mask = aml_int(TPM_PPI_FUNC_MASK);
> > +    not_implemented = aml_int(TPM_PPI_FUNC_NOT_IMPLEMENTED);
> > +
> > +    /*
> > +     * TPP2 is for the registers that ACPI code used to pass
> > +     * the PPI code and parameter (PPRQ, PPRM) to the firmware.
> > +     */
> > +    aml_append(dev,
> > +               aml_operation_region("TPP2", AML_SYSTEM_MEMORY,
> > +                                    aml_int(TPM_PPI_ADDR_BASE + 0x100),
> > +                                    0x5A));
> > +    field = aml_field("TPP2", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
> > +    aml_append(field, aml_named_field("PPIN", 8));
> > +    aml_append(field, aml_named_field("PPIP", 32));
> > +    aml_append(field, aml_named_field("PPRP", 32));
> > +    aml_append(field, aml_named_field("PPRQ", 32));
> > +    aml_append(field, aml_named_field("PPRM", 32));
> > +    aml_append(field, aml_named_field("LPPR", 32));
> > +    aml_append(dev, field);
> > +    pprq = aml_name("PPRQ");
> > +    pprm = aml_name("PPRM");
> > +
> > +    /*
> > +     * DerefOf in Windows is broken with SYSTEM_MEMORY.  Use a dynamic
> > +     * operation region inside of a method for getting FUNC[op].
> > +     */
> > +    method = aml_method("TPFN", 1, AML_SERIALIZED);
> > +    {
> > +        Aml *op = aml_arg(0);
> > +        ifctx = aml_if(aml_lgreater_equal(op, aml_int(0x100)));
> > +        {
> > +            aml_append(ifctx, aml_return(zero));
> > +        }
> > +        aml_append(method, ifctx);
> > +
> > +        aml_append(method,
> > +            aml_operation_region("TPP1", AML_SYSTEM_MEMORY,
> > +                aml_add(aml_int(TPM_PPI_ADDR_BASE), op, NULL), 0x1));
> > +        field = aml_field("TPP1", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
> > +        aml_append(field, aml_named_field("TPPF", 8));
> > +        aml_append(method, field);
> > +        aml_append(method, aml_return(aml_name("TPPF")));
> > +    }
> > +    aml_append(dev, method);
> > +
> > +    /*
> > +     * Use global TPM2 & TPM3 variables to workaround Windows ACPI bug
> > +     * when returning packages.
> > +     */
> > +    pak = aml_package(2);
> > +    aml_append(pak, zero);
> > +    aml_append(pak, zero);
> > +    aml_append(dev, aml_name_decl("TPM2", pak));
> > +    tpm2 = aml_name("TPM2");
> > +
> > +    pak = aml_package(3);
> > +    aml_append(pak, zero);
> > +    aml_append(pak, zero);
> > +    aml_append(pak, zero);
> > +    aml_append(dev, aml_name_decl("TPM3", pak));
> > +    tpm3 = aml_name("TPM3");
> > +
> > +    method = aml_method("_DSM", 4, AML_SERIALIZED);
> > +    {
> > +        uint8_t zerobyte[1] = { 0 };
> > +        Aml *function, *arguments, *rev, *op, *op_arg, *op_flags, *uuid;
> > +
> > +        uuid = aml_arg(0);
> > +        rev = aml_arg(1);
> > +        function = aml_arg(2);
> > +        arguments = aml_arg(3);
> > +        op = aml_local(0);
> > +        op_flags = aml_local(1);
> > +
> > +        ifctx = aml_if(
> > +            aml_equal(uuid,
> > +                      aml_touuid("3DDDFAA6-361B-4EB4-A424-8D10089D1653")));
> > +        {
> > +            /* standard DSM query function */
> > +            ifctx2 = aml_if(aml_equal(function, zero));
> > +            {
> > +                uint8_t byte_list[2] = { 0xff, 0x01 };
> > +                aml_append(ifctx2, aml_return(aml_buffer(2, byte_list)));
> > +            }
> > +            aml_append(ifctx, ifctx2);
> > +
> > +            /*
> > +             * PPI 1.0: 2.1.1 Get Physical Presence Interface Version
> > +             *
> > +             * Arg 2 (Integer): Function Index = 1
> > +             * Arg 3 (Package): Arguments = Empty Package
> > +             * Returns: Type: String
> > +             */
> > +            ifctx2 = aml_if(aml_equal(function, one));
> > +            {
> > +                aml_append(ifctx2, aml_return(aml_string("1.3")));
> > +            }
> > +            aml_append(ifctx, ifctx2);
> > +
> > +            /*
> > +             * PPI 1.0: 2.1.3 Submit TPM Operation Request to Pre-OS Environment
> > +             *
> > +             * Arg 2 (Integer): Function Index = 2
> > +             * Arg 3 (Package): Arguments = Package: Type: Integer
> > +             *                              Operation Value of the Request
> > +             * Returns: Type: Integer
> > +             *          0: Success
> > +             *          1: Operation Value of the Request Not Supported
> > +             *          2: General Failure
> > +             */
> > +            ifctx2 = aml_if(aml_equal(function, aml_int(2)));
> > +            {
> > +                /* get opcode */
> > +                aml_append(ifctx2,
> > +                           aml_store(aml_derefof(aml_index(arguments,
> > +                                                           zero)), op));
> > +
> > +                /* get opcode flags */
> > +                aml_append(ifctx2,
> > +                           aml_store(aml_call1("TPFN", op), op_flags));
> > +
> > +                /* if func[opcode] & TPM_PPI_FUNC_NOT_IMPLEMENTED */
> > +                ifctx3 = aml_if(
> > +                    aml_equal(
> > +                        aml_and(op_flags, func_mask, NULL),
> > +                        not_implemented));
> > +                {
> > +                    /* 1: Operation Value of the Request Not Supported */
> > +                    aml_append(ifctx3, aml_return(one));
> > +                }
> > +                aml_append(ifctx2, ifctx3);
> > +
> > +                aml_append(ifctx2, aml_store(op, pprq));
> > +                aml_append(ifctx2, aml_store(zero, pprm));
> > +                /* 0: success */
> > +                aml_append(ifctx2, aml_return(zero));
> > +            }
> > +            aml_append(ifctx, ifctx2);
> > +
> > +            /*
> > +             * PPI 1.0: 2.1.4 Get Pending TPM Operation Requested By the OS
> > +             *
> > +             * Arg 2 (Integer): Function Index = 3
> > +             * Arg 3 (Package): Arguments = Empty Package
> > +             * Returns: Type: Package of Integers
> > +             *          Integer 1: Function Return code
> > +             *                     0: Success
> > +             *                     1: General Failure
> > +             *          Integer 2: Pending operation requested by the OS
> > +             *                     0: None
> > +             *                    >0: Operation Value of the Pending Request
> > +             *          Integer 3: Optional argument to pending operation
> > +             *                     requested by the OS
> > +             *                     0: None
> > +             *                    >0: Argument Value of the Pending Request
> > +             */
> > +            ifctx2 = aml_if(aml_equal(function, aml_int(3)));
> > +            {
> > +                /*
> > +                 * Revision ID of 1, no integer parameter beyond
> > +                 * parameter two are expected
> > +                 */
> > +                ifctx3 = aml_if(aml_equal(rev, one));
> > +                {
> > +                    /* TPM2[1] = PPRQ */
> > +                    aml_append(ifctx3,
> > +                               aml_store(pprq, aml_index(tpm2, one)));
> > +                    aml_append(ifctx3, aml_return(tpm2));
> > +                }
> > +                aml_append(ifctx2, ifctx3);
> > +
> > +                /*
> > +                 * A return value of {0, 23, 1} indicates that
> > +                 * operation 23 with argument 1 is pending.
> > +                 */
> > +                ifctx3 = aml_if(aml_equal(rev, aml_int(2)));
> > +                {
> > +                    /* TPM3[1] = PPRQ */
> > +                    aml_append(ifctx3,
> > +                               aml_store(pprq, aml_index(tpm3, one)));
> > +                    /* TPM3[2] = PPRM */
> > +                    aml_append(ifctx3,
> > +                               aml_store(pprm, aml_index(tpm3, aml_int(2))));
> > +                    aml_append(ifctx3, aml_return(tpm3));
> > +                }
> > +                aml_append(ifctx2, ifctx3);
> > +            }
> > +            aml_append(ifctx, ifctx2);
> > +
> > +            /*
> > +             * PPI 1.0: 2.1.5 Get Platform-Specific Action to Transition to
> > +             *     Pre-OS Environment
> > +             *
> > +             * Arg 2 (Integer): Function Index = 4
> > +             * Arg 3 (Package): Arguments = Empty Package
> > +             * Returns: Type: Integer
> > +             *          0: None
> > +             *          1: Shutdown
> > +             *          2: Reboot
> > +             *          3: OS Vendor-specific
> > +             */
> > +            ifctx2 = aml_if(aml_equal(function, aml_int(4)));
> > +            {
> > +                /* reboot */
> > +                aml_append(ifctx2, aml_return(aml_int(2)));
> > +            }
> > +            aml_append(ifctx, ifctx2);
> > +
> > +            /*
> > +             * PPI 1.0: 2.1.6 Return TPM Operation Response to OS Environment
> > +             *
> > +             * Arg 2 (Integer): Function Index = 5
> > +             * Arg 3 (Package): Arguments = Empty Package
> > +             * Returns: Type: Package of Integer
> > +             *          Integer 1: Function Return code
> > +             *                     0: Success
> > +             *                     1: General Failure
> > +             *          Integer 2: Most recent operation request
> > +             *                     0: None
> > +             *                    >0: Operation Value of the most recent request
> > +             *          Integer 3: Response to the most recent operation request
> > +             *                     0: Success
> > +             *                     0x00000001..0x00000FFF: Corresponding TPM
> > +             *                                             error code
> > +             *                     0xFFFFFFF0: User Abort or timeout of dialog
> > +             *                     0xFFFFFFF1: firmware Failure
> > +             */
> > +            ifctx2 = aml_if(aml_equal(function, aml_int(5)));
> > +            {
> > +                /* TPM3[1] = LPPR */
> > +                aml_append(ifctx2,
> > +                           aml_store(aml_name("LPPR"),
> > +                                     aml_index(tpm3, one)));
> > +                /* TPM3[2] = PPRP */
> > +                aml_append(ifctx2,
> > +                           aml_store(aml_name("PPRP"),
> > +                                     aml_index(tpm3, aml_int(2))));
> > +                aml_append(ifctx2, aml_return(tpm3));
> > +            }
> > +            aml_append(ifctx, ifctx2);
> > +
> > +            /*
> > +             * PPI 1.0: 2.1.7 Submit preferred user language
> > +             *
> > +             * Arg 2 (Integer): Function Index = 6
> > +             * Arg 3 (Package): Arguments = String Package
> > +             *                  Preferred language code
> > +             * Returns: Type: Integer
> > +             * Function Return Code
> > +             *          3: Not implemented
> > +             */
> > +            ifctx2 = aml_if(aml_equal(function, aml_int(6)));
> > +            {
> > +                /* 3 = not implemented */
> > +                aml_append(ifctx2, aml_return(aml_int(3)));
> > +            }
> > +            aml_append(ifctx, ifctx2);
> > +
> > +            /*
> > +             * PPI 1.1: 2.1.7 Submit TPM Operation Request to
> > +             *     Pre-OS Environment 2
> > +             *
> > +             * Arg 2 (Integer): Function Index = 7
> > +             * Arg 3 (Package): Arguments = Package: Type: Integer
> > +             *                  Integer 1: Operation Value of the Request
> > +             *                  Integer 2: Argument for Operation (optional)
> > +             * Returns: Type: Integer
> > +             *          0: Success
> > +             *          1: Not Implemented
> > +             *          2: General Failure
> > +             *          3: Operation blocked by current firmware settings
> > +             */
> > +            ifctx2 = aml_if(aml_equal(function, aml_int(7)));
> > +            {
> > +                /* get opcode */
> > +                aml_append(ifctx2, aml_store(aml_derefof(aml_index(arguments,
> > +                                                                   zero)),
> > +                                             op));
> > +
> > +                /* get opcode flags */
> > +                aml_append(ifctx2, aml_store(aml_call1("TPFN", op),
> > +                                             op_flags));
> > +                /* if func[opcode] & TPM_PPI_FUNC_NOT_IMPLEMENTED */
> > +                ifctx3 = aml_if(
> > +                    aml_equal(
> > +                        aml_and(op_flags, func_mask, NULL),
> > +                        not_implemented));
> > +                {
> > +                    /* 1: not implemented */
> > +                    aml_append(ifctx3, aml_return(one));
> > +                }
> > +                aml_append(ifctx2, ifctx3);
> > +
> > +                /* if func[opcode] & TPM_PPI_FUNC_BLOCKED */
> > +                ifctx3 = aml_if(
> > +                    aml_equal(
> > +                        aml_and(op_flags, func_mask, NULL),
> > +                        aml_int(TPM_PPI_FUNC_BLOCKED)));
> > +                {
> > +                    /* 3: blocked by firmware */
> > +                    aml_append(ifctx3, aml_return(aml_int(3)));
> > +                }
> > +                aml_append(ifctx2, ifctx3);
> > +
> > +                /* revision to integer */
> > +                ifctx3 = aml_if(aml_equal(rev, one));
> > +                {
> > +                    /* revision 1 */
> > +                    /* PPRQ = op */
> > +                    aml_append(ifctx3, aml_store(op, pprq));
> > +                    /* no argument, PPRM = 0 */
> > +                    aml_append(ifctx3, aml_store(zero, pprm));
> > +                }
> > +                aml_append(ifctx2, ifctx3);
> > +
> > +                ifctx3 = aml_if(aml_equal(rev, aml_int(2)));
> > +                {
> > +                    /* revision 2 */
> > +                    /* PPRQ = op */
> > +                    op_arg = aml_derefof(aml_index(arguments, one));
> > +                    aml_append(ifctx3, aml_store(op, pprq));
> > +                    /* PPRM = arg3[1] */
> > +                    aml_append(ifctx3, aml_store(op_arg, pprm));
> > +                }
> > +                aml_append(ifctx2, ifctx3);
> > +                /* 0: success */
> > +                aml_append(ifctx2, aml_return(zero));
> > +            }
> > +            aml_append(ifctx, ifctx2);
> > +
> > +            /*
> > +             * PPI 1.1: 2.1.8 Get User Confirmation Status for Operation
> > +             *
> > +             * Arg 2 (Integer): Function Index = 8
> > +             * Arg 3 (Package): Arguments = Package: Type: Integer
> > +             *                  Operation Value that may need user confirmation
> > +             * Returns: Type: Integer
> > +             *          0: Not implemented
> > +             *          1: Firmware only
> > +             *          2: Blocked for OS by firmware configuration
> > +             *          3: Allowed and physically present user required
> > +             *          4: Allowed and physically present user not required
> > +             */
> > +            ifctx2 = aml_if(aml_equal(function, aml_int(8)));
> > +            {
> > +                /* get opcode */
> > +                aml_append(ifctx2,
> > +                           aml_store(aml_derefof(aml_index(arguments,
> > +                                                           zero)),
> > +                                     op));
> > +
> > +                /* get opcode flags */
> > +                aml_append(ifctx2, aml_store(aml_call1("TPFN", op),
> > +                                             op_flags));
> > +                /* return confirmation status code */
> > +                aml_append(ifctx2,
> > +                           aml_return(
> > +                               aml_and(op_flags, func_mask, NULL)));
> > +            }
> > +            aml_append(ifctx, ifctx2);
> > +
> > +            aml_append(ifctx, aml_return(aml_buffer(1, zerobyte)));
> > +        }
> > +        aml_append(method, ifctx);
> > +    }
> > +    aml_append(dev, method);
> > +}
> > +
> >  static void
> >  build_dsdt(GArray *table_data, BIOSLinker *linker,
> >             AcpiPmInfo *pm, AcpiMiscInfo *misc,
> > @@ -1802,6 +2183,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> >      uint32_t nr_mem = machine->ram_slots;
> >      int root_bus_limit = 0xFF;
> >      PCIBus *bus = NULL;
> > +    TPMIf *tpm = tpm_find();
> >      int i;
> >
> >      dsdt = init_aml_allocator();
> > @@ -2139,7 +2521,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> >              /* Scan all PCI buses. Generate tables to support hotplug. */
> >              build_append_pci_bus_devices(scope, bus, pm->pcihp_bridge_en);
> >
> > -            if (TPM_IS_TIS(tpm_find())) {
> > +            if (TPM_IS_TIS(tpm)) {
> >                  dev = aml_device("ISA.TPM");
> >                  aml_append(dev, aml_name_decl("_HID", aml_eisaid("PNP0C31")));
> >                  aml_append(dev, aml_name_decl("_STA", aml_int(0xF)));
> > @@ -2153,6 +2535,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> >                   */
> >                  /* aml_append(crs, aml_irq_no_flags(TPM_TIS_IRQ)); */
> >                  aml_append(dev, aml_name_decl("_CRS", crs));
> > +
> > +                build_tpm_ppi(tpm, dev);
> > +
> >                  aml_append(scope, dev);
> >              }
> >
> > @@ -2160,7 +2545,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> >          }
> >      }
> >
> > -    if (TPM_IS_CRB(tpm_find())) {
> > +    if (TPM_IS_CRB(tpm)) {
> >          dev = aml_device("TPM");
> >          aml_append(dev, aml_name_decl("_HID", aml_string("MSFT0101")));
> >          crs = aml_resource_template();
> > @@ -2172,6 +2557,8 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> >          aml_append(method, aml_return(aml_int(0x0f)));
> >          aml_append(dev, method);
> >
> > +        build_tpm_ppi(tpm, dev);
> > +
> >          aml_append(sb_scope, dev);
> >      }
> >
> > @@ -2924,7 +3311,7 @@ void acpi_setup(void)
> >          tpm_config = (FWCfgTPMConfig) {
> >              .tpmppi_address = cpu_to_le32(TPM_PPI_ADDR_BASE),
> >              .tpm_version = tpm_get_version(tpm_find()),
> > -            .tpmppi_version = TPM_PPI_VERSION_NONE
> > +            .tpmppi_version = TPM_PPI_VERSION_1_30
> >          };
> >          fw_cfg_add_file(pcms->fw_cfg, "etc/tpm/config",
> >                          &tpm_config, sizeof tpm_config);
> > diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
> > index a5bdd5f26e..332c2ae597 100644
> > --- a/docs/specs/tpm.txt
> > +++ b/docs/specs/tpm.txt
> > @@ -62,6 +62,89 @@ 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.                                                   |
> > + +----------+-------------------------------------------------------------+
> > +
> > +The location of the table is given by the fw_cfg tpmppi_address field.
> > +The PPI memory region size is 0x400 (TPM_PPI_ADDR_SIZE) to leave
> > +enough room for future updates.
> > +
> >
> >  QEMU files related to TPM ACPI tables:
> >   - hw/i386/acpi-build.c
> >
>


-- 
Marc-André Lureau

[Qemu-devel] [PATCH v12 6/6] tpm: add ACPI memory clear interface

[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". According
to Laszlo, it's not so easy to implement in OVMF, he suggested to do
it in qemu instead.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/tpm/tpm_ppi.h     |  2 ++
 hw/i386/acpi-build.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
 hw/tpm/tpm_crb.c     |  1 +
 hw/tpm/tpm_ppi.c     | 22 +++++++++++++++++++++
 hw/tpm/tpm_tis.c     |  1 +
 docs/specs/tpm.txt   |  2 ++
 hw/tpm/trace-events  |  3 +++
 7 files changed, 77 insertions(+)

diff --git a/hw/tpm/tpm_ppi.h b/hw/tpm/tpm_ppi.h
index c2ab2ed300..b8f67962c7 100644
--- a/hw/tpm/tpm_ppi.h
+++ b/hw/tpm/tpm_ppi.h
@@ -23,4 +23,6 @@ typedef struct TPMPPI {
 bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
                   hwaddr addr, Object *obj, Error **errp);
 
+void tpm_ppi_reset(TPMPPI *tpmppi);
+
 #endif /* TPM_TPM_PPI_H */
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index c5e9a6e11d..2ab3e8fae7 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1824,6 +1824,13 @@ build_tpm_ppi(TPMIf *tpm, Aml *dev)
     pprq = aml_name("PPRQ");
     pprm = aml_name("PPRM");
 
+    aml_append(dev,
+               aml_operation_region("TPP3", AML_SYSTEM_MEMORY,
+                                    aml_int(TPM_PPI_ADDR_BASE + 0x15a),
+                                    0x1));
+    field = aml_field("TPP3", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
+    aml_append(field, aml_named_field("MOVV", 8));
+    aml_append(dev, field);
     /*
      * DerefOf in Windows is broken with SYSTEM_MEMORY.  Use a dynamic
      * operation region inside of a method for getting FUNC[op].
@@ -2166,7 +2173,46 @@ build_tpm_ppi(TPMIf *tpm, Aml *dev)
             aml_append(ifctx, aml_return(aml_buffer(1, zerobyte)));
         }
         aml_append(method, ifctx);
+
+        ifctx = aml_if(
+            aml_equal(uuid,
+                      aml_touuid("376054ED-CC13-4675-901C-4756D7F2D45D")));
+        {
+            /* standard DSM query function */
+            ifctx2 = aml_if(aml_equal(function, zero));
+            {
+                uint8_t byte_list[1] = { 0x03 };
+                aml_append(ifctx2, aml_return(aml_buffer(1, byte_list)));
+            }
+            aml_append(ifctx, ifctx2);
+
+            /*
+             * TCG Platform Reset Attack Mitigation Specification 1.0 Ch.6
+             *
+             * Arg 2 (Integer): Function Index = 1
+             * Arg 3 (Package): Arguments = Package: Type: Integer
+             *                  Operation Value of the Request
+             * Returns: Type: Integer
+             *          0: Success
+             *          1: General Failure
+             */
+            ifctx2 = aml_if(aml_equal(function, one));
+            {
+                aml_append(ifctx2,
+                           aml_store(aml_derefof(aml_index(arguments, zero)),
+                                     op));
+                {
+                    aml_append(ifctx2, aml_store(op, aml_name("MOVV")));
+
+                    /* 0: success */
+                    aml_append(ifctx2, aml_return(zero));
+                }
+            }
+            aml_append(ifctx, ifctx2);
+        }
+        aml_append(method, ifctx);
     }
+
     aml_append(dev, method);
 }
 
diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
index b243222fd6..48f6a716ad 100644
--- a/hw/tpm/tpm_crb.c
+++ b/hw/tpm/tpm_crb.c
@@ -233,6 +233,7 @@ static void tpm_crb_reset(void *dev)
 {
     CRBState *s = CRB(dev);
 
+    tpm_ppi_reset(&s->ppi);
     tpm_backend_reset(s->tpmbe);
 
     memset(s->regs, 0, sizeof(s->regs));
diff --git a/hw/tpm/tpm_ppi.c b/hw/tpm/tpm_ppi.c
index f2f07f895e..ac05ba8d3c 100644
--- a/hw/tpm/tpm_ppi.c
+++ b/hw/tpm/tpm_ppi.c
@@ -16,8 +16,30 @@
 #include "qapi/error.h"
 #include "cpu.h"
 #include "sysemu/memory_mapping.h"
+#include "sysemu/reset.h"
 #include "migration/vmstate.h"
 #include "tpm_ppi.h"
+#include "trace.h"
+
+void tpm_ppi_reset(TPMPPI *tpmppi)
+{
+    if (tpmppi->buf[0x15a] & 0x1) {
+        GuestPhysBlockList guest_phys_blocks;
+        GuestPhysBlock *block;
+
+        guest_phys_blocks_init(&guest_phys_blocks);
+        guest_phys_blocks_append(&guest_phys_blocks);
+        QTAILQ_FOREACH(block, &guest_phys_blocks.head, next) {
+            trace_tpm_ppi_memset(block->host_addr,
+                             block->target_end - block->target_start);
+            memset(block->host_addr, 0,
+                   block->target_end - block->target_start);
+            memory_region_set_dirty(block->mr, 0,
+                                    block->target_end - block->target_start);
+        }
+        guest_phys_blocks_free(&guest_phys_blocks);
+    }
+}
 
 bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
                   hwaddr addr, Object *obj, Error **errp)
diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index 70432ffe8b..d9bfa956cc 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -868,6 +868,7 @@ static void tpm_tis_reset(DeviceState *dev)
     s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->be_driver),
                             TPM_TIS_BUFFER_MAX);
 
+    tpm_ppi_reset(&s->ppi);
     tpm_backend_reset(s->be_driver);
 
     s->active_locty = TPM_TIS_NO_LOCALITY;
diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
index 332c2ae597..ce9bda3c89 100644
--- a/docs/specs/tpm.txt
+++ b/docs/specs/tpm.txt
@@ -121,6 +121,8 @@ layout:
  +----------+--------+--------+-------------------------------------------+
  | next_step|   0x1  |  0x159 | Operation to execute after reboot by      |
  |          |        |        | firmware. Used by firmware.               |
+ +----------+--------+--------+-------------------------------------------+
+ | movv     |   0x1  |  0x15a | Memory overwrite variable                 |
  +----------+--------+--------+-------------------------------------------+
 
    The following values are supported for the 'func' field. They correspond
diff --git a/hw/tpm/trace-events b/hw/tpm/trace-events
index 25bee0cecf..920d32ad55 100644
--- a/hw/tpm/trace-events
+++ b/hw/tpm/trace-events
@@ -51,3 +51,6 @@ tpm_tis_mmio_write_init_abort(void) "Initiating abort"
 tpm_tis_mmio_write_lowering_irq(void) "Lowering IRQ"
 tpm_tis_mmio_write_data2send(uint32_t value, unsigned size) "Data to send to TPM: 0x%08x (size=%d)"
 tpm_tis_pre_save(uint8_t locty, uint32_t rw_offset) "locty: %d, rw_offset = %u"
+
+# hw/tpm/tpm_ppi.c
+tpm_ppi_memset(uint8_t *ptr, size_t size) "memset: %p %zu"
-- 
2.19.0.rc1

Re: [Qemu-devel] [PATCH v12 6/6] tpm: add ACPI memory clear interface

[Philippe Mathieu-Daudé]

On 9/10/18 10:32 AM, Marc-André Lureau wrote:
> 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". According
> to Laszlo, it's not so easy to implement in OVMF, he suggested to do
> it in qemu instead.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  hw/tpm/tpm_ppi.h     |  2 ++
>  hw/i386/acpi-build.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
>  hw/tpm/tpm_crb.c     |  1 +
>  hw/tpm/tpm_ppi.c     | 22 +++++++++++++++++++++
>  hw/tpm/tpm_tis.c     |  1 +
>  docs/specs/tpm.txt   |  2 ++
>  hw/tpm/trace-events  |  3 +++
>  7 files changed, 77 insertions(+)
> 
> diff --git a/hw/tpm/tpm_ppi.h b/hw/tpm/tpm_ppi.h
> index c2ab2ed300..b8f67962c7 100644
> --- a/hw/tpm/tpm_ppi.h
> +++ b/hw/tpm/tpm_ppi.h
> @@ -23,4 +23,6 @@ typedef struct TPMPPI {
>  bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
>                    hwaddr addr, Object *obj, Error **errp);
>  

Can you add documentation?

> +void tpm_ppi_reset(TPMPPI *tpmppi);
> +
>  #endif /* TPM_TPM_PPI_H */
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index c5e9a6e11d..2ab3e8fae7 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1824,6 +1824,13 @@ build_tpm_ppi(TPMIf *tpm, Aml *dev)
>      pprq = aml_name("PPRQ");
>      pprm = aml_name("PPRM");
>  
> +    aml_append(dev,
> +               aml_operation_region("TPP3", AML_SYSTEM_MEMORY,
> +                                    aml_int(TPM_PPI_ADDR_BASE + 0x15a),

Can you add a definition for this magic value?
Maybe:

#define TPM_PPI_MEMORY_OVERWRITE_OFFSET 0x15a

or

#define TPM_PPI_MOVV_OFFSET 0x15a

> +                                    0x1));
> +    field = aml_field("TPP3", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
> +    aml_append(field, aml_named_field("MOVV", 8));
> +    aml_append(dev, field);
>      /*
>       * DerefOf in Windows is broken with SYSTEM_MEMORY.  Use a dynamic
>       * operation region inside of a method for getting FUNC[op].
> @@ -2166,7 +2173,46 @@ build_tpm_ppi(TPMIf *tpm, Aml *dev)
>              aml_append(ifctx, aml_return(aml_buffer(1, zerobyte)));
>          }
>          aml_append(method, ifctx);
> +
> +        ifctx = aml_if(
> +            aml_equal(uuid,
> +                      aml_touuid("376054ED-CC13-4675-901C-4756D7F2D45D")));
> +        {
> +            /* standard DSM query function */
> +            ifctx2 = aml_if(aml_equal(function, zero));
> +            {
> +                uint8_t byte_list[1] = { 0x03 };
> +                aml_append(ifctx2, aml_return(aml_buffer(1, byte_list)));
> +            }
> +            aml_append(ifctx, ifctx2);
> +
> +            /*
> +             * TCG Platform Reset Attack Mitigation Specification 1.0 Ch.6
> +             *
> +             * Arg 2 (Integer): Function Index = 1
> +             * Arg 3 (Package): Arguments = Package: Type: Integer
> +             *                  Operation Value of the Request
> +             * Returns: Type: Integer
> +             *          0: Success
> +             *          1: General Failure
> +             */
> +            ifctx2 = aml_if(aml_equal(function, one));
> +            {
> +                aml_append(ifctx2,
> +                           aml_store(aml_derefof(aml_index(arguments, zero)),
> +                                     op));
> +                {
> +                    aml_append(ifctx2, aml_store(op, aml_name("MOVV")));
> +
> +                    /* 0: success */
> +                    aml_append(ifctx2, aml_return(zero));
> +                }
> +            }
> +            aml_append(ifctx, ifctx2);
> +        }
> +        aml_append(method, ifctx);
>      }
> +
>      aml_append(dev, method);
>  }
>  
> diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> index b243222fd6..48f6a716ad 100644
> --- a/hw/tpm/tpm_crb.c
> +++ b/hw/tpm/tpm_crb.c
> @@ -233,6 +233,7 @@ static void tpm_crb_reset(void *dev)
>  {
>      CRBState *s = CRB(dev);
>  
> +    tpm_ppi_reset(&s->ppi);
>      tpm_backend_reset(s->tpmbe);
>  
>      memset(s->regs, 0, sizeof(s->regs));
> diff --git a/hw/tpm/tpm_ppi.c b/hw/tpm/tpm_ppi.c
> index f2f07f895e..ac05ba8d3c 100644
> --- a/hw/tpm/tpm_ppi.c
> +++ b/hw/tpm/tpm_ppi.c
> @@ -16,8 +16,30 @@
>  #include "qapi/error.h"
>  #include "cpu.h"
>  #include "sysemu/memory_mapping.h"
> +#include "sysemu/reset.h"
>  #include "migration/vmstate.h"
>  #include "tpm_ppi.h"
> +#include "trace.h"
> +
> +void tpm_ppi_reset(TPMPPI *tpmppi)
> +{
> +    if (tpmppi->buf[0x15a] & 0x1) {

TPM_PPI_MOVV_OFFSET

> +        GuestPhysBlockList guest_phys_blocks;
> +        GuestPhysBlock *block;
> +
> +        guest_phys_blocks_init(&guest_phys_blocks);
> +        guest_phys_blocks_append(&guest_phys_blocks);
> +        QTAILQ_FOREACH(block, &guest_phys_blocks.head, next) {
> +            trace_tpm_ppi_memset(block->host_addr,
> +                             block->target_end - block->target_start);
> +            memset(block->host_addr, 0,
> +                   block->target_end - block->target_start);
> +            memory_region_set_dirty(block->mr, 0,
> +                                    block->target_end - block->target_start);
> +        }
> +        guest_phys_blocks_free(&guest_phys_blocks);
> +    }
> +}
>  
>  bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
>                    hwaddr addr, Object *obj, Error **errp)
> diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
> index 70432ffe8b..d9bfa956cc 100644
> --- a/hw/tpm/tpm_tis.c
> +++ b/hw/tpm/tpm_tis.c
> @@ -868,6 +868,7 @@ static void tpm_tis_reset(DeviceState *dev)
>      s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->be_driver),
>                              TPM_TIS_BUFFER_MAX);
>  
> +    tpm_ppi_reset(&s->ppi);
>      tpm_backend_reset(s->be_driver);
>  
>      s->active_locty = TPM_TIS_NO_LOCALITY;
> diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
> index 332c2ae597..ce9bda3c89 100644
> --- a/docs/specs/tpm.txt
> +++ b/docs/specs/tpm.txt
> @@ -121,6 +121,8 @@ layout:
>   +----------+--------+--------+-------------------------------------------+
>   | next_step|   0x1  |  0x159 | Operation to execute after reboot by      |
>   |          |        |        | firmware. Used by firmware.               |
> + +----------+--------+--------+-------------------------------------------+
> + | movv     |   0x1  |  0x15a | Memory overwrite variable                 |
>   +----------+--------+--------+-------------------------------------------+
>  
>     The following values are supported for the 'func' field. They correspond
> diff --git a/hw/tpm/trace-events b/hw/tpm/trace-events
> index 25bee0cecf..920d32ad55 100644
> --- a/hw/tpm/trace-events
> +++ b/hw/tpm/trace-events
> @@ -51,3 +51,6 @@ tpm_tis_mmio_write_init_abort(void) "Initiating abort"
>  tpm_tis_mmio_write_lowering_irq(void) "Lowering IRQ"
>  tpm_tis_mmio_write_data2send(uint32_t value, unsigned size) "Data to send to TPM: 0x%08x (size=%d)"
>  tpm_tis_pre_save(uint8_t locty, uint32_t rw_offset) "locty: %d, rw_offset = %u"
> +
> +# hw/tpm/tpm_ppi.c
> +tpm_ppi_memset(uint8_t *ptr, size_t size) "memset: %p %zu"
> 

I'd split this patch in 2: Add ACPI, Add tpm_ppi_reset.

Re: [Qemu-devel] [PATCH v12 6/6] tpm: add ACPI memory clear interface

[Igor Mammedov]

On Wed, 12 Dec 2018 17:22:21 +0100
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> On 9/10/18 10:32 AM, Marc-André Lureau wrote:
> > 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". According
> > to Laszlo, it's not so easy to implement in OVMF, he suggested to do
> > it in qemu instead.
> > 
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  hw/tpm/tpm_ppi.h     |  2 ++
> >  hw/i386/acpi-build.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
> >  hw/tpm/tpm_crb.c     |  1 +
> >  hw/tpm/tpm_ppi.c     | 22 +++++++++++++++++++++
> >  hw/tpm/tpm_tis.c     |  1 +
> >  docs/specs/tpm.txt   |  2 ++
> >  hw/tpm/trace-events  |  3 +++
> >  7 files changed, 77 insertions(+)
> > 
> > diff --git a/hw/tpm/tpm_ppi.h b/hw/tpm/tpm_ppi.h
> > index c2ab2ed300..b8f67962c7 100644
> > --- a/hw/tpm/tpm_ppi.h
> > +++ b/hw/tpm/tpm_ppi.h
> > @@ -23,4 +23,6 @@ typedef struct TPMPPI {
> >  bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
> >                    hwaddr addr, Object *obj, Error **errp);
> >    
> 
> Can you add documentation?
> 
> > +void tpm_ppi_reset(TPMPPI *tpmppi);
> > +
> >  #endif /* TPM_TPM_PPI_H */
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index c5e9a6e11d..2ab3e8fae7 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -1824,6 +1824,13 @@ build_tpm_ppi(TPMIf *tpm, Aml *dev)
> >      pprq = aml_name("PPRQ");
> >      pprm = aml_name("PPRM");
> >  
> > +    aml_append(dev,
> > +               aml_operation_region("TPP3", AML_SYSTEM_MEMORY,
> > +                                    aml_int(TPM_PPI_ADDR_BASE + 0x15a),  
> 
> Can you add a definition for this magic value?
> Maybe:
> 
> #define TPM_PPI_MEMORY_OVERWRITE_OFFSET 0x15a
> 
> or
> 
> #define TPM_PPI_MOVV_OFFSET 0x15a
I'd prefer explicit numeric values there as it's a bit easier to review
and grep in 'spec' since one doesn't have to jump to define definition
first to find out value to compare with spec.
What I'd add though is verbatim comment from spec like we do
in APCI code so we could jump to the field of interest in spec
just doing copy&paste&search

> 
> > +                                    0x1));
> > +    field = aml_field("TPP3", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
> > +    aml_append(field, aml_named_field("MOVV", 8));
> > +    aml_append(dev, field);
> >      /*
> >       * DerefOf in Windows is broken with SYSTEM_MEMORY.  Use a dynamic
> >       * operation region inside of a method for getting FUNC[op].
> > @@ -2166,7 +2173,46 @@ build_tpm_ppi(TPMIf *tpm, Aml *dev)
> >              aml_append(ifctx, aml_return(aml_buffer(1, zerobyte)));
> >          }
> >          aml_append(method, ifctx);
> > +
> > +        ifctx = aml_if(
> > +            aml_equal(uuid,
> > +                      aml_touuid("376054ED-CC13-4675-901C-4756D7F2D45D")));
> > +        {
> > +            /* standard DSM query function */
> > +            ifctx2 = aml_if(aml_equal(function, zero));
> > +            {
> > +                uint8_t byte_list[1] = { 0x03 };
> > +                aml_append(ifctx2, aml_return(aml_buffer(1, byte_list)));
> > +            }
> > +            aml_append(ifctx, ifctx2);
> > +
> > +            /*
> > +             * TCG Platform Reset Attack Mitigation Specification 1.0 Ch.6
> > +             *
> > +             * Arg 2 (Integer): Function Index = 1
> > +             * Arg 3 (Package): Arguments = Package: Type: Integer
> > +             *                  Operation Value of the Request
> > +             * Returns: Type: Integer
> > +             *          0: Success
> > +             *          1: General Failure
> > +             */
> > +            ifctx2 = aml_if(aml_equal(function, one));
> > +            {
> > +                aml_append(ifctx2,
> > +                           aml_store(aml_derefof(aml_index(arguments, zero)),
> > +                                     op));
> > +                {
> > +                    aml_append(ifctx2, aml_store(op, aml_name("MOVV")));
> > +
> > +                    /* 0: success */
> > +                    aml_append(ifctx2, aml_return(zero));
> > +                }
> > +            }
> > +            aml_append(ifctx, ifctx2);
> > +        }
> > +        aml_append(method, ifctx);
> >      }
> > +
> >      aml_append(dev, method);
> >  }
> >  
> > diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> > index b243222fd6..48f6a716ad 100644
> > --- a/hw/tpm/tpm_crb.c
> > +++ b/hw/tpm/tpm_crb.c
> > @@ -233,6 +233,7 @@ static void tpm_crb_reset(void *dev)
> >  {
> >      CRBState *s = CRB(dev);
> >  
> > +    tpm_ppi_reset(&s->ppi);
> >      tpm_backend_reset(s->tpmbe);
> >  
> >      memset(s->regs, 0, sizeof(s->regs));
> > diff --git a/hw/tpm/tpm_ppi.c b/hw/tpm/tpm_ppi.c
> > index f2f07f895e..ac05ba8d3c 100644
> > --- a/hw/tpm/tpm_ppi.c
> > +++ b/hw/tpm/tpm_ppi.c
> > @@ -16,8 +16,30 @@
> >  #include "qapi/error.h"
> >  #include "cpu.h"
> >  #include "sysemu/memory_mapping.h"
> > +#include "sysemu/reset.h"
> >  #include "migration/vmstate.h"
> >  #include "tpm_ppi.h"
> > +#include "trace.h"
> > +
> > +void tpm_ppi_reset(TPMPPI *tpmppi)
> > +{
> > +    if (tpmppi->buf[0x15a] & 0x1) {  
> 
> TPM_PPI_MOVV_OFFSET
> 
> > +        GuestPhysBlockList guest_phys_blocks;
> > +        GuestPhysBlock *block;
> > +
> > +        guest_phys_blocks_init(&guest_phys_blocks);
> > +        guest_phys_blocks_append(&guest_phys_blocks);
> > +        QTAILQ_FOREACH(block, &guest_phys_blocks.head, next) {
> > +            trace_tpm_ppi_memset(block->host_addr,
> > +                             block->target_end - block->target_start);
> > +            memset(block->host_addr, 0,
> > +                   block->target_end - block->target_start);
> > +            memory_region_set_dirty(block->mr, 0,
> > +                                    block->target_end - block->target_start);
> > +        }
> > +        guest_phys_blocks_free(&guest_phys_blocks);
> > +    }
> > +}
> >  
> >  bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
> >                    hwaddr addr, Object *obj, Error **errp)
> > diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
> > index 70432ffe8b..d9bfa956cc 100644
> > --- a/hw/tpm/tpm_tis.c
> > +++ b/hw/tpm/tpm_tis.c
> > @@ -868,6 +868,7 @@ static void tpm_tis_reset(DeviceState *dev)
> >      s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->be_driver),
> >                              TPM_TIS_BUFFER_MAX);
> >  
> > +    tpm_ppi_reset(&s->ppi);
> >      tpm_backend_reset(s->be_driver);
> >  
> >      s->active_locty = TPM_TIS_NO_LOCALITY;
> > diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
> > index 332c2ae597..ce9bda3c89 100644
> > --- a/docs/specs/tpm.txt
> > +++ b/docs/specs/tpm.txt
> > @@ -121,6 +121,8 @@ layout:
> >   +----------+--------+--------+-------------------------------------------+
> >   | next_step|   0x1  |  0x159 | Operation to execute after reboot by      |
> >   |          |        |        | firmware. Used by firmware.               |
> > + +----------+--------+--------+-------------------------------------------+
> > + | movv     |   0x1  |  0x15a | Memory overwrite variable                 |
> >   +----------+--------+--------+-------------------------------------------+
> >  
> >     The following values are supported for the 'func' field. They correspond
> > diff --git a/hw/tpm/trace-events b/hw/tpm/trace-events
> > index 25bee0cecf..920d32ad55 100644
> > --- a/hw/tpm/trace-events
> > +++ b/hw/tpm/trace-events
> > @@ -51,3 +51,6 @@ tpm_tis_mmio_write_init_abort(void) "Initiating abort"
> >  tpm_tis_mmio_write_lowering_irq(void) "Lowering IRQ"
> >  tpm_tis_mmio_write_data2send(uint32_t value, unsigned size) "Data to send to TPM: 0x%08x (size=%d)"
> >  tpm_tis_pre_save(uint8_t locty, uint32_t rw_offset) "locty: %d, rw_offset = %u"
> > +
> > +# hw/tpm/tpm_ppi.c
> > +tpm_ppi_memset(uint8_t *ptr, size_t size) "memset: %p %zu"
> >   
> 
> I'd split this patch in 2: Add ACPI, Add tpm_ppi_reset.

Re: [Qemu-devel] [PATCH v12 6/6] tpm: add ACPI memory clear interface

[Marc-André Lureau]

On Thu, Dec 13, 2018 at 4:01 PM Igor Mammedov <imammedo@redhat.com> wrote:
>
> On Wed, 12 Dec 2018 17:22:21 +0100
> Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> > On 9/10/18 10:32 AM, Marc-André Lureau wrote:
> > > 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". According
> > > to Laszlo, it's not so easy to implement in OVMF, he suggested to do
> > > it in qemu instead.
> > >
> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > ---
> > >  hw/tpm/tpm_ppi.h     |  2 ++
> > >  hw/i386/acpi-build.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
> > >  hw/tpm/tpm_crb.c     |  1 +
> > >  hw/tpm/tpm_ppi.c     | 22 +++++++++++++++++++++
> > >  hw/tpm/tpm_tis.c     |  1 +
> > >  docs/specs/tpm.txt   |  2 ++
> > >  hw/tpm/trace-events  |  3 +++
> > >  7 files changed, 77 insertions(+)
> > >
> > > diff --git a/hw/tpm/tpm_ppi.h b/hw/tpm/tpm_ppi.h
> > > index c2ab2ed300..b8f67962c7 100644
> > > --- a/hw/tpm/tpm_ppi.h
> > > +++ b/hw/tpm/tpm_ppi.h
> > > @@ -23,4 +23,6 @@ typedef struct TPMPPI {
> > >  bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
> > >                    hwaddr addr, Object *obj, Error **errp);
> > >
> >
> > Can you add documentation?
> >
> > > +void tpm_ppi_reset(TPMPPI *tpmppi);
> > > +
> > >  #endif /* TPM_TPM_PPI_H */
> > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > > index c5e9a6e11d..2ab3e8fae7 100644
> > > --- a/hw/i386/acpi-build.c
> > > +++ b/hw/i386/acpi-build.c
> > > @@ -1824,6 +1824,13 @@ build_tpm_ppi(TPMIf *tpm, Aml *dev)
> > >      pprq = aml_name("PPRQ");
> > >      pprm = aml_name("PPRM");
> > >
> > > +    aml_append(dev,
> > > +               aml_operation_region("TPP3", AML_SYSTEM_MEMORY,
> > > +                                    aml_int(TPM_PPI_ADDR_BASE + 0x15a),
> >
> > Can you add a definition for this magic value?
> > Maybe:
> >
> > #define TPM_PPI_MEMORY_OVERWRITE_OFFSET 0x15a
> >
> > or
> >
> > #define TPM_PPI_MOVV_OFFSET 0x15a
> I'd prefer explicit numeric values there as it's a bit easier to review
> and grep in 'spec' since one doesn't have to jump to define definition
> first to find out value to compare with spec.
> What I'd add though is verbatim comment from spec like we do
> in APCI code so we could jump to the field of interest in spec
> just doing copy&paste&search

I don't mind.
Philippe, are you okay going back to numeric values?

Igor, is this comment what you would expect where the 0x15a value is used:
/* 0x15a: movv field in ACPI PPI Interface, see specs/tpm.txt */

thanks
>
> >
> > > +                                    0x1));
> > > +    field = aml_field("TPP3", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
> > > +    aml_append(field, aml_named_field("MOVV", 8));
> > > +    aml_append(dev, field);
> > >      /*
> > >       * DerefOf in Windows is broken with SYSTEM_MEMORY.  Use a dynamic
> > >       * operation region inside of a method for getting FUNC[op].
> > > @@ -2166,7 +2173,46 @@ build_tpm_ppi(TPMIf *tpm, Aml *dev)
> > >              aml_append(ifctx, aml_return(aml_buffer(1, zerobyte)));
> > >          }
> > >          aml_append(method, ifctx);
> > > +
> > > +        ifctx = aml_if(
> > > +            aml_equal(uuid,
> > > +                      aml_touuid("376054ED-CC13-4675-901C-4756D7F2D45D")));
> > > +        {
> > > +            /* standard DSM query function */
> > > +            ifctx2 = aml_if(aml_equal(function, zero));
> > > +            {
> > > +                uint8_t byte_list[1] = { 0x03 };
> > > +                aml_append(ifctx2, aml_return(aml_buffer(1, byte_list)));
> > > +            }
> > > +            aml_append(ifctx, ifctx2);
> > > +
> > > +            /*
> > > +             * TCG Platform Reset Attack Mitigation Specification 1.0 Ch.6
> > > +             *
> > > +             * Arg 2 (Integer): Function Index = 1
> > > +             * Arg 3 (Package): Arguments = Package: Type: Integer
> > > +             *                  Operation Value of the Request
> > > +             * Returns: Type: Integer
> > > +             *          0: Success
> > > +             *          1: General Failure
> > > +             */
> > > +            ifctx2 = aml_if(aml_equal(function, one));
> > > +            {
> > > +                aml_append(ifctx2,
> > > +                           aml_store(aml_derefof(aml_index(arguments, zero)),
> > > +                                     op));
> > > +                {
> > > +                    aml_append(ifctx2, aml_store(op, aml_name("MOVV")));
> > > +
> > > +                    /* 0: success */
> > > +                    aml_append(ifctx2, aml_return(zero));
> > > +                }
> > > +            }
> > > +            aml_append(ifctx, ifctx2);
> > > +        }
> > > +        aml_append(method, ifctx);
> > >      }
> > > +
> > >      aml_append(dev, method);
> > >  }
> > >
> > > diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> > > index b243222fd6..48f6a716ad 100644
> > > --- a/hw/tpm/tpm_crb.c
> > > +++ b/hw/tpm/tpm_crb.c
> > > @@ -233,6 +233,7 @@ static void tpm_crb_reset(void *dev)
> > >  {
> > >      CRBState *s = CRB(dev);
> > >
> > > +    tpm_ppi_reset(&s->ppi);
> > >      tpm_backend_reset(s->tpmbe);
> > >
> > >      memset(s->regs, 0, sizeof(s->regs));
> > > diff --git a/hw/tpm/tpm_ppi.c b/hw/tpm/tpm_ppi.c
> > > index f2f07f895e..ac05ba8d3c 100644
> > > --- a/hw/tpm/tpm_ppi.c
> > > +++ b/hw/tpm/tpm_ppi.c
> > > @@ -16,8 +16,30 @@
> > >  #include "qapi/error.h"
> > >  #include "cpu.h"
> > >  #include "sysemu/memory_mapping.h"
> > > +#include "sysemu/reset.h"
> > >  #include "migration/vmstate.h"
> > >  #include "tpm_ppi.h"
> > > +#include "trace.h"
> > > +
> > > +void tpm_ppi_reset(TPMPPI *tpmppi)
> > > +{
> > > +    if (tpmppi->buf[0x15a] & 0x1) {
> >
> > TPM_PPI_MOVV_OFFSET
> >
> > > +        GuestPhysBlockList guest_phys_blocks;
> > > +        GuestPhysBlock *block;
> > > +
> > > +        guest_phys_blocks_init(&guest_phys_blocks);
> > > +        guest_phys_blocks_append(&guest_phys_blocks);
> > > +        QTAILQ_FOREACH(block, &guest_phys_blocks.head, next) {
> > > +            trace_tpm_ppi_memset(block->host_addr,
> > > +                             block->target_end - block->target_start);
> > > +            memset(block->host_addr, 0,
> > > +                   block->target_end - block->target_start);
> > > +            memory_region_set_dirty(block->mr, 0,
> > > +                                    block->target_end - block->target_start);
> > > +        }
> > > +        guest_phys_blocks_free(&guest_phys_blocks);
> > > +    }
> > > +}
> > >
> > >  bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
> > >                    hwaddr addr, Object *obj, Error **errp)
> > > diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
> > > index 70432ffe8b..d9bfa956cc 100644
> > > --- a/hw/tpm/tpm_tis.c
> > > +++ b/hw/tpm/tpm_tis.c
> > > @@ -868,6 +868,7 @@ static void tpm_tis_reset(DeviceState *dev)
> > >      s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->be_driver),
> > >                              TPM_TIS_BUFFER_MAX);
> > >
> > > +    tpm_ppi_reset(&s->ppi);
> > >      tpm_backend_reset(s->be_driver);
> > >
> > >      s->active_locty = TPM_TIS_NO_LOCALITY;
> > > diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
> > > index 332c2ae597..ce9bda3c89 100644
> > > --- a/docs/specs/tpm.txt
> > > +++ b/docs/specs/tpm.txt
> > > @@ -121,6 +121,8 @@ layout:
> > >   +----------+--------+--------+-------------------------------------------+
> > >   | next_step|   0x1  |  0x159 | Operation to execute after reboot by      |
> > >   |          |        |        | firmware. Used by firmware.               |
> > > + +----------+--------+--------+-------------------------------------------+
> > > + | movv     |   0x1  |  0x15a | Memory overwrite variable                 |
> > >   +----------+--------+--------+-------------------------------------------+
> > >
> > >     The following values are supported for the 'func' field. They correspond
> > > diff --git a/hw/tpm/trace-events b/hw/tpm/trace-events
> > > index 25bee0cecf..920d32ad55 100644
> > > --- a/hw/tpm/trace-events
> > > +++ b/hw/tpm/trace-events
> > > @@ -51,3 +51,6 @@ tpm_tis_mmio_write_init_abort(void) "Initiating abort"
> > >  tpm_tis_mmio_write_lowering_irq(void) "Lowering IRQ"
> > >  tpm_tis_mmio_write_data2send(uint32_t value, unsigned size) "Data to send to TPM: 0x%08x (size=%d)"
> > >  tpm_tis_pre_save(uint8_t locty, uint32_t rw_offset) "locty: %d, rw_offset = %u"
> > > +
> > > +# hw/tpm/tpm_ppi.c
> > > +tpm_ppi_memset(uint8_t *ptr, size_t size) "memset: %p %zu"
> > >
> >
> > I'd split this patch in 2: Add ACPI, Add tpm_ppi_reset.
>

Re: [Qemu-devel] [PATCH v12 6/6] tpm: add ACPI memory clear interface

[Philippe Mathieu-Daudé]

On 12/13/18 1:18 PM, Marc-André Lureau wrote:
> On Thu, Dec 13, 2018 at 4:01 PM Igor Mammedov <imammedo@redhat.com> wrote:
>>
>> On Wed, 12 Dec 2018 17:22:21 +0100
>> Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>
>>> On 9/10/18 10:32 AM, Marc-André Lureau wrote:
>>>> 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". According
>>>> to Laszlo, it's not so easy to implement in OVMF, he suggested to do
>>>> it in qemu instead.
>>>>
>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>> ---
>>>>  hw/tpm/tpm_ppi.h     |  2 ++
>>>>  hw/i386/acpi-build.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
>>>>  hw/tpm/tpm_crb.c     |  1 +
>>>>  hw/tpm/tpm_ppi.c     | 22 +++++++++++++++++++++
>>>>  hw/tpm/tpm_tis.c     |  1 +
>>>>  docs/specs/tpm.txt   |  2 ++
>>>>  hw/tpm/trace-events  |  3 +++
>>>>  7 files changed, 77 insertions(+)
>>>>
>>>> diff --git a/hw/tpm/tpm_ppi.h b/hw/tpm/tpm_ppi.h
>>>> index c2ab2ed300..b8f67962c7 100644
>>>> --- a/hw/tpm/tpm_ppi.h
>>>> +++ b/hw/tpm/tpm_ppi.h
>>>> @@ -23,4 +23,6 @@ typedef struct TPMPPI {
>>>>  bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
>>>>                    hwaddr addr, Object *obj, Error **errp);
>>>>
>>>
>>> Can you add documentation?
>>>
>>>> +void tpm_ppi_reset(TPMPPI *tpmppi);
>>>> +
>>>>  #endif /* TPM_TPM_PPI_H */
>>>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>>>> index c5e9a6e11d..2ab3e8fae7 100644
>>>> --- a/hw/i386/acpi-build.c
>>>> +++ b/hw/i386/acpi-build.c
>>>> @@ -1824,6 +1824,13 @@ build_tpm_ppi(TPMIf *tpm, Aml *dev)
>>>>      pprq = aml_name("PPRQ");
>>>>      pprm = aml_name("PPRM");
>>>>
>>>> +    aml_append(dev,
>>>> +               aml_operation_region("TPP3", AML_SYSTEM_MEMORY,
>>>> +                                    aml_int(TPM_PPI_ADDR_BASE + 0x15a),
>>>
>>> Can you add a definition for this magic value?
>>> Maybe:
>>>
>>> #define TPM_PPI_MEMORY_OVERWRITE_OFFSET 0x15a
>>>
>>> or
>>>
>>> #define TPM_PPI_MOVV_OFFSET 0x15a
>> I'd prefer explicit numeric values there as it's a bit easier to review
>> and grep in 'spec' since one doesn't have to jump to define definition
>> first to find out value to compare with spec.
>> What I'd add though is verbatim comment from spec like we do
>> in APCI code so we could jump to the field of interest in spec
>> just doing copy&paste&search
> 
> I don't mind.
> Philippe, are you okay going back to numeric values?

Can we have a consensus using:

#define TPM_PPI_MEMORY_OVERWRITE_OFFSET_0x15A 0x15a

Joking!

I prefer definitions but since Igor maintains ACPI, he rules and I have
no problem accepting this numeric value :)

> 
> Igor, is this comment what you would expect where the 0x15a value is used:
> /* 0x15a: movv field in ACPI PPI Interface, see specs/tpm.txt */
> 
> thanks
>>
>>>
>>>> +                                    0x1));
>>>> +    field = aml_field("TPP3", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
>>>> +    aml_append(field, aml_named_field("MOVV", 8));
>>>> +    aml_append(dev, field);
>>>>      /*
>>>>       * DerefOf in Windows is broken with SYSTEM_MEMORY.  Use a dynamic
>>>>       * operation region inside of a method for getting FUNC[op].
>>>> @@ -2166,7 +2173,46 @@ build_tpm_ppi(TPMIf *tpm, Aml *dev)
>>>>              aml_append(ifctx, aml_return(aml_buffer(1, zerobyte)));
>>>>          }
>>>>          aml_append(method, ifctx);
>>>> +
>>>> +        ifctx = aml_if(
>>>> +            aml_equal(uuid,
>>>> +                      aml_touuid("376054ED-CC13-4675-901C-4756D7F2D45D")));
>>>> +        {
>>>> +            /* standard DSM query function */
>>>> +            ifctx2 = aml_if(aml_equal(function, zero));
>>>> +            {
>>>> +                uint8_t byte_list[1] = { 0x03 };
>>>> +                aml_append(ifctx2, aml_return(aml_buffer(1, byte_list)));
>>>> +            }
>>>> +            aml_append(ifctx, ifctx2);
>>>> +
>>>> +            /*
>>>> +             * TCG Platform Reset Attack Mitigation Specification 1.0 Ch.6
>>>> +             *
>>>> +             * Arg 2 (Integer): Function Index = 1
>>>> +             * Arg 3 (Package): Arguments = Package: Type: Integer
>>>> +             *                  Operation Value of the Request
>>>> +             * Returns: Type: Integer
>>>> +             *          0: Success
>>>> +             *          1: General Failure
>>>> +             */
>>>> +            ifctx2 = aml_if(aml_equal(function, one));
>>>> +            {
>>>> +                aml_append(ifctx2,
>>>> +                           aml_store(aml_derefof(aml_index(arguments, zero)),
>>>> +                                     op));
>>>> +                {
>>>> +                    aml_append(ifctx2, aml_store(op, aml_name("MOVV")));
>>>> +
>>>> +                    /* 0: success */
>>>> +                    aml_append(ifctx2, aml_return(zero));
>>>> +                }
>>>> +            }
>>>> +            aml_append(ifctx, ifctx2);
>>>> +        }
>>>> +        aml_append(method, ifctx);
>>>>      }
>>>> +
>>>>      aml_append(dev, method);
>>>>  }
>>>>
>>>> diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
>>>> index b243222fd6..48f6a716ad 100644
>>>> --- a/hw/tpm/tpm_crb.c
>>>> +++ b/hw/tpm/tpm_crb.c
>>>> @@ -233,6 +233,7 @@ static void tpm_crb_reset(void *dev)
>>>>  {
>>>>      CRBState *s = CRB(dev);
>>>>
>>>> +    tpm_ppi_reset(&s->ppi);
>>>>      tpm_backend_reset(s->tpmbe);
>>>>
>>>>      memset(s->regs, 0, sizeof(s->regs));
>>>> diff --git a/hw/tpm/tpm_ppi.c b/hw/tpm/tpm_ppi.c
>>>> index f2f07f895e..ac05ba8d3c 100644
>>>> --- a/hw/tpm/tpm_ppi.c
>>>> +++ b/hw/tpm/tpm_ppi.c
>>>> @@ -16,8 +16,30 @@
>>>>  #include "qapi/error.h"
>>>>  #include "cpu.h"
>>>>  #include "sysemu/memory_mapping.h"
>>>> +#include "sysemu/reset.h"
>>>>  #include "migration/vmstate.h"
>>>>  #include "tpm_ppi.h"
>>>> +#include "trace.h"
>>>> +
>>>> +void tpm_ppi_reset(TPMPPI *tpmppi)
>>>> +{
>>>> +    if (tpmppi->buf[0x15a] & 0x1) {
>>>
>>> TPM_PPI_MOVV_OFFSET
>>>
>>>> +        GuestPhysBlockList guest_phys_blocks;
>>>> +        GuestPhysBlock *block;
>>>> +
>>>> +        guest_phys_blocks_init(&guest_phys_blocks);
>>>> +        guest_phys_blocks_append(&guest_phys_blocks);
>>>> +        QTAILQ_FOREACH(block, &guest_phys_blocks.head, next) {
>>>> +            trace_tpm_ppi_memset(block->host_addr,
>>>> +                             block->target_end - block->target_start);
>>>> +            memset(block->host_addr, 0,
>>>> +                   block->target_end - block->target_start);
>>>> +            memory_region_set_dirty(block->mr, 0,
>>>> +                                    block->target_end - block->target_start);
>>>> +        }
>>>> +        guest_phys_blocks_free(&guest_phys_blocks);
>>>> +    }
>>>> +}
>>>>
>>>>  bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
>>>>                    hwaddr addr, Object *obj, Error **errp)
>>>> diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
>>>> index 70432ffe8b..d9bfa956cc 100644
>>>> --- a/hw/tpm/tpm_tis.c
>>>> +++ b/hw/tpm/tpm_tis.c
>>>> @@ -868,6 +868,7 @@ static void tpm_tis_reset(DeviceState *dev)
>>>>      s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->be_driver),
>>>>                              TPM_TIS_BUFFER_MAX);
>>>>
>>>> +    tpm_ppi_reset(&s->ppi);
>>>>      tpm_backend_reset(s->be_driver);
>>>>
>>>>      s->active_locty = TPM_TIS_NO_LOCALITY;
>>>> diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
>>>> index 332c2ae597..ce9bda3c89 100644
>>>> --- a/docs/specs/tpm.txt
>>>> +++ b/docs/specs/tpm.txt
>>>> @@ -121,6 +121,8 @@ layout:
>>>>   +----------+--------+--------+-------------------------------------------+
>>>>   | next_step|   0x1  |  0x159 | Operation to execute after reboot by      |
>>>>   |          |        |        | firmware. Used by firmware.               |
>>>> + +----------+--------+--------+-------------------------------------------+
>>>> + | movv     |   0x1  |  0x15a | Memory overwrite variable                 |
>>>>   +----------+--------+--------+-------------------------------------------+
>>>>
>>>>     The following values are supported for the 'func' field. They correspond
>>>> diff --git a/hw/tpm/trace-events b/hw/tpm/trace-events
>>>> index 25bee0cecf..920d32ad55 100644
>>>> --- a/hw/tpm/trace-events
>>>> +++ b/hw/tpm/trace-events
>>>> @@ -51,3 +51,6 @@ tpm_tis_mmio_write_init_abort(void) "Initiating abort"
>>>>  tpm_tis_mmio_write_lowering_irq(void) "Lowering IRQ"
>>>>  tpm_tis_mmio_write_data2send(uint32_t value, unsigned size) "Data to send to TPM: 0x%08x (size=%d)"
>>>>  tpm_tis_pre_save(uint8_t locty, uint32_t rw_offset) "locty: %d, rw_offset = %u"
>>>> +
>>>> +# hw/tpm/tpm_ppi.c
>>>> +tpm_ppi_memset(uint8_t *ptr, size_t size) "memset: %p %zu"
>>>>
>>>
>>> I'd split this patch in 2: Add ACPI, Add tpm_ppi_reset.
>>

Re: [Qemu-devel] [PATCH v12 6/6] tpm: add ACPI memory clear interface

[Marc-André Lureau]

Hi

On Thu, Dec 13, 2018 at 6:08 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> On 12/13/18 1:18 PM, Marc-André Lureau wrote:
> > On Thu, Dec 13, 2018 at 4:01 PM Igor Mammedov <imammedo@redhat.com> wrote:
> >>
> >> On Wed, 12 Dec 2018 17:22:21 +0100
> >> Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> >>
> >>> On 9/10/18 10:32 AM, Marc-André Lureau wrote:
> >>>> 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". According
> >>>> to Laszlo, it's not so easy to implement in OVMF, he suggested to do
> >>>> it in qemu instead.
> >>>>
> >>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> >>>> ---
> >>>>  hw/tpm/tpm_ppi.h     |  2 ++
> >>>>  hw/i386/acpi-build.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
> >>>>  hw/tpm/tpm_crb.c     |  1 +
> >>>>  hw/tpm/tpm_ppi.c     | 22 +++++++++++++++++++++
> >>>>  hw/tpm/tpm_tis.c     |  1 +
> >>>>  docs/specs/tpm.txt   |  2 ++
> >>>>  hw/tpm/trace-events  |  3 +++
> >>>>  7 files changed, 77 insertions(+)
> >>>>
> >>>> diff --git a/hw/tpm/tpm_ppi.h b/hw/tpm/tpm_ppi.h
> >>>> index c2ab2ed300..b8f67962c7 100644
> >>>> --- a/hw/tpm/tpm_ppi.h
> >>>> +++ b/hw/tpm/tpm_ppi.h
> >>>> @@ -23,4 +23,6 @@ typedef struct TPMPPI {
> >>>>  bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
> >>>>                    hwaddr addr, Object *obj, Error **errp);
> >>>>
> >>>
> >>> Can you add documentation?
> >>>
> >>>> +void tpm_ppi_reset(TPMPPI *tpmppi);
> >>>> +
> >>>>  #endif /* TPM_TPM_PPI_H */
> >>>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> >>>> index c5e9a6e11d..2ab3e8fae7 100644
> >>>> --- a/hw/i386/acpi-build.c
> >>>> +++ b/hw/i386/acpi-build.c
> >>>> @@ -1824,6 +1824,13 @@ build_tpm_ppi(TPMIf *tpm, Aml *dev)
> >>>>      pprq = aml_name("PPRQ");
> >>>>      pprm = aml_name("PPRM");
> >>>>
> >>>> +    aml_append(dev,
> >>>> +               aml_operation_region("TPP3", AML_SYSTEM_MEMORY,
> >>>> +                                    aml_int(TPM_PPI_ADDR_BASE + 0x15a),
> >>>
> >>> Can you add a definition for this magic value?
> >>> Maybe:
> >>>
> >>> #define TPM_PPI_MEMORY_OVERWRITE_OFFSET 0x15a
> >>>
> >>> or
> >>>
> >>> #define TPM_PPI_MOVV_OFFSET 0x15a
> >> I'd prefer explicit numeric values there as it's a bit easier to review
> >> and grep in 'spec' since one doesn't have to jump to define definition
> >> first to find out value to compare with spec.
> >> What I'd add though is verbatim comment from spec like we do
> >> in APCI code so we could jump to the field of interest in spec
> >> just doing copy&paste&search
> >
> > I don't mind.
> > Philippe, are you okay going back to numeric values?
>
> Can we have a consensus using:
>
> #define TPM_PPI_MEMORY_OVERWRITE_OFFSET_0x15A 0x15a
>
> Joking!
>
> I prefer definitions but since Igor maintains ACPI, he rules and I have
> no problem accepting this numeric value :)

ok, with Igor comments, does it get you review-by?
thanks

>
> >
> > Igor, is this comment what you would expect where the 0x15a value is used:
> > /* 0x15a: movv field in ACPI PPI Interface, see specs/tpm.txt */
> >
> > thanks
> >>
> >>>
> >>>> +                                    0x1));
> >>>> +    field = aml_field("TPP3", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
> >>>> +    aml_append(field, aml_named_field("MOVV", 8));
> >>>> +    aml_append(dev, field);
> >>>>      /*
> >>>>       * DerefOf in Windows is broken with SYSTEM_MEMORY.  Use a dynamic
> >>>>       * operation region inside of a method for getting FUNC[op].
> >>>> @@ -2166,7 +2173,46 @@ build_tpm_ppi(TPMIf *tpm, Aml *dev)
> >>>>              aml_append(ifctx, aml_return(aml_buffer(1, zerobyte)));
> >>>>          }
> >>>>          aml_append(method, ifctx);
> >>>> +
> >>>> +        ifctx = aml_if(
> >>>> +            aml_equal(uuid,
> >>>> +                      aml_touuid("376054ED-CC13-4675-901C-4756D7F2D45D")));
> >>>> +        {
> >>>> +            /* standard DSM query function */
> >>>> +            ifctx2 = aml_if(aml_equal(function, zero));
> >>>> +            {
> >>>> +                uint8_t byte_list[1] = { 0x03 };
> >>>> +                aml_append(ifctx2, aml_return(aml_buffer(1, byte_list)));
> >>>> +            }
> >>>> +            aml_append(ifctx, ifctx2);
> >>>> +
> >>>> +            /*
> >>>> +             * TCG Platform Reset Attack Mitigation Specification 1.0 Ch.6
> >>>> +             *
> >>>> +             * Arg 2 (Integer): Function Index = 1
> >>>> +             * Arg 3 (Package): Arguments = Package: Type: Integer
> >>>> +             *                  Operation Value of the Request
> >>>> +             * Returns: Type: Integer
> >>>> +             *          0: Success
> >>>> +             *          1: General Failure
> >>>> +             */
> >>>> +            ifctx2 = aml_if(aml_equal(function, one));
> >>>> +            {
> >>>> +                aml_append(ifctx2,
> >>>> +                           aml_store(aml_derefof(aml_index(arguments, zero)),
> >>>> +                                     op));
> >>>> +                {
> >>>> +                    aml_append(ifctx2, aml_store(op, aml_name("MOVV")));
> >>>> +
> >>>> +                    /* 0: success */
> >>>> +                    aml_append(ifctx2, aml_return(zero));
> >>>> +                }
> >>>> +            }
> >>>> +            aml_append(ifctx, ifctx2);
> >>>> +        }
> >>>> +        aml_append(method, ifctx);
> >>>>      }
> >>>> +
> >>>>      aml_append(dev, method);
> >>>>  }
> >>>>
> >>>> diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> >>>> index b243222fd6..48f6a716ad 100644
> >>>> --- a/hw/tpm/tpm_crb.c
> >>>> +++ b/hw/tpm/tpm_crb.c
> >>>> @@ -233,6 +233,7 @@ static void tpm_crb_reset(void *dev)
> >>>>  {
> >>>>      CRBState *s = CRB(dev);
> >>>>
> >>>> +    tpm_ppi_reset(&s->ppi);
> >>>>      tpm_backend_reset(s->tpmbe);
> >>>>
> >>>>      memset(s->regs, 0, sizeof(s->regs));
> >>>> diff --git a/hw/tpm/tpm_ppi.c b/hw/tpm/tpm_ppi.c
> >>>> index f2f07f895e..ac05ba8d3c 100644
> >>>> --- a/hw/tpm/tpm_ppi.c
> >>>> +++ b/hw/tpm/tpm_ppi.c
> >>>> @@ -16,8 +16,30 @@
> >>>>  #include "qapi/error.h"
> >>>>  #include "cpu.h"
> >>>>  #include "sysemu/memory_mapping.h"
> >>>> +#include "sysemu/reset.h"
> >>>>  #include "migration/vmstate.h"
> >>>>  #include "tpm_ppi.h"
> >>>> +#include "trace.h"
> >>>> +
> >>>> +void tpm_ppi_reset(TPMPPI *tpmppi)
> >>>> +{
> >>>> +    if (tpmppi->buf[0x15a] & 0x1) {
> >>>
> >>> TPM_PPI_MOVV_OFFSET
> >>>
> >>>> +        GuestPhysBlockList guest_phys_blocks;
> >>>> +        GuestPhysBlock *block;
> >>>> +
> >>>> +        guest_phys_blocks_init(&guest_phys_blocks);
> >>>> +        guest_phys_blocks_append(&guest_phys_blocks);
> >>>> +        QTAILQ_FOREACH(block, &guest_phys_blocks.head, next) {
> >>>> +            trace_tpm_ppi_memset(block->host_addr,
> >>>> +                             block->target_end - block->target_start);
> >>>> +            memset(block->host_addr, 0,
> >>>> +                   block->target_end - block->target_start);
> >>>> +            memory_region_set_dirty(block->mr, 0,
> >>>> +                                    block->target_end - block->target_start);
> >>>> +        }
> >>>> +        guest_phys_blocks_free(&guest_phys_blocks);
> >>>> +    }
> >>>> +}
> >>>>
> >>>>  bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
> >>>>                    hwaddr addr, Object *obj, Error **errp)
> >>>> diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
> >>>> index 70432ffe8b..d9bfa956cc 100644
> >>>> --- a/hw/tpm/tpm_tis.c
> >>>> +++ b/hw/tpm/tpm_tis.c
> >>>> @@ -868,6 +868,7 @@ static void tpm_tis_reset(DeviceState *dev)
> >>>>      s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->be_driver),
> >>>>                              TPM_TIS_BUFFER_MAX);
> >>>>
> >>>> +    tpm_ppi_reset(&s->ppi);
> >>>>      tpm_backend_reset(s->be_driver);
> >>>>
> >>>>      s->active_locty = TPM_TIS_NO_LOCALITY;
> >>>> diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
> >>>> index 332c2ae597..ce9bda3c89 100644
> >>>> --- a/docs/specs/tpm.txt
> >>>> +++ b/docs/specs/tpm.txt
> >>>> @@ -121,6 +121,8 @@ layout:
> >>>>   +----------+--------+--------+-------------------------------------------+
> >>>>   | next_step|   0x1  |  0x159 | Operation to execute after reboot by      |
> >>>>   |          |        |        | firmware. Used by firmware.               |
> >>>> + +----------+--------+--------+-------------------------------------------+
> >>>> + | movv     |   0x1  |  0x15a | Memory overwrite variable                 |
> >>>>   +----------+--------+--------+-------------------------------------------+
> >>>>
> >>>>     The following values are supported for the 'func' field. They correspond
> >>>> diff --git a/hw/tpm/trace-events b/hw/tpm/trace-events
> >>>> index 25bee0cecf..920d32ad55 100644
> >>>> --- a/hw/tpm/trace-events
> >>>> +++ b/hw/tpm/trace-events
> >>>> @@ -51,3 +51,6 @@ tpm_tis_mmio_write_init_abort(void) "Initiating abort"
> >>>>  tpm_tis_mmio_write_lowering_irq(void) "Lowering IRQ"
> >>>>  tpm_tis_mmio_write_data2send(uint32_t value, unsigned size) "Data to send to TPM: 0x%08x (size=%d)"
> >>>>  tpm_tis_pre_save(uint8_t locty, uint32_t rw_offset) "locty: %d, rw_offset = %u"
> >>>> +
> >>>> +# hw/tpm/tpm_ppi.c
> >>>> +tpm_ppi_memset(uint8_t *ptr, size_t size) "memset: %p %zu"
> >>>>
> >>>
> >>> I'd split this patch in 2: Add ACPI, Add tpm_ppi_reset.
> >>
>


-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v12 6/6] tpm: add ACPI memory clear interface

[Igor Mammedov]

On Thu, 13 Dec 2018 16:18:09 +0400
Marc-André Lureau <marcandre.lureau@redhat.com> wrote:

> On Thu, Dec 13, 2018 at 4:01 PM Igor Mammedov <imammedo@redhat.com> wrote:
> >
> > On Wed, 12 Dec 2018 17:22:21 +0100
> > Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> >  
> > > On 9/10/18 10:32 AM, Marc-André Lureau wrote:  
> > > > 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". According
> > > > to Laszlo, it's not so easy to implement in OVMF, he suggested to do
> > > > it in qemu instead.
> > > >
> > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > > ---
> > > >  hw/tpm/tpm_ppi.h     |  2 ++
> > > >  hw/i386/acpi-build.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
> > > >  hw/tpm/tpm_crb.c     |  1 +
> > > >  hw/tpm/tpm_ppi.c     | 22 +++++++++++++++++++++
> > > >  hw/tpm/tpm_tis.c     |  1 +
> > > >  docs/specs/tpm.txt   |  2 ++
> > > >  hw/tpm/trace-events  |  3 +++
> > > >  7 files changed, 77 insertions(+)
> > > >
> > > > diff --git a/hw/tpm/tpm_ppi.h b/hw/tpm/tpm_ppi.h
> > > > index c2ab2ed300..b8f67962c7 100644
> > > > --- a/hw/tpm/tpm_ppi.h
> > > > +++ b/hw/tpm/tpm_ppi.h
> > > > @@ -23,4 +23,6 @@ typedef struct TPMPPI {
> > > >  bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
> > > >                    hwaddr addr, Object *obj, Error **errp);
> > > >  
> > >
> > > Can you add documentation?
> > >  
> > > > +void tpm_ppi_reset(TPMPPI *tpmppi);
> > > > +
> > > >  #endif /* TPM_TPM_PPI_H */
> > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > > > index c5e9a6e11d..2ab3e8fae7 100644
> > > > --- a/hw/i386/acpi-build.c
> > > > +++ b/hw/i386/acpi-build.c
> > > > @@ -1824,6 +1824,13 @@ build_tpm_ppi(TPMIf *tpm, Aml *dev)
> > > >      pprq = aml_name("PPRQ");
> > > >      pprm = aml_name("PPRM");
> > > >
> > > > +    aml_append(dev,
> > > > +               aml_operation_region("TPP3", AML_SYSTEM_MEMORY,
> > > > +                                    aml_int(TPM_PPI_ADDR_BASE + 0x15a),  
> > >
> > > Can you add a definition for this magic value?
> > > Maybe:
> > >
> > > #define TPM_PPI_MEMORY_OVERWRITE_OFFSET 0x15a
> > >
> > > or
> > >
> > > #define TPM_PPI_MOVV_OFFSET 0x15a  
> > I'd prefer explicit numeric values there as it's a bit easier to review
> > and grep in 'spec' since one doesn't have to jump to define definition
> > first to find out value to compare with spec.
> > What I'd add though is verbatim comment from spec like we do
> > in APCI code so we could jump to the field of interest in spec
> > just doing copy&paste&search  
> 
> I don't mind.
> Philippe, are you okay going back to numeric values?
> 
> Igor, is this comment what you would expect where the 0x15a value is used:
> /* 0x15a: movv field in ACPI PPI Interface, see specs/tpm.txt */
> 
usually field name is sufficient, so something like:
 foo(0.15a /* movv, docs/specs/tpm.txt */)


> thanks
> >  
> > >  
> > > > +                                    0x1));
> > > > +    field = aml_field("TPP3", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
> > > > +    aml_append(field, aml_named_field("MOVV", 8));
> > > > +    aml_append(dev, field);
> > > >      /*
> > > >       * DerefOf in Windows is broken with SYSTEM_MEMORY.  Use a dynamic
> > > >       * operation region inside of a method for getting FUNC[op].
> > > > @@ -2166,7 +2173,46 @@ build_tpm_ppi(TPMIf *tpm, Aml *dev)
> > > >              aml_append(ifctx, aml_return(aml_buffer(1, zerobyte)));
> > > >          }
> > > >          aml_append(method, ifctx);
> > > > +
> > > > +        ifctx = aml_if(
> > > > +            aml_equal(uuid,
> > > > +                      aml_touuid("376054ED-CC13-4675-901C-4756D7F2D45D")));
> > > > +        {
> > > > +            /* standard DSM query function */
> > > > +            ifctx2 = aml_if(aml_equal(function, zero));
> > > > +            {
> > > > +                uint8_t byte_list[1] = { 0x03 };
> > > > +                aml_append(ifctx2, aml_return(aml_buffer(1, byte_list)));
> > > > +            }
> > > > +            aml_append(ifctx, ifctx2);
> > > > +
> > > > +            /*
> > > > +             * TCG Platform Reset Attack Mitigation Specification 1.0 Ch.6
> > > > +             *
> > > > +             * Arg 2 (Integer): Function Index = 1
> > > > +             * Arg 3 (Package): Arguments = Package: Type: Integer
> > > > +             *                  Operation Value of the Request
> > > > +             * Returns: Type: Integer
> > > > +             *          0: Success
> > > > +             *          1: General Failure
> > > > +             */
> > > > +            ifctx2 = aml_if(aml_equal(function, one));
> > > > +            {
> > > > +                aml_append(ifctx2,
> > > > +                           aml_store(aml_derefof(aml_index(arguments, zero)),
> > > > +                                     op));
> > > > +                {
> > > > +                    aml_append(ifctx2, aml_store(op, aml_name("MOVV")));
> > > > +
> > > > +                    /* 0: success */
> > > > +                    aml_append(ifctx2, aml_return(zero));
> > > > +                }
> > > > +            }
> > > > +            aml_append(ifctx, ifctx2);
> > > > +        }
> > > > +        aml_append(method, ifctx);
> > > >      }
> > > > +
> > > >      aml_append(dev, method);
> > > >  }
> > > >
> > > > diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> > > > index b243222fd6..48f6a716ad 100644
> > > > --- a/hw/tpm/tpm_crb.c
> > > > +++ b/hw/tpm/tpm_crb.c
> > > > @@ -233,6 +233,7 @@ static void tpm_crb_reset(void *dev)
> > > >  {
> > > >      CRBState *s = CRB(dev);
> > > >
> > > > +    tpm_ppi_reset(&s->ppi);
> > > >      tpm_backend_reset(s->tpmbe);
> > > >
> > > >      memset(s->regs, 0, sizeof(s->regs));
> > > > diff --git a/hw/tpm/tpm_ppi.c b/hw/tpm/tpm_ppi.c
> > > > index f2f07f895e..ac05ba8d3c 100644
> > > > --- a/hw/tpm/tpm_ppi.c
> > > > +++ b/hw/tpm/tpm_ppi.c
> > > > @@ -16,8 +16,30 @@
> > > >  #include "qapi/error.h"
> > > >  #include "cpu.h"
> > > >  #include "sysemu/memory_mapping.h"
> > > > +#include "sysemu/reset.h"
> > > >  #include "migration/vmstate.h"
> > > >  #include "tpm_ppi.h"
> > > > +#include "trace.h"
> > > > +
> > > > +void tpm_ppi_reset(TPMPPI *tpmppi)
> > > > +{
> > > > +    if (tpmppi->buf[0x15a] & 0x1) {  
> > >
> > > TPM_PPI_MOVV_OFFSET
> > >  
> > > > +        GuestPhysBlockList guest_phys_blocks;
> > > > +        GuestPhysBlock *block;
> > > > +
> > > > +        guest_phys_blocks_init(&guest_phys_blocks);
> > > > +        guest_phys_blocks_append(&guest_phys_blocks);
> > > > +        QTAILQ_FOREACH(block, &guest_phys_blocks.head, next) {
> > > > +            trace_tpm_ppi_memset(block->host_addr,
> > > > +                             block->target_end - block->target_start);
> > > > +            memset(block->host_addr, 0,
> > > > +                   block->target_end - block->target_start);
> > > > +            memory_region_set_dirty(block->mr, 0,
> > > > +                                    block->target_end - block->target_start);
> > > > +        }
> > > > +        guest_phys_blocks_free(&guest_phys_blocks);
> > > > +    }
> > > > +}
> > > >
> > > >  bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
> > > >                    hwaddr addr, Object *obj, Error **errp)
> > > > diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
> > > > index 70432ffe8b..d9bfa956cc 100644
> > > > --- a/hw/tpm/tpm_tis.c
> > > > +++ b/hw/tpm/tpm_tis.c
> > > > @@ -868,6 +868,7 @@ static void tpm_tis_reset(DeviceState *dev)
> > > >      s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->be_driver),
> > > >                              TPM_TIS_BUFFER_MAX);
> > > >
> > > > +    tpm_ppi_reset(&s->ppi);
> > > >      tpm_backend_reset(s->be_driver);
> > > >
> > > >      s->active_locty = TPM_TIS_NO_LOCALITY;
> > > > diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
> > > > index 332c2ae597..ce9bda3c89 100644
> > > > --- a/docs/specs/tpm.txt
> > > > +++ b/docs/specs/tpm.txt
> > > > @@ -121,6 +121,8 @@ layout:
> > > >   +----------+--------+--------+-------------------------------------------+
> > > >   | next_step|   0x1  |  0x159 | Operation to execute after reboot by      |
> > > >   |          |        |        | firmware. Used by firmware.               |
> > > > + +----------+--------+--------+-------------------------------------------+
> > > > + | movv     |   0x1  |  0x15a | Memory overwrite variable                 |
> > > >   +----------+--------+--------+-------------------------------------------+
> > > >
> > > >     The following values are supported for the 'func' field. They correspond
> > > > diff --git a/hw/tpm/trace-events b/hw/tpm/trace-events
> > > > index 25bee0cecf..920d32ad55 100644
> > > > --- a/hw/tpm/trace-events
> > > > +++ b/hw/tpm/trace-events
> > > > @@ -51,3 +51,6 @@ tpm_tis_mmio_write_init_abort(void) "Initiating abort"
> > > >  tpm_tis_mmio_write_lowering_irq(void) "Lowering IRQ"
> > > >  tpm_tis_mmio_write_data2send(uint32_t value, unsigned size) "Data to send to TPM: 0x%08x (size=%d)"
> > > >  tpm_tis_pre_save(uint8_t locty, uint32_t rw_offset) "locty: %d, rw_offset = %u"
> > > > +
> > > > +# hw/tpm/tpm_ppi.c
> > > > +tpm_ppi_memset(uint8_t *ptr, size_t size) "memset: %p %zu"
> > > >  
> > >
> > > I'd split this patch in 2: Add ACPI, Add tpm_ppi_reset.  
> >  
> 

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

[Philippe Mathieu-Daudé]

Hi Marc-André,

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

I made trivial review comments in indivial patches.
Can you respin a rebased version for further testing?

Thanks,

Phil.