This allows to pass the last failing test from the Windows HLK TPM 2.0
TCG PPI 1.3 tests.
The interface is described in the "TCG Platform Reset Attack
Mitigation Specification", chapter 6 "ACPI _DSM Function". According
to Laszlo, it's not so easy to implement in OVMF, he suggested to do
it in qemu instead.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
hw/tpm/tpm_ppi.h | 2 ++
hw/i386/acpi-build.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
hw/tpm/tpm_crb.c | 1 +
hw/tpm/tpm_ppi.c | 22 +++++++++++++++++++++
hw/tpm/tpm_tis.c | 1 +
docs/specs/tpm.txt | 2 ++
hw/tpm/trace-events | 3 +++
7 files changed, 77 insertions(+)
diff --git a/hw/tpm/tpm_ppi.h b/hw/tpm/tpm_ppi.h
index c2ab2ed300..b8f67962c7 100644
--- a/hw/tpm/tpm_ppi.h
+++ b/hw/tpm/tpm_ppi.h
@@ -23,4 +23,6 @@ typedef struct TPMPPI {
bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
hwaddr addr, Object *obj, Error **errp);
+void tpm_ppi_reset(TPMPPI *tpmppi);
+
#endif /* TPM_TPM_PPI_H */
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index c5e9a6e11d..2ab3e8fae7 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1824,6 +1824,13 @@ build_tpm_ppi(TPMIf *tpm, Aml *dev)
pprq = aml_name("PPRQ");
pprm = aml_name("PPRM");
+ aml_append(dev,
+ aml_operation_region("TPP3", AML_SYSTEM_MEMORY,
+ aml_int(TPM_PPI_ADDR_BASE + 0x15a),
+ 0x1));
+ field = aml_field("TPP3", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
+ aml_append(field, aml_named_field("MOVV", 8));
+ aml_append(dev, field);
/*
* DerefOf in Windows is broken with SYSTEM_MEMORY. Use a dynamic
* operation region inside of a method for getting FUNC[op].
@@ -2166,7 +2173,46 @@ build_tpm_ppi(TPMIf *tpm, Aml *dev)
aml_append(ifctx, aml_return(aml_buffer(1, zerobyte)));
}
aml_append(method, ifctx);
+
+ ifctx = aml_if(
+ aml_equal(uuid,
+ aml_touuid("376054ED-CC13-4675-901C-4756D7F2D45D")));
+ {
+ /* standard DSM query function */
+ ifctx2 = aml_if(aml_equal(function, zero));
+ {
+ uint8_t byte_list[1] = { 0x03 };
+ aml_append(ifctx2, aml_return(aml_buffer(1, byte_list)));
+ }
+ aml_append(ifctx, ifctx2);
+
+ /*
+ * TCG Platform Reset Attack Mitigation Specification 1.0 Ch.6
+ *
+ * Arg 2 (Integer): Function Index = 1
+ * Arg 3 (Package): Arguments = Package: Type: Integer
+ * Operation Value of the Request
+ * Returns: Type: Integer
+ * 0: Success
+ * 1: General Failure
+ */
+ ifctx2 = aml_if(aml_equal(function, one));
+ {
+ aml_append(ifctx2,
+ aml_store(aml_derefof(aml_index(arguments, zero)),
+ op));
+ {
+ aml_append(ifctx2, aml_store(op, aml_name("MOVV")));
+
+ /* 0: success */
+ aml_append(ifctx2, aml_return(zero));
+ }
+ }
+ aml_append(ifctx, ifctx2);
+ }
+ aml_append(method, ifctx);
}
+
aml_append(dev, method);
}
diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
index b243222fd6..48f6a716ad 100644
--- a/hw/tpm/tpm_crb.c
+++ b/hw/tpm/tpm_crb.c
@@ -233,6 +233,7 @@ static void tpm_crb_reset(void *dev)
{
CRBState *s = CRB(dev);
+ tpm_ppi_reset(&s->ppi);
tpm_backend_reset(s->tpmbe);
memset(s->regs, 0, sizeof(s->regs));
diff --git a/hw/tpm/tpm_ppi.c b/hw/tpm/tpm_ppi.c
index f2f07f895e..ac05ba8d3c 100644
--- a/hw/tpm/tpm_ppi.c
+++ b/hw/tpm/tpm_ppi.c
@@ -16,8 +16,30 @@
#include "qapi/error.h"
#include "cpu.h"
#include "sysemu/memory_mapping.h"
+#include "sysemu/reset.h"
#include "migration/vmstate.h"
#include "tpm_ppi.h"
+#include "trace.h"
+
+void tpm_ppi_reset(TPMPPI *tpmppi)
+{
+ if (tpmppi->buf[0x15a] & 0x1) {
+ GuestPhysBlockList guest_phys_blocks;
+ GuestPhysBlock *block;
+
+ guest_phys_blocks_init(&guest_phys_blocks);
+ guest_phys_blocks_append(&guest_phys_blocks);
+ QTAILQ_FOREACH(block, &guest_phys_blocks.head, next) {
+ trace_tpm_ppi_memset(block->host_addr,
+ block->target_end - block->target_start);
+ memset(block->host_addr, 0,
+ block->target_end - block->target_start);
+ memory_region_set_dirty(block->mr, 0,
+ block->target_end - block->target_start);
+ }
+ guest_phys_blocks_free(&guest_phys_blocks);
+ }
+}
bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
hwaddr addr, Object *obj, Error **errp)
diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
index 70432ffe8b..d9bfa956cc 100644
--- a/hw/tpm/tpm_tis.c
+++ b/hw/tpm/tpm_tis.c
@@ -868,6 +868,7 @@ static void tpm_tis_reset(DeviceState *dev)
s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->be_driver),
TPM_TIS_BUFFER_MAX);
+ tpm_ppi_reset(&s->ppi);
tpm_backend_reset(s->be_driver);
s->active_locty = TPM_TIS_NO_LOCALITY;
diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
index 332c2ae597..ce9bda3c89 100644
--- a/docs/specs/tpm.txt
+++ b/docs/specs/tpm.txt
@@ -121,6 +121,8 @@ layout:
+----------+--------+--------+-------------------------------------------+
| next_step| 0x1 | 0x159 | Operation to execute after reboot by |
| | | | firmware. Used by firmware. |
+ +----------+--------+--------+-------------------------------------------+
+ | movv | 0x1 | 0x15a | Memory overwrite variable |
+----------+--------+--------+-------------------------------------------+
The following values are supported for the 'func' field. They correspond
diff --git a/hw/tpm/trace-events b/hw/tpm/trace-events
index 25bee0cecf..920d32ad55 100644
--- a/hw/tpm/trace-events
+++ b/hw/tpm/trace-events
@@ -51,3 +51,6 @@ tpm_tis_mmio_write_init_abort(void) "Initiating abort"
tpm_tis_mmio_write_lowering_irq(void) "Lowering IRQ"
tpm_tis_mmio_write_data2send(uint32_t value, unsigned size) "Data to send to TPM: 0x%08x (size=%d)"
tpm_tis_pre_save(uint8_t locty, uint32_t rw_offset) "locty: %d, rw_offset = %u"
+
+# hw/tpm/tpm_ppi.c
+tpm_ppi_memset(uint8_t *ptr, size_t size) "memset: %p %zu"
--
2.19.0.rc1
On 9/10/18 10:32 AM, Marc-André Lureau wrote:
> This allows to pass the last failing test from the Windows HLK TPM 2.0
> TCG PPI 1.3 tests.
>
> The interface is described in the "TCG Platform Reset Attack
> Mitigation Specification", chapter 6 "ACPI _DSM Function". According
> to Laszlo, it's not so easy to implement in OVMF, he suggested to do
> it in qemu instead.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> hw/tpm/tpm_ppi.h | 2 ++
> hw/i386/acpi-build.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
> hw/tpm/tpm_crb.c | 1 +
> hw/tpm/tpm_ppi.c | 22 +++++++++++++++++++++
> hw/tpm/tpm_tis.c | 1 +
> docs/specs/tpm.txt | 2 ++
> hw/tpm/trace-events | 3 +++
> 7 files changed, 77 insertions(+)
>
> diff --git a/hw/tpm/tpm_ppi.h b/hw/tpm/tpm_ppi.h
> index c2ab2ed300..b8f67962c7 100644
> --- a/hw/tpm/tpm_ppi.h
> +++ b/hw/tpm/tpm_ppi.h
> @@ -23,4 +23,6 @@ typedef struct TPMPPI {
> bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
> hwaddr addr, Object *obj, Error **errp);
>
Can you add documentation?
> +void tpm_ppi_reset(TPMPPI *tpmppi);
> +
> #endif /* TPM_TPM_PPI_H */
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index c5e9a6e11d..2ab3e8fae7 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1824,6 +1824,13 @@ build_tpm_ppi(TPMIf *tpm, Aml *dev)
> pprq = aml_name("PPRQ");
> pprm = aml_name("PPRM");
>
> + aml_append(dev,
> + aml_operation_region("TPP3", AML_SYSTEM_MEMORY,
> + aml_int(TPM_PPI_ADDR_BASE + 0x15a),
Can you add a definition for this magic value?
Maybe:
#define TPM_PPI_MEMORY_OVERWRITE_OFFSET 0x15a
or
#define TPM_PPI_MOVV_OFFSET 0x15a
> + 0x1));
> + field = aml_field("TPP3", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
> + aml_append(field, aml_named_field("MOVV", 8));
> + aml_append(dev, field);
> /*
> * DerefOf in Windows is broken with SYSTEM_MEMORY. Use a dynamic
> * operation region inside of a method for getting FUNC[op].
> @@ -2166,7 +2173,46 @@ build_tpm_ppi(TPMIf *tpm, Aml *dev)
> aml_append(ifctx, aml_return(aml_buffer(1, zerobyte)));
> }
> aml_append(method, ifctx);
> +
> + ifctx = aml_if(
> + aml_equal(uuid,
> + aml_touuid("376054ED-CC13-4675-901C-4756D7F2D45D")));
> + {
> + /* standard DSM query function */
> + ifctx2 = aml_if(aml_equal(function, zero));
> + {
> + uint8_t byte_list[1] = { 0x03 };
> + aml_append(ifctx2, aml_return(aml_buffer(1, byte_list)));
> + }
> + aml_append(ifctx, ifctx2);
> +
> + /*
> + * TCG Platform Reset Attack Mitigation Specification 1.0 Ch.6
> + *
> + * Arg 2 (Integer): Function Index = 1
> + * Arg 3 (Package): Arguments = Package: Type: Integer
> + * Operation Value of the Request
> + * Returns: Type: Integer
> + * 0: Success
> + * 1: General Failure
> + */
> + ifctx2 = aml_if(aml_equal(function, one));
> + {
> + aml_append(ifctx2,
> + aml_store(aml_derefof(aml_index(arguments, zero)),
> + op));
> + {
> + aml_append(ifctx2, aml_store(op, aml_name("MOVV")));
> +
> + /* 0: success */
> + aml_append(ifctx2, aml_return(zero));
> + }
> + }
> + aml_append(ifctx, ifctx2);
> + }
> + aml_append(method, ifctx);
> }
> +
> aml_append(dev, method);
> }
>
> diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> index b243222fd6..48f6a716ad 100644
> --- a/hw/tpm/tpm_crb.c
> +++ b/hw/tpm/tpm_crb.c
> @@ -233,6 +233,7 @@ static void tpm_crb_reset(void *dev)
> {
> CRBState *s = CRB(dev);
>
> + tpm_ppi_reset(&s->ppi);
> tpm_backend_reset(s->tpmbe);
>
> memset(s->regs, 0, sizeof(s->regs));
> diff --git a/hw/tpm/tpm_ppi.c b/hw/tpm/tpm_ppi.c
> index f2f07f895e..ac05ba8d3c 100644
> --- a/hw/tpm/tpm_ppi.c
> +++ b/hw/tpm/tpm_ppi.c
> @@ -16,8 +16,30 @@
> #include "qapi/error.h"
> #include "cpu.h"
> #include "sysemu/memory_mapping.h"
> +#include "sysemu/reset.h"
> #include "migration/vmstate.h"
> #include "tpm_ppi.h"
> +#include "trace.h"
> +
> +void tpm_ppi_reset(TPMPPI *tpmppi)
> +{
> + if (tpmppi->buf[0x15a] & 0x1) {
TPM_PPI_MOVV_OFFSET
> + GuestPhysBlockList guest_phys_blocks;
> + GuestPhysBlock *block;
> +
> + guest_phys_blocks_init(&guest_phys_blocks);
> + guest_phys_blocks_append(&guest_phys_blocks);
> + QTAILQ_FOREACH(block, &guest_phys_blocks.head, next) {
> + trace_tpm_ppi_memset(block->host_addr,
> + block->target_end - block->target_start);
> + memset(block->host_addr, 0,
> + block->target_end - block->target_start);
> + memory_region_set_dirty(block->mr, 0,
> + block->target_end - block->target_start);
> + }
> + guest_phys_blocks_free(&guest_phys_blocks);
> + }
> +}
>
> bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
> hwaddr addr, Object *obj, Error **errp)
> diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
> index 70432ffe8b..d9bfa956cc 100644
> --- a/hw/tpm/tpm_tis.c
> +++ b/hw/tpm/tpm_tis.c
> @@ -868,6 +868,7 @@ static void tpm_tis_reset(DeviceState *dev)
> s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->be_driver),
> TPM_TIS_BUFFER_MAX);
>
> + tpm_ppi_reset(&s->ppi);
> tpm_backend_reset(s->be_driver);
>
> s->active_locty = TPM_TIS_NO_LOCALITY;
> diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
> index 332c2ae597..ce9bda3c89 100644
> --- a/docs/specs/tpm.txt
> +++ b/docs/specs/tpm.txt
> @@ -121,6 +121,8 @@ layout:
> +----------+--------+--------+-------------------------------------------+
> | next_step| 0x1 | 0x159 | Operation to execute after reboot by |
> | | | | firmware. Used by firmware. |
> + +----------+--------+--------+-------------------------------------------+
> + | movv | 0x1 | 0x15a | Memory overwrite variable |
> +----------+--------+--------+-------------------------------------------+
>
> The following values are supported for the 'func' field. They correspond
> diff --git a/hw/tpm/trace-events b/hw/tpm/trace-events
> index 25bee0cecf..920d32ad55 100644
> --- a/hw/tpm/trace-events
> +++ b/hw/tpm/trace-events
> @@ -51,3 +51,6 @@ tpm_tis_mmio_write_init_abort(void) "Initiating abort"
> tpm_tis_mmio_write_lowering_irq(void) "Lowering IRQ"
> tpm_tis_mmio_write_data2send(uint32_t value, unsigned size) "Data to send to TPM: 0x%08x (size=%d)"
> tpm_tis_pre_save(uint8_t locty, uint32_t rw_offset) "locty: %d, rw_offset = %u"
> +
> +# hw/tpm/tpm_ppi.c
> +tpm_ppi_memset(uint8_t *ptr, size_t size) "memset: %p %zu"
>
I'd split this patch in 2: Add ACPI, Add tpm_ppi_reset.
On Wed, 12 Dec 2018 17:22:21 +0100
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> On 9/10/18 10:32 AM, Marc-André Lureau wrote:
> > This allows to pass the last failing test from the Windows HLK TPM 2.0
> > TCG PPI 1.3 tests.
> >
> > The interface is described in the "TCG Platform Reset Attack
> > Mitigation Specification", chapter 6 "ACPI _DSM Function". According
> > to Laszlo, it's not so easy to implement in OVMF, he suggested to do
> > it in qemu instead.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> > hw/tpm/tpm_ppi.h | 2 ++
> > hw/i386/acpi-build.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
> > hw/tpm/tpm_crb.c | 1 +
> > hw/tpm/tpm_ppi.c | 22 +++++++++++++++++++++
> > hw/tpm/tpm_tis.c | 1 +
> > docs/specs/tpm.txt | 2 ++
> > hw/tpm/trace-events | 3 +++
> > 7 files changed, 77 insertions(+)
> >
> > diff --git a/hw/tpm/tpm_ppi.h b/hw/tpm/tpm_ppi.h
> > index c2ab2ed300..b8f67962c7 100644
> > --- a/hw/tpm/tpm_ppi.h
> > +++ b/hw/tpm/tpm_ppi.h
> > @@ -23,4 +23,6 @@ typedef struct TPMPPI {
> > bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
> > hwaddr addr, Object *obj, Error **errp);
> >
>
> Can you add documentation?
>
> > +void tpm_ppi_reset(TPMPPI *tpmppi);
> > +
> > #endif /* TPM_TPM_PPI_H */
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index c5e9a6e11d..2ab3e8fae7 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -1824,6 +1824,13 @@ build_tpm_ppi(TPMIf *tpm, Aml *dev)
> > pprq = aml_name("PPRQ");
> > pprm = aml_name("PPRM");
> >
> > + aml_append(dev,
> > + aml_operation_region("TPP3", AML_SYSTEM_MEMORY,
> > + aml_int(TPM_PPI_ADDR_BASE + 0x15a),
>
> Can you add a definition for this magic value?
> Maybe:
>
> #define TPM_PPI_MEMORY_OVERWRITE_OFFSET 0x15a
>
> or
>
> #define TPM_PPI_MOVV_OFFSET 0x15a
I'd prefer explicit numeric values there as it's a bit easier to review
and grep in 'spec' since one doesn't have to jump to define definition
first to find out value to compare with spec.
What I'd add though is verbatim comment from spec like we do
in APCI code so we could jump to the field of interest in spec
just doing copy&paste&search
>
> > + 0x1));
> > + field = aml_field("TPP3", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
> > + aml_append(field, aml_named_field("MOVV", 8));
> > + aml_append(dev, field);
> > /*
> > * DerefOf in Windows is broken with SYSTEM_MEMORY. Use a dynamic
> > * operation region inside of a method for getting FUNC[op].
> > @@ -2166,7 +2173,46 @@ build_tpm_ppi(TPMIf *tpm, Aml *dev)
> > aml_append(ifctx, aml_return(aml_buffer(1, zerobyte)));
> > }
> > aml_append(method, ifctx);
> > +
> > + ifctx = aml_if(
> > + aml_equal(uuid,
> > + aml_touuid("376054ED-CC13-4675-901C-4756D7F2D45D")));
> > + {
> > + /* standard DSM query function */
> > + ifctx2 = aml_if(aml_equal(function, zero));
> > + {
> > + uint8_t byte_list[1] = { 0x03 };
> > + aml_append(ifctx2, aml_return(aml_buffer(1, byte_list)));
> > + }
> > + aml_append(ifctx, ifctx2);
> > +
> > + /*
> > + * TCG Platform Reset Attack Mitigation Specification 1.0 Ch.6
> > + *
> > + * Arg 2 (Integer): Function Index = 1
> > + * Arg 3 (Package): Arguments = Package: Type: Integer
> > + * Operation Value of the Request
> > + * Returns: Type: Integer
> > + * 0: Success
> > + * 1: General Failure
> > + */
> > + ifctx2 = aml_if(aml_equal(function, one));
> > + {
> > + aml_append(ifctx2,
> > + aml_store(aml_derefof(aml_index(arguments, zero)),
> > + op));
> > + {
> > + aml_append(ifctx2, aml_store(op, aml_name("MOVV")));
> > +
> > + /* 0: success */
> > + aml_append(ifctx2, aml_return(zero));
> > + }
> > + }
> > + aml_append(ifctx, ifctx2);
> > + }
> > + aml_append(method, ifctx);
> > }
> > +
> > aml_append(dev, method);
> > }
> >
> > diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> > index b243222fd6..48f6a716ad 100644
> > --- a/hw/tpm/tpm_crb.c
> > +++ b/hw/tpm/tpm_crb.c
> > @@ -233,6 +233,7 @@ static void tpm_crb_reset(void *dev)
> > {
> > CRBState *s = CRB(dev);
> >
> > + tpm_ppi_reset(&s->ppi);
> > tpm_backend_reset(s->tpmbe);
> >
> > memset(s->regs, 0, sizeof(s->regs));
> > diff --git a/hw/tpm/tpm_ppi.c b/hw/tpm/tpm_ppi.c
> > index f2f07f895e..ac05ba8d3c 100644
> > --- a/hw/tpm/tpm_ppi.c
> > +++ b/hw/tpm/tpm_ppi.c
> > @@ -16,8 +16,30 @@
> > #include "qapi/error.h"
> > #include "cpu.h"
> > #include "sysemu/memory_mapping.h"
> > +#include "sysemu/reset.h"
> > #include "migration/vmstate.h"
> > #include "tpm_ppi.h"
> > +#include "trace.h"
> > +
> > +void tpm_ppi_reset(TPMPPI *tpmppi)
> > +{
> > + if (tpmppi->buf[0x15a] & 0x1) {
>
> TPM_PPI_MOVV_OFFSET
>
> > + GuestPhysBlockList guest_phys_blocks;
> > + GuestPhysBlock *block;
> > +
> > + guest_phys_blocks_init(&guest_phys_blocks);
> > + guest_phys_blocks_append(&guest_phys_blocks);
> > + QTAILQ_FOREACH(block, &guest_phys_blocks.head, next) {
> > + trace_tpm_ppi_memset(block->host_addr,
> > + block->target_end - block->target_start);
> > + memset(block->host_addr, 0,
> > + block->target_end - block->target_start);
> > + memory_region_set_dirty(block->mr, 0,
> > + block->target_end - block->target_start);
> > + }
> > + guest_phys_blocks_free(&guest_phys_blocks);
> > + }
> > +}
> >
> > bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
> > hwaddr addr, Object *obj, Error **errp)
> > diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
> > index 70432ffe8b..d9bfa956cc 100644
> > --- a/hw/tpm/tpm_tis.c
> > +++ b/hw/tpm/tpm_tis.c
> > @@ -868,6 +868,7 @@ static void tpm_tis_reset(DeviceState *dev)
> > s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->be_driver),
> > TPM_TIS_BUFFER_MAX);
> >
> > + tpm_ppi_reset(&s->ppi);
> > tpm_backend_reset(s->be_driver);
> >
> > s->active_locty = TPM_TIS_NO_LOCALITY;
> > diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
> > index 332c2ae597..ce9bda3c89 100644
> > --- a/docs/specs/tpm.txt
> > +++ b/docs/specs/tpm.txt
> > @@ -121,6 +121,8 @@ layout:
> > +----------+--------+--------+-------------------------------------------+
> > | next_step| 0x1 | 0x159 | Operation to execute after reboot by |
> > | | | | firmware. Used by firmware. |
> > + +----------+--------+--------+-------------------------------------------+
> > + | movv | 0x1 | 0x15a | Memory overwrite variable |
> > +----------+--------+--------+-------------------------------------------+
> >
> > The following values are supported for the 'func' field. They correspond
> > diff --git a/hw/tpm/trace-events b/hw/tpm/trace-events
> > index 25bee0cecf..920d32ad55 100644
> > --- a/hw/tpm/trace-events
> > +++ b/hw/tpm/trace-events
> > @@ -51,3 +51,6 @@ tpm_tis_mmio_write_init_abort(void) "Initiating abort"
> > tpm_tis_mmio_write_lowering_irq(void) "Lowering IRQ"
> > tpm_tis_mmio_write_data2send(uint32_t value, unsigned size) "Data to send to TPM: 0x%08x (size=%d)"
> > tpm_tis_pre_save(uint8_t locty, uint32_t rw_offset) "locty: %d, rw_offset = %u"
> > +
> > +# hw/tpm/tpm_ppi.c
> > +tpm_ppi_memset(uint8_t *ptr, size_t size) "memset: %p %zu"
> >
>
> I'd split this patch in 2: Add ACPI, Add tpm_ppi_reset.
On Thu, Dec 13, 2018 at 4:01 PM Igor Mammedov <imammedo@redhat.com> wrote:
>
> On Wed, 12 Dec 2018 17:22:21 +0100
> Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> > On 9/10/18 10:32 AM, Marc-André Lureau wrote:
> > > This allows to pass the last failing test from the Windows HLK TPM 2.0
> > > TCG PPI 1.3 tests.
> > >
> > > The interface is described in the "TCG Platform Reset Attack
> > > Mitigation Specification", chapter 6 "ACPI _DSM Function". According
> > > to Laszlo, it's not so easy to implement in OVMF, he suggested to do
> > > it in qemu instead.
> > >
> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > ---
> > > hw/tpm/tpm_ppi.h | 2 ++
> > > hw/i386/acpi-build.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
> > > hw/tpm/tpm_crb.c | 1 +
> > > hw/tpm/tpm_ppi.c | 22 +++++++++++++++++++++
> > > hw/tpm/tpm_tis.c | 1 +
> > > docs/specs/tpm.txt | 2 ++
> > > hw/tpm/trace-events | 3 +++
> > > 7 files changed, 77 insertions(+)
> > >
> > > diff --git a/hw/tpm/tpm_ppi.h b/hw/tpm/tpm_ppi.h
> > > index c2ab2ed300..b8f67962c7 100644
> > > --- a/hw/tpm/tpm_ppi.h
> > > +++ b/hw/tpm/tpm_ppi.h
> > > @@ -23,4 +23,6 @@ typedef struct TPMPPI {
> > > bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
> > > hwaddr addr, Object *obj, Error **errp);
> > >
> >
> > Can you add documentation?
> >
> > > +void tpm_ppi_reset(TPMPPI *tpmppi);
> > > +
> > > #endif /* TPM_TPM_PPI_H */
> > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > > index c5e9a6e11d..2ab3e8fae7 100644
> > > --- a/hw/i386/acpi-build.c
> > > +++ b/hw/i386/acpi-build.c
> > > @@ -1824,6 +1824,13 @@ build_tpm_ppi(TPMIf *tpm, Aml *dev)
> > > pprq = aml_name("PPRQ");
> > > pprm = aml_name("PPRM");
> > >
> > > + aml_append(dev,
> > > + aml_operation_region("TPP3", AML_SYSTEM_MEMORY,
> > > + aml_int(TPM_PPI_ADDR_BASE + 0x15a),
> >
> > Can you add a definition for this magic value?
> > Maybe:
> >
> > #define TPM_PPI_MEMORY_OVERWRITE_OFFSET 0x15a
> >
> > or
> >
> > #define TPM_PPI_MOVV_OFFSET 0x15a
> I'd prefer explicit numeric values there as it's a bit easier to review
> and grep in 'spec' since one doesn't have to jump to define definition
> first to find out value to compare with spec.
> What I'd add though is verbatim comment from spec like we do
> in APCI code so we could jump to the field of interest in spec
> just doing copy&paste&search
I don't mind.
Philippe, are you okay going back to numeric values?
Igor, is this comment what you would expect where the 0x15a value is used:
/* 0x15a: movv field in ACPI PPI Interface, see specs/tpm.txt */
thanks
>
> >
> > > + 0x1));
> > > + field = aml_field("TPP3", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
> > > + aml_append(field, aml_named_field("MOVV", 8));
> > > + aml_append(dev, field);
> > > /*
> > > * DerefOf in Windows is broken with SYSTEM_MEMORY. Use a dynamic
> > > * operation region inside of a method for getting FUNC[op].
> > > @@ -2166,7 +2173,46 @@ build_tpm_ppi(TPMIf *tpm, Aml *dev)
> > > aml_append(ifctx, aml_return(aml_buffer(1, zerobyte)));
> > > }
> > > aml_append(method, ifctx);
> > > +
> > > + ifctx = aml_if(
> > > + aml_equal(uuid,
> > > + aml_touuid("376054ED-CC13-4675-901C-4756D7F2D45D")));
> > > + {
> > > + /* standard DSM query function */
> > > + ifctx2 = aml_if(aml_equal(function, zero));
> > > + {
> > > + uint8_t byte_list[1] = { 0x03 };
> > > + aml_append(ifctx2, aml_return(aml_buffer(1, byte_list)));
> > > + }
> > > + aml_append(ifctx, ifctx2);
> > > +
> > > + /*
> > > + * TCG Platform Reset Attack Mitigation Specification 1.0 Ch.6
> > > + *
> > > + * Arg 2 (Integer): Function Index = 1
> > > + * Arg 3 (Package): Arguments = Package: Type: Integer
> > > + * Operation Value of the Request
> > > + * Returns: Type: Integer
> > > + * 0: Success
> > > + * 1: General Failure
> > > + */
> > > + ifctx2 = aml_if(aml_equal(function, one));
> > > + {
> > > + aml_append(ifctx2,
> > > + aml_store(aml_derefof(aml_index(arguments, zero)),
> > > + op));
> > > + {
> > > + aml_append(ifctx2, aml_store(op, aml_name("MOVV")));
> > > +
> > > + /* 0: success */
> > > + aml_append(ifctx2, aml_return(zero));
> > > + }
> > > + }
> > > + aml_append(ifctx, ifctx2);
> > > + }
> > > + aml_append(method, ifctx);
> > > }
> > > +
> > > aml_append(dev, method);
> > > }
> > >
> > > diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> > > index b243222fd6..48f6a716ad 100644
> > > --- a/hw/tpm/tpm_crb.c
> > > +++ b/hw/tpm/tpm_crb.c
> > > @@ -233,6 +233,7 @@ static void tpm_crb_reset(void *dev)
> > > {
> > > CRBState *s = CRB(dev);
> > >
> > > + tpm_ppi_reset(&s->ppi);
> > > tpm_backend_reset(s->tpmbe);
> > >
> > > memset(s->regs, 0, sizeof(s->regs));
> > > diff --git a/hw/tpm/tpm_ppi.c b/hw/tpm/tpm_ppi.c
> > > index f2f07f895e..ac05ba8d3c 100644
> > > --- a/hw/tpm/tpm_ppi.c
> > > +++ b/hw/tpm/tpm_ppi.c
> > > @@ -16,8 +16,30 @@
> > > #include "qapi/error.h"
> > > #include "cpu.h"
> > > #include "sysemu/memory_mapping.h"
> > > +#include "sysemu/reset.h"
> > > #include "migration/vmstate.h"
> > > #include "tpm_ppi.h"
> > > +#include "trace.h"
> > > +
> > > +void tpm_ppi_reset(TPMPPI *tpmppi)
> > > +{
> > > + if (tpmppi->buf[0x15a] & 0x1) {
> >
> > TPM_PPI_MOVV_OFFSET
> >
> > > + GuestPhysBlockList guest_phys_blocks;
> > > + GuestPhysBlock *block;
> > > +
> > > + guest_phys_blocks_init(&guest_phys_blocks);
> > > + guest_phys_blocks_append(&guest_phys_blocks);
> > > + QTAILQ_FOREACH(block, &guest_phys_blocks.head, next) {
> > > + trace_tpm_ppi_memset(block->host_addr,
> > > + block->target_end - block->target_start);
> > > + memset(block->host_addr, 0,
> > > + block->target_end - block->target_start);
> > > + memory_region_set_dirty(block->mr, 0,
> > > + block->target_end - block->target_start);
> > > + }
> > > + guest_phys_blocks_free(&guest_phys_blocks);
> > > + }
> > > +}
> > >
> > > bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
> > > hwaddr addr, Object *obj, Error **errp)
> > > diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
> > > index 70432ffe8b..d9bfa956cc 100644
> > > --- a/hw/tpm/tpm_tis.c
> > > +++ b/hw/tpm/tpm_tis.c
> > > @@ -868,6 +868,7 @@ static void tpm_tis_reset(DeviceState *dev)
> > > s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->be_driver),
> > > TPM_TIS_BUFFER_MAX);
> > >
> > > + tpm_ppi_reset(&s->ppi);
> > > tpm_backend_reset(s->be_driver);
> > >
> > > s->active_locty = TPM_TIS_NO_LOCALITY;
> > > diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
> > > index 332c2ae597..ce9bda3c89 100644
> > > --- a/docs/specs/tpm.txt
> > > +++ b/docs/specs/tpm.txt
> > > @@ -121,6 +121,8 @@ layout:
> > > +----------+--------+--------+-------------------------------------------+
> > > | next_step| 0x1 | 0x159 | Operation to execute after reboot by |
> > > | | | | firmware. Used by firmware. |
> > > + +----------+--------+--------+-------------------------------------------+
> > > + | movv | 0x1 | 0x15a | Memory overwrite variable |
> > > +----------+--------+--------+-------------------------------------------+
> > >
> > > The following values are supported for the 'func' field. They correspond
> > > diff --git a/hw/tpm/trace-events b/hw/tpm/trace-events
> > > index 25bee0cecf..920d32ad55 100644
> > > --- a/hw/tpm/trace-events
> > > +++ b/hw/tpm/trace-events
> > > @@ -51,3 +51,6 @@ tpm_tis_mmio_write_init_abort(void) "Initiating abort"
> > > tpm_tis_mmio_write_lowering_irq(void) "Lowering IRQ"
> > > tpm_tis_mmio_write_data2send(uint32_t value, unsigned size) "Data to send to TPM: 0x%08x (size=%d)"
> > > tpm_tis_pre_save(uint8_t locty, uint32_t rw_offset) "locty: %d, rw_offset = %u"
> > > +
> > > +# hw/tpm/tpm_ppi.c
> > > +tpm_ppi_memset(uint8_t *ptr, size_t size) "memset: %p %zu"
> > >
> >
> > I'd split this patch in 2: Add ACPI, Add tpm_ppi_reset.
>
On 12/13/18 1:18 PM, Marc-André Lureau wrote:
> On Thu, Dec 13, 2018 at 4:01 PM Igor Mammedov <imammedo@redhat.com> wrote:
>>
>> On Wed, 12 Dec 2018 17:22:21 +0100
>> Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>
>>> On 9/10/18 10:32 AM, Marc-André Lureau wrote:
>>>> This allows to pass the last failing test from the Windows HLK TPM 2.0
>>>> TCG PPI 1.3 tests.
>>>>
>>>> The interface is described in the "TCG Platform Reset Attack
>>>> Mitigation Specification", chapter 6 "ACPI _DSM Function". According
>>>> to Laszlo, it's not so easy to implement in OVMF, he suggested to do
>>>> it in qemu instead.
>>>>
>>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>>>> ---
>>>> hw/tpm/tpm_ppi.h | 2 ++
>>>> hw/i386/acpi-build.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
>>>> hw/tpm/tpm_crb.c | 1 +
>>>> hw/tpm/tpm_ppi.c | 22 +++++++++++++++++++++
>>>> hw/tpm/tpm_tis.c | 1 +
>>>> docs/specs/tpm.txt | 2 ++
>>>> hw/tpm/trace-events | 3 +++
>>>> 7 files changed, 77 insertions(+)
>>>>
>>>> diff --git a/hw/tpm/tpm_ppi.h b/hw/tpm/tpm_ppi.h
>>>> index c2ab2ed300..b8f67962c7 100644
>>>> --- a/hw/tpm/tpm_ppi.h
>>>> +++ b/hw/tpm/tpm_ppi.h
>>>> @@ -23,4 +23,6 @@ typedef struct TPMPPI {
>>>> bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
>>>> hwaddr addr, Object *obj, Error **errp);
>>>>
>>>
>>> Can you add documentation?
>>>
>>>> +void tpm_ppi_reset(TPMPPI *tpmppi);
>>>> +
>>>> #endif /* TPM_TPM_PPI_H */
>>>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>>>> index c5e9a6e11d..2ab3e8fae7 100644
>>>> --- a/hw/i386/acpi-build.c
>>>> +++ b/hw/i386/acpi-build.c
>>>> @@ -1824,6 +1824,13 @@ build_tpm_ppi(TPMIf *tpm, Aml *dev)
>>>> pprq = aml_name("PPRQ");
>>>> pprm = aml_name("PPRM");
>>>>
>>>> + aml_append(dev,
>>>> + aml_operation_region("TPP3", AML_SYSTEM_MEMORY,
>>>> + aml_int(TPM_PPI_ADDR_BASE + 0x15a),
>>>
>>> Can you add a definition for this magic value?
>>> Maybe:
>>>
>>> #define TPM_PPI_MEMORY_OVERWRITE_OFFSET 0x15a
>>>
>>> or
>>>
>>> #define TPM_PPI_MOVV_OFFSET 0x15a
>> I'd prefer explicit numeric values there as it's a bit easier to review
>> and grep in 'spec' since one doesn't have to jump to define definition
>> first to find out value to compare with spec.
>> What I'd add though is verbatim comment from spec like we do
>> in APCI code so we could jump to the field of interest in spec
>> just doing copy&paste&search
>
> I don't mind.
> Philippe, are you okay going back to numeric values?
Can we have a consensus using:
#define TPM_PPI_MEMORY_OVERWRITE_OFFSET_0x15A 0x15a
Joking!
I prefer definitions but since Igor maintains ACPI, he rules and I have
no problem accepting this numeric value :)
>
> Igor, is this comment what you would expect where the 0x15a value is used:
> /* 0x15a: movv field in ACPI PPI Interface, see specs/tpm.txt */
>
> thanks
>>
>>>
>>>> + 0x1));
>>>> + field = aml_field("TPP3", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
>>>> + aml_append(field, aml_named_field("MOVV", 8));
>>>> + aml_append(dev, field);
>>>> /*
>>>> * DerefOf in Windows is broken with SYSTEM_MEMORY. Use a dynamic
>>>> * operation region inside of a method for getting FUNC[op].
>>>> @@ -2166,7 +2173,46 @@ build_tpm_ppi(TPMIf *tpm, Aml *dev)
>>>> aml_append(ifctx, aml_return(aml_buffer(1, zerobyte)));
>>>> }
>>>> aml_append(method, ifctx);
>>>> +
>>>> + ifctx = aml_if(
>>>> + aml_equal(uuid,
>>>> + aml_touuid("376054ED-CC13-4675-901C-4756D7F2D45D")));
>>>> + {
>>>> + /* standard DSM query function */
>>>> + ifctx2 = aml_if(aml_equal(function, zero));
>>>> + {
>>>> + uint8_t byte_list[1] = { 0x03 };
>>>> + aml_append(ifctx2, aml_return(aml_buffer(1, byte_list)));
>>>> + }
>>>> + aml_append(ifctx, ifctx2);
>>>> +
>>>> + /*
>>>> + * TCG Platform Reset Attack Mitigation Specification 1.0 Ch.6
>>>> + *
>>>> + * Arg 2 (Integer): Function Index = 1
>>>> + * Arg 3 (Package): Arguments = Package: Type: Integer
>>>> + * Operation Value of the Request
>>>> + * Returns: Type: Integer
>>>> + * 0: Success
>>>> + * 1: General Failure
>>>> + */
>>>> + ifctx2 = aml_if(aml_equal(function, one));
>>>> + {
>>>> + aml_append(ifctx2,
>>>> + aml_store(aml_derefof(aml_index(arguments, zero)),
>>>> + op));
>>>> + {
>>>> + aml_append(ifctx2, aml_store(op, aml_name("MOVV")));
>>>> +
>>>> + /* 0: success */
>>>> + aml_append(ifctx2, aml_return(zero));
>>>> + }
>>>> + }
>>>> + aml_append(ifctx, ifctx2);
>>>> + }
>>>> + aml_append(method, ifctx);
>>>> }
>>>> +
>>>> aml_append(dev, method);
>>>> }
>>>>
>>>> diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
>>>> index b243222fd6..48f6a716ad 100644
>>>> --- a/hw/tpm/tpm_crb.c
>>>> +++ b/hw/tpm/tpm_crb.c
>>>> @@ -233,6 +233,7 @@ static void tpm_crb_reset(void *dev)
>>>> {
>>>> CRBState *s = CRB(dev);
>>>>
>>>> + tpm_ppi_reset(&s->ppi);
>>>> tpm_backend_reset(s->tpmbe);
>>>>
>>>> memset(s->regs, 0, sizeof(s->regs));
>>>> diff --git a/hw/tpm/tpm_ppi.c b/hw/tpm/tpm_ppi.c
>>>> index f2f07f895e..ac05ba8d3c 100644
>>>> --- a/hw/tpm/tpm_ppi.c
>>>> +++ b/hw/tpm/tpm_ppi.c
>>>> @@ -16,8 +16,30 @@
>>>> #include "qapi/error.h"
>>>> #include "cpu.h"
>>>> #include "sysemu/memory_mapping.h"
>>>> +#include "sysemu/reset.h"
>>>> #include "migration/vmstate.h"
>>>> #include "tpm_ppi.h"
>>>> +#include "trace.h"
>>>> +
>>>> +void tpm_ppi_reset(TPMPPI *tpmppi)
>>>> +{
>>>> + if (tpmppi->buf[0x15a] & 0x1) {
>>>
>>> TPM_PPI_MOVV_OFFSET
>>>
>>>> + GuestPhysBlockList guest_phys_blocks;
>>>> + GuestPhysBlock *block;
>>>> +
>>>> + guest_phys_blocks_init(&guest_phys_blocks);
>>>> + guest_phys_blocks_append(&guest_phys_blocks);
>>>> + QTAILQ_FOREACH(block, &guest_phys_blocks.head, next) {
>>>> + trace_tpm_ppi_memset(block->host_addr,
>>>> + block->target_end - block->target_start);
>>>> + memset(block->host_addr, 0,
>>>> + block->target_end - block->target_start);
>>>> + memory_region_set_dirty(block->mr, 0,
>>>> + block->target_end - block->target_start);
>>>> + }
>>>> + guest_phys_blocks_free(&guest_phys_blocks);
>>>> + }
>>>> +}
>>>>
>>>> bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
>>>> hwaddr addr, Object *obj, Error **errp)
>>>> diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
>>>> index 70432ffe8b..d9bfa956cc 100644
>>>> --- a/hw/tpm/tpm_tis.c
>>>> +++ b/hw/tpm/tpm_tis.c
>>>> @@ -868,6 +868,7 @@ static void tpm_tis_reset(DeviceState *dev)
>>>> s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->be_driver),
>>>> TPM_TIS_BUFFER_MAX);
>>>>
>>>> + tpm_ppi_reset(&s->ppi);
>>>> tpm_backend_reset(s->be_driver);
>>>>
>>>> s->active_locty = TPM_TIS_NO_LOCALITY;
>>>> diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
>>>> index 332c2ae597..ce9bda3c89 100644
>>>> --- a/docs/specs/tpm.txt
>>>> +++ b/docs/specs/tpm.txt
>>>> @@ -121,6 +121,8 @@ layout:
>>>> +----------+--------+--------+-------------------------------------------+
>>>> | next_step| 0x1 | 0x159 | Operation to execute after reboot by |
>>>> | | | | firmware. Used by firmware. |
>>>> + +----------+--------+--------+-------------------------------------------+
>>>> + | movv | 0x1 | 0x15a | Memory overwrite variable |
>>>> +----------+--------+--------+-------------------------------------------+
>>>>
>>>> The following values are supported for the 'func' field. They correspond
>>>> diff --git a/hw/tpm/trace-events b/hw/tpm/trace-events
>>>> index 25bee0cecf..920d32ad55 100644
>>>> --- a/hw/tpm/trace-events
>>>> +++ b/hw/tpm/trace-events
>>>> @@ -51,3 +51,6 @@ tpm_tis_mmio_write_init_abort(void) "Initiating abort"
>>>> tpm_tis_mmio_write_lowering_irq(void) "Lowering IRQ"
>>>> tpm_tis_mmio_write_data2send(uint32_t value, unsigned size) "Data to send to TPM: 0x%08x (size=%d)"
>>>> tpm_tis_pre_save(uint8_t locty, uint32_t rw_offset) "locty: %d, rw_offset = %u"
>>>> +
>>>> +# hw/tpm/tpm_ppi.c
>>>> +tpm_ppi_memset(uint8_t *ptr, size_t size) "memset: %p %zu"
>>>>
>>>
>>> I'd split this patch in 2: Add ACPI, Add tpm_ppi_reset.
>>
Hi
On Thu, Dec 13, 2018 at 6:08 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> On 12/13/18 1:18 PM, Marc-André Lureau wrote:
> > On Thu, Dec 13, 2018 at 4:01 PM Igor Mammedov <imammedo@redhat.com> wrote:
> >>
> >> On Wed, 12 Dec 2018 17:22:21 +0100
> >> Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> >>
> >>> On 9/10/18 10:32 AM, Marc-André Lureau wrote:
> >>>> This allows to pass the last failing test from the Windows HLK TPM 2.0
> >>>> TCG PPI 1.3 tests.
> >>>>
> >>>> The interface is described in the "TCG Platform Reset Attack
> >>>> Mitigation Specification", chapter 6 "ACPI _DSM Function". According
> >>>> to Laszlo, it's not so easy to implement in OVMF, he suggested to do
> >>>> it in qemu instead.
> >>>>
> >>>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> >>>> ---
> >>>> hw/tpm/tpm_ppi.h | 2 ++
> >>>> hw/i386/acpi-build.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
> >>>> hw/tpm/tpm_crb.c | 1 +
> >>>> hw/tpm/tpm_ppi.c | 22 +++++++++++++++++++++
> >>>> hw/tpm/tpm_tis.c | 1 +
> >>>> docs/specs/tpm.txt | 2 ++
> >>>> hw/tpm/trace-events | 3 +++
> >>>> 7 files changed, 77 insertions(+)
> >>>>
> >>>> diff --git a/hw/tpm/tpm_ppi.h b/hw/tpm/tpm_ppi.h
> >>>> index c2ab2ed300..b8f67962c7 100644
> >>>> --- a/hw/tpm/tpm_ppi.h
> >>>> +++ b/hw/tpm/tpm_ppi.h
> >>>> @@ -23,4 +23,6 @@ typedef struct TPMPPI {
> >>>> bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
> >>>> hwaddr addr, Object *obj, Error **errp);
> >>>>
> >>>
> >>> Can you add documentation?
> >>>
> >>>> +void tpm_ppi_reset(TPMPPI *tpmppi);
> >>>> +
> >>>> #endif /* TPM_TPM_PPI_H */
> >>>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> >>>> index c5e9a6e11d..2ab3e8fae7 100644
> >>>> --- a/hw/i386/acpi-build.c
> >>>> +++ b/hw/i386/acpi-build.c
> >>>> @@ -1824,6 +1824,13 @@ build_tpm_ppi(TPMIf *tpm, Aml *dev)
> >>>> pprq = aml_name("PPRQ");
> >>>> pprm = aml_name("PPRM");
> >>>>
> >>>> + aml_append(dev,
> >>>> + aml_operation_region("TPP3", AML_SYSTEM_MEMORY,
> >>>> + aml_int(TPM_PPI_ADDR_BASE + 0x15a),
> >>>
> >>> Can you add a definition for this magic value?
> >>> Maybe:
> >>>
> >>> #define TPM_PPI_MEMORY_OVERWRITE_OFFSET 0x15a
> >>>
> >>> or
> >>>
> >>> #define TPM_PPI_MOVV_OFFSET 0x15a
> >> I'd prefer explicit numeric values there as it's a bit easier to review
> >> and grep in 'spec' since one doesn't have to jump to define definition
> >> first to find out value to compare with spec.
> >> What I'd add though is verbatim comment from spec like we do
> >> in APCI code so we could jump to the field of interest in spec
> >> just doing copy&paste&search
> >
> > I don't mind.
> > Philippe, are you okay going back to numeric values?
>
> Can we have a consensus using:
>
> #define TPM_PPI_MEMORY_OVERWRITE_OFFSET_0x15A 0x15a
>
> Joking!
>
> I prefer definitions but since Igor maintains ACPI, he rules and I have
> no problem accepting this numeric value :)
ok, with Igor comments, does it get you review-by?
thanks
>
> >
> > Igor, is this comment what you would expect where the 0x15a value is used:
> > /* 0x15a: movv field in ACPI PPI Interface, see specs/tpm.txt */
> >
> > thanks
> >>
> >>>
> >>>> + 0x1));
> >>>> + field = aml_field("TPP3", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
> >>>> + aml_append(field, aml_named_field("MOVV", 8));
> >>>> + aml_append(dev, field);
> >>>> /*
> >>>> * DerefOf in Windows is broken with SYSTEM_MEMORY. Use a dynamic
> >>>> * operation region inside of a method for getting FUNC[op].
> >>>> @@ -2166,7 +2173,46 @@ build_tpm_ppi(TPMIf *tpm, Aml *dev)
> >>>> aml_append(ifctx, aml_return(aml_buffer(1, zerobyte)));
> >>>> }
> >>>> aml_append(method, ifctx);
> >>>> +
> >>>> + ifctx = aml_if(
> >>>> + aml_equal(uuid,
> >>>> + aml_touuid("376054ED-CC13-4675-901C-4756D7F2D45D")));
> >>>> + {
> >>>> + /* standard DSM query function */
> >>>> + ifctx2 = aml_if(aml_equal(function, zero));
> >>>> + {
> >>>> + uint8_t byte_list[1] = { 0x03 };
> >>>> + aml_append(ifctx2, aml_return(aml_buffer(1, byte_list)));
> >>>> + }
> >>>> + aml_append(ifctx, ifctx2);
> >>>> +
> >>>> + /*
> >>>> + * TCG Platform Reset Attack Mitigation Specification 1.0 Ch.6
> >>>> + *
> >>>> + * Arg 2 (Integer): Function Index = 1
> >>>> + * Arg 3 (Package): Arguments = Package: Type: Integer
> >>>> + * Operation Value of the Request
> >>>> + * Returns: Type: Integer
> >>>> + * 0: Success
> >>>> + * 1: General Failure
> >>>> + */
> >>>> + ifctx2 = aml_if(aml_equal(function, one));
> >>>> + {
> >>>> + aml_append(ifctx2,
> >>>> + aml_store(aml_derefof(aml_index(arguments, zero)),
> >>>> + op));
> >>>> + {
> >>>> + aml_append(ifctx2, aml_store(op, aml_name("MOVV")));
> >>>> +
> >>>> + /* 0: success */
> >>>> + aml_append(ifctx2, aml_return(zero));
> >>>> + }
> >>>> + }
> >>>> + aml_append(ifctx, ifctx2);
> >>>> + }
> >>>> + aml_append(method, ifctx);
> >>>> }
> >>>> +
> >>>> aml_append(dev, method);
> >>>> }
> >>>>
> >>>> diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> >>>> index b243222fd6..48f6a716ad 100644
> >>>> --- a/hw/tpm/tpm_crb.c
> >>>> +++ b/hw/tpm/tpm_crb.c
> >>>> @@ -233,6 +233,7 @@ static void tpm_crb_reset(void *dev)
> >>>> {
> >>>> CRBState *s = CRB(dev);
> >>>>
> >>>> + tpm_ppi_reset(&s->ppi);
> >>>> tpm_backend_reset(s->tpmbe);
> >>>>
> >>>> memset(s->regs, 0, sizeof(s->regs));
> >>>> diff --git a/hw/tpm/tpm_ppi.c b/hw/tpm/tpm_ppi.c
> >>>> index f2f07f895e..ac05ba8d3c 100644
> >>>> --- a/hw/tpm/tpm_ppi.c
> >>>> +++ b/hw/tpm/tpm_ppi.c
> >>>> @@ -16,8 +16,30 @@
> >>>> #include "qapi/error.h"
> >>>> #include "cpu.h"
> >>>> #include "sysemu/memory_mapping.h"
> >>>> +#include "sysemu/reset.h"
> >>>> #include "migration/vmstate.h"
> >>>> #include "tpm_ppi.h"
> >>>> +#include "trace.h"
> >>>> +
> >>>> +void tpm_ppi_reset(TPMPPI *tpmppi)
> >>>> +{
> >>>> + if (tpmppi->buf[0x15a] & 0x1) {
> >>>
> >>> TPM_PPI_MOVV_OFFSET
> >>>
> >>>> + GuestPhysBlockList guest_phys_blocks;
> >>>> + GuestPhysBlock *block;
> >>>> +
> >>>> + guest_phys_blocks_init(&guest_phys_blocks);
> >>>> + guest_phys_blocks_append(&guest_phys_blocks);
> >>>> + QTAILQ_FOREACH(block, &guest_phys_blocks.head, next) {
> >>>> + trace_tpm_ppi_memset(block->host_addr,
> >>>> + block->target_end - block->target_start);
> >>>> + memset(block->host_addr, 0,
> >>>> + block->target_end - block->target_start);
> >>>> + memory_region_set_dirty(block->mr, 0,
> >>>> + block->target_end - block->target_start);
> >>>> + }
> >>>> + guest_phys_blocks_free(&guest_phys_blocks);
> >>>> + }
> >>>> +}
> >>>>
> >>>> bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
> >>>> hwaddr addr, Object *obj, Error **errp)
> >>>> diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
> >>>> index 70432ffe8b..d9bfa956cc 100644
> >>>> --- a/hw/tpm/tpm_tis.c
> >>>> +++ b/hw/tpm/tpm_tis.c
> >>>> @@ -868,6 +868,7 @@ static void tpm_tis_reset(DeviceState *dev)
> >>>> s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->be_driver),
> >>>> TPM_TIS_BUFFER_MAX);
> >>>>
> >>>> + tpm_ppi_reset(&s->ppi);
> >>>> tpm_backend_reset(s->be_driver);
> >>>>
> >>>> s->active_locty = TPM_TIS_NO_LOCALITY;
> >>>> diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
> >>>> index 332c2ae597..ce9bda3c89 100644
> >>>> --- a/docs/specs/tpm.txt
> >>>> +++ b/docs/specs/tpm.txt
> >>>> @@ -121,6 +121,8 @@ layout:
> >>>> +----------+--------+--------+-------------------------------------------+
> >>>> | next_step| 0x1 | 0x159 | Operation to execute after reboot by |
> >>>> | | | | firmware. Used by firmware. |
> >>>> + +----------+--------+--------+-------------------------------------------+
> >>>> + | movv | 0x1 | 0x15a | Memory overwrite variable |
> >>>> +----------+--------+--------+-------------------------------------------+
> >>>>
> >>>> The following values are supported for the 'func' field. They correspond
> >>>> diff --git a/hw/tpm/trace-events b/hw/tpm/trace-events
> >>>> index 25bee0cecf..920d32ad55 100644
> >>>> --- a/hw/tpm/trace-events
> >>>> +++ b/hw/tpm/trace-events
> >>>> @@ -51,3 +51,6 @@ tpm_tis_mmio_write_init_abort(void) "Initiating abort"
> >>>> tpm_tis_mmio_write_lowering_irq(void) "Lowering IRQ"
> >>>> tpm_tis_mmio_write_data2send(uint32_t value, unsigned size) "Data to send to TPM: 0x%08x (size=%d)"
> >>>> tpm_tis_pre_save(uint8_t locty, uint32_t rw_offset) "locty: %d, rw_offset = %u"
> >>>> +
> >>>> +# hw/tpm/tpm_ppi.c
> >>>> +tpm_ppi_memset(uint8_t *ptr, size_t size) "memset: %p %zu"
> >>>>
> >>>
> >>> I'd split this patch in 2: Add ACPI, Add tpm_ppi_reset.
> >>
>
--
Marc-André Lureau
On Thu, 13 Dec 2018 16:18:09 +0400
Marc-André Lureau <marcandre.lureau@redhat.com> wrote:
> On Thu, Dec 13, 2018 at 4:01 PM Igor Mammedov <imammedo@redhat.com> wrote:
> >
> > On Wed, 12 Dec 2018 17:22:21 +0100
> > Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> >
> > > On 9/10/18 10:32 AM, Marc-André Lureau wrote:
> > > > This allows to pass the last failing test from the Windows HLK TPM 2.0
> > > > TCG PPI 1.3 tests.
> > > >
> > > > The interface is described in the "TCG Platform Reset Attack
> > > > Mitigation Specification", chapter 6 "ACPI _DSM Function". According
> > > > to Laszlo, it's not so easy to implement in OVMF, he suggested to do
> > > > it in qemu instead.
> > > >
> > > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > > ---
> > > > hw/tpm/tpm_ppi.h | 2 ++
> > > > hw/i386/acpi-build.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
> > > > hw/tpm/tpm_crb.c | 1 +
> > > > hw/tpm/tpm_ppi.c | 22 +++++++++++++++++++++
> > > > hw/tpm/tpm_tis.c | 1 +
> > > > docs/specs/tpm.txt | 2 ++
> > > > hw/tpm/trace-events | 3 +++
> > > > 7 files changed, 77 insertions(+)
> > > >
> > > > diff --git a/hw/tpm/tpm_ppi.h b/hw/tpm/tpm_ppi.h
> > > > index c2ab2ed300..b8f67962c7 100644
> > > > --- a/hw/tpm/tpm_ppi.h
> > > > +++ b/hw/tpm/tpm_ppi.h
> > > > @@ -23,4 +23,6 @@ typedef struct TPMPPI {
> > > > bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
> > > > hwaddr addr, Object *obj, Error **errp);
> > > >
> > >
> > > Can you add documentation?
> > >
> > > > +void tpm_ppi_reset(TPMPPI *tpmppi);
> > > > +
> > > > #endif /* TPM_TPM_PPI_H */
> > > > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > > > index c5e9a6e11d..2ab3e8fae7 100644
> > > > --- a/hw/i386/acpi-build.c
> > > > +++ b/hw/i386/acpi-build.c
> > > > @@ -1824,6 +1824,13 @@ build_tpm_ppi(TPMIf *tpm, Aml *dev)
> > > > pprq = aml_name("PPRQ");
> > > > pprm = aml_name("PPRM");
> > > >
> > > > + aml_append(dev,
> > > > + aml_operation_region("TPP3", AML_SYSTEM_MEMORY,
> > > > + aml_int(TPM_PPI_ADDR_BASE + 0x15a),
> > >
> > > Can you add a definition for this magic value?
> > > Maybe:
> > >
> > > #define TPM_PPI_MEMORY_OVERWRITE_OFFSET 0x15a
> > >
> > > or
> > >
> > > #define TPM_PPI_MOVV_OFFSET 0x15a
> > I'd prefer explicit numeric values there as it's a bit easier to review
> > and grep in 'spec' since one doesn't have to jump to define definition
> > first to find out value to compare with spec.
> > What I'd add though is verbatim comment from spec like we do
> > in APCI code so we could jump to the field of interest in spec
> > just doing copy&paste&search
>
> I don't mind.
> Philippe, are you okay going back to numeric values?
>
> Igor, is this comment what you would expect where the 0x15a value is used:
> /* 0x15a: movv field in ACPI PPI Interface, see specs/tpm.txt */
>
usually field name is sufficient, so something like:
foo(0.15a /* movv, docs/specs/tpm.txt */)
> thanks
> >
> > >
> > > > + 0x1));
> > > > + field = aml_field("TPP3", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
> > > > + aml_append(field, aml_named_field("MOVV", 8));
> > > > + aml_append(dev, field);
> > > > /*
> > > > * DerefOf in Windows is broken with SYSTEM_MEMORY. Use a dynamic
> > > > * operation region inside of a method for getting FUNC[op].
> > > > @@ -2166,7 +2173,46 @@ build_tpm_ppi(TPMIf *tpm, Aml *dev)
> > > > aml_append(ifctx, aml_return(aml_buffer(1, zerobyte)));
> > > > }
> > > > aml_append(method, ifctx);
> > > > +
> > > > + ifctx = aml_if(
> > > > + aml_equal(uuid,
> > > > + aml_touuid("376054ED-CC13-4675-901C-4756D7F2D45D")));
> > > > + {
> > > > + /* standard DSM query function */
> > > > + ifctx2 = aml_if(aml_equal(function, zero));
> > > > + {
> > > > + uint8_t byte_list[1] = { 0x03 };
> > > > + aml_append(ifctx2, aml_return(aml_buffer(1, byte_list)));
> > > > + }
> > > > + aml_append(ifctx, ifctx2);
> > > > +
> > > > + /*
> > > > + * TCG Platform Reset Attack Mitigation Specification 1.0 Ch.6
> > > > + *
> > > > + * Arg 2 (Integer): Function Index = 1
> > > > + * Arg 3 (Package): Arguments = Package: Type: Integer
> > > > + * Operation Value of the Request
> > > > + * Returns: Type: Integer
> > > > + * 0: Success
> > > > + * 1: General Failure
> > > > + */
> > > > + ifctx2 = aml_if(aml_equal(function, one));
> > > > + {
> > > > + aml_append(ifctx2,
> > > > + aml_store(aml_derefof(aml_index(arguments, zero)),
> > > > + op));
> > > > + {
> > > > + aml_append(ifctx2, aml_store(op, aml_name("MOVV")));
> > > > +
> > > > + /* 0: success */
> > > > + aml_append(ifctx2, aml_return(zero));
> > > > + }
> > > > + }
> > > > + aml_append(ifctx, ifctx2);
> > > > + }
> > > > + aml_append(method, ifctx);
> > > > }
> > > > +
> > > > aml_append(dev, method);
> > > > }
> > > >
> > > > diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> > > > index b243222fd6..48f6a716ad 100644
> > > > --- a/hw/tpm/tpm_crb.c
> > > > +++ b/hw/tpm/tpm_crb.c
> > > > @@ -233,6 +233,7 @@ static void tpm_crb_reset(void *dev)
> > > > {
> > > > CRBState *s = CRB(dev);
> > > >
> > > > + tpm_ppi_reset(&s->ppi);
> > > > tpm_backend_reset(s->tpmbe);
> > > >
> > > > memset(s->regs, 0, sizeof(s->regs));
> > > > diff --git a/hw/tpm/tpm_ppi.c b/hw/tpm/tpm_ppi.c
> > > > index f2f07f895e..ac05ba8d3c 100644
> > > > --- a/hw/tpm/tpm_ppi.c
> > > > +++ b/hw/tpm/tpm_ppi.c
> > > > @@ -16,8 +16,30 @@
> > > > #include "qapi/error.h"
> > > > #include "cpu.h"
> > > > #include "sysemu/memory_mapping.h"
> > > > +#include "sysemu/reset.h"
> > > > #include "migration/vmstate.h"
> > > > #include "tpm_ppi.h"
> > > > +#include "trace.h"
> > > > +
> > > > +void tpm_ppi_reset(TPMPPI *tpmppi)
> > > > +{
> > > > + if (tpmppi->buf[0x15a] & 0x1) {
> > >
> > > TPM_PPI_MOVV_OFFSET
> > >
> > > > + GuestPhysBlockList guest_phys_blocks;
> > > > + GuestPhysBlock *block;
> > > > +
> > > > + guest_phys_blocks_init(&guest_phys_blocks);
> > > > + guest_phys_blocks_append(&guest_phys_blocks);
> > > > + QTAILQ_FOREACH(block, &guest_phys_blocks.head, next) {
> > > > + trace_tpm_ppi_memset(block->host_addr,
> > > > + block->target_end - block->target_start);
> > > > + memset(block->host_addr, 0,
> > > > + block->target_end - block->target_start);
> > > > + memory_region_set_dirty(block->mr, 0,
> > > > + block->target_end - block->target_start);
> > > > + }
> > > > + guest_phys_blocks_free(&guest_phys_blocks);
> > > > + }
> > > > +}
> > > >
> > > > bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
> > > > hwaddr addr, Object *obj, Error **errp)
> > > > diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
> > > > index 70432ffe8b..d9bfa956cc 100644
> > > > --- a/hw/tpm/tpm_tis.c
> > > > +++ b/hw/tpm/tpm_tis.c
> > > > @@ -868,6 +868,7 @@ static void tpm_tis_reset(DeviceState *dev)
> > > > s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->be_driver),
> > > > TPM_TIS_BUFFER_MAX);
> > > >
> > > > + tpm_ppi_reset(&s->ppi);
> > > > tpm_backend_reset(s->be_driver);
> > > >
> > > > s->active_locty = TPM_TIS_NO_LOCALITY;
> > > > diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
> > > > index 332c2ae597..ce9bda3c89 100644
> > > > --- a/docs/specs/tpm.txt
> > > > +++ b/docs/specs/tpm.txt
> > > > @@ -121,6 +121,8 @@ layout:
> > > > +----------+--------+--------+-------------------------------------------+
> > > > | next_step| 0x1 | 0x159 | Operation to execute after reboot by |
> > > > | | | | firmware. Used by firmware. |
> > > > + +----------+--------+--------+-------------------------------------------+
> > > > + | movv | 0x1 | 0x15a | Memory overwrite variable |
> > > > +----------+--------+--------+-------------------------------------------+
> > > >
> > > > The following values are supported for the 'func' field. They correspond
> > > > diff --git a/hw/tpm/trace-events b/hw/tpm/trace-events
> > > > index 25bee0cecf..920d32ad55 100644
> > > > --- a/hw/tpm/trace-events
> > > > +++ b/hw/tpm/trace-events
> > > > @@ -51,3 +51,6 @@ tpm_tis_mmio_write_init_abort(void) "Initiating abort"
> > > > tpm_tis_mmio_write_lowering_irq(void) "Lowering IRQ"
> > > > tpm_tis_mmio_write_data2send(uint32_t value, unsigned size) "Data to send to TPM: 0x%08x (size=%d)"
> > > > tpm_tis_pre_save(uint8_t locty, uint32_t rw_offset) "locty: %d, rw_offset = %u"
> > > > +
> > > > +# hw/tpm/tpm_ppi.c
> > > > +tpm_ppi_memset(uint8_t *ptr, size_t size) "memset: %p %zu"
> > > >
> > >
> > > I'd split this patch in 2: Add ACPI, Add tpm_ppi_reset.
> >
>
Hi
On Mon, Sep 10, 2018 at 12:47 PM 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.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> hw/tpm/tpm_ppi.h | 2 ++
> hw/i386/acpi-build.c | 46 ++++++++++++++++++++++++++++++++++++++++++++
> hw/tpm/tpm_crb.c | 1 +
> hw/tpm/tpm_ppi.c | 22 +++++++++++++++++++++
> hw/tpm/tpm_tis.c | 1 +
> docs/specs/tpm.txt | 2 ++
> hw/tpm/trace-events | 3 +++
> 7 files changed, 77 insertions(+)
>
> diff --git a/hw/tpm/tpm_ppi.h b/hw/tpm/tpm_ppi.h
> index c2ab2ed300..b8f67962c7 100644
> --- a/hw/tpm/tpm_ppi.h
> +++ b/hw/tpm/tpm_ppi.h
> @@ -23,4 +23,6 @@ typedef struct TPMPPI {
> bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
> hwaddr addr, Object *obj, Error **errp);
>
> +void tpm_ppi_reset(TPMPPI *tpmppi);
> +
> #endif /* TPM_TPM_PPI_H */
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index c5e9a6e11d..2ab3e8fae7 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1824,6 +1824,13 @@ build_tpm_ppi(TPMIf *tpm, Aml *dev)
> pprq = aml_name("PPRQ");
> pprm = aml_name("PPRM");
>
> + aml_append(dev,
> + aml_operation_region("TPP3", AML_SYSTEM_MEMORY,
> + aml_int(TPM_PPI_ADDR_BASE + 0x15a),
> + 0x1));
> + field = aml_field("TPP3", AML_BYTE_ACC, AML_NOLOCK, AML_PRESERVE);
> + aml_append(field, aml_named_field("MOVV", 8));
> + aml_append(dev, field);
> /*
> * DerefOf in Windows is broken with SYSTEM_MEMORY. Use a dynamic
> * operation region inside of a method for getting FUNC[op].
> @@ -2166,7 +2173,46 @@ build_tpm_ppi(TPMIf *tpm, Aml *dev)
> aml_append(ifctx, aml_return(aml_buffer(1, zerobyte)));
> }
> aml_append(method, ifctx);
> +
> + ifctx = aml_if(
> + aml_equal(uuid,
> + aml_touuid("376054ED-CC13-4675-901C-4756D7F2D45D")));
> + {
> + /* standard DSM query function */
> + ifctx2 = aml_if(aml_equal(function, zero));
> + {
> + uint8_t byte_list[1] = { 0x03 };
> + aml_append(ifctx2, aml_return(aml_buffer(1, byte_list)));
> + }
> + aml_append(ifctx, ifctx2);
> +
> + /*
> + * TCG Platform Reset Attack Mitigation Specification 1.0 Ch.6
> + *
> + * Arg 2 (Integer): Function Index = 1
> + * Arg 3 (Package): Arguments = Package: Type: Integer
> + * Operation Value of the Request
> + * Returns: Type: Integer
> + * 0: Success
> + * 1: General Failure
> + */
> + ifctx2 = aml_if(aml_equal(function, one));
> + {
> + aml_append(ifctx2,
> + aml_store(aml_derefof(aml_index(arguments, zero)),
> + op));
> + {
> + aml_append(ifctx2, aml_store(op, aml_name("MOVV")));
> +
> + /* 0: success */
> + aml_append(ifctx2, aml_return(zero));
> + }
> + }
> + aml_append(ifctx, ifctx2);
> + }
> + aml_append(method, ifctx);
> }
> +
> aml_append(dev, method);
> }
>
> diff --git a/hw/tpm/tpm_crb.c b/hw/tpm/tpm_crb.c
> index b243222fd6..48f6a716ad 100644
> --- a/hw/tpm/tpm_crb.c
> +++ b/hw/tpm/tpm_crb.c
> @@ -233,6 +233,7 @@ static void tpm_crb_reset(void *dev)
> {
> CRBState *s = CRB(dev);
>
> + tpm_ppi_reset(&s->ppi);
> tpm_backend_reset(s->tpmbe);
>
> memset(s->regs, 0, sizeof(s->regs));
> diff --git a/hw/tpm/tpm_ppi.c b/hw/tpm/tpm_ppi.c
> index f2f07f895e..ac05ba8d3c 100644
> --- a/hw/tpm/tpm_ppi.c
> +++ b/hw/tpm/tpm_ppi.c
> @@ -16,8 +16,30 @@
> #include "qapi/error.h"
> #include "cpu.h"
> #include "sysemu/memory_mapping.h"
> +#include "sysemu/reset.h"
> #include "migration/vmstate.h"
> #include "tpm_ppi.h"
> +#include "trace.h"
> +
> +void tpm_ppi_reset(TPMPPI *tpmppi)
> +{
> + if (tpmppi->buf[0x15a] & 0x1) {
> + GuestPhysBlockList guest_phys_blocks;
> + GuestPhysBlock *block;
> +
> + guest_phys_blocks_init(&guest_phys_blocks);
> + guest_phys_blocks_append(&guest_phys_blocks);
> + QTAILQ_FOREACH(block, &guest_phys_blocks.head, next) {
> + trace_tpm_ppi_memset(block->host_addr,
> + block->target_end - block->target_start);
> + memset(block->host_addr, 0,
> + block->target_end - block->target_start);
> + memory_region_set_dirty(block->mr, 0,
> + block->target_end - block->target_start);
> + }
The specification also mentions CPU cache clearing.
Is this enough to flush the CPU caches as well?
Maybe the CPU cache is already cleared on reset?
> + guest_phys_blocks_free(&guest_phys_blocks);
> + }
> +}
>
> bool tpm_ppi_init(TPMPPI *tpmppi, struct MemoryRegion *m,
> hwaddr addr, Object *obj, Error **errp)
> diff --git a/hw/tpm/tpm_tis.c b/hw/tpm/tpm_tis.c
> index 70432ffe8b..d9bfa956cc 100644
> --- a/hw/tpm/tpm_tis.c
> +++ b/hw/tpm/tpm_tis.c
> @@ -868,6 +868,7 @@ static void tpm_tis_reset(DeviceState *dev)
> s->be_buffer_size = MIN(tpm_backend_get_buffer_size(s->be_driver),
> TPM_TIS_BUFFER_MAX);
>
> + tpm_ppi_reset(&s->ppi);
> tpm_backend_reset(s->be_driver);
>
> s->active_locty = TPM_TIS_NO_LOCALITY;
> diff --git a/docs/specs/tpm.txt b/docs/specs/tpm.txt
> index 332c2ae597..ce9bda3c89 100644
> --- a/docs/specs/tpm.txt
> +++ b/docs/specs/tpm.txt
> @@ -121,6 +121,8 @@ layout:
> +----------+--------+--------+-------------------------------------------+
> | next_step| 0x1 | 0x159 | Operation to execute after reboot by |
> | | | | firmware. Used by firmware. |
> + +----------+--------+--------+-------------------------------------------+
> + | movv | 0x1 | 0x15a | Memory overwrite variable |
> +----------+--------+--------+-------------------------------------------+
>
> The following values are supported for the 'func' field. They correspond
> diff --git a/hw/tpm/trace-events b/hw/tpm/trace-events
> index 25bee0cecf..920d32ad55 100644
> --- a/hw/tpm/trace-events
> +++ b/hw/tpm/trace-events
> @@ -51,3 +51,6 @@ tpm_tis_mmio_write_init_abort(void) "Initiating abort"
> tpm_tis_mmio_write_lowering_irq(void) "Lowering IRQ"
> tpm_tis_mmio_write_data2send(uint32_t value, unsigned size) "Data to send to TPM: 0x%08x (size=%d)"
> tpm_tis_pre_save(uint8_t locty, uint32_t rw_offset) "locty: %d, rw_offset = %u"
> +
> +# hw/tpm/tpm_ppi.c
> +tpm_ppi_memset(uint8_t *ptr, size_t size) "memset: %p %zu"
> --
> 2.19.0.rc1
>
>
--
Marc-André Lureau
© 2016 - 2026 Red Hat, Inc.