[Qemu-devel] [PATCH v8 0/5] Add support for TPM Physical Presence interface

[Qemu-devel] [PATCH v8 0/5] 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.

v8: ACPI code improvements
- replace various aml_int() with variables
- make op/op_flags common variables
- replace the switch field indexing hack by a dynamic operation region

v7:
- relace RAM with RAM device ptr
- a few ACPI code & comments improvements
- add back proof-of-concept ACPI memory clear interface, handled in
  qemu (pass again WHLK TCG tests)

Marc-André Lureau (2):
  tpm: add a "ppi" boolean property
  PoC: tpm: add ACPI memory clear interface

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

 hw/tpm/tpm_ppi.h      |  26 +++
 include/hw/acpi/tpm.h |  17 ++
 include/hw/compat.h   |  10 +
 hw/i386/acpi-build.c  | 456 +++++++++++++++++++++++++++++++++++++++++-
 hw/tpm/tpm_crb.c      |  11 +
 hw/tpm/tpm_ppi.c      |  56 ++++++
 hw/tpm/tpm_tis.c      |  11 +
 docs/specs/tpm.txt    | 101 ++++++++++
 hw/tpm/Makefile.objs  |   1 +
 hw/tpm/trace-events   |   3 +
 10 files changed, 690 insertions(+), 2 deletions(-)
 create mode 100644 hw/tpm/tpm_ppi.h
 create mode 100644 hw/tpm/tpm_ppi.c

-- 
2.18.0.129.ge3331758f1

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

[Marc-André Lureau]

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

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

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

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

[Marc-André Lureau]

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

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

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

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

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/tpm/tpm_ppi.h      | 26 ++++++++++++++++++++++++++
 include/hw/acpi/tpm.h |  6 ++++++
 hw/tpm/tpm_crb.c      |  8 ++++++++
 hw/tpm/tpm_ppi.c      | 31 +++++++++++++++++++++++++++++++
 hw/tpm/tpm_tis.c      |  8 ++++++++
 hw/tpm/Makefile.objs  |  1 +
 6 files changed, 80 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..f6458bf87e
