Re: [PATCH v4 26/41] backends/iommufd: Introduce the iommufd object

[Markus Armbruster]

Cédric Le Goater <clg@redhat.com> writes:

> On 11/2/23 08:12, Zhenzhong Duan wrote:
>> 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>
>> ---
>> 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-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..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?

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?

> There might be a way to remove the documentation also. Not a big
> issue for now.
>
>
>> +
>>   ##
>>   # @RngProperties:
>>   #
>> @@ -934,6 +952,8 @@
>>      'input-barrier',
>>      { 'name': 'input-linux',
>>        'if': 'CONFIG_LINUX' },
>> +    { 'name': 'iommufd',
>> +      'if': 'CONFIG_IOMMUFD' },
>>      'iothread',
>>      'main-loop',
>>      { 'name': 'memory-backend-epc',
>> @@ -1003,6 +1023,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',

[...]