Re: [Qemu-devel] [PATCH v16 5/6] acpi: add ACPI memory clear interface

[Marc-André Lureau]

Hi

On Mon, Jan 14, 2019 at 2:17 PM Igor Mammedov <address@hidden> wrote:
>
> On Wed,  9 Jan 2019 23:31:14 +0400
> Marc-André Lureau <address@hidden> wrote:
>
> > The interface is described in the "TCG Platform Reset Attack
> > Mitigation Specification", chapter 6 "ACPI _DSM Function". According
> I'd add a spec version here as well adn maybe a link like it's
> in 4/6 (but links are often go stale so it's not must have, just name
> and version should be sufficient to find the spec)
>
> > to Laszlo, it's not so easy to implement in OVMF, he suggested to do
> > it in qemu instead.
> >
> > See specification documentation for more details, and next commit for
> > memory clear on reset handling.
> >
> > Signed-off-by: Marc-André Lureau <address@hidden>
> > Reviewed-by: Michael S. Tsirkin <address@hidden>
> > Tested-by: Stefan Berger <address@hidden>
> With commit amended
>
> Reviewed-by: Igor Mammedov <address@hidden>

Specification Version 1.00
https://www.trustedcomputinggroup.org/wp-content/uploads/Platform-Reset-Attack-Mitigation-Specification.pdf

Michael, do you need me to resend the series or can you update the
commit message?

thanks

>
> > ---
> >  hw/acpi/tpm.c      | 55 ++++++++++++++++++++++++++++++++++++++++++++++
> >  docs/specs/tpm.txt |  2 ++
> >  2 files changed, 57 insertions(+)
> >
> > diff --git a/hw/acpi/tpm.c b/hw/acpi/tpm.c
> > index 9f205378f2..b96459e45b 100644
> > --- a/hw/acpi/tpm.c
> > +++ b/hw/acpi/tpm.c
> > @@ -53,6 +53,16 @@ void tpm_build_ppi_acpi(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 /* movv, docs/specs/tpm.txt */),
> > +                           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].
> > @@ -399,6 +409,51 @@ void tpm_build_ppi_acpi(TPMIf *tpm, Aml *dev)
> >              aml_append(ifctx, aml_return(aml_buffer(1, zerobyte)));
> >          }
> >          aml_append(method, ifctx);
> > +
> > +        /*
> > +         * "TCG Platform Reset Attack Mitigation Specification 1.00",
> > +         * Chapter 6 "ACPI _DSM Function"
> > +         */
> > +        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 }; /* functions 1-2 
> > supported */
> > +
> > +                aml_append(ifctx2,
> > +                           aml_return(aml_buffer(sizeof(byte_list),
> > +                                                 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/docs/specs/tpm.txt b/docs/specs/tpm.txt
> > index 424d1511fc..5d8c26b1ad 100644
> > --- a/docs/specs/tpm.txt
> > +++ b/docs/specs/tpm.txt
> > @@ -135,6 +135,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
>