[PULL,20/42] spapr: initial implementation for H_TPM_COMM/spapr-tpm-proxy

Commit Message

David Gibson Aug. 21, 2019, 7:25 a.m. UTC

From: Michael Roth <[e-mail]>

This implements the H_TPM_COMM hypercall, which is used by an
Ultravisor to pass TPM commands directly to the host's TPM device, or
a TPM Resource Manager associated with the device.

This also introduces a new virtual device, spapr-tpm-proxy, which
is used to configure the host TPM path to be used to service
requests sent by H_TPM_COMM hcalls, for example:

  -device spapr-tpm-proxy,id=tpmp0,host-path=/dev/tpmrm0

By default, no spapr-tpm-proxy will be created, and hcalls will return
H_FUNCTION.

The full specification for this hypercall can be found in
docs/specs/ppc-spapr-uv-hcalls.txt

Since SVM-related hcalls like H_TPM_COMM use a reserved range of
0xEF00-0xEF80, we introduce a separate hcall table here to handle
them.

Signed-off-by: Michael Roth <[e-mail]
Message-Id: <[e-mail]>
[dwg: Corrected #include for upstream change]
Signed-off-by: David Gibson <[e-mail]>
---
 hw/ppc/Makefile.objs             |   1 +
 hw/ppc/spapr.c                   |  33 +++++-
 hw/ppc/spapr_hcall.c             |  13 +++
 hw/ppc/spapr_tpm_proxy.c         | 178 +++++++++++++++++++++++++++++++
 hw/ppc/trace-events              |   4 +
 include/hw/ppc/spapr.h           |  11 ++
 include/hw/ppc/spapr_tpm_proxy.h |  31 ++++++
 7 files changed, 270 insertions(+), 1 deletion(-)
 create mode 100644 hw/ppc/spapr_tpm_proxy.c
 create mode 100644 include/hw/ppc/spapr_tpm_proxy.h

Comments

Peter Maydell Sept. 9, 2019, 5:23 p.m. UTC | #1

On Wed, 21 Aug 2019 at 08:26, David Gibson <[e-mail]> wrote:
>
> From: Michael Roth <[e-mail]>
>
> This implements the H_TPM_COMM hypercall, which is used by an
> Ultravisor to pass TPM commands directly to the host's TPM device, or
> a TPM Resource Manager associated with the device.
>
> This also introduces a new virtual device, spapr-tpm-proxy, which
> is used to configure the host TPM path to be used to service
> requests sent by H_TPM_COMM hcalls, for example:
>
>   -device spapr-tpm-proxy,id=tpmp0,host-path=/dev/tpmrm0
>
> By default, no spapr-tpm-proxy will be created, and hcalls will return
> H_FUNCTION.
>
> The full specification for this hypercall can be found in
> docs/specs/ppc-spapr-uv-hcalls.txt
>
> Since SVM-related hcalls like H_TPM_COMM use a reserved range of
> 0xEF00-0xEF80, we introduce a separate hcall table here to handle
> them.
>
> Signed-off-by: Michael Roth <[e-mail]
> Message-Id: <[e-mail]>
> [dwg: Corrected #include for upstream change]
> Signed-off-by: David Gibson <[e-mail]>

Hi; Coverity reports an issue in this change (CID 1405304):

> +static ssize_t tpm_execute(SpaprTpmProxy *tpm_proxy, target_ulong *args)
> +{
> +    uint64_t data_in = ppc64_phys_to_real(args[1]);
> +    target_ulong data_in_size = args[2];
> +    uint64_t data_out = ppc64_phys_to_real(args[3]);
> +    target_ulong data_out_size = args[4];
> +    uint8_t buf_in[TPM_SPAPR_BUFSIZE];
> +    uint8_t buf_out[TPM_SPAPR_BUFSIZE];
> +    ssize_t ret;
> +
> +    trace_spapr_tpm_execute(data_in, data_in_size, data_out, data_out_size);
> +
> +    if (data_in_size > TPM_SPAPR_BUFSIZE) {
> +        error_report("invalid TPM input buffer size: " TARGET_FMT_lu,
> +                     data_in_size);
> +        return H_P3;
> +    }
> +
> +    if (data_out_size < TPM_SPAPR_BUFSIZE) {
> +        error_report("invalid TPM output buffer size: " TARGET_FMT_lu,
> +                     data_out_size);
> +        return H_P5;
> +    }
> +
> +    if (tpm_proxy->host_fd == -1) {
> +        tpm_proxy->host_fd = open(tpm_proxy->host_path, O_RDWR);
> +        if (tpm_proxy->host_fd == -1) {
> +            error_report("failed to open TPM device %s: %d",
> +                         tpm_proxy->host_path, errno);
> +            return H_RESOURCE;
> +        }
> +    }
> +
> +    cpu_physical_memory_read(data_in, buf_in, data_in_size);
> +
> +    do {
> +        ret = write(tpm_proxy->host_fd, buf_in, data_in_size);
> +        if (ret > 0) {
> +            data_in_size -= ret;
> +        }
> +    } while ((ret >= 0 && data_in_size > 0) || (ret == -1 && errno == EINTR));
> +
> +    if (ret == -1) {
> +        error_report("failed to write to TPM device %s: %d",
> +                     tpm_proxy->host_path, errno);
> +        return H_RESOURCE;
> +    }
> +
> +    do {
> +        ret = read(tpm_proxy->host_fd, buf_out, data_out_size);
> +    } while (ret == 0 || (ret == -1 && errno == EINTR));
> +
> +    if (ret == -1) {
> +        error_report("failed to read from TPM device %s: %d",
> +                     tpm_proxy->host_path, errno);

The tpm_execute() function can unconditionally dereference
tpm_proxy->host_path, implying it can never be NULL...

> +        return H_RESOURCE;
> +    }
> +
> +    cpu_physical_memory_write(data_out, buf_out, ret);
> +    args[0] = ret;
> +
> +    return H_SUCCESS;
> +}
> +
> +static target_ulong h_tpm_comm(PowerPCCPU *cpu,
> +                               SpaprMachineState *spapr,
> +                               target_ulong opcode,
> +                               target_ulong *args)
> +{
> +    target_ulong op = args[0];
> +    SpaprTpmProxy *tpm_proxy = spapr->tpm_proxy;
> +
> +    if (!tpm_proxy) {
> +        error_report("TPM proxy not available");
> +        return H_FUNCTION;
> +    }
> +
> +    trace_spapr_h_tpm_comm(tpm_proxy->host_path ?: "null", op);

...but in this tracing at the callsite we check for whether
it is NULL or not, implying that it can be NULL.

> +
> +    switch (op) {
> +    case TPM_COMM_OP_EXECUTE:
> +        return tpm_execute(tpm_proxy, args);
> +    case TPM_COMM_OP_CLOSE_SESSION:
> +        spapr_tpm_proxy_reset(tpm_proxy);
> +        return H_SUCCESS;
> +    default:
> +        return H_PARAMETER;
> +    }
> +}

Coverity isn't happy about the possible use-after-NULL-check.

thanks
-- PMM

Greg Kurz Sept. 9, 2019, 5:51 p.m. UTC | #2

On Mon, 9 Sep 2019 18:23:20 +0100
Peter Maydell <[e-mail]> wrote:

> On Wed, 21 Aug 2019 at 08:26, David Gibson <[e-mail]> wrote:
> >
> > From: Michael Roth <[e-mail]>
> >
> > This implements the H_TPM_COMM hypercall, which is used by an
> > Ultravisor to pass TPM commands directly to the host's TPM device, or
> > a TPM Resource Manager associated with the device.
> >
> > This also introduces a new virtual device, spapr-tpm-proxy, which
> > is used to configure the host TPM path to be used to service
> > requests sent by H_TPM_COMM hcalls, for example:
> >
> >   -device spapr-tpm-proxy,id=tpmp0,host-path=/dev/tpmrm0
> >
> > By default, no spapr-tpm-proxy will be created, and hcalls will return
> > H_FUNCTION.
> >
> > The full specification for this hypercall can be found in
> > docs/specs/ppc-spapr-uv-hcalls.txt
> >
> > Since SVM-related hcalls like H_TPM_COMM use a reserved range of
> > 0xEF00-0xEF80, we introduce a separate hcall table here to handle
> > them.
> >
> > Signed-off-by: Michael Roth <[e-mail]
> > Message-Id: <[e-mail]>
> > [dwg: Corrected #include for upstream change]
> > Signed-off-by: David Gibson <[e-mail]>
> 
> Hi; Coverity reports an issue in this change (CID 1405304):
> 
> > +static ssize_t tpm_execute(SpaprTpmProxy *tpm_proxy, target_ulong *args)
> > +{
> > +    uint64_t data_in = ppc64_phys_to_real(args[1]);
> > +    target_ulong data_in_size = args[2];
> > +    uint64_t data_out = ppc64_phys_to_real(args[3]);
> > +    target_ulong data_out_size = args[4];
> > +    uint8_t buf_in[TPM_SPAPR_BUFSIZE];
> > +    uint8_t buf_out[TPM_SPAPR_BUFSIZE];
> > +    ssize_t ret;
> > +
> > +    trace_spapr_tpm_execute(data_in, data_in_size, data_out, data_out_size);
> > +
> > +    if (data_in_size > TPM_SPAPR_BUFSIZE) {
> > +        error_report("invalid TPM input buffer size: " TARGET_FMT_lu,
> > +                     data_in_size);
> > +        return H_P3;
> > +    }
> > +
> > +    if (data_out_size < TPM_SPAPR_BUFSIZE) {
> > +        error_report("invalid TPM output buffer size: " TARGET_FMT_lu,
> > +                     data_out_size);
> > +        return H_P5;
> > +    }
> > +
> > +    if (tpm_proxy->host_fd == -1) {
> > +        tpm_proxy->host_fd = open(tpm_proxy->host_path, O_RDWR);
> > +        if (tpm_proxy->host_fd == -1) {
> > +            error_report("failed to open TPM device %s: %d",
> > +                         tpm_proxy->host_path, errno);
> > +            return H_RESOURCE;
> > +        }
> > +    }
> > +
> > +    cpu_physical_memory_read(data_in, buf_in, data_in_size);
> > +
> > +    do {
> > +        ret = write(tpm_proxy->host_fd, buf_in, data_in_size);
> > +        if (ret > 0) {
> > +            data_in_size -= ret;
> > +        }
> > +    } while ((ret >= 0 && data_in_size > 0) || (ret == -1 && errno == EINTR));
> > +
> > +    if (ret == -1) {
> > +        error_report("failed to write to TPM device %s: %d",
> > +                     tpm_proxy->host_path, errno);
> > +        return H_RESOURCE;
> > +    }
> > +
> > +    do {
> > +        ret = read(tpm_proxy->host_fd, buf_out, data_out_size);
> > +    } while (ret == 0 || (ret == -1 && errno == EINTR));
> > +
> > +    if (ret == -1) {
> > +        error_report("failed to read from TPM device %s: %d",
> > +                     tpm_proxy->host_path, errno);
> 
> The tpm_execute() function can unconditionally dereference
> tpm_proxy->host_path, implying it can never be NULL...
> 
> > +        return H_RESOURCE;
> > +    }
> > +
> > +    cpu_physical_memory_write(data_out, buf_out, ret);
> > +    args[0] = ret;
> > +
> > +    return H_SUCCESS;
> > +}
> > +
> > +static target_ulong h_tpm_comm(PowerPCCPU *cpu,
> > +                               SpaprMachineState *spapr,
> > +                               target_ulong opcode,
> > +                               target_ulong *args)
> > +{
> > +    target_ulong op = args[0];
> > +    SpaprTpmProxy *tpm_proxy = spapr->tpm_proxy;
> > +
> > +    if (!tpm_proxy) {
> > +        error_report("TPM proxy not available");
> > +        return H_FUNCTION;
> > +    }
> > +
> > +    trace_spapr_h_tpm_comm(tpm_proxy->host_path ?: "null", op);
> 
> ...but in this tracing at the callsite we check for whether
> it is NULL or not, implying that it can be NULL.
> 

And we have this in the device realize function:

static void spapr_tpm_proxy_realize(DeviceState *d, Error **errp)
{
    SpaprTpmProxy *tpm_proxy = SPAPR_TPM_PROXY(d);

    if (tpm_proxy->host_path == NULL) {
        error_setg(errp, "must specify 'host-path' option for device");
        return;
    }
[...]
}

so we can safely assume host_path is never NULL. I guess the fix is to
stop checking tpm_proxy->host_path in the trace callsite above.

> > +
> > +    switch (op) {
> > +    case TPM_COMM_OP_EXECUTE:
> > +        return tpm_execute(tpm_proxy, args);
> > +    case TPM_COMM_OP_CLOSE_SESSION:
> > +        spapr_tpm_proxy_reset(tpm_proxy);
> > +        return H_SUCCESS;
> > +    default:
> > +        return H_PARAMETER;
> > +    }
> > +}
> 
> Coverity isn't happy about the possible use-after-NULL-check.
> 
> thanks
> -- PMM

Patch

diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs
index 9da93af905..2c4e1c8de0 100644
--- a/hw/ppc/Makefile.objs
+++ b/hw/ppc/Makefile.objs
@@ -5,6 +5,7 @@  obj-$(CONFIG_PSERIES) += spapr.o spapr_caps.o spapr_vio.o spapr_events.o
 obj-$(CONFIG_PSERIES) += spapr_hcall.o spapr_iommu.o spapr_rtas.o
 obj-$(CONFIG_PSERIES) += spapr_pci.o spapr_rtc.o spapr_drc.o
 obj-$(CONFIG_PSERIES) += spapr_cpu_core.o spapr_ovec.o spapr_irq.o
+obj-$(CONFIG_PSERIES) += spapr_tpm_proxy.o
 obj-$(CONFIG_SPAPR_RNG) +=  spapr_rng.o
 # IBM PowerNV
 obj-$(CONFIG_POWERNV) += pnv.o pnv_xscom.o pnv_core.o pnv_lpc.o pnv_psi.o pnv_occ.o pnv_bmc.o
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 15e27d9a10..56b33571c5 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -79,6 +79,7 @@ 
 #include "qemu/cutils.h"
 #include "hw/ppc/spapr_cpu_core.h"
 #include "hw/mem/memory-device.h"
+#include "hw/ppc/spapr_tpm_proxy.h"
 
 #include <libfdt.h>
 
@@ -4036,6 +4037,29 @@  static void spapr_phb_unplug_request(HotplugHandler *hotplug_dev,
     }
 }
 
+static void spapr_tpm_proxy_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
+                                 Error **errp)
+{
+    SpaprMachineState *spapr = SPAPR_MACHINE(OBJECT(hotplug_dev));
+    SpaprTpmProxy *tpm_proxy = SPAPR_TPM_PROXY(dev);
+
+    if (spapr->tpm_proxy != NULL) {
+        error_setg(errp, "Only one TPM proxy can be specified for this machine");
+        return;
+    }
+
+    spapr->tpm_proxy = tpm_proxy;
+}
+
+static void spapr_tpm_proxy_unplug(HotplugHandler *hotplug_dev, DeviceState *dev)
+{
+    SpaprMachineState *spapr = SPAPR_MACHINE(OBJECT(hotplug_dev));
+
+    object_property_set_bool(OBJECT(dev), false, "realized", NULL);
+    object_unparent(OBJECT(dev));
+    spapr->tpm_proxy = NULL;
+}
+
 static void spapr_machine_device_plug(HotplugHandler *hotplug_dev,
                                       DeviceState *dev, Error **errp)
 {
@@ -4045,6 +4069,8 @@  static void spapr_machine_device_plug(HotplugHandler *hotplug_dev,
         spapr_core_plug(hotplug_dev, dev, errp);
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_PCI_HOST_BRIDGE)) {
         spapr_phb_plug(hotplug_dev, dev, errp);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_TPM_PROXY)) {
+        spapr_tpm_proxy_plug(hotplug_dev, dev, errp);
     }
 }
 
