[PATCH] hw/rdma: Deprecate the pvrdma device and the rdma subsystem
This subsystem is said to be in a bad shape (see e.g. [1], [2] and [3]), and nobody seems to feel responsible to pick up patches for this and send them via a pull request. For example there is a patch for a CVE-worthy bug posted more than half a year ago [4] which has never been merged. Quoting Markus: "Given the shape it is in, I wouldn't let friends use it in production" - we shouldn't expose this to our users in the current state. Thus let's mark it as deprecated and finally remove it unless somebody steps up and improves the code quality and adds proper regression tests. [1] https://lore.kernel.org/qemu-devel/20230918144206.560120-1-armbru@redhat.com... [2] https://lore.kernel.org/qemu-devel/ZQnojJOqoFu73995@redhat.com/ [3] https://lore.kernel.org/qemu-devel/1054981c-e8ae-c676-3b04-eeb030e11f65@tls.... [4] https://lore.kernel.org/qemu-devel/20230301142926.18686-1-yuval.shaia.ml@gma... [5] https://lore.kernel.org/qemu-devel/8734z9f086.fsf@pond.sub.org/ Signed-off-by: Thomas Huth <thuth@redhat.com> --- MAINTAINERS | 2 +- docs/about/deprecated.rst | 8 ++++++++ hw/rdma/vmw/pvrdma_main.c | 2 ++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index 355b1960ce..ca42b89ef8 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3815,7 +3815,7 @@ F: docs/block-replication.txt PVRDMA M: Yuval Shaia <yuval.shaia.ml@gmail.com> M: Marcel Apfelbaum <marcel.apfelbaum@gmail.com> -S: Maintained +S: Odd Fixes F: hw/rdma/* F: hw/rdma/vmw/* F: docs/pvrdma.txt diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index dc4da95329..f0c7addb1f 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -365,6 +365,14 @@ QEMU's ``vhost`` feature, which would eliminate the high latency costs under which the 9p ``proxy`` backend currently suffers. However as of to date nobody has indicated plans for such kind of reimplementation unfortunately. +``-device pvrdma`` and the rdma subsystem (since 8.2) +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +The pvrdma device and the whole rdma subsystem are in a bad shape and +without active maintenance. The QEMU project intends to remove this +device and subsystem from the code base in a future release without +replacement unless somebody steps up and improves the situation. + Block device options '''''''''''''''''''' diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c index 4fc6712025..ed49ce1e72 100644 --- a/hw/rdma/vmw/pvrdma_main.c +++ b/hw/rdma/vmw/pvrdma_main.c @@ -601,6 +601,8 @@ static void pvrdma_realize(PCIDevice *pdev, Error **errp) bool ram_shared = false; PCIDevice *func0; + warn_report_once("pvrdma is deprecated and will be removed in a future release"); + rdma_info_report("Initializing device %s %x.%x", pdev->name, PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn)); -- 2.41.0
Daniel P. Berrangé <berrange@redhat.com> writes:
On Wed, Sep 27, 2023 at 12:49:08PM -0400, James Bottomley wrote:
From: James Bottomley <James.Bottomley@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@linux.ibm.com> Acked-by: Markus Armbruster <armbru@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@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. [...]
Thomas Huth <thuth@redhat.com> wrote:
This subsystem is said to be in a bad shape (see e.g. [1], [2] and [3]), and nobody seems to feel responsible to pick up patches for this and send them via a pull request. For example there is a patch for a CVE-worthy bug posted more than half a year ago [4] which has never been merged.
Quoting Markus: "Given the shape it is in, I wouldn't let friends use it in production" - we shouldn't expose this to our users in the current state. Thus let's mark it as deprecated and finally remove it unless somebody steps up and improves the code quality and adds proper regression tests.
[1] https://lore.kernel.org/qemu-devel/20230918144206.560120-1-armbru@redhat.com... [2] https://lore.kernel.org/qemu-devel/ZQnojJOqoFu73995@redhat.com/ [3] https://lore.kernel.org/qemu-devel/1054981c-e8ae-c676-3b04-eeb030e11f65@tls.... [4] https://lore.kernel.org/qemu-devel/20230301142926.18686-1-yuval.shaia.ml@gma... [5] https://lore.kernel.org/qemu-devel/8734z9f086.fsf@pond.sub.org/
Signed-off-by: Thomas Huth <thuth@redhat.com>
Acked-by: Juan Quintela <quintela@redhat.com>
On 27/9/23 15:30, Thomas Huth wrote:
This subsystem is said to be in a bad shape (see e.g. [1], [2] and [3]), and nobody seems to feel responsible to pick up patches for this and send them via a pull request. For example there is a patch for a CVE-worthy bug posted more than half a year ago [4] which has never been merged.
Quoting Markus: "Given the shape it is in, I wouldn't let friends use it in production" - we shouldn't expose this to our users in the current state. Thus let's mark it as deprecated and finally remove it unless somebody steps up and improves the code quality and adds proper regression tests.
[1] https://lore.kernel.org/qemu-devel/20230918144206.560120-1-armbru@redhat.com... [2] https://lore.kernel.org/qemu-devel/ZQnojJOqoFu73995@redhat.com/ [3] https://lore.kernel.org/qemu-devel/1054981c-e8ae-c676-3b04-eeb030e11f65@tls.... [4] https://lore.kernel.org/qemu-devel/20230301142926.18686-1-yuval.shaia.ml@gma... [5] https://lore.kernel.org/qemu-devel/8734z9f086.fsf@pond.sub.org/
Signed-off-by: Thomas Huth <thuth@redhat.com> --- MAINTAINERS | 2 +- docs/about/deprecated.rst | 8 ++++++++ hw/rdma/vmw/pvrdma_main.c | 2 ++ 3 files changed, 11 insertions(+), 1 deletion(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
participants (4)
-
Juan Quintela -
Markus Armbruster -
Philippe Mathieu-Daudé -
Thomas Huth