Re: [PATCH v1 15/22] Add iommufd configure option

[Eric Auger]

On 9/20/23 15:02, Cédric Le Goater wrote:
> On 9/20/23 14:51, Jason Gunthorpe wrote:
>> On Wed, Sep 20, 2023 at 02:19:42PM +0200, Cédric Le Goater wrote:
>>> On 9/20/23 05:42, Duan, Zhenzhong wrote:
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Cédric Le Goater <clg@redhat.com>
>>>>> Sent: Wednesday, September 20, 2023 1:08 AM
>>>>> Subject: Re: [PATCH v1 15/22] Add iommufd configure option
>>>>>
>>>>> On 8/30/23 12:37, Zhenzhong Duan wrote:
>>>>>> This adds "--enable-iommufd/--disable-iommufd" to enable or disable
>>>>>> iommufd support, enabled by default.
>>>>>
>>>>> Why would someone want to disable support at compile time ? It might
>>>>
>>>> For those users who only want to support legacy container feature?
>>>> Let me know if you still prefer to drop this patch, I'm fine with
>>>> that.
>>>
>>> I think it is too early.
>>>
>>>>> have been useful for dev but now QEMU should self-adjust at runtime
>>>>> depending only on the host capabilities AFAIUI. Am I missing
>>>>> something ?
>>>>
>>>> IOMMUFD doesn't support all features of legacy container, so QEMU
>>>> doesn't self-adjust at runtime by checking if host supports IOMMUFD.
>>>> We need to specify it explicitly to use IOMMUFD as below:
>>>>
>>>>       -object iommufd,id=iommufd0
>>>>       -device vfio-pci,host=0000:02:00.0,iommufd=iommufd0
>>>
>>> OK. I am not sure this is the correct interface yet. At first glance,
>>> I wouldn't introduce a new object for a simple backend depending on a
>>> kernel interface. I would tend to prefer a "iommu-something" property
>>> of the vfio-pci device with string values: "legacy", "iommufd",
>>> "default"
>>> and define the various interfaces (the ops you proposed) for each
>>> depending on the user preference and the capabilities of the host and
>>> possibly the device.
>>
>> I think the idea came from Alex? The major point is to be able to have
>> libvirt open /dev/iommufd and FD pass it into qemu 
>
> ok.
>
>> and then share that single FD across all VFIOs. 
>
> I will ask Alex to help me catch up on the topic.
>
>> qemu will typically not be able to
>> self-open /dev/iommufd as it is root-only.
>
> I don't understand, we open multiple fds to KVM devices. This is the
> same.
Actually qemu opens the /dev/iommu in case no fd is passed along with
the iommufd object. This is done in
[PATCH v1 16/22] backends/iommufd: Introduce the iommufd object, in

iommufd_backend_connect(). I don't understand either.

Thanks

Eric

>
>>
>> So the object is not exactly for the backend, the object is for the
>> file descriptor.
> got it.
>
>>
>> Adding a legacy/iommufd option to the vfio-pci device string doesn't
>> address these needs.
>
> I agree.
>
> Thanks,
>
> C.
>
>> Jason
>>
>