>-----Original Message-----
>From: Markus Armbruster <arm...@redhat.com>
>Sent: Friday, October 27, 2023 4:31 PM
>Subject: Re: [PATCH v3 24/37] backends/iommufd: Introduce the iommufd object
>
>"Duan, Zhenzhong" <zhenzhong.d...@intel.com> writes:
>
>>>-----Original Message-----
>>> From: Markus Armbruster <arm...@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.d...@intel.com> writes:
>>>
>>>> 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>
>>>> Signed-off-by: Cédric Le Goater <c...@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-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..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.
>
>I'd like you to format the doc comment like this:
>
>
> # @fd: file descriptor name previously passed via 'getfd' command,
> # which represents a pre-opened /dev/iommu. This allows the
>
>Note the two spaces after the period in "/dev/iommu. This allows".
Understood.
>
>>>
>>>> +# 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.
>
>When something is optional, we pretty much always need to document
>behavior when it's absent.
>
>Ideally, behavior when absent is identical to behavior when present with
>a certain default value. Then documenting the default value suffices.
>We commonly do this like (default: VALUE).
Got it.
Thanks
Zhenzhong