--- /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[TPM_PPI_ADDR_SIZE];
+} 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..8b46b9dd4b
--- /dev/null
+++ b/hw/tpm/tpm_ppi.c
@@ -0,0 +1,31 @@
+/*
+ * 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)
+{
+    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.18.0.129.ge3331758f1

Re: [Qemu-devel] [PATCH v8 2/5] tpm: implement virtual memory device for TPM PPI

[Igor Mammedov]

On Mon, 16 Jul 2018 14:59:45 +0200
Marc-André Lureau <marcandre.lureau@redhat.com> 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.
it doesn't implement anything, maybe something like this would be better?

tpm: allocate/map buffer for TPM Physical Presence interface

...

> 
> This device should be used by all TPM interfaces on x86 and can be added
> by calling tpm_ppi_init_io().
Did you mean:
 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>
> ---
>  hw/tpm/tpm_ppi.h      | 26 ++++++++++++++++++++++++++
>  include/hw/acpi/tpm.h |  6 ++++++
>  hw/tpm/tpm_crb.c      |  8 ++++++++
>  hw/tpm/tpm_ppi.c      | 31 +++++++++++++++++++++++++++++++
>  hw/tpm/tpm_tis.c      |  8 ++++++++
>  hw/tpm/Makefile.objs  |  1 +
>  6 files changed, 80 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..f6458bf87e
> --- /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[TPM_PPI_ADDR_SIZE];
> +} 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..8b46b9dd4b
> --- /dev/null
> +++ b/hw/tpm/tpm_ppi.c
> @@ -0,0 +1,31 @@
> +/*
> + * 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)
> +{
> +    memory_region_init_ram_device_ptr(&tpmppi->ram, obj, "tpm-ppi",
> +                                      TPM_PPI_ADDR_SIZE, tpmppi->buf);
why use buf and not a plain ram memory region?

> +    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

Re: [Qemu-devel] [PATCH v8 2/5] tpm: implement virtual memory device for TPM PPI

[Marc-André Lureau]

Hi

On Mon, Jul 16, 2018 at 4:44 PM, Igor Mammedov <imammedo@redhat.com> wrote:
> On Mon, 16 Jul 2018 14:59:45 +0200
> Marc-André Lureau <marcandre.lureau@redhat.com> 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.
> it doesn't implement anything, maybe something like this would be better?
>
> tpm: allocate/map buffer for TPM Physical Presence interface
>
> ...
>
>>
>> This device should be used by all TPM interfaces on x86 and can be added
>> by calling tpm_ppi_init_io().
> Did you mean:
>  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>
>> ---
>>  hw/tpm/tpm_ppi.h      | 26 ++++++++++++++++++++++++++
>>  include/hw/acpi/tpm.h |  6 ++++++
>>  hw/tpm/tpm_crb.c      |  8 ++++++++
>>  hw/tpm/tpm_ppi.c      | 31 +++++++++++++++++++++++++++++++
>>  hw/tpm/tpm_tis.c      |  8 ++++++++
>>  hw/tpm/Makefile.objs  |  1 +
>>  6 files changed, 80 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..f6458bf87e
>> --- /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[TPM_PPI_ADDR_SIZE];
>> +} 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..8b46b9dd4b
>> --- /dev/null
>> +++ b/hw/tpm/tpm_ppi.c
>> @@ -0,0 +1,31 @@
>> +/*
>> + * 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)
>> +{
>> +    memory_region_init_ram_device_ptr(&tpmppi->ram, obj, "tpm-ppi",
>> +                                      TPM_PPI_ADDR_SIZE, tpmppi->buf);
> why use buf and not a plain ram memory region?

It shouldn't be treated as regular RAM. (for example when iterating
guest_phys_blocks for dump or memory clear in last patch)

>
>> +    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
>
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v8 2/5] tpm: implement virtual memory device for TPM PPI

[Igor Mammedov]

On Mon, 16 Jul 2018 16:56:36 +0200
Marc-André Lureau <marcandre.lureau@gmail.com> wrote:

> Hi
> 
> On Mon, Jul 16, 2018 at 4:44 PM, Igor Mammedov <imammedo@redhat.com> wrote:
> > On Mon, 16 Jul 2018 14:59:45 +0200
> > Marc-André Lureau <marcandre.lureau@redhat.com> 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.  
> > it doesn't implement anything, maybe something like this would be better?
> >
> > tpm: allocate/map buffer for TPM Physical Presence interface
> >
> > ...
> >  
> >>
> >> This device should be used by all TPM interfaces on x86 and can be added
> >> by calling tpm_ppi_init_io().  
> > Did you mean:
> >  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>
> >> ---
> >>  hw/tpm/tpm_ppi.h      | 26 ++++++++++++++++++++++++++
> >>  include/hw/acpi/tpm.h |  6 ++++++
> >>  hw/tpm/tpm_crb.c      |  8 ++++++++
> >>  hw/tpm/tpm_ppi.c      | 31 +++++++++++++++++++++++++++++++
> >>  hw/tpm/tpm_tis.c      |  8 ++++++++
> >>  hw/tpm/Makefile.objs  |  1 +
> >>  6 files changed, 80 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..f6458bf87e
> >> --- /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[TPM_PPI_ADDR_SIZE];
> >> +} 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..8b46b9dd4b
> >> --- /dev/null
> >> +++ b/hw/tpm/tpm_ppi.c
> >> @@ -0,0 +1,31 @@
> >> +/*
> >> + * 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)
> >> +{
> >> +    memory_region_init_ram_device_ptr(&tpmppi->ram, obj, "tpm-ppi",
> >> +                                      TPM_PPI_ADDR_SIZE, tpmppi->buf);  
> > why use buf and not a plain ram memory region?  
> 
> It shouldn't be treated as regular RAM. (for example when iterating
> guest_phys_blocks for dump or memory clear in last patch)
why it shouldn't, I'd think having ppi flags in dump would be useful
and why it shouldn't be cleared on reset along with all other ram?

> 
> >  
> >> +    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  
> >
> >  
> 
> 
> 

Re: [Qemu-devel] [PATCH v8 2/5] tpm: implement virtual memory device for TPM PPI

[Marc-André Lureau]

On Tue, Jul 17, 2018 at 9:59 AM, Igor Mammedov <imammedo@redhat.com> wrote:
> On Mon, 16 Jul 2018 16:56:36 +0200
> Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
>
>> Hi
>>
>> On Mon, Jul 16, 2018 at 4:44 PM, Igor Mammedov <imammedo@redhat.com> wrote:
>> > On Mon, 16 Jul 2018 14:59:45 +0200
>> > Marc-André Lureau <marcandre.lureau@redhat.com> 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.
>> > it doesn't implement anything, maybe something like this would be better?
>> >
>> > tpm: allocate/map buffer for TPM Physical Presence interface
>> >
>> > ...
>> >
>> >>
>> >> This device should be used by all TPM interfaces on x86 and can be added
>> >> by calling tpm_ppi_init_io().
>> > Did you mean:
>> >  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>
>> >> ---
>> >>  hw/tpm/tpm_ppi.h      | 26 ++++++++++++++++++++++++++
>> >>  include/hw/acpi/tpm.h |  6 ++++++
>> >>  hw/tpm/tpm_crb.c      |  8 ++++++++
>> >>  hw/tpm/tpm_ppi.c      | 31 +++++++++++++++++++++++++++++++
>> >>  hw/tpm/tpm_tis.c      |  8 ++++++++
>> >>  hw/tpm/Makefile.objs  |  1 +
>> >>  6 files changed, 80 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..f6458bf87e
>> >> --- /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[TPM_PPI_ADDR_SIZE];
>> >> +} 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..8b46b9dd4b
>> >> --- /dev/null
>> >> +++ b/hw/tpm/tpm_ppi.c
>> >> @@ -0,0 +1,31 @@
>> >> +/*
>> >> + * 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)
>> >> +{
>> >> +    memory_region_init_ram_device_ptr(&tpmppi->ram, obj, "tpm-ppi",
>> >> +                                      TPM_PPI_ADDR_SIZE, tpmppi->buf);
>> > why use buf and not a plain ram memory region?
>>
>> It shouldn't be treated as regular RAM. (for example when iterating
>> guest_phys_blocks for dump or memory clear in last patch)
> why it shouldn't, I'd think having ppi flags in dump would be useful

We don't dump device memory.

> and why it shouldn't be cleared on reset along with all other ram?

It's a device memory, not system memory. The spec talks only about
system memory RAM, and Stefan also mentioned that we should not touch
device/tpm RAM in previous review.

However, we may clear the MOR bit after reset (shown in Figure 2 in
the spec - "the MemoryOverwriteRequestControl EFI variable described
in Section 5 can be updated to clear the MOR bit after secrets have
been removed from memory")

>>
>> >
>> >> +    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
>> >
>> >
>>
>>
>>
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v8 2/5] tpm: implement virtual memory device for TPM PPI

[Igor Mammedov]

On Tue, 17 Jul 2018 12:22:49 +0200
Marc-André Lureau <marcandre.lureau@gmail.com> wrote:

> On Tue, Jul 17, 2018 at 9:59 AM, Igor Mammedov <imammedo@redhat.com> wrote:
> > On Mon, 16 Jul 2018 16:56:36 +0200
> > Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
> >  
> >> Hi
> >>
> >> On Mon, Jul 16, 2018 at 4:44 PM, Igor Mammedov <imammedo@redhat.com> wrote:  
> >> > On Mon, 16 Jul 2018 14:59:45 +0200
> >> > Marc-André Lureau <marcandre.lureau@redhat.com> 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.  
> >> > it doesn't implement anything, maybe something like this would be better?
> >> >
> >> > tpm: allocate/map buffer for TPM Physical Presence interface
> >> >
> >> > ...
> >> >  
> >> >>
> >> >> This device should be used by all TPM interfaces on x86 and can be added
> >> >> by calling tpm_ppi_init_io().  
> >> > Did you mean:
> >> >  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>
> >> >> ---
> >> >>  hw/tpm/tpm_ppi.h      | 26 ++++++++++++++++++++++++++
> >> >>  include/hw/acpi/tpm.h |  6 ++++++
> >> >>  hw/tpm/tpm_crb.c      |  8 ++++++++
> >> >>  hw/tpm/tpm_ppi.c      | 31 +++++++++++++++++++++++++++++++
> >> >>  hw/tpm/tpm_tis.c      |  8 ++++++++
> >> >>  hw/tpm/Makefile.objs  |  1 +
> >> >>  6 files changed, 80 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..f6458bf87e
> >> >> --- /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[TPM_PPI_ADDR_SIZE];
> >> >> +} 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..8b46b9dd4b
> >> >> --- /dev/null
> >> >> +++ b/hw/tpm/tpm_ppi.c
> >> >> @@ -0,0 +1,31 @@
> >> >> +/*
> >> >> + * 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)
> >> >> +{
> >> >> +    memory_region_init_ram_device_ptr(&tpmppi->ram, obj, "tpm-ppi",
> >> >> +                                      TPM_PPI_ADDR_SIZE, tpmppi->buf);  
> >> > why use buf and not a plain ram memory region?  
> >>
> >> It shouldn't be treated as regular RAM. (for example when iterating
> >> guest_phys_blocks for dump or memory clear in last patch)  
> > why it shouldn't, I'd think having ppi flags in dump would be useful  
> 
> We don't dump device memory.
> 
> > and why it shouldn't be cleared on reset along with all other ram?  
> 
> It's a device memory, not system memory. The spec talks only about
> system memory RAM, and Stefan also mentioned that we should not touch
> device/tpm RAM in previous review.
ok


> However, we may clear the MOR bit after reset (shown in Figure 2 in
> the spec - "the MemoryOverwriteRequestControl EFI variable described
> in Section 5 can be updated to clear the MOR bit after secrets have
> been removed from memory")
Also spec talks about MOR flags being saved in nonvolatile memory
so it would survive S4/S5 states. It's not the case with this
series but we probably should care since backend for ram/nvdimm
could be a persistent file.

Interesting, what should we do in case of nvdimm if it holds
an image of OS and aren't supposed to be cleared on reboot when
platform still sees it as part of RAM?

> 
> >>  
> >> >  
> >> >> +    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  
> >> >
> >> >  
> >>
> >>
> >>  
> >  
> 
> 
> 

Re: [Qemu-devel] [PATCH v8 2/5] tpm: implement virtual memory device for TPM PPI

[Igor Mammedov]

On Mon, 16 Jul 2018 14:59:45 +0200
Marc-André Lureau <marcandre.lureau@redhat.com> 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.
...
another question wrt E820 table, is firmware putting a reservation record
for this RAM area overlay or should we do it QEMU?

Re: [Qemu-devel] [PATCH v8 2/5] tpm: implement virtual memory device for TPM PPI

[Marc-André Lureau]

Hi

On Tue, Jul 17, 2018 at 12:03 PM, Igor Mammedov <imammedo@redhat.com> wrote:
> On Mon, 16 Jul 2018 14:59:45 +0200
> Marc-André Lureau <marcandre.lureau@redhat.com> 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.
> ...
> another question wrt E820 table, is firmware putting a reservation record
> for this RAM area overlay or should we do it QEMU?

There doesn't seem to be any reservation done in ovmf, or on my
laptop. I also wonder if it should be done. And how OS deal with that
today (/proc/iomem is correct).

Laszlo, any idea?

Re: [Qemu-devel] [PATCH v8 2/5] tpm: implement virtual memory device for TPM PPI

[Laszlo Ersek]

On 07/17/18 12:39, Marc-André Lureau wrote:
> Hi
> 
> On Tue, Jul 17, 2018 at 12:03 PM, Igor Mammedov <imammedo@redhat.com> wrote:
>> On Mon, 16 Jul 2018 14:59:45 +0200
>> Marc-André Lureau <marcandre.lureau@redhat.com> 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.
>> ...
>> another question wrt E820 table, is firmware putting a reservation record
>> for this RAM area overlay or should we do it QEMU?

What OS requires this memmap entry?

> There doesn't seem to be any reservation done in ovmf, or on my
> laptop.

Relevant point, about your laptop.

> I also wonder if it should be done.

IMO: no, it shouldn't.

> And how OS deal with that today (/proc/iomem is correct).
> 
> Laszlo, any idea?

In my experience, exposing such MMIO areas that belong to *platform*
devices, in the UEFI memory map, is an inconsistent practice. I can list
at least four different cases:

(1) A UEFI runtime driver is using the MMIO area for providing a UEFI
runtime service. In this case, the firmware must expose the area as
"MMIO" type memory, and set the "runtime" attribute on it. In response,
the OS will map the area with virtual addresses, directly in response to
the memmap entry.

(2) A firmware SMM driver is using the MMIO area for providing a
privileged part of a UEFI runtime service. Because SMM uses its own page
table anyway (protected from the OS), advertizing the area in the UEFI
memmap is unneeded. It can simply be left as a gap.

(3) Linux requires the firmware to cover every MMCONFIG area (each of
which belongs to a PCIe host bridge, which is a platform device) with a
"reserved" type memmap entry. To me this looks like an outlier, although
I *very* vaguely recall that some spec recommended this.

Ah, right, the PCI Firmware Specification says, under "MCFG Table
Description": "*If* the operating system does not natively comprehend
reserving the MMCFG region, the MMCFG region must be reserved by
firmware." (Emphasis mine.) In other words, this is a provision in the
PCI firmware spec so that platform firmware paper over the fact that the
ACPI subsystem in the OS kernel is limited. Linux, given that it
understand MCFG just fine, totally should not *require* the firmware to
reserve this region in the UEFI memory map as well. Anyway, Linux does
require it (incorrectly, in my opinion), so OVMF complies.

(4) To my understanding, unless given Linux or Windows quirks like (3),
or UEFI runtime drivers like (1), we shouldn't expose, in the UEFI
memmap, MMIO areas related to platform devices. In particular, the
"MMIO" type memory *without* the "runtime" attribute (see (1)) should
never be used in the UEFI memmap (and edk2 will never create such
entries) -- the UEFI spec says that all such areas (resources) should be
communicated through ACPI tables ("All system memory-mapped IO
information should come from ACPI tables" -- see EfiMemoryMappedIO in
"Table 26. Memory Type Usage after ExitBootServices()").


In brief, the TPM PPI MMIO area should just be left as a gap in the UEFI
memmap. (Case (4) applies.)

Thanks,
Laszlo

Re: [Qemu-devel] [PATCH v8 2/5] tpm: implement virtual memory device for TPM PPI

[Igor Mammedov]

On Tue, 17 Jul 2018 15:04:25 +0200
Laszlo Ersek <lersek@redhat.com> wrote:

> On 07/17/18 12:39, Marc-André Lureau wrote:
> > Hi
> > 
> > On Tue, Jul 17, 2018 at 12:03 PM, Igor Mammedov <imammedo@redhat.com> wrote:  
> >> On Mon, 16 Jul 2018 14:59:45 +0200
> >> Marc-André Lureau <marcandre.lureau@redhat.com> 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.  
> >> ...
> >> another question wrt E820 table, is firmware putting a reservation record
> >> for this RAM area overlay or should we do it QEMU?  
> 
> What OS requires this memmap entry?
> 
> > There doesn't seem to be any reservation done in ovmf, or on my
> > laptop.  
> 
> Relevant point, about your laptop.
> 
> > I also wonder if it should be done.  
> 
> IMO: no, it shouldn't.
> 
> > And how OS deal with that today (/proc/iomem is correct).
> > 
> > Laszlo, any idea?  
> 
> In my experience, exposing such MMIO areas that belong to *platform*
> devices, in the UEFI memory map, is an inconsistent practice. I can list
> at least four different cases:
> 
> (1) A UEFI runtime driver is using the MMIO area for providing a UEFI
> runtime service. In this case, the firmware must expose the area as
> "MMIO" type memory, and set the "runtime" attribute on it. In response,
> the OS will map the area with virtual addresses, directly in response to
> the memmap entry.
> 
> (2) A firmware SMM driver is using the MMIO area for providing a
> privileged part of a UEFI runtime service. Because SMM uses its own page
> table anyway (protected from the OS), advertizing the area in the UEFI
> memmap is unneeded. It can simply be left as a gap.
> 
> (3) Linux requires the firmware to cover every MMCONFIG area (each of
> which belongs to a PCIe host bridge, which is a platform device) with a
> "reserved" type memmap entry. To me this looks like an outlier, although
> I *very* vaguely recall that some spec recommended this.
> 
> Ah, right, the PCI Firmware Specification says, under "MCFG Table
> Description": "*If* the operating system does not natively comprehend
> reserving the MMCFG region, the MMCFG region must be reserved by
> firmware." (Emphasis mine.) In other words, this is a provision in the
> PCI firmware spec so that platform firmware paper over the fact that the
> ACPI subsystem in the OS kernel is limited. Linux, given that it
> understand MCFG just fine, totally should not *require* the firmware to
> reserve this region in the UEFI memory map as well. Anyway, Linux does
> require it (incorrectly, in my opinion), so OVMF complies.
> 
> (4) To my understanding, unless given Linux or Windows quirks like (3),
> or UEFI runtime drivers like (1), we shouldn't expose, in the UEFI
> memmap, MMIO areas related to platform devices. In particular, the
> "MMIO" type memory *without* the "runtime" attribute (see (1)) should
> never be used in the UEFI memmap (and edk2 will never create such
> entries) -- the UEFI spec says that all such areas (resources) should be
> communicated through ACPI tables ("All system memory-mapped IO
> information should come from ACPI tables" -- see EfiMemoryMappedIO in
> "Table 26. Memory Type Usage after ExitBootServices()").
> 
> 
> In brief, the TPM PPI MMIO area should just be left as a gap in the UEFI
> memmap. (Case (4) applies.)
so I guess we are good as the patch should fit the bill for (4)

> 
> Thanks,
> Laszlo

Re: [Qemu-devel] [PATCH v8 2/5] tpm: implement virtual memory device for TPM PPI

[Igor Mammedov]

On Mon, 16 Jul 2018 14:59:45 +0200
Marc-André Lureau <marcandre.lureau@redhat.com> 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 device should be used by all TPM interfaces on x86 and can be added
> by calling tpm_ppi_init_io().
> 
> Note: bios_linker cannot be used to allocate the PPI memory region,
> since the reserved memory should stay stable across reboots, and might
> be needed before the ACPI tables are installed.
> 
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>

with commit message fixed as suggested earlier

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      | 31 +++++++++++++++++++++++++++++++
>  hw/tpm/tpm_tis.c      |  8 ++++++++
>  hw/tpm/Makefile.objs  |  1 +
>  6 files changed, 80 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..f6458bf87e
> --- /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[TPM_PPI_ADDR_SIZE];
> +} 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..8b46b9dd4b
> --- /dev/null
> +++ b/hw/tpm/tpm_ppi.c
> @@ -0,0 +1,31 @@
> +/*
> + * 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)
> +{
> +    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

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

[Marc-André Lureau]

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

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

Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
[ Marc-André: renamed to etc/tpm/config, made it static, document it ]
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 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 9e8350c55d..b19575681b 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -119,6 +119,12 @@ typedef struct AcpiBuildPciBusHotplugState {
     bool pcihp_bridge_en;
 } AcpiBuildPciBusHotplugState;
 
+typedef struct FWCfgTPMConfig {
+    uint32_t tpmppi_address;
+    uint8_t tpm_version;
+    uint8_t tpmppi_version;
+} QEMU_PACKED FWCfgTPMConfig;
+
 static void init_common_fadt_data(Object *o, AcpiFadtData *data)
 {
     uint32_t io = object_property_get_uint(o, ACPI_PM_PROP_PM_IO_BASE, NULL);
@@ -2873,6 +2879,8 @@ void acpi_setup(void)
     AcpiBuildTables tables;
     AcpiBuildState *build_state;
     Object *vmgenid_dev;
+    TPMIf *tpm;
+    static FWCfgTPMConfig tpm_config;
 
     if (!pcms->fw_cfg) {
         ACPI_BUILD_DPRINTF("No fw cfg. Bailing out.\n");
@@ -2907,6 +2915,17 @@ void acpi_setup(void)
     fw_cfg_add_file(pcms->fw_cfg, ACPI_BUILD_TPMLOG_FILE,
                     tables.tcpalog->data, acpi_data_len(tables.tcpalog));
 
+    tpm = tpm_find();
+    if (tpm && object_property_get_bool(OBJECT(tpm), "ppi", &error_abort)) {
+        tpm_config = (FWCfgTPMConfig) {
+            .tpmppi_address = cpu_to_le32(TPM_PPI_ADDR_BASE),
+            .tpm_version = tpm_get_version(tpm_find()),
+            .tpmppi_version = TPM_PPI_VERSION_NONE
+        };
+        fw_cfg_add_file(pcms->fw_cfg, "etc/tpm/config",
+                        &tpm_config, sizeof tpm_config);
+    }
+
     vmgenid_dev = find_vmgenid_dev();
     if (vmgenid_dev) {
         vmgenid_add_fw_cfg(VMGENID(vmgenid_dev), pcms->fw_cfg,
diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
index 0e9bbebe1d..de3f8bda56 100644
--- a/docs/specs/tpm.txt
+++ b/docs/specs/tpm.txt
@@ -20,6 +20,26 @@ QEMU files related to TPM TIS interface:
  - hw/tpm/tpm_tis.h
 
 
+= fw_cfg interface =
+
+The bios/firmware may use the "etc/tpm/config" fw_cfg entry for
+configuring the guest appropriately.
+
+The entry of 6 bytes has the following content, in little-endian:
+
+    #define TPM_VERSION_UNSPEC          0
+    #define TPM_VERSION_1_2             1
+    #define TPM_VERSION_2_0             2
+
+    #define TPM_PPI_VERSION_NONE        0
+    #define TPM_PPI_VERSION_1_30        1
+
+    struct FWCfgTPMConfig {
+        uint32_t tpmppi_address;         /* PPI memory location */
+        uint8_t tpm_version;             /* TPM version */
+        uint8_t tpmppi_version;          /* PPI version */
+    };
+
 = ACPI Interface =
 
 The TPM device is defined with ACPI ID "PNP0C31". QEMU builds a SSDT and passes
