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

[Duan, Zhenzhong]

>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Sent: Wednesday, September 20, 2023 9:02 PM
>Subject: Re: [PATCH v1 15/22] Add iommufd configure option
>
>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.

There are two slight differences:

1. Different group:
$ ll /dev/kvm
crw-rw----+ 1 root kvm 10, 232  9月 18 14:23 /dev/kvm
$ ll /dev/iommu
crw-rw---- 1 root root 10, 124  9月 12 14:14 /dev/iommu

2. Default cgroup device allowed list:
#cgroup_device_acl = [
#    "/dev/null", "/dev/full", "/dev/zero",
#    "/dev/random", "/dev/urandom",
#    "/dev/ptmx", "/dev/kvm"
#]

By default, libvirt creates qemu instance with usr/group libvirt-qemu/kvm
So qemu has permission to open /dev/kvm, but not for /dev/iommu.

When a general user wants to open /dev/kvm, it's not permitted:

duan@duan-server-S2600BT:~$ qemu-system-x86_64 -accel kvm
Could not access KVM kernel module: Permission denied
qemu-system-x86_64: -accel kvm: failed to initialize kvm: Permission denied

Thanks
Zhenzhong