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

[Duan, Zhenzhong]

>-----Original Message-----
>From: Markus Armbruster <armbru@redhat.com>
>Sent: Thursday, October 26, 2023 9:28 PM
>Subject: Re: [PATCH v3 24/37] backends/iommufd: Introduce the iommufd object
>
>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.

Presume you mean "  'data': { '*fd': 'str' } }" line, not above line.

>
>> +#     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."

Will fix.

>
>> +#
>> +# Since: 8.2
>> +##
>> +{ 'struct': 'IOMMUFDProperties',
>> +        'data': { '*fd': 'str' } }
>
>@fd is optional.  How does the iommufd object behave when @fd is absent?

If no fd is passed along with the iommufd object, the /dev/iommu
is opened by the qemu code. Let me know if this also needs to be documented.

>
>> +
>>  ##
>>  # @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'?

Yes, thanks for point out, will fix.

BRs.
Zhenzhong