-- 
2.18.0.129.ge3331758f1

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

[Igor Mammedov]

On Mon, 16 Jul 2018 14:59:46 +0200
Marc-André Lureau <marcandre.lureau@redhat.com> 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>
> ---
>  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 9e8350c55d..b19575681b 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -119,6 +119,12 @@ typedef struct AcpiBuildPciBusHotplugState {
>      bool pcihp_bridge_en;
>  } AcpiBuildPciBusHotplugState;
>  
> +typedef struct FWCfgTPMConfig {
> +    uint32_t tpmppi_address;
> +    uint8_t tpm_version;
> +    uint8_t tpmppi_version;
> +} QEMU_PACKED FWCfgTPMConfig;
> +
>  static void init_common_fadt_data(Object *o, AcpiFadtData *data)
>  {
>      uint32_t io = object_property_get_uint(o, ACPI_PM_PROP_PM_IO_BASE, NULL);
> @@ -2873,6 +2879,8 @@ void acpi_setup(void)
>      AcpiBuildTables tables;
>      AcpiBuildState *build_state;
>      Object *vmgenid_dev;
> +    TPMIf *tpm;
> +    static FWCfgTPMConfig tpm_config;
>  
>      if (!pcms->fw_cfg) {
>          ACPI_BUILD_DPRINTF("No fw cfg. Bailing out.\n");
> @@ -2907,6 +2915,17 @@ void acpi_setup(void)
>      fw_cfg_add_file(pcms->fw_cfg, ACPI_BUILD_TPMLOG_FILE,
>                      tables.tcpalog->data, acpi_data_len(tables.tcpalog));
>  
> +    tpm = tpm_find();
> +    if (tpm && object_property_get_bool(OBJECT(tpm), "ppi", &error_abort)) {
> +        tpm_config = (FWCfgTPMConfig) {
> +            .tpmppi_address = cpu_to_le32(TPM_PPI_ADDR_BASE),
> +            .tpm_version = tpm_get_version(tpm_find()),
> +            .tpmppi_version = TPM_PPI_VERSION_NONE
> +        };
> +        fw_cfg_add_file(pcms->fw_cfg, "etc/tpm/config",
> +                        &tpm_config, sizeof tpm_config);
> +    }
> +
>      vmgenid_dev = find_vmgenid_dev();
>      if (vmgenid_dev) {
>          vmgenid_add_fw_cfg(VMGENID(vmgenid_dev), pcms->fw_cfg,
> diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
> index 0e9bbebe1d..de3f8bda56 100644
> --- a/docs/specs/tpm.txt
> +++ b/docs/specs/tpm.txt
> @@ -20,6 +20,26 @@ QEMU files related to TPM TIS interface:
>   - hw/tpm/tpm_tis.h
>  
>  
> += fw_cfg interface =
> +
> +The bios/firmware may use the "etc/tpm/config" fw_cfg entry for
s/use/read/ to make clear fw may not write into it contract

> +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 v8 3/5] acpi: add fw_cfg file for TPM and PPI virtual memory device

[Igor Mammedov]

On Mon, 16 Jul 2018 16:46:59 +0200
Igor Mammedov <imammedo@redhat.com> wrote:

> On Mon, 16 Jul 2018 14:59:46 +0200
> Marc-André Lureau <marcandre.lureau@redhat.com> 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.

subj has nothing to do with acpi anymore, probably  we should change it to:

expose TPM/PPI configuration parameters to firmware via fw_cfg file 

> > 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>
> > ---
> >  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 9e8350c55d..b19575681b 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -119,6 +119,12 @@ typedef struct AcpiBuildPciBusHotplugState {
> >      bool pcihp_bridge_en;
> >  } AcpiBuildPciBusHotplugState;
> >  
> > +typedef struct FWCfgTPMConfig {
> > +    uint32_t tpmppi_address;
> > +    uint8_t tpm_version;
> > +    uint8_t tpmppi_version;
> > +} QEMU_PACKED FWCfgTPMConfig;
> > +
> >  static void init_common_fadt_data(Object *o, AcpiFadtData *data)
> >  {
> >      uint32_t io = object_property_get_uint(o, ACPI_PM_PROP_PM_IO_BASE, NULL);
> > @@ -2873,6 +2879,8 @@ void acpi_setup(void)
> >      AcpiBuildTables tables;
> >      AcpiBuildState *build_state;
> >      Object *vmgenid_dev;
> > +    TPMIf *tpm;
> > +    static FWCfgTPMConfig tpm_config;
> >  
> >      if (!pcms->fw_cfg) {
> >          ACPI_BUILD_DPRINTF("No fw cfg. Bailing out.\n");
> > @@ -2907,6 +2915,17 @@ void acpi_setup(void)
> >      fw_cfg_add_file(pcms->fw_cfg, ACPI_BUILD_TPMLOG_FILE,
> >                      tables.tcpalog->data, acpi_data_len(tables.tcpalog));
> >  
> > +    tpm = tpm_find();
> > +    if (tpm && object_property_get_bool(OBJECT(tpm), "ppi", &error_abort)) {
> > +        tpm_config = (FWCfgTPMConfig) {
> > +            .tpmppi_address = cpu_to_le32(TPM_PPI_ADDR_BASE),
> > +            .tpm_version = tpm_get_version(tpm_find()),
> > +            .tpmppi_version = TPM_PPI_VERSION_NONE
> > +        };
> > +        fw_cfg_add_file(pcms->fw_cfg, "etc/tpm/config",
> > +                        &tpm_config, sizeof tpm_config);
> > +    }
> > +
> >      vmgenid_dev = find_vmgenid_dev();
> >      if (vmgenid_dev) {
> >          vmgenid_add_fw_cfg(VMGENID(vmgenid_dev), pcms->fw_cfg,
> > diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
> > index 0e9bbebe1d..de3f8bda56 100644
> > --- a/docs/specs/tpm.txt
> > +++ b/docs/specs/tpm.txt
> > @@ -20,6 +20,26 @@ QEMU files related to TPM TIS interface:
> >   - hw/tpm/tpm_tis.h
> >  
> >  
> > += fw_cfg interface =
> > +
> > +The bios/firmware may use the "etc/tpm/config" fw_cfg entry for  
> s/use/read/ to make clear fw may not write into it contract
> 
> > +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  
> 
> 

[Qemu-devel] [PATCH v8 4/5] 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>
---
 include/hw/acpi/tpm.h |   8 +
 hw/i386/acpi-build.c  | 393 +++++++++++++++++++++++++++++++++++++++++-
 docs/specs/tpm.txt    |  79 +++++++++
 3 files changed, 477 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 b19575681b..882fc27846 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);
     }
 
@@ -2920,7 +3307,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 de3f8bda56..16b210c1f2 100644
--- a/docs/specs/tpm.txt
+++ b/docs/specs/tpm.txt
@@ -62,6 +62,85 @@ URL:
 
 https://trustedcomputinggroup.org/tcg-acpi-specification/
 
+== ACPI PPI Interface ==
+
+QEMU supports the Physical Presence Interface (PPI) for TPM 1.2 and TPM 2. This
+interface requires ACPI and firmware support. The specification can be found at
+the following URL:
+
+https://trustedcomputinggroup.org/resource/tcg-physical-presence-interface-specification/
+
+PPI enables a system administrator (root) to request a modification to the
+TPM upon reboot. The PPI specification defines the operation requests and the
+actions the firmware has to take. The system administrator passes the operation
+request number to the firmware through an ACPI interface which writes this
+number to a memory location that the firmware knows. Upon reboot, the firmware
+finds the number and sends commands to the the TPM. The firmware writes the TPM
+result code and the operation request number to a memory location that ACPI can
+read from and pass the result on to the administrator.
+
+The PPI specification defines a set of mandatory and optional operations for
+the firmware to implement. The ACPI interface also allows an administrator to
+list the supported operations. In QEMU the ACPI code is generated by QEMU, yet
+the firmware needs to implement support on a per-operations basis, and
+different firmwares may support a different subset. Therefore, QEMU introduces
+the virtual memory device for PPI where the firmware can indicate which
+operations it supports and ACPI can enable the ones that are supported and
+disable all others. This interface lies in main memory and has the following
+layout:
+
+ +----------+--------+--------+-------------------------------------------+
+ |  Field   | Length | Offset | Description                               |
+ +----------+--------+--------+-------------------------------------------+
+ | func     |  0x100 |  0x000 | Firmware sets values for each supported   |
+ |          |        |        | operation. See defined values below.      |
+ +----------+--------+--------+-------------------------------------------+
+ | ppin     |   0x1  |  0x100 | SMI interrupt to use. Set by firmware.    |
+ |          |        |        | Not supported.                            |
+ +----------+--------+--------+-------------------------------------------+
+ | ppip     |   0x4  |  0x101 | ACPI function index to pass to SMM code.  |
+ |          |        |        | Set by ACPI. Not supported.               |
+ +----------+--------+--------+-------------------------------------------+
+ | pprp     |   0x4  |  0x105 | Result of last executed operation. Set by |
+ |          |        |        | firmware. See function index 5 for values.|
+ +----------+--------+--------+-------------------------------------------+
+ | pprq     |   0x4  |  0x109 | Operation request number to execute. See  |
+ |          |        |        | 'Physical Presence Interface Operation    |
+ |          |        |        | Summary' tables in specs. Set by ACPI.    |
+ +----------+--------+--------+-------------------------------------------+
+ | pprm     |   0x4  |  0x10d | Operation request optional parameter.     |
+ |          |        |        | Values depend on operation. Set by ACPI.  |
+ +----------+--------+--------+-------------------------------------------+
+ | lppr     |   0x4  |  0x111 | Last executed operation request number.   |
+ |          |        |        | Copied from pprq field by firmware.       |
+ +----------+--------+--------+-------------------------------------------+
+ | fret     |   0x4  |  0x115 | Result code from SMM function.            |
+ |          |        |        | Not supported.                            |
+ +----------+--------+--------+-------------------------------------------+
+ | res1     |  0x40  |  0x119 | Reserved for future use                   |
+ +----------+--------+--------+-------------------------------------------+
+ | next_step|   0x1  |  0x159 | Operation to execute after reboot by      |
+ |          |        |        | firmware. Used by firmware.               |
+ +----------+--------+--------+-------------------------------------------+
+
+   The following values are supported for the 'func' field. They correspond
+   to the values used by ACPI function index 8.
+
+ +----------+-------------------------------------------------------------+
+ | value    | Description                                                 |
+ +----------+-------------------------------------------------------------+
+ | 0        | Operation is not implemented.                               |
+ +----------+-------------------------------------------------------------+
+ | 1        | Operation is only accessible through firmware.              |
+ +----------+-------------------------------------------------------------+
+ | 2        | Operation is blocked for OS by firmware configuration.      |
+ +----------+-------------------------------------------------------------+
+ | 3        | Operation is allowed and physically present user required.  |
+ +----------+-------------------------------------------------------------+
+ | 4        | Operation is allowed and physically present user is not     |
+ |          | required.                                                   |
+ +----------+-------------------------------------------------------------+
+
 
 QEMU files related to TPM ACPI tables:
  - hw/i386/acpi-build.c
-- 
2.18.0.129.ge3331758f1

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

[Igor Mammedov]

On Mon, 16 Jul 2018 14:59:47 +0200
Marc-André Lureau <marcandre.lureau@redhat.com> wrote:

> From: Stefan Berger <stefanb@linux.vnet.ibm.com>
> 
> The TPM Physical Presence interface consists of an ACPI part, a shared
> memory part, and code in the firmware. Users can send messages to the
> firmware by writing a code into the shared memory through invoking the
> ACPI code. When a reboot happens, the firmware looks for the code and
> acts on it by sending sequences of commands to the TPM.
> 
> This patch adds the ACPI code. It is similar to the one in EDK2 but doesn't
> assume that SMIs are necessary to use. It uses a similar datastructure for
> the shared memory as EDK2 does so that EDK2 and SeaBIOS could both make use
> of it. I extended the shared memory data structure with an array of 256
> bytes, one for each code that could be implemented. The array contains
> flags describing the individual codes. This decouples the ACPI implementation
> from the firmware implementation.
> 
> The underlying TCG specification is accessible from the following page.
> 
> https://trustedcomputinggroup.org/tcg-physical-presence-interface-specification/
> 
> This patch implements version 1.30.
> 
> Signed-off-by: Stefan Berger <stefanb@linux.vnet.ibm.com>
> [ Marc-André - ACPI code improvements and windows fixes ]
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
haven't tested but acpi code looks good to me, hence

Reviewed-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  include/hw/acpi/tpm.h |   8 +
>  hw/i386/acpi-build.c  | 393 +++++++++++++++++++++++++++++++++++++++++-
>  docs/specs/tpm.txt    |  79 +++++++++
>  3 files changed, 477 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 b19575681b..882fc27846 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);
>      }
>  
> @@ -2920,7 +3307,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 de3f8bda56..16b210c1f2 100644
> --- a/docs/specs/tpm.txt
> +++ b/docs/specs/tpm.txt
> @@ -62,6 +62,85 @@ URL:
>  
>  https://trustedcomputinggroup.org/tcg-acpi-specification/
>  
> +== ACPI PPI Interface ==
> +
> +QEMU supports the Physical Presence Interface (PPI) for TPM 1.2 and TPM 2. This
> +interface requires ACPI and firmware support. The specification can be found at
> +the following URL:
> +
> +https://trustedcomputinggroup.org/resource/tcg-physical-presence-interface-specification/
> +
> +PPI enables a system administrator (root) to request a modification to the
> +TPM upon reboot. The PPI specification defines the operation requests and the
> +actions the firmware has to take. The system administrator passes the operation
> +request number to the firmware through an ACPI interface which writes this
> +number to a memory location that the firmware knows. Upon reboot, the firmware
> +finds the number and sends commands to the the TPM. The firmware writes the TPM
> +result code and the operation request number to a memory location that ACPI can
> +read from and pass the result on to the administrator.
> +
> +The PPI specification defines a set of mandatory and optional operations for
> +the firmware to implement. The ACPI interface also allows an administrator to
> +list the supported operations. In QEMU the ACPI code is generated by QEMU, yet
> +the firmware needs to implement support on a per-operations basis, and
> +different firmwares may support a different subset. Therefore, QEMU introduces
> +the virtual memory device for PPI where the firmware can indicate which
> +operations it supports and ACPI can enable the ones that are supported and
> +disable all others. This interface lies in main memory and has the following
> +layout:
> +
> + +----------+--------+--------+-------------------------------------------+
> + |  Field   | Length | Offset | Description                               |
> + +----------+--------+--------+-------------------------------------------+
> + | func     |  0x100 |  0x000 | Firmware sets values for each supported   |
> + |          |        |        | operation. See defined values below.      |
> + +----------+--------+--------+-------------------------------------------+
> + | ppin     |   0x1  |  0x100 | SMI interrupt to use. Set by firmware.    |
> + |          |        |        | Not supported.                            |
> + +----------+--------+--------+-------------------------------------------+
> + | ppip     |   0x4  |  0x101 | ACPI function index to pass to SMM code.  |
> + |          |        |        | Set by ACPI. Not supported.               |
> + +----------+--------+--------+-------------------------------------------+
> + | pprp     |   0x4  |  0x105 | Result of last executed operation. Set by |
> + |          |        |        | firmware. See function index 5 for values.|
> + +----------+--------+--------+-------------------------------------------+
> + | pprq     |   0x4  |  0x109 | Operation request number to execute. See  |
> + |          |        |        | 'Physical Presence Interface Operation    |
> + |          |        |        | Summary' tables in specs. Set by ACPI.    |
> + +----------+--------+--------+-------------------------------------------+
> + | pprm     |   0x4  |  0x10d | Operation request optional parameter.     |
> + |          |        |        | Values depend on operation. Set by ACPI.  |
> + +----------+--------+--------+-------------------------------------------+
> + | lppr     |   0x4  |  0x111 | Last executed operation request number.   |
> + |          |        |        | Copied from pprq field by firmware.       |
> + +----------+--------+--------+-------------------------------------------+
> + | fret     |   0x4  |  0x115 | Result code from SMM function.            |
> + |          |        |        | Not supported.                            |
> + +----------+--------+--------+-------------------------------------------+
> + | res1     |  0x40  |  0x119 | Reserved for future use                   |
> + +----------+--------+--------+-------------------------------------------+
> + | next_step|   0x1  |  0x159 | Operation to execute after reboot by      |
> + |          |        |        | firmware. Used by firmware.               |
> + +----------+--------+--------+-------------------------------------------+
> +
> +   The following values are supported for the 'func' field. They correspond
> +   to the values used by ACPI function index 8.
> +
> + +----------+-------------------------------------------------------------+
> + | value    | Description                                                 |
> + +----------+-------------------------------------------------------------+
> + | 0        | Operation is not implemented.                               |
> + +----------+-------------------------------------------------------------+
> + | 1        | Operation is only accessible through firmware.              |
> + +----------+-------------------------------------------------------------+
> + | 2        | Operation is blocked for OS by firmware configuration.      |
> + +----------+-------------------------------------------------------------+
> + | 3        | Operation is allowed and physically present user required.  |
> + +----------+-------------------------------------------------------------+
> + | 4        | Operation is allowed and physically present user is not     |
> + |          | required.                                                   |
> + +----------+-------------------------------------------------------------+
> +
>  
>  QEMU files related to TPM ACPI tables:
>   - hw/i386/acpi-build.c

[Qemu-devel] [PATCH v8 5/5] PoC: 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. It is relatively simple to implement in QEMU, but
make this a proof-of-concept PATCH for discussion, since this is not
a conventional approach.

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

diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 882fc27846..7e3c217076 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 + 0x200),
+                                    0x1));
+    field = aml_field("TPP3", AML_ANY_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_ppi.c b/hw/tpm/tpm_ppi.c
index 8b46b9dd4b..64e8d828e8 100644
--- a/hw/tpm/tpm_ppi.c
+++ b/hw/tpm/tpm_ppi.c
@@ -16,8 +16,31 @@
 #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"
+
+static void tpm_ppi_reset(void *opaque)
+{
+    TPMPPI *tpmppi = opaque;
+    char *ptr = memory_region_get_ram_ptr(&tpmppi->ram);
+
+    if (ptr[0x200] & 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);
+        }
+        guest_phys_blocks_free(&guest_phys_blocks);
+    }
+}
 
 bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
                   hwaddr addr, Object *obj, Error **errp)
