2:13 p.m.
Daniel P. Berrangé <berrange(a)redhat.com> writes:
On Wed, Sep 27, 2023 at 12:49:08PM -0400, James Bottomley wrote:
> From: James Bottomley <James.Bottomley(a)HansenPartnership.com>
>
> The Microsoft Simulator (mssim) is the reference emulation platform
> for the TCG TPM 2.0 specification.
>
> https://github.com/Microsoft/ms-tpm-20-ref.git
>
> It exports a fairly simple network socket based protocol on two
> sockets, one for command (default 2321) and one for control (default
> 2322). This patch adds a simple backend that can speak the mssim
> protocol over the network. It also allows the two sockets to be
> specified on the command line. The benefits are twofold: firstly it
> gives us a backend that actually speaks a standard TPM emulation
> protocol instead of the linux specific TPM driver format of the
> current emulated TPM backend and secondly, using the microsoft
> protocol, the end point of the emulator can be anywhere on the
> network, facilitating the cloud use case where a central TPM service
> can be used over a control network.
>
> The implementation does basic control commands like power off/on, but
> doesn't implement cancellation or startup. The former because
> cancellation is pretty much useless on a fast operating TPM emulator
> and the latter because this emulator is designed to be used with OVMF
> which itself does TPM startup and I wanted to validate that.
>
> To run this, simply download an emulator based on the MS specification
> (package ibmswtpm2 on openSUSE) and run it, then add these two lines
> to the qemu command and it will use the emulator.
>
> -tpmdev mssim,id=tpm0 \
> -device tpm-crb,tpmdev=tpm0 \
>
> to use a remote emulator replace the first line with
>
> -tpmdev
"{'type':'mssim','id':'tpm0','command':{'type':inet,'host':'remote','port':'2321'}}"
>
> tpm-tis also works as the backend.
>
> Signed-off-by: James Bottomley <jejb(a)linux.ibm.com>
> Acked-by: Markus Armbruster <armbru(a)redhat.com>
[...]
> diff --git a/backends/tpm/tpm_mssim.c b/backends/tpm/tpm_mssim.c
> new file mode 100644
> index 0000000000..b8a12dce04
> --- /dev/null
> +++ b/backends/tpm/tpm_mssim.c
> @@ -0,0 +1,290 @@
> +/*
> + * Emulator TPM driver which connects over the mssim protocol
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * Copyright (c) 2022
> + * Author: James Bottomley <jejb(a)linux.ibm.com>
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/error-report.h"
> +#include "qemu/sockets.h"
> +
> +#include "qapi/clone-visitor.h"
> +#include "qapi/qapi-visit-tpm.h"
> +
> +#include "io/channel-socket.h"
> +
> +#include "sysemu/runstate.h"
> +#include "sysemu/tpm_backend.h"
> +#include "sysemu/tpm_util.h"
> +
> +#include "qom/object.h"
> +
> +#include "tpm_int.h"
> +#include "tpm_mssim.h"
> +
> +#define ERROR_PREFIX "TPM mssim Emulator: "
> +
> +#define TYPE_TPM_MSSIM "tpm-mssim"
> +OBJECT_DECLARE_SIMPLE_TYPE(TPMMssim, TPM_MSSIM)
> +
> +struct TPMMssim {
> + TPMBackend parent;
> +
> + TPMMssimOptions opts;
> +
> + QIOChannelSocket *cmd_qc, *ctrl_qc;
> +};
> +
> +static int tpm_send_ctrl(TPMMssim *t, uint32_t cmd, Error **errp)
> +{
> + int ret;
> +
> + qio_channel_socket_connect_sync(t->ctrl_qc, t->opts.control, errp);
Need to assign to 'ret' and check for failure here, otherwise the
next call to write_all will overwrite the useful message in 'errp'
with a less helpful one.
No, it'll crash :)
An @errp argument must point to a null pointer. If it doesn't, setting
an error will trip error_setv()'s assertion.
+ cmd = htonl(cmd);
+ ret = qio_channel_write_all(QIO_CHANNEL(t->ctrl_qc),
+ (char *)&cmd, sizeof(cmd), errp);
+ if (ret != 0) {
+ goto out;
+ }
qapi/error.h's big comment advises:
* Receive and accumulate multiple errors (first one wins):
* Error *err = NULL, *local_err = NULL;
* foo(arg, &err);
* bar(arg, &local_err);
* error_propagate(&err, local_err);
* if (err) {
* handle the error...
* }
*
* Do *not* "optimize" this to
* Error *err = NULL;
* foo(arg, &err);
* bar(arg, &err); // WRONG!
* if (err) {
* handle the error...
* }
* because this may pass a non-null err to bar().
*
* Likewise, do *not*
* Error *err = NULL;
* if (cond1) {
* error_setg(&err, ...);
* }
* if (cond2) {
* error_setg(&err, ...); // WRONG!
* }
* because this may pass a non-null err to error_setg().
The quoted code is like the last example, except the error_setg() lurk
within the functions called.
[...]