1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH v15 0/6] Add support for TPM Physical Presence interface
  4. View patch

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

Marc-André Lureau posted 6 patches 6 years, 10 months ago
There is a newer version of this series
[Qemu-devel] [PATCH v15 5/6] acpi: add ACPI memory clear interface
Posted by Marc-André Lureau 6 years, 10 months ago 
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.

See specification documentation for more details, and next commit for
memory clear on reset handling.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 hw/acpi/tpm.c      | 48 ++++++++++++++++++++++++++++++++++++++++++++++
 docs/specs/tpm.txt |  2 ++
 2 files changed, 50 insertions(+)

diff --git a/hw/acpi/tpm.c b/hw/acpi/tpm.c
index fee9490306..61da1de97b 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].
@@ -395,6 +405,44 @@ void tpm_build_ppi_acpi(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/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
-- 
2.20.1.2.gb21ebb671b
Re: [Qemu-devel] [PATCH v15 5/6] acpi: add ACPI memory clear interface
Posted by Michael S. Tsirkin 6 years, 10 months ago 
On Wed, Jan 09, 2019 at 12:29:01PM +0400, Marc-André Lureau wrote:
> 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.
> 
> See specification documentation for more details, and next commit for
> memory clear on reset handling.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  hw/acpi/tpm.c      | 48 ++++++++++++++++++++++++++++++++++++++++++++++
>  docs/specs/tpm.txt |  2 ++
>  2 files changed, 50 insertions(+)
> 
> diff --git a/hw/acpi/tpm.c b/hw/acpi/tpm.c
> index fee9490306..61da1de97b 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].
> @@ -395,6 +405,44 @@ void tpm_build_ppi_acpi(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 */

refer to spec chapter here?

> +            ifctx2 = aml_if(aml_equal(function, zero));
> +            {
> +                uint8_t byte_list[1] = { 0x03 };

and explain 3 above?

> +                aml_append(ifctx2, aml_return(aml_buffer(1

1 -> 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
> -- 
> 2.20.1.2.gb21ebb671b
Re: [Qemu-devel] [PATCH v15 5/6] acpi: add ACPI memory clear interface
Posted by Marc-André Lureau 6 years, 10 months ago 
Hi

On Wed, Jan 9, 2019 at 6:47 PM Michael S. Tsirkin <mst@redhat.com> wrote:
>
> On Wed, Jan 09, 2019 at 12:29:01PM +0400, Marc-André Lureau wrote:
> > 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.
> >
> > See specification documentation for more details, and next commit for
> > memory clear on reset handling.
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > ---
> >  hw/acpi/tpm.c      | 48 ++++++++++++++++++++++++++++++++++++++++++++++
> >  docs/specs/tpm.txt |  2 ++
> >  2 files changed, 50 insertions(+)
> >
> > diff --git a/hw/acpi/tpm.c b/hw/acpi/tpm.c
> > index fee9490306..61da1de97b 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].
> > @@ -395,6 +405,44 @@ void tpm_build_ppi_acpi(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 */
>
> refer to spec chapter here?

Yes, we could add a code comment such as:
/* "TCG Platform Reset Attack Mitigation Specification", chapter 6
"ACPI _DSM Function" */


>
> > +            ifctx2 = aml_if(aml_equal(function, zero));
> > +            {
> > +                uint8_t byte_list[1] = { 0x03 };
>
> and explain 3 above?

/* functions 1-2 are supported */

>
> > +                aml_append(ifctx2, aml_return(aml_buffer(1
>
> 1 -> sizeof(byte_list)?

ok

>
> > + , 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
> > --
> > 2.20.1.2.gb21ebb671b
>


-- 
Marc-André Lureau
Re: [Qemu-devel] [PATCH v15 5/6] acpi: add ACPI memory clear interface
Posted by Michael S. Tsirkin 6 years, 10 months ago 
On Wed, Jan 09, 2019 at 06:55:56PM +0400, Marc-André Lureau wrote:
> Hi
> 
> On Wed, Jan 9, 2019 at 6:47 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> > On Wed, Jan 09, 2019 at 12:29:01PM +0400, Marc-André Lureau wrote:
> > > 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.
> > >
> > > See specification documentation for more details, and next commit for
> > > memory clear on reset handling.
> > >
> > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > > ---
> > >  hw/acpi/tpm.c      | 48 ++++++++++++++++++++++++++++++++++++++++++++++
> > >  docs/specs/tpm.txt |  2 ++
> > >  2 files changed, 50 insertions(+)
> > >
> > > diff --git a/hw/acpi/tpm.c b/hw/acpi/tpm.c
> > > index fee9490306..61da1de97b 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].
> > > @@ -395,6 +405,44 @@ void tpm_build_ppi_acpi(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 */
> >
> > refer to spec chapter here?
> 
> Yes, we could add a code comment such as:
> /* "TCG Platform Reset Attack Mitigation Specification", chapter 6
> "ACPI _DSM Function" */

You want to include the version as well.

> 
> >
> > > +            ifctx2 = aml_if(aml_equal(function, zero));
> > > +            {
> > > +                uint8_t byte_list[1] = { 0x03 };
> >
> > and explain 3 above?
> 
> /* functions 1-2 are supported */
> 
> >
> > > +                aml_append(ifctx2, aml_return(aml_buffer(1
> >
> > 1 -> sizeof(byte_list)?
> 
> ok
> 
> >
> > > + , 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
> > > --
> > > 2.20.1.2.gb21ebb671b
> >
> 
> 
> -- 
> Marc-André Lureau

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.