@@ -27,5 +50,7 @@ bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
     vmstate_register_ram(&tpmppi->ram, DEVICE(obj));
 
     memory_region_add_subregion(m, addr, &tpmppi->ram);
+    qemu_register_reset(tpm_ppi_reset, tpmppi);
+
     return true;
 }
diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
index 16b210c1f2..9c44384315 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  |  0x200 | 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.18.0.129.ge3331758f1

Re: [Qemu-devel] [PATCH v8 5/5] PoC: tpm: add ACPI memory clear interface

[Igor Mammedov]

On Mon, 16 Jul 2018 14:59:48 +0200
Marc-André Lureau <marcandre.lureau@redhat.com> 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. It is relatively simple to implement in QEMU, but
> make this a proof-of-concept PATCH for discussion, since this is not
> a conventional approach.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  hw/i386/acpi-build.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
>  hw/tpm/tpm_ppi.c     | 25 ++++++++++++++++++++++++
>  docs/specs/tpm.txt   |  2 ++
>  hw/tpm/trace-events  |  3 +++
>  4 files changed, 76 insertions(+)
> 
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 882fc27846..7e3c217076 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 + 0x200),
> +                                    0x1));
> +    field = aml_field("TPP3", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
                                 ^^^ AML_BYTE_ACC

> +    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_ppi.c b/hw/tpm/tpm_ppi.c
> index 8b46b9dd4b..64e8d828e8 100644
> --- a/hw/tpm/tpm_ppi.c
> +++ b/hw/tpm/tpm_ppi.c
> @@ -16,8 +16,31 @@
>  #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"
> +
> +static void tpm_ppi_reset(void *opaque)
> +{
> +    TPMPPI *tpmppi = opaque;
> +    char *ptr = memory_region_get_ram_ptr(&tpmppi->ram);
> +
> +    if (ptr[0x200] & 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);
> +        }
> +        guest_phys_blocks_free(&guest_phys_blocks);
> +    }
> +}
>  
>  bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
>                    hwaddr addr, Object *obj, Error **errp)
> @@ -27,5 +50,7 @@ bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
>      vmstate_register_ram(&tpmppi->ram, DEVICE(obj));
>  
>      memory_region_add_subregion(m, addr, &tpmppi->ram);
> +    qemu_register_reset(tpm_ppi_reset, tpmppi);
is it necessary,
shouldn't it be reset by bus initiated reset?

