Re: [PATCH v3 24/37] backends/iommufd: Introduce the iommufd object

[Markus Armbruster <armbru@redhat.com>]

Zhenzhong Duan <zhenzhong.duan@intel.com> writes:

> From: Eric Auger <eric.auger@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.williamson@redhat.com>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---
>  MAINTAINERS              |   7 +
>  qapi/qom.json            |  20 +++
>  include/sysemu/iommufd.h |  46 +++++++
>  backends/iommufd-stub.c  |  59 +++++++++
>  backends/iommufd.c       | 268 +++++++++++++++++++++++++++++++++++++++
>  backends/Kconfig         |   4 +
>  backends/meson.build     |   5 +
>  backends/trace-events    |  12 ++
>  qemu-options.hx          |  13 ++
>  9 files changed, 434 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 7f9912baa0..7aa57ab16f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2109,6 +2109,13 @@ F: hw/vfio/ap.c
>  F: docs/system/s390x/vfio-ap.rst
>  L: qemu-s390x@nongnu.org
>  
> +iommufd
> +M: Yi Liu <yi.l.liu@intel.com>
> +M: Eric Auger <eric.auger@redhat.com>
> +S: Supported
> +F: backends/iommufd.c
> +F: include/sysemu/iommufd.h
> +
>  vhost
>  M: Michael S. Tsirkin <mst@redhat.com>
>  S: Supported
> diff --git a/qapi/qom.json b/qapi/qom.json
> index c53ef978ff..ef0b50f107 100644
> --- a/qapi/qom.json
> +++ b/qapi/qom.json
> @@ -794,6 +794,22 @@
>  { '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

Two spaces between sentences for consistency, please.

> +#     iommufd object to be shared accross several subsystems
> +#     (VFIO, VDPA, ...) and file descriptor to be shared with

Comma before "and file".

Either "the file descriptor", or "file descriptors".

> +#     other process, e.g: DPDK.

e.g.

Alternatively "such as DPDK."

> +#
> +# Since: 8.2
> +##
> +{ 'struct': 'IOMMUFDProperties',
> +        'data': { '*fd': 'str' } }

@fd is optional.  How does the iommufd object behave when @fd is absent?

> +
>  ##
>  # @RngProperties:
>  #
> @@ -934,6 +950,8 @@
   ##
   # @ObjectType:

None of the enum members are documented.  I'm not asking you to fix that
now.

   #
   # Features:
   #
   # @unstable: Member @x-remote-object is experimental.
   #
   # Since: 6.0
   ##
   { 'enum': 'ObjectType',
     'data': [
       [...]
>      'input-barrier',
>      { 'name': 'input-linux',
>        'if': 'CONFIG_LINUX' },
> +    { 'name': 'iommufd',
> +      'if': 'CONFIG_IOMMUFD' },

Should struct IOMMUFDProperties also have 'if': 'CONFIG_IOMMUFD'?

>      'iothread',
>      'main-loop',
>      { 'name': 'memory-backend-epc',
> @@ -1003,6 +1021,8 @@
>        'input-barrier':              'InputBarrierProperties',
>        'input-linux':                { 'type': 'InputLinuxProperties',
>                                        'if': 'CONFIG_LINUX' },
> +      'iommufd':                    { 'type': 'IOMMUFDProperties',
> +                                      'if': 'CONFIG_IOMMUFD' },
>        'iothread':                   'IothreadProperties',
>        'main-loop':                  'MainLoopProperties',
>        'memory-backend-epc':         { 'type': 'MemoryBackendEpcProperties',

[...]