Re: [Qemu-devel] [PATCH for-4.2 2/2] spapr: initial implementation for H_TPM_COMM/spapr-tpm-proxy

2019-07-17 Thread Michael Roth
Quoting David Gibson (2019-07-16 21:01:15)
> On Tue, Jul 16, 2019 at 06:53:13PM -0500, Michael Roth wrote:
> > 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
> 
> Mostly LGTM, but..
> 
> [...]
> >  #define H_SUCCESS 0
> > @@ -490,8 +492,9 @@ struct SpaprMachineState {
> >  #define H_INT_ESB   0x3C8
> >  #define H_INT_SYNC  0x3CC
> >  #define H_INT_RESET 0x3D0
> > +#define H_TPM_COMM  0xEF10
> 
> This is vastly increasing the size of the hcall dispatch table, which
> isn't great.  Is the 0xE... range reserved for PEF related hypercalls?
> I'm wondering if we want to make a third table here (we already have a
> separate one for the qemu-specific hypercalls).

Yes, that's probably a good idea. SVM hcalls use a reserved range
0xEF00-0xEF80. I'll send a v2 that uses a separate table for these.

> 
> >  
> > -#define MAX_HCALL_OPCODEH_INT_RESET
> > +#define MAX_HCALL_OPCODEH_TPM_COMM
> >  
> >  /* The hcalls above are standardized in PAPR and implemented by pHyp
> >   * as well.
> > diff --git a/include/hw/ppc/spapr_tpm_proxy.h 
> > b/include/hw/ppc/spapr_tpm_proxy.h
> > new file mode 100644
> > index 00..4843cdaf58
> > --- /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  
> > + *
> > + * 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.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 */
> 
> -- 
> David Gibson| I'll have my music baroque, and my code
> david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
> | _way_ _around_!
> http://www.ozlabs.org/~dgibson



Re: [Qemu-devel] [PATCH for-4.2 2/2] spapr: initial implementation for H_TPM_COMM/spapr-tpm-proxy

2019-07-16 Thread David Gibson
On Tue, Jul 16, 2019 at 06:53:13PM -0500, Michael Roth wrote:
> 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

Mostly LGTM, but..

[...]
>  #define H_SUCCESS 0
> @@ -490,8 +492,9 @@ struct SpaprMachineState {
>  #define H_INT_ESB   0x3C8
>  #define H_INT_SYNC  0x3CC
>  #define H_INT_RESET 0x3D0
> +#define H_TPM_COMM  0xEF10

This is vastly increasing the size of the hcall dispatch table, which
isn't great.  Is the 0xE... range reserved for PEF related hypercalls?
I'm wondering if we want to make a third table here (we already have a
separate one for the qemu-specific hypercalls).

>  
> -#define MAX_HCALL_OPCODEH_INT_RESET
> +#define MAX_HCALL_OPCODEH_TPM_COMM
>  
>  /* The hcalls above are standardized in PAPR and implemented by pHyp
>   * as well.
> diff --git a/include/hw/ppc/spapr_tpm_proxy.h 
> b/include/hw/ppc/spapr_tpm_proxy.h
> new file mode 100644
> index 00..4843cdaf58
> --- /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  
> + *
> + * 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.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 */

-- 
David Gibson| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
| _way_ _around_!
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature


[Qemu-devel] [PATCH for-4.2 2/2] spapr: initial implementation for H_TPM_COMM/spapr-tpm-proxy

2019-07-16 Thread Michael Roth
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

Signed-off-by: Michael Roth 
 
@@ -4031,6 +4032,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)
 {
@@ -4040,6 +4064,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);
 }
 }
 
@@ -4052,6 +4078,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);
 }
 }
 
@@ -4086,6 +4114,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);
 }
 }
 
@@ -4106,7 +4136,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_tpm_proxy.c b/hw/ppc/spapr_tpm_proxy.c
new file mode 100644
index 00..00c2ecf547
--- /dev/null
+++ b/hw/ppc/spapr_tpm_proxy.c
@@ -0,0 +1,176 @@
+/*
+ * SPAPR TPM Proxy/Hypercall
+ *
+ * Copyright IBM Corp. 2019
+ *
+ * Authors:
+ *  Michael Roth  
+ *
+ * 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 "cpu.h"
+#include "hw/ppc/spapr.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,
+