| Message ID | 20190712011934.29863-3-mdroth@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | spapr: Implement H_TPM_COMM for accessing host TPM device | expand |
On Thu, Jul 11, 2019 at 08:19:34PM -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 pseries machine option which is used to > configure what TPM device to pass commands to, for example: > > -machine pseries,...,tpm-device-file=/dev/tmprm0 Bolting this into yet another machine parameter seems kind of ugly. Wouldn't it make more sense to treat this as an virtual device (say "spapr-vtpm"). Adding that device would enable the hcall, and would have properties for the back end host device. > By default, no tpm-device-file is defined and hcalls will return > H_RESOURCE. Wouldn't H_FUNCTION make more sense? > > The full specification for this hypercall can be found in > docs/specs/ppc-spapr-uv-hcalls.txt > > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com > --- > hw/ppc/Makefile.objs | 1 + > hw/ppc/spapr.c | 27 ++++++++ > hw/ppc/spapr_hcall_tpm.c | 135 +++++++++++++++++++++++++++++++++++++++ > hw/ppc/trace-events | 4 ++ > include/hw/ppc/spapr.h | 7 +- > 5 files changed, 173 insertions(+), 1 deletion(-) > create mode 100644 hw/ppc/spapr_hcall_tpm.c > > diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs > index 9da93af905..5aa120cae6 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_hcall_tpm.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 821f0d4a49..eb3421673b 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -1776,6 +1776,10 @@ static void spapr_machine_reset(MachineState *machine) > */ > object_child_foreach_recursive(object_get_root(), spapr_reset_drcs, NULL); > > + if (spapr->tpm_device_file) { > + spapr_hcall_tpm_reset(); > + } > + > spapr_clear_pending_events(spapr); > > /* > @@ -3340,6 +3344,21 @@ static void spapr_set_host_serial(Object *obj, const char *value, Error **errp) > spapr->host_serial = g_strdup(value); > } > > +static char *spapr_get_tpm_device_file(Object *obj, Error **errp) > +{ > + SpaprMachineState *spapr = SPAPR_MACHINE(obj); > + > + return g_strdup(spapr->tpm_device_file); > +} > + > +static void spapr_set_tpm_device_file(Object *obj, const char *value, Error **errp) > +{ > + SpaprMachineState *spapr = SPAPR_MACHINE(obj); > + > + g_free(spapr->tpm_device_file); > + spapr->tpm_device_file = g_strdup(value); > +} > + > static void spapr_instance_init(Object *obj) > { > SpaprMachineState *spapr = SPAPR_MACHINE(obj); > @@ -3396,6 +3415,14 @@ static void spapr_instance_init(Object *obj) > &error_abort); > object_property_set_description(obj, "host-serial", > "Host serial number to advertise in guest device tree", &error_abort); > + object_property_add_str(obj, "tpm-device-file", > + spapr_get_tpm_device_file, > + spapr_set_tpm_device_file, &error_abort); > + object_property_set_description(obj, "tpm-device-file", > + "Specifies the path to the TPM character device file to use" > + " for TPM communication via hypercalls (usually a TPM" > + " resource manager)", > + &error_abort); > } > > static void spapr_machine_finalizefn(Object *obj) > diff --git a/hw/ppc/spapr_hcall_tpm.c b/hw/ppc/spapr_hcall_tpm.c > new file mode 100644 > index 0000000000..75e2b6d594 > --- /dev/null > +++ b/hw/ppc/spapr_hcall_tpm.c > @@ -0,0 +1,135 @@ > +/* > + * SPAPR TPM Hypercall > + * > + * Copyright IBM Corp. 2019 > + * > + * Authors: > + * Michael Roth <mdroth@linux.vnet.ibm.com> > + * > + * 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 int tpm_devfd = -1; A global? Really? You can do better. > + > +static ssize_t tpm_execute(SpaprMachineState *spapr, 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 "\n", > + data_in_size); > + return H_P3; > + } > + > + if (data_out_size < TPM_SPAPR_BUFSIZE) { > + error_report("invalid TPM output buffer size: " TARGET_FMT_lu "\n", > + data_out_size); > + return H_P5; > + } > + > + if (tpm_devfd == -1) { > + tpm_devfd = open(spapr->tpm_device_file, O_RDWR); > + if (tpm_devfd == -1) { > + error_report("failed to open TPM device %s: %d", > + spapr->tpm_device_file, errno); > + return H_RESOURCE; > + } > + } > + > + cpu_physical_memory_read(data_in, buf_in, data_in_size); > + > + do { > + ret = write(tpm_devfd, 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", > + spapr->tpm_device_file, errno); > + return H_RESOURCE; > + } > + > + do { > + ret = read(tpm_devfd, 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", > + spapr->tpm_device_file, 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]; > + > + trace_spapr_h_tpm_comm(spapr->tpm_device_file ?: "null", op); > + > + if (!spapr->tpm_device_file) { > + error_report("TPM device not specified"); > + return H_RESOURCE; > + } > + > + switch (op) { > + case TPM_COMM_OP_EXECUTE: > + return tpm_execute(spapr, args); > + case TPM_COMM_OP_CLOSE_SESSION: > + if (tpm_devfd != -1) { > + close(tpm_devfd); > + tpm_devfd = -1; > + } > + return H_SUCCESS; > + default: > + return H_PARAMETER; > + } > +} > + > +void spapr_hcall_tpm_reset(void) > +{ > + if (tpm_devfd != -1) { > + close(tpm_devfd); > + tpm_devfd = -1; > + } > +} > + > +static void hypercall_register_types(void) > +{ > + spapr_register_hypercall(H_TPM_COMM, h_tpm_comm); > +} > + > +type_init(hypercall_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 60553d32c4..7bd47575d7 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -198,6 +198,7 @@ struct SpaprMachineState { > SpaprXive *xive; > SpaprIrq *irq; > qemu_irq *qirqs; > + char *tpm_device_file; > > bool cmd_line_caps[SPAPR_CAP_NUM]; > SpaprCapabilities def, eff, mig; > @@ -490,8 +491,9 @@ struct SpaprMachineState { > #define H_INT_ESB 0x3C8 > #define H_INT_SYNC 0x3CC > #define H_INT_RESET 0x3D0 > +#define H_TPM_COMM 0xEF10 > > -#define MAX_HCALL_OPCODE H_INT_RESET > +#define MAX_HCALL_OPCODE H_TPM_COMM > > /* The hcalls above are standardized in PAPR and implemented by pHyp > * as well. > @@ -864,6 +866,9 @@ int spapr_caps_post_migration(SpaprMachineState *spapr); > > void spapr_check_pagesize(SpaprMachineState *spapr, hwaddr pagesize, > Error **errp); > + > +void spapr_hcall_tpm_reset(void); > + > /* > * XIVE definitions > */
Quoting David Gibson (2019-07-12 01:46:19) > On Thu, Jul 11, 2019 at 08:19:34PM -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 pseries machine option which is used to > > configure what TPM device to pass commands to, for example: > > > > -machine pseries,...,tpm-device-file=/dev/tmprm0 > > Bolting this into yet another machine parameter seems kind of ugly. > Wouldn't it make more sense to treat this as an virtual device (say > "spapr-vtpm"). Adding that device would enable the hcall, and would > have properties for the back end host device. That does sound nicer. Originally I had SpaprMachineClass implement the TYPE_TPM_IF interface so we could define a TPM backend via -tpmdev passthrough,path=..., but after some discussion with the TPM maintainer it didn't quite work for the main use-case of passing through a TPM Resource Manager since it isn't suitable for full vTPM front-ends (since multiple guests can interfere with each other's operations when running the full gamut of TPM functionality). I hadn't consider a stand-alone -device implementation though. It's not a proper VIO or PCI device so there's no proper bus to attach it to. I guess we would just make it a direct child of SpaprMachineState (sort of like SpaprDrcClass), then we could define it via something like -object spapr-tpm-proxy,path=.... I'll go ahead and give that a shot, assuming it seems reasonable to you. > > > By default, no tpm-device-file is defined and hcalls will return > > H_RESOURCE. > > Wouldn't H_FUNCTION make more sense? Yes, for this case it probably would. Thanks for the suggestions! > > > > > The full specification for this hypercall can be found in > > docs/specs/ppc-spapr-uv-hcalls.txt > > > > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com > > --- > > hw/ppc/Makefile.objs | 1 + > > hw/ppc/spapr.c | 27 ++++++++ > > hw/ppc/spapr_hcall_tpm.c | 135 +++++++++++++++++++++++++++++++++++++++ > > hw/ppc/trace-events | 4 ++ > > include/hw/ppc/spapr.h | 7 +- > > 5 files changed, 173 insertions(+), 1 deletion(-) > > create mode 100644 hw/ppc/spapr_hcall_tpm.c > > > > diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs > > index 9da93af905..5aa120cae6 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_hcall_tpm.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 821f0d4a49..eb3421673b 100644 > > --- a/hw/ppc/spapr.c > > +++ b/hw/ppc/spapr.c > > @@ -1776,6 +1776,10 @@ static void spapr_machine_reset(MachineState *machine) > > */ > > object_child_foreach_recursive(object_get_root(), spapr_reset_drcs, NULL); > > > > + if (spapr->tpm_device_file) { > > + spapr_hcall_tpm_reset(); > > + } > > + > > spapr_clear_pending_events(spapr); > > > > /* > > @@ -3340,6 +3344,21 @@ static void spapr_set_host_serial(Object *obj, const char *value, Error **errp) > > spapr->host_serial = g_strdup(value); > > } > > > > +static char *spapr_get_tpm_device_file(Object *obj, Error **errp) > > +{ > > + SpaprMachineState *spapr = SPAPR_MACHINE(obj); > > + > > + return g_strdup(spapr->tpm_device_file); > > +} > > + > > +static void spapr_set_tpm_device_file(Object *obj, const char *value, Error **errp) > > +{ > > + SpaprMachineState *spapr = SPAPR_MACHINE(obj); > > + > > + g_free(spapr->tpm_device_file); > > + spapr->tpm_device_file = g_strdup(value); > > +} > > + > > static void spapr_instance_init(Object *obj) > > { > > SpaprMachineState *spapr = SPAPR_MACHINE(obj); > > @@ -3396,6 +3415,14 @@ static void spapr_instance_init(Object *obj) > > &error_abort); > > object_property_set_description(obj, "host-serial", > > "Host serial number to advertise in guest device tree", &error_abort); > > + object_property_add_str(obj, "tpm-device-file", > > + spapr_get_tpm_device_file, > > + spapr_set_tpm_device_file, &error_abort); > > + object_property_set_description(obj, "tpm-device-file", > > + "Specifies the path to the TPM character device file to use" > > + " for TPM communication via hypercalls (usually a TPM" > > + " resource manager)", > > + &error_abort); > > } > > > > static void spapr_machine_finalizefn(Object *obj) > > diff --git a/hw/ppc/spapr_hcall_tpm.c b/hw/ppc/spapr_hcall_tpm.c > > new file mode 100644 > > index 0000000000..75e2b6d594 > > --- /dev/null > > +++ b/hw/ppc/spapr_hcall_tpm.c > > @@ -0,0 +1,135 @@ > > +/* > > + * SPAPR TPM Hypercall > > + * > > + * Copyright IBM Corp. 2019 > > + * > > + * Authors: > > + * Michael Roth <mdroth@linux.vnet.ibm.com> > > + * > > + * 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 int tpm_devfd = -1; > > A global? Really? You can do better. > > > + > > +static ssize_t tpm_execute(SpaprMachineState *spapr, 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 "\n", > > + data_in_size); > > + return H_P3; > > + } > > + > > + if (data_out_size < TPM_SPAPR_BUFSIZE) { > > + error_report("invalid TPM output buffer size: " TARGET_FMT_lu "\n", > > + data_out_size); > > + return H_P5; > > + } > > + > > + if (tpm_devfd == -1) { > > + tpm_devfd = open(spapr->tpm_device_file, O_RDWR); > > + if (tpm_devfd == -1) { > > + error_report("failed to open TPM device %s: %d", > > + spapr->tpm_device_file, errno); > > + return H_RESOURCE; > > + } > > + } > > + > > + cpu_physical_memory_read(data_in, buf_in, data_in_size); > > + > > + do { > > + ret = write(tpm_devfd, 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", > > + spapr->tpm_device_file, errno); > > + return H_RESOURCE; > > + } > > + > > + do { > > + ret = read(tpm_devfd, 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", > > + spapr->tpm_device_file, 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]; > > + > > + trace_spapr_h_tpm_comm(spapr->tpm_device_file ?: "null", op); > > + > > + if (!spapr->tpm_device_file) { > > + error_report("TPM device not specified"); > > + return H_RESOURCE; > > + } > > + > > + switch (op) { > > + case TPM_COMM_OP_EXECUTE: > > + return tpm_execute(spapr, args); > > + case TPM_COMM_OP_CLOSE_SESSION: > > + if (tpm_devfd != -1) { > > + close(tpm_devfd); > > + tpm_devfd = -1; > > + } > > + return H_SUCCESS; > > + default: > > + return H_PARAMETER; > > + } > > +} > > + > > +void spapr_hcall_tpm_reset(void) > > +{ > > + if (tpm_devfd != -1) { > > + close(tpm_devfd); > > + tpm_devfd = -1; > > + } > > +} > > + > > +static void hypercall_register_types(void) > > +{ > > + spapr_register_hypercall(H_TPM_COMM, h_tpm_comm); > > +} > > + > > +type_init(hypercall_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 60553d32c4..7bd47575d7 100644 > > --- a/include/hw/ppc/spapr.h > > +++ b/include/hw/ppc/spapr.h > > @@ -198,6 +198,7 @@ struct SpaprMachineState { > > SpaprXive *xive; > > SpaprIrq *irq; > > qemu_irq *qirqs; > > + char *tpm_device_file; > > > > bool cmd_line_caps[SPAPR_CAP_NUM]; > > SpaprCapabilities def, eff, mig; > > @@ -490,8 +491,9 @@ struct SpaprMachineState { > > #define H_INT_ESB 0x3C8 > > #define H_INT_SYNC 0x3CC > > #define H_INT_RESET 0x3D0 > > +#define H_TPM_COMM 0xEF10 > > > > -#define MAX_HCALL_OPCODE H_INT_RESET > > +#define MAX_HCALL_OPCODE H_TPM_COMM > > > > /* The hcalls above are standardized in PAPR and implemented by pHyp > > * as well. > > @@ -864,6 +866,9 @@ int spapr_caps_post_migration(SpaprMachineState *spapr); > > > > void spapr_check_pagesize(SpaprMachineState *spapr, hwaddr pagesize, > > Error **errp); > > + > > +void spapr_hcall_tpm_reset(void); > > + > > /* > > * XIVE definitions > > */ > > -- > 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
On Fri, Jul 12, 2019 at 09:34:46AM -0500, Michael Roth wrote: > Quoting David Gibson (2019-07-12 01:46:19) > > On Thu, Jul 11, 2019 at 08:19:34PM -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 pseries machine option which is used to > > > configure what TPM device to pass commands to, for example: > > > > > > -machine pseries,...,tpm-device-file=/dev/tmprm0 > > > > Bolting this into yet another machine parameter seems kind of ugly. > > Wouldn't it make more sense to treat this as an virtual device (say > > "spapr-vtpm"). Adding that device would enable the hcall, and would > > have properties for the back end host device. > > That does sound nicer. > > Originally I had SpaprMachineClass implement the TYPE_TPM_IF interface so > we could define a TPM backend via -tpmdev passthrough,path=..., but after > some discussion with the TPM maintainer it didn't quite work for the main > use-case of passing through a TPM Resource Manager since it isn't suitable > for full vTPM front-ends (since multiple guests can interfere with each > other's operations when running the full gamut of TPM functionality). > > I hadn't consider a stand-alone -device implementation though. It's not > a proper VIO or PCI device so there's no proper bus to attach it to. I > guess we would just make it a direct child of SpaprMachineState (sort > of like SpaprDrcClass), then we could define it via something like > -object spapr-tpm-proxy,path=.... It should be -device not -object, but otherwise that looks ok. How does the TPM appear in the device tree? > I'll go ahead and give that a shot, assuming it seems reasonable to you. > > > > > > By default, no tpm-device-file is defined and hcalls will return > > > H_RESOURCE. > > > > Wouldn't H_FUNCTION make more sense? > > Yes, for this case it probably would. > > Thanks for the suggestions! > > > > > > > > > The full specification for this hypercall can be found in > > > docs/specs/ppc-spapr-uv-hcalls.txt > > > > > > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com > > > --- > > > hw/ppc/Makefile.objs | 1 + > > > hw/ppc/spapr.c | 27 ++++++++ > > > hw/ppc/spapr_hcall_tpm.c | 135 +++++++++++++++++++++++++++++++++++++++ > > > hw/ppc/trace-events | 4 ++ > > > include/hw/ppc/spapr.h | 7 +- > > > 5 files changed, 173 insertions(+), 1 deletion(-) > > > create mode 100644 hw/ppc/spapr_hcall_tpm.c > > > > > > diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs > > > index 9da93af905..5aa120cae6 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_hcall_tpm.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 821f0d4a49..eb3421673b 100644 > > > --- a/hw/ppc/spapr.c > > > +++ b/hw/ppc/spapr.c > > > @@ -1776,6 +1776,10 @@ static void spapr_machine_reset(MachineState *machine) > > > */ > > > object_child_foreach_recursive(object_get_root(), spapr_reset_drcs, NULL); > > > > > > + if (spapr->tpm_device_file) { > > > + spapr_hcall_tpm_reset(); > > > + } > > > + > > > spapr_clear_pending_events(spapr); > > > > > > /* > > > @@ -3340,6 +3344,21 @@ static void spapr_set_host_serial(Object *obj, const char *value, Error **errp) > > > spapr->host_serial = g_strdup(value); > > > } > > > > > > +static char *spapr_get_tpm_device_file(Object *obj, Error **errp) > > > +{ > > > + SpaprMachineState *spapr = SPAPR_MACHINE(obj); > > > + > > > + return g_strdup(spapr->tpm_device_file); > > > +} > > > + > > > +static void spapr_set_tpm_device_file(Object *obj, const char *value, Error **errp) > > > +{ > > > + SpaprMachineState *spapr = SPAPR_MACHINE(obj); > > > + > > > + g_free(spapr->tpm_device_file); > > > + spapr->tpm_device_file = g_strdup(value); > > > +} > > > + > > > static void spapr_instance_init(Object *obj) > > > { > > > SpaprMachineState *spapr = SPAPR_MACHINE(obj); > > > @@ -3396,6 +3415,14 @@ static void spapr_instance_init(Object *obj) > > > &error_abort); > > > object_property_set_description(obj, "host-serial", > > > "Host serial number to advertise in guest device tree", &error_abort); > > > + object_property_add_str(obj, "tpm-device-file", > > > + spapr_get_tpm_device_file, > > > + spapr_set_tpm_device_file, &error_abort); > > > + object_property_set_description(obj, "tpm-device-file", > > > + "Specifies the path to the TPM character device file to use" > > > + " for TPM communication via hypercalls (usually a TPM" > > > + " resource manager)", > > > + &error_abort); > > > } > > > > > > static void spapr_machine_finalizefn(Object *obj) > > > diff --git a/hw/ppc/spapr_hcall_tpm.c b/hw/ppc/spapr_hcall_tpm.c > > > new file mode 100644 > > > index 0000000000..75e2b6d594 > > > --- /dev/null > > > +++ b/hw/ppc/spapr_hcall_tpm.c > > > @@ -0,0 +1,135 @@ > > > +/* > > > + * SPAPR TPM Hypercall > > > + * > > > + * Copyright IBM Corp. 2019 > > > + * > > > + * Authors: > > > + * Michael Roth <mdroth@linux.vnet.ibm.com> > > > + * > > > + * 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 int tpm_devfd = -1; > > > > A global? Really? You can do better. > > > > > + > > > +static ssize_t tpm_execute(SpaprMachineState *spapr, 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 "\n", > > > + data_in_size); > > > + return H_P3; > > > + } > > > + > > > + if (data_out_size < TPM_SPAPR_BUFSIZE) { > > > + error_report("invalid TPM output buffer size: " TARGET_FMT_lu "\n", > > > + data_out_size); > > > + return H_P5; > > > + } > > > + > > > + if (tpm_devfd == -1) { > > > + tpm_devfd = open(spapr->tpm_device_file, O_RDWR); > > > + if (tpm_devfd == -1) { > > > + error_report("failed to open TPM device %s: %d", > > > + spapr->tpm_device_file, errno); > > > + return H_RESOURCE; > > > + } > > > + } > > > + > > > + cpu_physical_memory_read(data_in, buf_in, data_in_size); > > > + > > > + do { > > > + ret = write(tpm_devfd, 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", > > > + spapr->tpm_device_file, errno); > > > + return H_RESOURCE; > > > + } > > > + > > > + do { > > > + ret = read(tpm_devfd, 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", > > > + spapr->tpm_device_file, 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]; > > > + > > > + trace_spapr_h_tpm_comm(spapr->tpm_device_file ?: "null", op); > > > + > > > + if (!spapr->tpm_device_file) { > > > + error_report("TPM device not specified"); > > > + return H_RESOURCE; > > > + } > > > + > > > + switch (op) { > > > + case TPM_COMM_OP_EXECUTE: > > > + return tpm_execute(spapr, args); > > > + case TPM_COMM_OP_CLOSE_SESSION: > > > + if (tpm_devfd != -1) { > > > + close(tpm_devfd); > > > + tpm_devfd = -1; > > > + } > > > + return H_SUCCESS; > > > + default: > > > + return H_PARAMETER; > > > + } > > > +} > > > + > > > +void spapr_hcall_tpm_reset(void) > > > +{ > > > + if (tpm_devfd != -1) { > > > + close(tpm_devfd); > > > + tpm_devfd = -1; > > > + } > > > +} > > > + > > > +static void hypercall_register_types(void) > > > +{ > > > + spapr_register_hypercall(H_TPM_COMM, h_tpm_comm); > > > +} > > > + > > > +type_init(hypercall_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 60553d32c4..7bd47575d7 100644 > > > --- a/include/hw/ppc/spapr.h > > > +++ b/include/hw/ppc/spapr.h > > > @@ -198,6 +198,7 @@ struct SpaprMachineState { > > > SpaprXive *xive; > > > SpaprIrq *irq; > > > qemu_irq *qirqs; > > > + char *tpm_device_file; > > > > > > bool cmd_line_caps[SPAPR_CAP_NUM]; > > > SpaprCapabilities def, eff, mig; > > > @@ -490,8 +491,9 @@ struct SpaprMachineState { > > > #define H_INT_ESB 0x3C8 > > > #define H_INT_SYNC 0x3CC > > > #define H_INT_RESET 0x3D0 > > > +#define H_TPM_COMM 0xEF10 > > > > > > -#define MAX_HCALL_OPCODE H_INT_RESET > > > +#define MAX_HCALL_OPCODE H_TPM_COMM > > > > > > /* The hcalls above are standardized in PAPR and implemented by pHyp > > > * as well. > > > @@ -864,6 +866,9 @@ int spapr_caps_post_migration(SpaprMachineState *spapr); > > > > > > void spapr_check_pagesize(SpaprMachineState *spapr, hwaddr pagesize, > > > Error **errp); > > > + > > > +void spapr_hcall_tpm_reset(void); > > > + > > > /* > > > * XIVE definitions > > > */ > > >
Quoting David Gibson (2019-07-14 21:25:24) > On Fri, Jul 12, 2019 at 09:34:46AM -0500, Michael Roth wrote: > > Quoting David Gibson (2019-07-12 01:46:19) > > > On Thu, Jul 11, 2019 at 08:19:34PM -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 pseries machine option which is used to > > > > configure what TPM device to pass commands to, for example: > > > > > > > > -machine pseries,...,tpm-device-file=/dev/tmprm0 > > > > > > Bolting this into yet another machine parameter seems kind of ugly. > > > Wouldn't it make more sense to treat this as an virtual device (say > > > "spapr-vtpm"). Adding that device would enable the hcall, and would > > > have properties for the back end host device. > > > > That does sound nicer. > > > > Originally I had SpaprMachineClass implement the TYPE_TPM_IF interface so > > we could define a TPM backend via -tpmdev passthrough,path=..., but after > > some discussion with the TPM maintainer it didn't quite work for the main > > use-case of passing through a TPM Resource Manager since it isn't suitable > > for full vTPM front-ends (since multiple guests can interfere with each > > other's operations when running the full gamut of TPM functionality). > > > > I hadn't consider a stand-alone -device implementation though. It's not > > a proper VIO or PCI device so there's no proper bus to attach it to. I > > guess we would just make it a direct child of SpaprMachineState (sort > > of like SpaprDrcClass), then we could define it via something like > > -object spapr-tpm-proxy,path=.... > > It should be -device not -object, but otherwise that looks ok. Ok, for some reason I thought -device needed either a specific bus or needed to be a SysBusDevice to attach to main-system-bus, but maybe that was just for qdev-managed reset handling. I've re-worked the series to allow configuration via: -device spapr-tpm-proxy,host_path=/dev/tpmrmX > > How does the TPM appear in the device tree? Nothing in the guest, on the host it appears as: ./xscom@603fc00000000/i2cm@a2000/i2c-bus@0/tpm@57 ./xscom@603fc00000000/i2cm@a2000/i2c-bus@0/tpm@57/link-id ./xscom@603fc00000000/i2cm@a2000/i2c-bus@0/tpm@57/linux,sml-size ./xscom@603fc00000000/i2cm@a2000/i2c-bus@0/tpm@57/label ./xscom@603fc00000000/i2cm@a2000/i2c-bus@0/tpm@57/compatible ./xscom@603fc00000000/i2cm@a2000/i2c-bus@0/tpm@57/status ./xscom@603fc00000000/i2cm@a2000/i2c-bus@0/tpm@57/reg ./xscom@603fc00000000/i2cm@a2000/i2c-bus@0/tpm@57/phandle ./xscom@603fc00000000/i2cm@a2000/i2c-bus@0/tpm@57/linux,sml-base ./xscom@603fc00000000/i2cm@a2000/i2c-bus@0/tpm@57/name > > > I'll go ahead and give that a shot, assuming it seems reasonable to you. > > > > > > > > > By default, no tpm-device-file is defined and hcalls will return > > > > H_RESOURCE. > > > > > > Wouldn't H_FUNCTION make more sense? > > > > Yes, for this case it probably would. > > > > Thanks for the suggestions! > > > > > > > > > > > > > The full specification for this hypercall can be found in > > > > docs/specs/ppc-spapr-uv-hcalls.txt > > > > > > > > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com > > > > --- > > > > hw/ppc/Makefile.objs | 1 + > > > > hw/ppc/spapr.c | 27 ++++++++ > > > > hw/ppc/spapr_hcall_tpm.c | 135 +++++++++++++++++++++++++++++++++++++++ > > > > hw/ppc/trace-events | 4 ++ > > > > include/hw/ppc/spapr.h | 7 +- > > > > 5 files changed, 173 insertions(+), 1 deletion(-) > > > > create mode 100644 hw/ppc/spapr_hcall_tpm.c > > > > > > > > diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs > > > > index 9da93af905..5aa120cae6 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_hcall_tpm.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 821f0d4a49..eb3421673b 100644 > > > > --- a/hw/ppc/spapr.c > > > > +++ b/hw/ppc/spapr.c > > > > @@ -1776,6 +1776,10 @@ static void spapr_machine_reset(MachineState *machine) > > > > */ > > > > object_child_foreach_recursive(object_get_root(), spapr_reset_drcs, NULL); > > > > > > > > + if (spapr->tpm_device_file) { > > > > + spapr_hcall_tpm_reset(); > > > > + } > > > > + > > > > spapr_clear_pending_events(spapr); > > > > > > > > /* > > > > @@ -3340,6 +3344,21 @@ static void spapr_set_host_serial(Object *obj, const char *value, Error **errp) > > > > spapr->host_serial = g_strdup(value); > > > > } > > > > > > > > +static char *spapr_get_tpm_device_file(Object *obj, Error **errp) > > > > +{ > > > > + SpaprMachineState *spapr = SPAPR_MACHINE(obj); > > > > + > > > > + return g_strdup(spapr->tpm_device_file); > > > > +} > > > > + > > > > +static void spapr_set_tpm_device_file(Object *obj, const char *value, Error **errp) > > > > +{ > > > > + SpaprMachineState *spapr = SPAPR_MACHINE(obj); > > > > + > > > > + g_free(spapr->tpm_device_file); > > > > + spapr->tpm_device_file = g_strdup(value); > > > > +} > > > > + > > > > static void spapr_instance_init(Object *obj) > > > > { > > > > SpaprMachineState *spapr = SPAPR_MACHINE(obj); > > > > @@ -3396,6 +3415,14 @@ static void spapr_instance_init(Object *obj) > > > > &error_abort); > > > > object_property_set_description(obj, "host-serial", > > > > "Host serial number to advertise in guest device tree", &error_abort); > > > > + object_property_add_str(obj, "tpm-device-file", > > > > + spapr_get_tpm_device_file, > > > > + spapr_set_tpm_device_file, &error_abort); > > > > + object_property_set_description(obj, "tpm-device-file", > > > > + "Specifies the path to the TPM character device file to use" > > > > + " for TPM communication via hypercalls (usually a TPM" > > > > + " resource manager)", > > > > + &error_abort); > > > > } > > > > > > > > static void spapr_machine_finalizefn(Object *obj) > > > > diff --git a/hw/ppc/spapr_hcall_tpm.c b/hw/ppc/spapr_hcall_tpm.c > > > > new file mode 100644 > > > > index 0000000000..75e2b6d594 > > > > --- /dev/null > > > > +++ b/hw/ppc/spapr_hcall_tpm.c > > > > @@ -0,0 +1,135 @@ > > > > +/* > > > > + * SPAPR TPM Hypercall > > > > + * > > > > + * Copyright IBM Corp. 2019 > > > > + * > > > > + * Authors: > > > > + * Michael Roth <mdroth@linux.vnet.ibm.com> > > > > + * > > > > + * 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 int tpm_devfd = -1; > > > > > > A global? Really? You can do better. > > > > > > > + > > > > +static ssize_t tpm_execute(SpaprMachineState *spapr, 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 "\n", > > > > + data_in_size); > > > > + return H_P3; > > > > + } > > > > + > > > > + if (data_out_size < TPM_SPAPR_BUFSIZE) { > > > > + error_report("invalid TPM output buffer size: " TARGET_FMT_lu "\n", > > > > + data_out_size); > > > > + return H_P5; > > > > + } > > > > + > > > > + if (tpm_devfd == -1) { > > > > + tpm_devfd = open(spapr->tpm_device_file, O_RDWR); > > > > + if (tpm_devfd == -1) { > > > > + error_report("failed to open TPM device %s: %d", > > > > + spapr->tpm_device_file, errno); > > > > + return H_RESOURCE; > > > > + } > > > > + } > > > > + > > > > + cpu_physical_memory_read(data_in, buf_in, data_in_size); > > > > + > > > > + do { > > > > + ret = write(tpm_devfd, 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", > > > > + spapr->tpm_device_file, errno); > > > > + return H_RESOURCE; > > > > + } > > > > + > > > > + do { > > > > + ret = read(tpm_devfd, 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", > > > > + spapr->tpm_device_file, 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]; > > > > + > > > > + trace_spapr_h_tpm_comm(spapr->tpm_device_file ?: "null", op); > > > > + > > > > + if (!spapr->tpm_device_file) { > > > > + error_report("TPM device not specified"); > > > > + return H_RESOURCE; > > > > + } > > > > + > > > > + switch (op) { > > > > + case TPM_COMM_OP_EXECUTE: > > > > + return tpm_execute(spapr, args); > > > > + case TPM_COMM_OP_CLOSE_SESSION: > > > > + if (tpm_devfd != -1) { > > > > + close(tpm_devfd); > > > > + tpm_devfd = -1; > > > > + } > > > > + return H_SUCCESS; > > > > + default: > > > > + return H_PARAMETER; > > > > + } > > > > +} > > > > + > > > > +void spapr_hcall_tpm_reset(void) > > > > +{ > > > > + if (tpm_devfd != -1) { > > > > + close(tpm_devfd); > > > > + tpm_devfd = -1; > > > > + } > > > > +} > > > > + > > > > +static void hypercall_register_types(void) > > > > +{ > > > > + spapr_register_hypercall(H_TPM_COMM, h_tpm_comm); > > > > +} > > > > + > > > > +type_init(hypercall_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 60553d32c4..7bd47575d7 100644 > > > > --- a/include/hw/ppc/spapr.h > > > > +++ b/include/hw/ppc/spapr.h > > > > @@ -198,6 +198,7 @@ struct SpaprMachineState { > > > > SpaprXive *xive; > > > > SpaprIrq *irq; > > > > qemu_irq *qirqs; > > > > + char *tpm_device_file; > > > > > > > > bool cmd_line_caps[SPAPR_CAP_NUM]; > > > > SpaprCapabilities def, eff, mig; > > > > @@ -490,8 +491,9 @@ struct SpaprMachineState { > > > > #define H_INT_ESB 0x3C8 > > > > #define H_INT_SYNC 0x3CC > > > > #define H_INT_RESET 0x3D0 > > > > +#define H_TPM_COMM 0xEF10 > > > > > > > > -#define MAX_HCALL_OPCODE H_INT_RESET > > > > +#define MAX_HCALL_OPCODE H_TPM_COMM > > > > > > > > /* The hcalls above are standardized in PAPR and implemented by pHyp > > > > * as well. > > > > @@ -864,6 +866,9 @@ int spapr_caps_post_migration(SpaprMachineState *spapr); > > > > > > > > void spapr_check_pagesize(SpaprMachineState *spapr, hwaddr pagesize, > > > > Error **errp); > > > > + > > > > +void spapr_hcall_tpm_reset(void); > > > > + > > > > /* > > > > * XIVE definitions > > > > */ > > > > > > > -- > 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
On Tue, Jul 16, 2019 at 11:30:01AM -0500, Michael Roth wrote: > Quoting David Gibson (2019-07-14 21:25:24) > > On Fri, Jul 12, 2019 at 09:34:46AM -0500, Michael Roth wrote: > > > Quoting David Gibson (2019-07-12 01:46:19) > > > > On Thu, Jul 11, 2019 at 08:19:34PM -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 pseries machine option which is used to > > > > > configure what TPM device to pass commands to, for example: > > > > > > > > > > -machine pseries,...,tpm-device-file=/dev/tmprm0 > > > > > > > > Bolting this into yet another machine parameter seems kind of ugly. > > > > Wouldn't it make more sense to treat this as an virtual device (say > > > > "spapr-vtpm"). Adding that device would enable the hcall, and would > > > > have properties for the back end host device. > > > > > > That does sound nicer. > > > > > > Originally I had SpaprMachineClass implement the TYPE_TPM_IF interface so > > > we could define a TPM backend via -tpmdev passthrough,path=..., but after > > > some discussion with the TPM maintainer it didn't quite work for the main > > > use-case of passing through a TPM Resource Manager since it isn't suitable > > > for full vTPM front-ends (since multiple guests can interfere with each > > > other's operations when running the full gamut of TPM functionality). > > > > > > I hadn't consider a stand-alone -device implementation though. It's not > > > a proper VIO or PCI device so there's no proper bus to attach it to. I > > > guess we would just make it a direct child of SpaprMachineState (sort > > > of like SpaprDrcClass), then we could define it via something like > > > -object spapr-tpm-proxy,path=.... > > > > It should be -device not -object, but otherwise that looks ok. > > Ok, for some reason I thought -device needed either a specific bus or > needed to be a SysBusDevice to attach to main-system-bus, but maybe that > was just for qdev-managed reset handling. I've re-worked the series to > allow configuration via: > > -device spapr-tpm-proxy,host_path=/dev/tpmrmX That looks good. > > How does the TPM appear in the device tree? > > Nothing in the guest, on the host it appears as: Hrm. That seems unwise. I mean, I guess its being treated as a hypervisor facility rather than a device per se, but what if we ever need to advertise more metadata about it. > ./xscom@603fc00000000/i2cm@a2000/i2c-bus@0/tpm@57 > ./xscom@603fc00000000/i2cm@a2000/i2c-bus@0/tpm@57/link-id > ./xscom@603fc00000000/i2cm@a2000/i2c-bus@0/tpm@57/linux,sml-size > ./xscom@603fc00000000/i2cm@a2000/i2c-bus@0/tpm@57/label > ./xscom@603fc00000000/i2cm@a2000/i2c-bus@0/tpm@57/compatible > ./xscom@603fc00000000/i2cm@a2000/i2c-bus@0/tpm@57/status > ./xscom@603fc00000000/i2cm@a2000/i2c-bus@0/tpm@57/reg > ./xscom@603fc00000000/i2cm@a2000/i2c-bus@0/tpm@57/phandle > ./xscom@603fc00000000/i2cm@a2000/i2c-bus@0/tpm@57/linux,sml-base > ./xscom@603fc00000000/i2cm@a2000/i2c-bus@0/tpm@57/name > > > > > > I'll go ahead and give that a shot, assuming it seems reasonable to you. > > > > > > > > > > > > By default, no tpm-device-file is defined and hcalls will return > > > > > H_RESOURCE. > > > > > > > > Wouldn't H_FUNCTION make more sense? > > > > > > Yes, for this case it probably would. > > > > > > Thanks for the suggestions! > > > > > > > > > > > > > > > > > The full specification for this hypercall can be found in > > > > > docs/specs/ppc-spapr-uv-hcalls.txt > > > > > > > > > > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com > > > > > --- > > > > > hw/ppc/Makefile.objs | 1 + > > > > > hw/ppc/spapr.c | 27 ++++++++ > > > > > hw/ppc/spapr_hcall_tpm.c | 135 +++++++++++++++++++++++++++++++++++++++ > > > > > hw/ppc/trace-events | 4 ++ > > > > > include/hw/ppc/spapr.h | 7 +- > > > > > 5 files changed, 173 insertions(+), 1 deletion(-) > > > > > create mode 100644 hw/ppc/spapr_hcall_tpm.c > > > > > > > > > > diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs > > > > > index 9da93af905..5aa120cae6 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_hcall_tpm.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 821f0d4a49..eb3421673b 100644 > > > > > --- a/hw/ppc/spapr.c > > > > > +++ b/hw/ppc/spapr.c > > > > > @@ -1776,6 +1776,10 @@ static void spapr_machine_reset(MachineState *machine) > > > > > */ > > > > > object_child_foreach_recursive(object_get_root(), spapr_reset_drcs, NULL); > > > > > > > > > > + if (spapr->tpm_device_file) { > > > > > + spapr_hcall_tpm_reset(); > > > > > + } > > > > > + > > > > > spapr_clear_pending_events(spapr); > > > > > > > > > > /* > > > > > @@ -3340,6 +3344,21 @@ static void spapr_set_host_serial(Object *obj, const char *value, Error **errp) > > > > > spapr->host_serial = g_strdup(value); > > > > > } > > > > > > > > > > +static char *spapr_get_tpm_device_file(Object *obj, Error **errp) > > > > > +{ > > > > > + SpaprMachineState *spapr = SPAPR_MACHINE(obj); > > > > > + > > > > > + return g_strdup(spapr->tpm_device_file); > > > > > +} > > > > > + > > > > > +static void spapr_set_tpm_device_file(Object *obj, const char *value, Error **errp) > > > > > +{ > > > > > + SpaprMachineState *spapr = SPAPR_MACHINE(obj); > > > > > + > > > > > + g_free(spapr->tpm_device_file); > > > > > + spapr->tpm_device_file = g_strdup(value); > > > > > +} > > > > > + > > > > > static void spapr_instance_init(Object *obj) > > > > > { > > > > > SpaprMachineState *spapr = SPAPR_MACHINE(obj); > > > > > @@ -3396,6 +3415,14 @@ static void spapr_instance_init(Object *obj) > > > > > &error_abort); > > > > > object_property_set_description(obj, "host-serial", > > > > > "Host serial number to advertise in guest device tree", &error_abort); > > > > > + object_property_add_str(obj, "tpm-device-file", > > > > > + spapr_get_tpm_device_file, > > > > > + spapr_set_tpm_device_file, &error_abort); > > > > > + object_property_set_description(obj, "tpm-device-file", > > > > > + "Specifies the path to the TPM character device file to use" > > > > > + " for TPM communication via hypercalls (usually a TPM" > > > > > + " resource manager)", > > > > > + &error_abort); > > > > > } > > > > > > > > > > static void spapr_machine_finalizefn(Object *obj) > > > > > diff --git a/hw/ppc/spapr_hcall_tpm.c b/hw/ppc/spapr_hcall_tpm.c > > > > > new file mode 100644 > > > > > index 0000000000..75e2b6d594 > > > > > --- /dev/null > > > > > +++ b/hw/ppc/spapr_hcall_tpm.c > > > > > @@ -0,0 +1,135 @@ > > > > > +/* > > > > > + * SPAPR TPM Hypercall > > > > > + * > > > > > + * Copyright IBM Corp. 2019 > > > > > + * > > > > > + * Authors: > > > > > + * Michael Roth <mdroth@linux.vnet.ibm.com> > > > > > + * > > > > > + * 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 int tpm_devfd = -1; > > > > > > > > A global? Really? You can do better. > > > > > > > > > + > > > > > +static ssize_t tpm_execute(SpaprMachineState *spapr, 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 "\n", > > > > > + data_in_size); > > > > > + return H_P3; > > > > > + } > > > > > + > > > > > + if (data_out_size < TPM_SPAPR_BUFSIZE) { > > > > > + error_report("invalid TPM output buffer size: " TARGET_FMT_lu "\n", > > > > > + data_out_size); > > > > > + return H_P5; > > > > > + } > > > > > + > > > > > + if (tpm_devfd == -1) { > > > > > + tpm_devfd = open(spapr->tpm_device_file, O_RDWR); > > > > > + if (tpm_devfd == -1) { > > > > > + error_report("failed to open TPM device %s: %d", > > > > > + spapr->tpm_device_file, errno); > > > > > + return H_RESOURCE; > > > > > + } > > > > > + } > > > > > + > > > > > + cpu_physical_memory_read(data_in, buf_in, data_in_size); > > > > > + > > > > > + do { > > > > > + ret = write(tpm_devfd, 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", > > > > > + spapr->tpm_device_file, errno); > > > > > + return H_RESOURCE; > > > > > + } > > > > > + > > > > > + do { > > > > > + ret = read(tpm_devfd, 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", > > > > > + spapr->tpm_device_file, 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]; > > > > > + > > > > > + trace_spapr_h_tpm_comm(spapr->tpm_device_file ?: "null", op); > > > > > + > > > > > + if (!spapr->tpm_device_file) { > > > > > + error_report("TPM device not specified"); > > > > > + return H_RESOURCE; > > > > > + } > > > > > + > > > > > + switch (op) { > > > > > + case TPM_COMM_OP_EXECUTE: > > > > > + return tpm_execute(spapr, args); > > > > > + case TPM_COMM_OP_CLOSE_SESSION: > > > > > + if (tpm_devfd != -1) { > > > > > + close(tpm_devfd); > > > > > + tpm_devfd = -1; > > > > > + } > > > > > + return H_SUCCESS; > > > > > + default: > > > > > + return H_PARAMETER; > > > > > + } > > > > > +} > > > > > + > > > > > +void spapr_hcall_tpm_reset(void) > > > > > +{ > > > > > + if (tpm_devfd != -1) { > > > > > + close(tpm_devfd); > > > > > + tpm_devfd = -1; > > > > > + } > > > > > +} > > > > > + > > > > > +static void hypercall_register_types(void) > > > > > +{ > > > > > + spapr_register_hypercall(H_TPM_COMM, h_tpm_comm); > > > > > +} > > > > > + > > > > > +type_init(hypercall_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 60553d32c4..7bd47575d7 100644 > > > > > --- a/include/hw/ppc/spapr.h > > > > > +++ b/include/hw/ppc/spapr.h > > > > > @@ -198,6 +198,7 @@ struct SpaprMachineState { > > > > > SpaprXive *xive; > > > > > SpaprIrq *irq; > > > > > qemu_irq *qirqs; > > > > > + char *tpm_device_file; > > > > > > > > > > bool cmd_line_caps[SPAPR_CAP_NUM]; > > > > > SpaprCapabilities def, eff, mig; > > > > > @@ -490,8 +491,9 @@ struct SpaprMachineState { > > > > > #define H_INT_ESB 0x3C8 > > > > > #define H_INT_SYNC 0x3CC > > > > > #define H_INT_RESET 0x3D0 > > > > > +#define H_TPM_COMM 0xEF10 > > > > > > > > > > -#define MAX_HCALL_OPCODE H_INT_RESET > > > > > +#define MAX_HCALL_OPCODE H_TPM_COMM > > > > > > > > > > /* The hcalls above are standardized in PAPR and implemented by pHyp > > > > > * as well. > > > > > @@ -864,6 +866,9 @@ int spapr_caps_post_migration(SpaprMachineState *spapr); > > > > > > > > > > void spapr_check_pagesize(SpaprMachineState *spapr, hwaddr pagesize, > > > > > Error **errp); > > > > > + > > > > > +void spapr_hcall_tpm_reset(void); > > > > > + > > > > > /* > > > > > * XIVE definitions > > > > > */ > > > > > > > > > >
Quoting David Gibson (2019-07-16 20:29:12) > On Tue, Jul 16, 2019 at 11:30:01AM -0500, Michael Roth wrote: > > Quoting David Gibson (2019-07-14 21:25:24) > > > On Fri, Jul 12, 2019 at 09:34:46AM -0500, Michael Roth wrote: > > > > Quoting David Gibson (2019-07-12 01:46:19) > > > > > On Thu, Jul 11, 2019 at 08:19:34PM -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 pseries machine option which is used to > > > > > > configure what TPM device to pass commands to, for example: > > > > > > > > > > > > -machine pseries,...,tpm-device-file=/dev/tmprm0 > > > > > > > > > > Bolting this into yet another machine parameter seems kind of ugly. > > > > > Wouldn't it make more sense to treat this as an virtual device (say > > > > > "spapr-vtpm"). Adding that device would enable the hcall, and would > > > > > have properties for the back end host device. > > > > > > > > That does sound nicer. > > > > > > > > Originally I had SpaprMachineClass implement the TYPE_TPM_IF interface so > > > > we could define a TPM backend via -tpmdev passthrough,path=..., but after > > > > some discussion with the TPM maintainer it didn't quite work for the main > > > > use-case of passing through a TPM Resource Manager since it isn't suitable > > > > for full vTPM front-ends (since multiple guests can interfere with each > > > > other's operations when running the full gamut of TPM functionality). > > > > > > > > I hadn't consider a stand-alone -device implementation though. It's not > > > > a proper VIO or PCI device so there's no proper bus to attach it to. I > > > > guess we would just make it a direct child of SpaprMachineState (sort > > > > of like SpaprDrcClass), then we could define it via something like > > > > -object spapr-tpm-proxy,path=.... > > > > > > It should be -device not -object, but otherwise that looks ok. > > > > Ok, for some reason I thought -device needed either a specific bus or > > needed to be a SysBusDevice to attach to main-system-bus, but maybe that > > was just for qdev-managed reset handling. I've re-worked the series to > > allow configuration via: > > > > -device spapr-tpm-proxy,host_path=/dev/tpmrmX > > That looks good. > > > > How does the TPM appear in the device tree? > > > > Nothing in the guest, on the host it appears as: > > Hrm. That seems unwise. I mean, I guess its being treated as a > hypervisor facility rather than a device per se, but what if we ever > need to advertise more metadata about it. It's a little bit awkward using a device tree in this case since it's generally the ultravisor that will be making this hcall on behalf of a guest requesting switch-over to SVM mode. The TPM device itself has a GetCapabilities command that seems like it would cover most of the metadata we might need though: https://trustedcomputinggroup.org/wp-content/uploads/TPM-Rev-2.0-Part-3-Commands-01.38.pdf (page 340) and if we need to add a layer of metadata on top of that there's also the option of introducing support for an additional operation in H_TPM_COMM itself, e.g. TPM_COMM_OP_GET_CAPABILITIES. Unsupported or invalid operations have a unique H_PARAMETER return code so callers should be able to reliably probe for it in the future if they need more information. > > > ./xscom@603fc00000000/i2cm@a2000/i2c-bus@0/tpm@57 > > ./xscom@603fc00000000/i2cm@a2000/i2c-bus@0/tpm@57/link-id > > ./xscom@603fc00000000/i2cm@a2000/i2c-bus@0/tpm@57/linux,sml-size > > ./xscom@603fc00000000/i2cm@a2000/i2c-bus@0/tpm@57/label > > ./xscom@603fc00000000/i2cm@a2000/i2c-bus@0/tpm@57/compatible > > ./xscom@603fc00000000/i2cm@a2000/i2c-bus@0/tpm@57/status > > ./xscom@603fc00000000/i2cm@a2000/i2c-bus@0/tpm@57/reg > > ./xscom@603fc00000000/i2cm@a2000/i2c-bus@0/tpm@57/phandle > > ./xscom@603fc00000000/i2cm@a2000/i2c-bus@0/tpm@57/linux,sml-base > > ./xscom@603fc00000000/i2cm@a2000/i2c-bus@0/tpm@57/name > > > > > > > > > I'll go ahead and give that a shot, assuming it seems reasonable to you. > > > > > > > > > > > > > > > By default, no tpm-device-file is defined and hcalls will return > > > > > > H_RESOURCE. > > > > > > > > > > Wouldn't H_FUNCTION make more sense? > > > > > > > > Yes, for this case it probably would. > > > > > > > > Thanks for the suggestions! > > > > > > > > > > > > > > > > > > > > > The full specification for this hypercall can be found in > > > > > > docs/specs/ppc-spapr-uv-hcalls.txt > > > > > > > > > > > > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com > > > > > > --- > > > > > > hw/ppc/Makefile.objs | 1 + > > > > > > hw/ppc/spapr.c | 27 ++++++++ > > > > > > hw/ppc/spapr_hcall_tpm.c | 135 +++++++++++++++++++++++++++++++++++++++ > > > > > > hw/ppc/trace-events | 4 ++ > > > > > > include/hw/ppc/spapr.h | 7 +- > > > > > > 5 files changed, 173 insertions(+), 1 deletion(-) > > > > > > create mode 100644 hw/ppc/spapr_hcall_tpm.c > > > > > > > > > > > > diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs > > > > > > index 9da93af905..5aa120cae6 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_hcall_tpm.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 821f0d4a49..eb3421673b 100644 > > > > > > --- a/hw/ppc/spapr.c > > > > > > +++ b/hw/ppc/spapr.c > > > > > > @@ -1776,6 +1776,10 @@ static void spapr_machine_reset(MachineState *machine) > > > > > > */ > > > > > > object_child_foreach_recursive(object_get_root(), spapr_reset_drcs, NULL); > > > > > > > > > > > > + if (spapr->tpm_device_file) { > > > > > > + spapr_hcall_tpm_reset(); > > > > > > + } > > > > > > + > > > > > > spapr_clear_pending_events(spapr); > > > > > > > > > > > > /* > > > > > > @@ -3340,6 +3344,21 @@ static void spapr_set_host_serial(Object *obj, const char *value, Error **errp) > > > > > > spapr->host_serial = g_strdup(value); > > > > > > } > > > > > > > > > > > > +static char *spapr_get_tpm_device_file(Object *obj, Error **errp) > > > > > > +{ > > > > > > + SpaprMachineState *spapr = SPAPR_MACHINE(obj); > > > > > > + > > > > > > + return g_strdup(spapr->tpm_device_file); > > > > > > +} > > > > > > + > > > > > > +static void spapr_set_tpm_device_file(Object *obj, const char *value, Error **errp) > > > > > > +{ > > > > > > + SpaprMachineState *spapr = SPAPR_MACHINE(obj); > > > > > > + > > > > > > + g_free(spapr->tpm_device_file); > > > > > > + spapr->tpm_device_file = g_strdup(value); > > > > > > +} > > > > > > + > > > > > > static void spapr_instance_init(Object *obj) > > > > > > { > > > > > > SpaprMachineState *spapr = SPAPR_MACHINE(obj); > > > > > > @@ -3396,6 +3415,14 @@ static void spapr_instance_init(Object *obj) > > > > > > &error_abort); > > > > > > object_property_set_description(obj, "host-serial", > > > > > > "Host serial number to advertise in guest device tree", &error_abort); > > > > > > + object_property_add_str(obj, "tpm-device-file", > > > > > > + spapr_get_tpm_device_file, > > > > > > + spapr_set_tpm_device_file, &error_abort); > > > > > > + object_property_set_description(obj, "tpm-device-file", > > > > > > + "Specifies the path to the TPM character device file to use" > > > > > > + " for TPM communication via hypercalls (usually a TPM" > > > > > > + " resource manager)", > > > > > > + &error_abort); > > > > > > } > > > > > > > > > > > > static void spapr_machine_finalizefn(Object *obj) > > > > > > diff --git a/hw/ppc/spapr_hcall_tpm.c b/hw/ppc/spapr_hcall_tpm.c > > > > > > new file mode 100644 > > > > > > index 0000000000..75e2b6d594 > > > > > > --- /dev/null > > > > > > +++ b/hw/ppc/spapr_hcall_tpm.c > > > > > > @@ -0,0 +1,135 @@ > > > > > > +/* > > > > > > + * SPAPR TPM Hypercall > > > > > > + * > > > > > > + * Copyright IBM Corp. 2019 > > > > > > + * > > > > > > + * Authors: > > > > > > + * Michael Roth <mdroth@linux.vnet.ibm.com> > > > > > > + * > > > > > > + * 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 int tpm_devfd = -1; > > > > > > > > > > A global? Really? You can do better. > > > > > > > > > > > + > > > > > > +static ssize_t tpm_execute(SpaprMachineState *spapr, 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 "\n", > > > > > > + data_in_size); > > > > > > + return H_P3; > > > > > > + } > > > > > > + > > > > > > + if (data_out_size < TPM_SPAPR_BUFSIZE) { > > > > > > + error_report("invalid TPM output buffer size: " TARGET_FMT_lu "\n", > > > > > > + data_out_size); > > > > > > + return H_P5; > > > > > > + } > > > > > > + > > > > > > + if (tpm_devfd == -1) { > > > > > > + tpm_devfd = open(spapr->tpm_device_file, O_RDWR); > > > > > > + if (tpm_devfd == -1) { > > > > > > + error_report("failed to open TPM device %s: %d", > > > > > > + spapr->tpm_device_file, errno); > > > > > > + return H_RESOURCE; > > > > > > + } > > > > > > + } > > > > > > + > > > > > > + cpu_physical_memory_read(data_in, buf_in, data_in_size); > > > > > > + > > > > > > + do { > > > > > > + ret = write(tpm_devfd, 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", > > > > > > + spapr->tpm_device_file, errno); > > > > > > + return H_RESOURCE; > > > > > > + } > > > > > > + > > > > > > + do { > > > > > > + ret = read(tpm_devfd, 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", > > > > > > + spapr->tpm_device_file, 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]; > > > > > > + > > > > > > + trace_spapr_h_tpm_comm(spapr->tpm_device_file ?: "null", op); > > > > > > + > > > > > > + if (!spapr->tpm_device_file) { > > > > > > + error_report("TPM device not specified"); > > > > > > + return H_RESOURCE; > > > > > > + } > > > > > > + > > > > > > + switch (op) { > > > > > > + case TPM_COMM_OP_EXECUTE: > > > > > > + return tpm_execute(spapr, args); > > > > > > + case TPM_COMM_OP_CLOSE_SESSION: > > > > > > + if (tpm_devfd != -1) { > > > > > > + close(tpm_devfd); > > > > > > + tpm_devfd = -1; > > > > > > + } > > > > > > + return H_SUCCESS; > > > > > > + default: > > > > > > + return H_PARAMETER; > > > > > > + } > > > > > > +} > > > > > > + > > > > > > +void spapr_hcall_tpm_reset(void) > > > > > > +{ > > > > > > + if (tpm_devfd != -1) { > > > > > > + close(tpm_devfd); > > > > > > + tpm_devfd = -1; > > > > > > + } > > > > > > +} > > > > > > + > > > > > > +static void hypercall_register_types(void) > > > > > > +{ > > > > > > + spapr_register_hypercall(H_TPM_COMM, h_tpm_comm); > > > > > > +} > > > > > > + > > > > > > +type_init(hypercall_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 60553d32c4..7bd47575d7 100644 > > > > > > --- a/include/hw/ppc/spapr.h > > > > > > +++ b/include/hw/ppc/spapr.h > > > > > > @@ -198,6 +198,7 @@ struct SpaprMachineState { > > > > > > SpaprXive *xive; > > > > > > SpaprIrq *irq; > > > > > > qemu_irq *qirqs; > > > > > > + char *tpm_device_file; > > > > > > > > > > > > bool cmd_line_caps[SPAPR_CAP_NUM]; > > > > > > SpaprCapabilities def, eff, mig; > > > > > > @@ -490,8 +491,9 @@ struct SpaprMachineState { > > > > > > #define H_INT_ESB 0x3C8 > > > > > > #define H_INT_SYNC 0x3CC > > > > > > #define H_INT_RESET 0x3D0 > > > > > > +#define H_TPM_COMM 0xEF10 > > > > > > > > > > > > -#define MAX_HCALL_OPCODE H_INT_RESET > > > > > > +#define MAX_HCALL_OPCODE H_TPM_COMM > > > > > > > > > > > > /* The hcalls above are standardized in PAPR and implemented by pHyp > > > > > > * as well. > > > > > > @@ -864,6 +866,9 @@ int spapr_caps_post_migration(SpaprMachineState *spapr); > > > > > > > > > > > > void spapr_check_pagesize(SpaprMachineState *spapr, hwaddr pagesize, > > > > > > Error **errp); > > > > > > + > > > > > > +void spapr_hcall_tpm_reset(void); > > > > > > + > > > > > > /* > > > > > > * XIVE definitions > > > > > > */ > > > > > > > > > > > > > > > > -- > 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
On Wed, Jul 17, 2019 at 03:53:47PM -0500, Michael Roth wrote: > Quoting David Gibson (2019-07-16 20:29:12) > > On Tue, Jul 16, 2019 at 11:30:01AM -0500, Michael Roth wrote: > > > Quoting David Gibson (2019-07-14 21:25:24) > > > > On Fri, Jul 12, 2019 at 09:34:46AM -0500, Michael Roth wrote: > > > > > Quoting David Gibson (2019-07-12 01:46:19) > > > > > > On Thu, Jul 11, 2019 at 08:19:34PM -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 pseries machine option which is used to > > > > > > > configure what TPM device to pass commands to, for example: > > > > > > > > > > > > > > -machine pseries,...,tpm-device-file=/dev/tmprm0 > > > > > > > > > > > > Bolting this into yet another machine parameter seems kind of ugly. > > > > > > Wouldn't it make more sense to treat this as an virtual device (say > > > > > > "spapr-vtpm"). Adding that device would enable the hcall, and would > > > > > > have properties for the back end host device. > > > > > > > > > > That does sound nicer. > > > > > > > > > > Originally I had SpaprMachineClass implement the TYPE_TPM_IF interface so > > > > > we could define a TPM backend via -tpmdev passthrough,path=..., but after > > > > > some discussion with the TPM maintainer it didn't quite work for the main > > > > > use-case of passing through a TPM Resource Manager since it isn't suitable > > > > > for full vTPM front-ends (since multiple guests can interfere with each > > > > > other's operations when running the full gamut of TPM functionality). > > > > > > > > > > I hadn't consider a stand-alone -device implementation though. It's not > > > > > a proper VIO or PCI device so there's no proper bus to attach it to. I > > > > > guess we would just make it a direct child of SpaprMachineState (sort > > > > > of like SpaprDrcClass), then we could define it via something like > > > > > -object spapr-tpm-proxy,path=.... > > > > > > > > It should be -device not -object, but otherwise that looks ok. > > > > > > Ok, for some reason I thought -device needed either a specific bus or > > > needed to be a SysBusDevice to attach to main-system-bus, but maybe that > > > was just for qdev-managed reset handling. I've re-worked the series to > > > allow configuration via: > > > > > > -device spapr-tpm-proxy,host_path=/dev/tpmrmX > > > > That looks good. > > > > > > How does the TPM appear in the device tree? > > > > > > Nothing in the guest, on the host it appears as: > > > > Hrm. That seems unwise. I mean, I guess its being treated as a > > hypervisor facility rather than a device per se, but what if we ever > > need to advertise more metadata about it. > > It's a little bit awkward using a device tree in this case since it's > generally the ultravisor that will be making this hcall on behalf of > a guest requesting switch-over to SVM mode. The TPM device itself has > a GetCapabilities command that seems like it would cover most of the > metadata we might need though: > > https://trustedcomputinggroup.org/wp-content/uploads/TPM-Rev-2.0-Part-3-Commands-01.38.pdf > (page 340) > > and if we need to add a layer of metadata on top of that there's also > the option of introducing support for an additional operation in > H_TPM_COMM itself, e.g. TPM_COMM_OP_GET_CAPABILITIES. Unsupported or > invalid operations have a unique H_PARAMETER return code so callers > should be able to reliably probe for it in the future if they need > more information. Yeah, ok.
diff --git a/hw/ppc/Makefile.objs b/hw/ppc/Makefile.objs index 9da93af905..5aa120cae6 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_hcall_tpm.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 821f0d4a49..eb3421673b 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -1776,6 +1776,10 @@ static void spapr_machine_reset(MachineState *machine) */ object_child_foreach_recursive(object_get_root(), spapr_reset_drcs, NULL); + if (spapr->tpm_device_file) { + spapr_hcall_tpm_reset(); + } + spapr_clear_pending_events(spapr); /* @@ -3340,6 +3344,21 @@ static void spapr_set_host_serial(Object *obj, const char *value, Error **errp) spapr->host_serial = g_strdup(value); } +static char *spapr_get_tpm_device_file(Object *obj, Error **errp) +{ + SpaprMachineState *spapr = SPAPR_MACHINE(obj); + + return g_strdup(spapr->tpm_device_file); +} + +static void spapr_set_tpm_device_file(Object *obj, const char *value, Error **errp) +{ + SpaprMachineState *spapr = SPAPR_MACHINE(obj); + + g_free(spapr->tpm_device_file); + spapr->tpm_device_file = g_strdup(value); +} + static void spapr_instance_init(Object *obj) { SpaprMachineState *spapr = SPAPR_MACHINE(obj); @@ -3396,6 +3415,14 @@ static void spapr_instance_init(Object *obj) &error_abort); object_property_set_description(obj, "host-serial", "Host serial number to advertise in guest device tree", &error_abort); + object_property_add_str(obj, "tpm-device-file", + spapr_get_tpm_device_file, + spapr_set_tpm_device_file, &error_abort); + object_property_set_description(obj, "tpm-device-file", + "Specifies the path to the TPM character device file to use" + " for TPM communication via hypercalls (usually a TPM" + " resource manager)", + &error_abort); } static void spapr_machine_finalizefn(Object *obj) diff --git a/hw/ppc/spapr_hcall_tpm.c b/hw/ppc/spapr_hcall_tpm.c new file mode 100644 index 0000000000..75e2b6d594 --- /dev/null +++ b/hw/ppc/spapr_hcall_tpm.c @@ -0,0 +1,135 @@ +/* + * SPAPR TPM Hypercall + * + * Copyright IBM Corp. 2019 + * + * Authors: + * Michael Roth <mdroth@linux.vnet.ibm.com> + * + * 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 int tpm_devfd = -1; + +static ssize_t tpm_execute(SpaprMachineState *spapr, 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 "\n", + data_in_size); + return H_P3; + } + + if (data_out_size < TPM_SPAPR_BUFSIZE) { + error_report("invalid TPM output buffer size: " TARGET_FMT_lu "\n", + data_out_size); + return H_P5; + } + + if (tpm_devfd == -1) { + tpm_devfd = open(spapr->tpm_device_file, O_RDWR); + if (tpm_devfd == -1) { + error_report("failed to open TPM device %s: %d", + spapr->tpm_device_file, errno); + return H_RESOURCE; + } + } + + cpu_physical_memory_read(data_in, buf_in, data_in_size); + + do { + ret = write(tpm_devfd, 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", + spapr->tpm_device_file, errno); + return H_RESOURCE; + } + + do { + ret = read(tpm_devfd, 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", + spapr->tpm_device_file, 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]; + + trace_spapr_h_tpm_comm(spapr->tpm_device_file ?: "null", op); + + if (!spapr->tpm_device_file) { + error_report("TPM device not specified"); + return H_RESOURCE; + } + + switch (op) { + case TPM_COMM_OP_EXECUTE: + return tpm_execute(spapr, args); + case TPM_COMM_OP_CLOSE_SESSION: + if (tpm_devfd != -1) { + close(tpm_devfd); + tpm_devfd = -1; + } + return H_SUCCESS; + default: + return H_PARAMETER; + } +} + +void spapr_hcall_tpm_reset(void) +{ + if (tpm_devfd != -1) { + close(tpm_devfd); + tpm_devfd = -1; + } +} + +static void hypercall_register_types(void) +{ + spapr_register_hypercall(H_TPM_COMM, h_tpm_comm); +} + +type_init(hypercall_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 60553d32c4..7bd47575d7 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -198,6 +198,7 @@ struct SpaprMachineState { SpaprXive *xive; SpaprIrq *irq; qemu_irq *qirqs; + char *tpm_device_file; bool cmd_line_caps[SPAPR_CAP_NUM]; SpaprCapabilities def, eff, mig; @@ -490,8 +491,9 @@ struct SpaprMachineState { #define H_INT_ESB 0x3C8 #define H_INT_SYNC 0x3CC #define H_INT_RESET 0x3D0 +#define H_TPM_COMM 0xEF10 -#define MAX_HCALL_OPCODE H_INT_RESET +#define MAX_HCALL_OPCODE H_TPM_COMM /* The hcalls above are standardized in PAPR and implemented by pHyp * as well. @@ -864,6 +866,9 @@ int spapr_caps_post_migration(SpaprMachineState *spapr); void spapr_check_pagesize(SpaprMachineState *spapr, hwaddr pagesize, Error **errp); + +void spapr_hcall_tpm_reset(void); + /* * XIVE definitions */
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 pseries machine option which is used to configure what TPM device to pass commands to, for example: -machine pseries,...,tpm-device-file=/dev/tmprm0 By default, no tpm-device-file is defined and hcalls will return H_RESOURCE. The full specification for this hypercall can be found in docs/specs/ppc-spapr-uv-hcalls.txt Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com --- hw/ppc/Makefile.objs | 1 + hw/ppc/spapr.c | 27 ++++++++ hw/ppc/spapr_hcall_tpm.c | 135 +++++++++++++++++++++++++++++++++++++++ hw/ppc/trace-events | 4 ++ include/hw/ppc/spapr.h | 7 +- 5 files changed, 173 insertions(+), 1 deletion(-) create mode 100644 hw/ppc/spapr_hcall_tpm.c