> +
>      return true;
>  }
> diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
> index 16b210c1f2..9c44384315 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  |  0x200 | 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"

Re: [Qemu-devel] [PATCH v8 5/5] PoC: tpm: add ACPI memory clear interface

[Marc-André Lureau]

Hi

On Tue, Jul 17, 2018 at 9:57 AM, Igor Mammedov <imammedo@redhat.com> wrote:
> On Mon, 16 Jul 2018 14:59:48 +0200
> Marc-André Lureau <marcandre.lureau@redhat.com> 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. It is relatively simple to implement in QEMU, but
>> make this a proof-of-concept PATCH for discussion, since this is not
>> a conventional approach.
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
>>  hw/i386/acpi-build.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
>>  hw/tpm/tpm_ppi.c     | 25 ++++++++++++++++++++++++
>>  docs/specs/tpm.txt   |  2 ++
>>  hw/tpm/trace-events  |  3 +++
>>  4 files changed, 76 insertions(+)
>>
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index 882fc27846..7e3c217076 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 + 0x200),
>> +                                    0x1));
>> +    field = aml_field("TPP3", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
>                                  ^^^ AML_BYTE_ACC

ok


>
>> +    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_ppi.c b/hw/tpm/tpm_ppi.c
>> index 8b46b9dd4b..64e8d828e8 100644
>> --- a/hw/tpm/tpm_ppi.c
>> +++ b/hw/tpm/tpm_ppi.c
>> @@ -16,8 +16,31 @@
>>  #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"
>> +
>> +static void tpm_ppi_reset(void *opaque)
>> +{
>> +    TPMPPI *tpmppi = opaque;
>> +    char *ptr = memory_region_get_ram_ptr(&tpmppi->ram);
>> +
>> +    if (ptr[0x200] & 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);
>> +        }
>> +        guest_phys_blocks_free(&guest_phys_blocks);
>> +    }
>> +}
>>
>>  bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
>>                    hwaddr addr, Object *obj, Error **errp)
>> @@ -27,5 +50,7 @@ bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
>>      vmstate_register_ram(&tpmppi->ram, DEVICE(obj));
>>
>>      memory_region_add_subregion(m, addr, &tpmppi->ram);
>> +    qemu_register_reset(tpm_ppi_reset, tpmppi);
> is it necessary,
> shouldn't it be reset by bus initiated reset?