@@ -4057,6 +4083,8 @@  static void spapr_machine_device_unplug(HotplugHandler *hotplug_dev,
         spapr_core_unplug(hotplug_dev, dev);
     } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_PCI_HOST_BRIDGE)) {
         spapr_phb_unplug(hotplug_dev, dev);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_TPM_PROXY)) {
+        spapr_tpm_proxy_unplug(hotplug_dev, dev);
     }
 }
 
@@ -4091,6 +4119,8 @@  static void spapr_machine_device_unplug_request(HotplugHandler *hotplug_dev,
             return;
         }
         spapr_phb_unplug_request(hotplug_dev, dev, errp);
+    } else if (object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_TPM_PROXY)) {
+        spapr_tpm_proxy_unplug(hotplug_dev, dev);
     }
 }
 
@@ -4111,7 +4141,8 @@  static HotplugHandler *spapr_get_hotplug_handler(MachineState *machine,
 {
     if (object_dynamic_cast(OBJECT(dev), TYPE_PC_DIMM) ||
         object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_CPU_CORE) ||
-        object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_PCI_HOST_BRIDGE)) {
+        object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_PCI_HOST_BRIDGE) ||
+        object_dynamic_cast(OBJECT(dev), TYPE_SPAPR_TPM_PROXY)) {
         return HOTPLUG_HANDLER(machine);
     }
     if (object_dynamic_cast(OBJECT(dev), TYPE_PCI_DEVICE)) {
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 10c0b53339..e20a946b99 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1963,6 +1963,7 @@  static target_ulong h_update_dt(PowerPCCPU *cpu, SpaprMachineState *spapr,
 
 static spapr_hcall_fn papr_hypercall_table[(MAX_HCALL_OPCODE / 4) + 1];
 static spapr_hcall_fn kvmppc_hypercall_table[KVMPPC_HCALL_MAX - KVMPPC_HCALL_BASE + 1];
+static spapr_hcall_fn svm_hypercall_table[(SVM_HCALL_MAX - SVM_HCALL_BASE) / 4 + 1];
 
 void spapr_register_hypercall(target_ulong opcode, spapr_hcall_fn fn)
 {
@@ -1972,6 +1973,11 @@  void spapr_register_hypercall(target_ulong opcode, spapr_hcall_fn fn)
         assert((opcode & 0x3) == 0);
 
         slot = &papr_hypercall_table[opcode / 4];
+    } else if (opcode >= SVM_HCALL_BASE && opcode <= SVM_HCALL_MAX) {
+        /* we only have SVM-related hcall numbers assigned in multiples of 4 */
+        assert((opcode & 0x3) == 0);
+
+        slot = &svm_hypercall_table[(opcode - SVM_HCALL_BASE) / 4];
     } else {
         assert((opcode >= KVMPPC_HCALL_BASE) && (opcode <= KVMPPC_HCALL_MAX));
 
@@ -1991,6 +1997,13 @@  target_ulong spapr_hypercall(PowerPCCPU *cpu, target_ulong opcode,
         && ((opcode & 0x3) == 0)) {
         spapr_hcall_fn fn = papr_hypercall_table[opcode / 4];
 
+        if (fn) {
+            return fn(cpu, spapr, opcode, args);
+        }
+    } else if ((opcode >= SVM_HCALL_BASE) &&
+               (opcode <= SVM_HCALL_MAX)) {
+        spapr_hcall_fn fn = svm_hypercall_table[(opcode - SVM_HCALL_BASE) / 4];
+
         if (fn) {
             return fn(cpu, spapr, opcode, args);
         }
diff --git a/hw/ppc/spapr_tpm_proxy.c b/hw/ppc/spapr_tpm_proxy.c
new file mode 100644
index 0000000000..b835d25be6
--- /dev/null
+++ b/hw/ppc/spapr_tpm_proxy.c
@@ -0,0 +1,178 @@ 
+/*
+ * SPAPR TPM Proxy/Hypercall
+ *
+ * Copyright IBM Corp. 2019
+ *
+ * Authors:
+ *  Michael Roth      <[e-mail]>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "qapi/error.h"
+#include "qemu/error-report.h"
+#include "sysemu/reset.h"
+#include "cpu.h"
+#include "hw/ppc/spapr.h"
+#include "hw/qdev-properties.h"
+#include "trace.h"
+
+#define TPM_SPAPR_BUFSIZE 4096
+
+enum {
+    TPM_COMM_OP_EXECUTE = 1,
+    TPM_COMM_OP_CLOSE_SESSION = 2,
+};
+
+static void spapr_tpm_proxy_reset(void *opaque)
+{
+    SpaprTpmProxy *tpm_proxy = SPAPR_TPM_PROXY(opaque);
+
+    if (tpm_proxy->host_fd != -1) {
+        close(tpm_proxy->host_fd);
+        tpm_proxy->host_fd = -1;
+    }
+}
+
+static ssize_t tpm_execute(SpaprTpmProxy *tpm_proxy, target_ulong *args)
+{
+    uint64_t data_in = ppc64_phys_to_real(args[1]);
+    target_ulong data_in_size = args[2];
+    uint64_t data_out = ppc64_phys_to_real(args[3]);
+    target_ulong data_out_size = args[4];
+    uint8_t buf_in[TPM_SPAPR_BUFSIZE];
+    uint8_t buf_out[TPM_SPAPR_BUFSIZE];
+    ssize_t ret;
+
+    trace_spapr_tpm_execute(data_in, data_in_size, data_out, data_out_size);
+
+    if (data_in_size > TPM_SPAPR_BUFSIZE) {
+        error_report("invalid TPM input buffer size: " TARGET_FMT_lu,
+                     data_in_size);
+        return H_P3;
+    }
+
+    if (data_out_size < TPM_SPAPR_BUFSIZE) {
+        error_report("invalid TPM output buffer size: " TARGET_FMT_lu,
+                     data_out_size);
+        return H_P5;
+    }
+
+    if (tpm_proxy->host_fd == -1) {
+        tpm_proxy->host_fd = open(tpm_proxy->host_path, O_RDWR);
+        if (tpm_proxy->host_fd == -1) {
+            error_report("failed to open TPM device %s: %d",
+                         tpm_proxy->host_path, errno);
+            return H_RESOURCE;
+        }
+    }
+
+    cpu_physical_memory_read(data_in, buf_in, data_in_size);
+
+    do {
+        ret = write(tpm_proxy->host_fd, buf_in, data_in_size);
+        if (ret > 0) {
+            data_in_size -= ret;
+        }
+    } while ((ret >= 0 && data_in_size > 0) || (ret == -1 && errno == EINTR));
+
+    if (ret == -1) {
+        error_report("failed to write to TPM device %s: %d",
+                     tpm_proxy->host_path, errno);
+        return H_RESOURCE;
+    }
+
+    do {
+        ret = read(tpm_proxy->host_fd, buf_out, data_out_size);
+    } while (ret == 0 || (ret == -1 && errno == EINTR));
+
+    if (ret == -1) {
+        error_report("failed to read from TPM device %s: %d",
+                     tpm_proxy->host_path, errno);
+        return H_RESOURCE;
+    }
+
+    cpu_physical_memory_write(data_out, buf_out, ret);
+    args[0] = ret;
+
+    return H_SUCCESS;
+}
+
+static target_ulong h_tpm_comm(PowerPCCPU *cpu,
+                               SpaprMachineState *spapr,
+                               target_ulong opcode,
+                               target_ulong *args)
+{
+    target_ulong op = args[0];
+    SpaprTpmProxy *tpm_proxy = spapr->tpm_proxy;
+
+    if (!tpm_proxy) {
+        error_report("TPM proxy not available");
+        return H_FUNCTION;
+    }
+
+    trace_spapr_h_tpm_comm(tpm_proxy->host_path ?: "null", op);
+
+    switch (op) {
+    case TPM_COMM_OP_EXECUTE:
+        return tpm_execute(tpm_proxy, args);
+    case TPM_COMM_OP_CLOSE_SESSION:
+        spapr_tpm_proxy_reset(tpm_proxy);
+        return H_SUCCESS;
+    default:
+        return H_PARAMETER;
+    }
+}
+
+static void spapr_tpm_proxy_realize(DeviceState *d, Error **errp)
+{
+    SpaprTpmProxy *tpm_proxy = SPAPR_TPM_PROXY(d);
+
+    if (tpm_proxy->host_path == NULL) {
+        error_setg(errp, "must specify 'host-path' option for device");
+        return;
+    }
+
+    tpm_proxy->host_fd = -1;
+    qemu_register_reset(spapr_tpm_proxy_reset, tpm_proxy);
+}
+
+static void spapr_tpm_proxy_unrealize(DeviceState *d, Error **errp)
+{
+    SpaprTpmProxy *tpm_proxy = SPAPR_TPM_PROXY(d);
+
+    qemu_unregister_reset(spapr_tpm_proxy_reset, tpm_proxy);
+}
+
+static Property spapr_tpm_proxy_properties[] = {
+    DEFINE_PROP_STRING("host-path", SpaprTpmProxy, host_path),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static void spapr_tpm_proxy_class_init(ObjectClass *k, void *data)
+{
+    DeviceClass *dk = DEVICE_CLASS(k);
+
+    dk->realize = spapr_tpm_proxy_realize;
+    dk->unrealize = spapr_tpm_proxy_unrealize;
+    dk->user_creatable = true;
+    dk->props = spapr_tpm_proxy_properties;
+}
+
+static const TypeInfo spapr_tpm_proxy_info = {
+    .name          = TYPE_SPAPR_TPM_PROXY,
+    .parent        = TYPE_DEVICE,
+    .instance_size = sizeof(SpaprTpmProxy),
+    .class_init    = spapr_tpm_proxy_class_init,
+};
+
+static void spapr_tpm_proxy_register_types(void)
+{
+    type_register_static(&spapr_tpm_proxy_info);
+    spapr_register_hypercall(SVM_H_TPM_COMM, h_tpm_comm);
+}
+
+type_init(spapr_tpm_proxy_register_types)
diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
index f76448f532..96dad767a1 100644
--- a/hw/ppc/trace-events
+++ b/hw/ppc/trace-events
@@ -25,6 +25,10 @@  spapr_update_dt(unsigned cb) "New blob %u bytes"
 spapr_update_dt_failed_size(unsigned cbold, unsigned cbnew, unsigned magic) "Old blob %u bytes, new blob %u bytes, magic 0x%x"
 spapr_update_dt_failed_check(unsigned cbold, unsigned cbnew, unsigned magic) "Old blob %u bytes, new blob %u bytes, magic 0x%x"
 
+# spapr_hcall_tpm.c
+spapr_h_tpm_comm(const char *device_path, uint64_t operation) "tpm_device_path=%s operation=0x%"PRIu64
+spapr_tpm_execute(uint64_t data_in, uint64_t data_in_sz, uint64_t data_out, uint64_t data_out_sz) "data_in=0x%"PRIx64", data_in_sz=%"PRIu64", data_out=0x%"PRIx64", data_out_sz=%"PRIu64
+
 # spapr_iommu.c
 spapr_iommu_put(uint64_t liobn, uint64_t ioba, uint64_t tce, uint64_t ret) "liobn=0x%"PRIx64" ioba=0x%"PRIx64" tce=0x%"PRIx64" ret=%"PRId64
 spapr_iommu_get(uint64_t liobn, uint64_t ioba, uint64_t ret, uint64_t tce) "liobn=0x%"PRIx64" ioba=0x%"PRIx64" ret=%"PRId64" tce=0x%"PRIx64
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 5d36eec9d0..c79bc6a123 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -10,6 +10,7 @@ 
 #include "hw/ppc/spapr_irq.h"
 #include "hw/ppc/spapr_xive.h"  /* For SpaprXive */
 #include "hw/ppc/xics.h"        /* For ICSState */
+#include "hw/ppc/spapr_tpm_proxy.h"
 
 struct SpaprVioBus;
 struct SpaprPhbState;
@@ -203,6 +204,7 @@  struct SpaprMachineState {
     SpaprCapabilities def, eff, mig;
 
     unsigned gpu_numa_id;
+    SpaprTpmProxy *tpm_proxy;
 };
 
 #define H_SUCCESS         0
@@ -508,6 +510,15 @@  struct SpaprMachineState {
 #define KVMPPC_H_UPDATE_DT      (KVMPPC_HCALL_BASE + 0x3)
 #define KVMPPC_HCALL_MAX        KVMPPC_H_UPDATE_DT
 
+/*
+ * The hcall range 0xEF00 to 0xEF80 is reserved for use in facilitating
+ * Secure VM mode via an Ultravisor / Protected Execution Facility
+ */
+#define SVM_HCALL_BASE              0xEF00
+#define SVM_H_TPM_COMM              0xEF10
+#define SVM_HCALL_MAX               SVM_H_TPM_COMM
+
+
 typedef struct SpaprDeviceTreeUpdateHeader {
     uint32_t version_id;
 } SpaprDeviceTreeUpdateHeader;
diff --git a/include/hw/ppc/spapr_tpm_proxy.h b/include/hw/ppc/spapr_tpm_proxy.h
new file mode 100644
index 0000000000..c574e22ba4
--- /dev/null
+++ b/include/hw/ppc/spapr_tpm_proxy.h
@@ -0,0 +1,31 @@ 
+/*
+ * SPAPR TPM Proxy/Hypercall
+ *
+ * Copyright IBM Corp. 2019
+ *
+ * Authors:
+ *  Michael Roth      <[e-mail]>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef HW_SPAPR_TPM_PROXY_H
+#define HW_SPAPR_TPM_PROXY_H
+
+#include "qom/object.h"
+#include "hw/qdev-core.h"
+
+#define TYPE_SPAPR_TPM_PROXY "spapr-tpm-proxy"
+#define SPAPR_TPM_PROXY(obj) OBJECT_CHECK(SpaprTpmProxy, (obj), \
+                                          TYPE_SPAPR_TPM_PROXY)
+
+typedef struct SpaprTpmProxy {
+    /*< private >*/
+    DeviceState parent;
+
+    char *host_path;
+    int host_fd;
+} SpaprTpmProxy;
+
+#endif /* HW_SPAPR_TPM_PROXY_H */