Messages in this thread | | | From | "Tian, Kevin" <> | Subject | RE: [RFC 08/20] vfio/pci: Add VFIO_DEVICE_BIND_IOMMUFD | Date | Wed, 29 Sep 2021 06:41:00 +0000 |
| |
> From: David Gibson <david@gibson.dropbear.id.au> > Sent: Wednesday, September 29, 2021 2:01 PM > > On Sun, Sep 19, 2021 at 02:38:36PM +0800, Liu Yi L wrote: > > This patch adds VFIO_DEVICE_BIND_IOMMUFD for userspace to bind the > vfio > > device to an iommufd. No VFIO_DEVICE_UNBIND_IOMMUFD interface is > provided > > because it's implicitly done when the device fd is closed. > > > > In concept a vfio device can be bound to multiple iommufds, each hosting > > a subset of I/O address spaces attached by this device. > > I really feel like this many<->many mapping between devices is going > to be super-confusing, and therefore make it really hard to be > confident we have all the rules right for proper isolation.
Based on new discussion on group ownership part (patch06), I feel this many<->many relationship will disappear. The context fd (either container or iommufd) will uniquely mark the ownership on a physical device and its group. With this design it's impractical to have one device bound to multiple iommufds. Actually I don't think this is a compelling usage in reality. The previous rationale was that no need to impose such restriction if no special reason... and now we have a reason. 😊
Jason, are you OK with this simplification?
> > That's why I was suggesting a concept like endpoints, to break this > into two many<->one relationships. I'm ok if that isn't visible in > the user API, but I think this is going to be really hard to keep > track of if it isn't explicit somewhere in the internals. >
I think this endpoint concept is represented by ioas_device_info in patch14:
+/* + * An ioas_device_info object is created per each successful attaching + * request. A list of objects are maintained per ioas when the address + * space is shared by multiple devices. + */ +struct ioas_device_info { + struct iommufd_device *idev; + struct list_head next; };
currently it's 1:1 mapping before this object and iommufd_device, because no pasid support yet.
We can rename it to struct ioas_endpoint if it makes you feel better.
Thanks Kevin
| |