There is no bus for CRB device (we could eventually move it to SYSBUS,
but there was some complication with piix iirc).

Other than that, the role for BusClass.reset() isn't so clear to me,
but it seems to be linked with the same qemu reset handler.

>
>> +
>>      return true;
>>  }
>> diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
>> index 16b210c1f2..9c44384315 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  |  0x200 | 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"
>
>



-- 
Marc-André Lureau

Re: [Qemu-devel] [PATCH v8 5/5] PoC: tpm: add ACPI memory clear interface

[Igor Mammedov]

On Tue, 17 Jul 2018 17:39:12 +0200
Marc-André Lureau <marcandre.lureau@gmail.com> wrote:

> Hi
> 
> On Tue, Jul 17, 2018 at 9:57 AM, Igor Mammedov <imammedo@redhat.com> wrote:
> > On Mon, 16 Jul 2018 14:59:48 +0200
> > Marc-André Lureau <marcandre.lureau@redhat.com> 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. It is relatively simple to implement in QEMU, but
> >> make this a proof-of-concept PATCH for discussion, since this is not
> >> a conventional approach.
> >>
> >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> >> ---
> >>  hw/i386/acpi-build.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
> >>  hw/tpm/tpm_ppi.c     | 25 ++++++++++++++++++++++++
> >>  docs/specs/tpm.txt   |  2 ++
> >>  hw/tpm/trace-events  |  3 +++
> >>  4 files changed, 76 insertions(+)
> >>
> >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> >> index 882fc27846..7e3c217076 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 + 0x200),
> >> +                                    0x1));
> >> +    field = aml_field("TPP3", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);  
> >                                  ^^^ AML_BYTE_ACC  
> 
> ok
> 
> 
> >  
> >> +    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_ppi.c b/hw/tpm/tpm_ppi.c
> >> index 8b46b9dd4b..64e8d828e8 100644
> >> --- a/hw/tpm/tpm_ppi.c
> >> +++ b/hw/tpm/tpm_ppi.c
> >> @@ -16,8 +16,31 @@
> >>  #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"
> >> +
> >> +static void tpm_ppi_reset(void *opaque)
> >> +{
> >> +    TPMPPI *tpmppi = opaque;
> >> +    char *ptr = memory_region_get_ram_ptr(&tpmppi->ram);
> >> +
> >> +    if (ptr[0x200] & 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);
> >> +        }
> >> +        guest_phys_blocks_free(&guest_phys_blocks);
> >> +    }
> >> +}
> >>
> >>  bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
> >>                    hwaddr addr, Object *obj, Error **errp)
> >> @@ -27,5 +50,7 @@ bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
> >>      vmstate_register_ram(&tpmppi->ram, DEVICE(obj));
> >>
> >>      memory_region_add_subregion(m, addr, &tpmppi->ram);
> >> +    qemu_register_reset(tpm_ppi_reset, tpmppi);  
> > is it necessary,
> > shouldn't it be reset by bus initiated reset?  
> 
> There is no bus for CRB device (we could eventually move it to SYSBUS,
> but there was some complication with piix iirc).
> 
> Other than that, the role for BusClass.reset() isn't so clear to me,
> but it seems to be linked with the same qemu reset handler.

