On 11/8/23 11:30, Markus Armbruster wrote:
Cédric Le Goater <c...@redhat.com> writes:
Hello Markus,
On 11/8/23 06:50, Markus Armbruster wrote:
Cédric Le Goater <c...@redhat.com> writes:
On 11/2/23 08:12, Zhenzhong Duan wrote:
From: Eric Auger <eric.au...@redhat.com>
Introduce an iommufd object which allows the interaction
with the host /dev/iommu device.
The /dev/iommu can have been already pre-opened outside of qemu,
in which case the fd can be passed directly along with the
iommufd object:
This allows the iommufd object to be shared accross several
subsystems (VFIO, VDPA, ...). For example, libvirt would open
the /dev/iommu once.
If no fd is passed along with the iommufd object, the /dev/iommu
is opened by the qemu code.
The CONFIG_IOMMUFD option must be set to compile this new object.
Suggested-by: Alex Williamson <alex.william...@redhat.com>
Signed-off-by: Eric Auger <eric.au...@redhat.com>
Signed-off-by: Yi Liu <yi.l....@intel.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
---
v4: add CONFIG_IOMMUFD check, document default case
MAINTAINERS | 7 ++
qapi/qom.json | 22 ++++
include/sysemu/iommufd.h | 46 +++++++
backends/iommufd-stub.c | 59 +++++++++
backends/iommufd.c | 257 +++++++++++++++++++++++++++++++++++++++
backends/Kconfig | 4 +
backends/meson.build | 5 +
backends/trace-events | 12 ++
qemu-options.hx | 13 ++
9 files changed, 425 insertions(+)
create mode 100644 include/sysemu/iommufd.h
create mode 100644 backends/iommufd-stub.c
create mode 100644 backends/iommufd.c
diff --git a/MAINTAINERS b/MAINTAINERS
index cd8d6b140f..6f35159255 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2135,6 +2135,13 @@ F: hw/vfio/ap.c
F: docs/system/s390x/vfio-ap.rst
L: qemu-s3...@nongnu.org
+iommufd
+M: Yi Liu <yi.l....@intel.com>
+M: Eric Auger <eric.au...@redhat.com>
+S: Supported
+F: backends/iommufd.c
+F: include/sysemu/iommufd.h
+
vhost
M: Michael S. Tsirkin <m...@redhat.com>
S: Supported
diff --git a/qapi/qom.json b/qapi/qom.json
index c53ef978ff..27300add48 100644
--- a/qapi/qom.json
+++ b/qapi/qom.json
@@ -794,6 +794,24 @@
{ 'struct': 'VfioUserServerProperties',
'data': { 'socket': 'SocketAddress', 'device': 'str' } }
+##
+# @IOMMUFDProperties:
+#
+# Properties for iommufd objects.
+#
+# @fd: file descriptor name previously passed via 'getfd' command,
+# which represents a pre-opened /dev/iommu. This allows the
+# iommufd object to be shared accross several subsystems
+# (VFIO, VDPA, ...), and the file descriptor to be shared
+# with other process, e.g. DPDK. (default: QEMU opens
+# /dev/iommu by itself)
+#
+# Since: 8.2
+##
+{ 'struct': 'IOMMUFDProperties',
+ 'data': { '*fd': 'str' },
+ 'if': 'CONFIG_IOMMUFD' }
Activating or not IOMMUFD on a platform is a configuration choice
and it is not a dependency on an external resource. I would make
things simpler and drop all the #ifdef in the documentation files.
What exactly are you proposing?
I would like to simplify the configuration part of this new IOMMUFD
feature and avoid a ./configure option to enable/disable the feature
since it has no external dependencies and can be compiled on all
platforms.
However, we know that it only makes sense to have the IOMMUFD backend
on platforms s390x, aarch64, x86_64. So I am proposing as an improvement
to enable IOMMUFD only on these platforms with this addition :
imply IOMMUFD
to hw/{i386,s390x,arm}/Kconfig files.
This gives us the possibility to compile out the feature downstream
if something goes wrong, using the files under : configs/devices/.
Shouldn't we then compile out the relevant parts of the QAPI schema,
too?
Is it possible with Kconfig options ?
Given that the IOMMUFD feature doesn't have any external dependencies
and that the IOMMUFD backend object is common to all platforms, I am
also proposing to remove all the CONFIG_IOMMUFD define usage in the
documentation file "qemu-options.hx" and the schema file "qapi/qom.json".
Any CONFIG_IOMMUFD left elsewhere?
There are. To expose or not vfio properties. Stuff like :
ifdef CONFIG_IOMMUFD
DEFINE_PROP_LINK("iommufd", VFIOPCIDevice, vbasedev.iommufd,
TYPE_IOMMUFD_BACKEND, IOMMUFDBackend *),
#endif
DEFINE_PROP_END_OF_LIST(),
and
#ifdef CONFIG_IOMMUFD
object_class_property_add_str(klass, "fd", NULL, vfio_pci_set_fd);
#endif
The use of 'if': 'CONFIG_IOMMUFD' in the QAPI schema enables
introspection with query-qmp-schema: when ObjectType @iommufd exists,
QEMU supports creating the object. Or am I confused?
Object iommufd should always exist since it is common to all.
Is that acceptable ?
Perhaps the question to ask is whether a management application needs to
know whether this version of QEMU supports iommufd objects. If yes,
then query-qmp-schema is an obvious way to find out.
Thanks for reminding me of this possibility. In that case, we should
indeed avoid returning iommufd support when !CONFIG_IOMMUFD.
Can it be implemented in qapi/qom.json with Kconfig options ?
What could go
wrong when this returns "supported" when it actually isn't?
The management layer would build an invalid QEMU command line and
QEMU would return "invalid object type: iommufd"
Thanks,
C.
