On Tue, 17 Jul 2018 17:39:12 +0200
Marc-André Lureau <marcandre.lur...@gmail.com> wrote:
> Hi
>
> On Tue, Jul 17, 2018 at 9:57 AM, Igor Mammedov <imamm...@redhat.com> wrote:
> > On Mon, 16 Jul 2018 14:59:48 +0200
> > Marc-André Lureau <marcandre.lur...@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.lur...@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"
> >
> >
>
>
>