I'd suggest to drop qemu_register_reset() here, export tpm_ppi_reset()
and call it explicitly from tpm_crb_reset() and tpm_tis_reset() if
ppi is enabled.


> >> +
> >>      return true;
> >>  }
> >> diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
> >> index 16b210c1f2..9c44384315 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  |  0x200 | 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"  
> >
> >  
> 
> 
> 

Re: [Qemu-devel] [PATCH v8 5/5] PoC: tpm: add ACPI memory clear interface

[Marc-André Lureau]

Hi

On Mon, Jul 23, 2018 at 1:08 PM, Igor Mammedov <imammedo@redhat.com> wrote:
> On Tue, 17 Jul 2018 17:39:12 +0200
> Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
>
>> Hi
>>
>> On Tue, Jul 17, 2018 at 9:57 AM, Igor Mammedov <imammedo@redhat.com> wrote:
>> > On Mon, 16 Jul 2018 14:59:48 +0200
>> > Marc-André Lureau <marcandre.lureau@redhat.com> 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. It is relatively simple to implement in QEMU, but
>> >> make this a proof-of-concept PATCH for discussion, since this is not
>> >> a conventional approach.
>> >>
>> >> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> >> ---
>> >>  hw/i386/acpi-build.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
>> >>  hw/tpm/tpm_ppi.c     | 25 ++++++++++++++++++++++++
>> >>  docs/specs/tpm.txt   |  2 ++
>> >>  hw/tpm/trace-events  |  3 +++
>> >>  4 files changed, 76 insertions(+)
>> >>
>> >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> >> index 882fc27846..7e3c217076 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 + 0x200),
>> >> +                                    0x1));
>> >> +    field = aml_field("TPP3", AML_ANY_ACC, AML_NOLOCK, AML_PRESERVE);
>> >                                  ^^^ AML_BYTE_ACC
>>
>> ok
>>
>>
>> >
>> >> +    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_ppi.c b/hw/tpm/tpm_ppi.c
>> >> index 8b46b9dd4b..64e8d828e8 100644
>> >> --- a/hw/tpm/tpm_ppi.c
>> >> +++ b/hw/tpm/tpm_ppi.c
>> >> @@ -16,8 +16,31 @@
>> >>  #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"
>> >> +
>> >> +static void tpm_ppi_reset(void *opaque)
>> >> +{
>> >> +    TPMPPI *tpmppi = opaque;
>> >> +    char *ptr = memory_region_get_ram_ptr(&tpmppi->ram);
>> >> +
>> >> +    if (ptr[0x200] & 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);
>> >> +        }
>> >> +        guest_phys_blocks_free(&guest_phys_blocks);
>> >> +    }
>> >> +}
>> >>
>> >>  bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
>> >>                    hwaddr addr, Object *obj, Error **errp)
>> >> @@ -27,5 +50,7 @@ bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
>> >>      vmstate_register_ram(&tpmppi->ram, DEVICE(obj));
>> >>
>> >>      memory_region_add_subregion(m, addr, &tpmppi->ram);
>> >> +    qemu_register_reset(tpm_ppi_reset, tpmppi);
>> > is it necessary,
>> > shouldn't it be reset by bus initiated reset?
>>
>> There is no bus for CRB device (we could eventually move it to SYSBUS,
>> but there was some complication with piix iirc).
>>
>> Other than that, the role for BusClass.reset() isn't so clear to me,
>> but it seems to be linked with the same qemu reset handler.
>
> I'd suggest to drop qemu_register_reset() here, export tpm_ppi_reset()
> and call it explicitly from tpm_crb_reset() and tpm_tis_reset() if
> ppi is enabled.
>

