On Thu, Apr 22, 2021 at 04:38:08PM -0600, Alex Williamson wrote: > Because it's fundamental to the isolation of the device? What you're > proposing doesn't get around the group issue, it just makes it implicit > rather than explicit in the uapi. I'm not even sure it makes it explicit or implicit, it just takes away the FD. There are four group IOCTLs, I see them mapping to /dev/ioasid follows: VFIO_GROUP_GET_STATUS - + VFIO_GROUP_FLAGS_CONTAINER_SET is fairly redundant + VFIO_GROUP_FLAGS_VIABLE could be in a new sysfs under kernel/iomm_groups, or could be an IOCTL on /dev/ioasid IOASID_ALL_DEVICES_VIABLE VFIO_GROUP_SET_CONTAINER - + This happens implicitly when the device joins the IOASID so it gets moved to the vfio_device FD: ioctl(vifo_device_fd, JOIN_IOASID_FD, ioasifd) VFIO_GROUP_UNSET_CONTAINER - + Also moved to the vfio_device FD, opposite of JOIN_IOASID_FD VFIO_GROUP_GET_DEVICE_FD - + Replaced by opening /dev/vfio/deviceX Learn the deviceX which will be the cdev sysfs shows as: /sys/devices/pci0000:00/0000:00:01.0/0000:01:00.0/vfio/deviceX/dev Open /dev/vfio/deviceX > > How do we model the VFIO group security concept to something like > > VDPA? > > Is it really a "VFIO group security concept"? We're reflecting the > reality of the hardware, not all devices are fully isolated. Well, exactly. /dev/ioasid should understand the group concept somehow, otherwise it is incomplete and maybe even security broken. So, how do I add groups to, say, VDPA in a way that makes sense? The only answer I come to is broadly what I outlined here - make /dev/ioasid do all the group operations, and do them when we enjoin the VDPA device to the ioasid. Once I have solved all the groups problems with the non-VFIO users, then where does that leave VFIO? Why does VFIO need a group FD if everyone else doesn't? > IOMMU group. This is the reality that any userspace driver needs to > play in, it doesn't magically go away because we drop the group file > descriptor. I'm not saying it does, I'm saying it makes the uAPI more regular and easier to fit into /dev/ioasid without the group FD. > It only makes the uapi more difficult to use correctly because > userspace drivers need to go outside of the uapi to have any idea > that this restriction exists. I don't think it makes any substantive difference one way or the other. With the group FD: the userspace has to read sysfs, find the list of devices in the group, open the group fd, create device FDs for each device using the name from sysfs. Starting from a BDF the general pseudo code is group_path = readlink("/sys/bus/pci/devices/BDF/iommu_group") group_name = basename(group_path) group_fd = open("/dev/vfio/"+group_name) device_fd = ioctl(VFIO_GROUP_GET_DEVICE_FD, BDF); Without the group FD: the userspace has to read sysfs, find the list of devices in the group and then open the device-specific cdev (found via sysfs) and link them to a /dev/ioasid FD. Starting from a BDF the general pseudo code is: device_name = first_directory_of("/sys/bus/pci/devices/BDF/vfio/") device_fd = open("/dev/vfio/"+device_name) ioasidfd = open("/dev/ioasid") ioctl(device_fd, JOIN_IOASID_FD, ioasidfd) These two routes can have identical outcomes and identical security checks. In both cases if userspace wants a list of BDFs in the same group as the BDF it is interested in: readdir("/sys/bus/pci/devices/BDF/iommu_group/devices") It seems like a very small difference to me. I still don't see how the group restriction gets surfaced to the application through the group FD. The applications I looked through just treat the group FD as a step on their way to get the device_fd. Jason