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

[Igor Mammedov <imammedo@redhat.com>]

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"