should be fine as well

thanks

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

[Michael S. Tsirkin]

On Mon, Jul 16, 2018 at 02:59:43PM +0200, 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.

ACPI bits:

Acked-by: Michael S. Tsirkin <mst@redhat.com>


> v8: ACPI code improvements
> - replace various aml_int() with variables
> - make op/op_flags common variables
> - replace the switch field indexing hack by a dynamic operation region
> 
> v7:
> - relace RAM with RAM device ptr
> - a few ACPI code & comments improvements
> - add back proof-of-concept ACPI memory clear interface, handled in
>   qemu (pass again WHLK TCG tests)
> 
> Marc-André Lureau (2):
>   tpm: add a "ppi" boolean property
>   PoC: tpm: add ACPI memory clear interface
> 
> Stefan Berger (3):
>   tpm: implement virtual memory device for TPM PPI
>   acpi: add fw_cfg file for TPM and PPI virtual memory device
>   acpi: build TPM Physical Presence interface
> 
>  hw/tpm/tpm_ppi.h      |  26 +++
>  include/hw/acpi/tpm.h |  17 ++
>  include/hw/compat.h   |  10 +
>  hw/i386/acpi-build.c  | 456 +++++++++++++++++++++++++++++++++++++++++-
>  hw/tpm/tpm_crb.c      |  11 +
>  hw/tpm/tpm_ppi.c      |  56 ++++++
>  hw/tpm/tpm_tis.c      |  11 +
>  docs/specs/tpm.txt    | 101 ++++++++++
>  hw/tpm/Makefile.objs  |   1 +
>  hw/tpm/trace-events   |   3 +
>  10 files changed, 690 insertions(+), 2 deletions(-)
>  create mode 100644 hw/tpm/tpm_ppi.h
>  create mode 100644 hw/tpm/tpm_ppi.c
> 
> -- 
> 2.18.0.129.ge3331758f1