> From: Alex Williamson [mailto:alex.williamson@xxxxxxxxxx] > Sent: Tuesday, December 3, 2019 8:12 AM > To: Liu, Yi L <yi.l.liu@xxxxxxxxx> > Subject: Re: [RFC v2 3/3] vfio/type1: bind guest pasid (guest page tables) to host > > On Mon, 25 Nov 2019 07:45:18 +0000 > "Liu, Yi L" <yi.l.liu@xxxxxxxxx> wrote: > > > Hi Alex, > > > > Thanks for the review. Here I'd like to conclude the major opens in this > > thread and see if we can get some agreements to prepare a new version. > > > > a) IOCTLs for BIND_GPASID and BIND_PROCESS, share a single IOCTL or two > > separate IOCTLs? > > Yi: It may be helpful to have separate IOCTLs. The bind data conveyed > > for BIND_GPASID and BIND_PROCESS are totally different, and the struct > > iommu_gpasid_bind_data has vendor specific data and may even have more > > versions in future. To better maintain it, I guess separate IOCTLs for > > the two bind types would be better. The structure for BIND_GPASID is > > as below: > > > > struct vfio_iommu_type1_bind { > > __u32 argsz; > > struct iommu_gpasid_bind_data bind_data; > > }; > > > We've been rather successful at extending ioctls in vfio and I'm > generally opposed to rampant ioctl proliferation. If we added @flags > to the above struct (as pretty much the standard for vfio ioctls), then > we could use it to describe the type of binding to perform and > therefore the type of data provided. I think my major complaint here > was that we were defining PROCESS but not implementing it. We can > design the ioctl to enable it, but not define it until it's implemented. sure, so I'll pull back the @flags field. BTW. Regards to the payloads, what would be preferred? @data[] or a wrapper structure like below? union { struct iommu_gpasid_bind_data bind_gpasid; }bind_data; > > b) how kernel-space learns the number of bytes to be copied (a.k.a. the > > usage of @version field and @format field of struct > > iommu_gpasid_bind_data) > > Yi: Jean has an excellent recap in prior reply on the plan of future > > extensions regards to @version field and @format field. Based on the > > plan, kernel space needs to parse the @version field and @format field > > to get the length of the current BIND_GPASID request. Also kernel needs > > to maintain the new and old structure versions. Follow specific > > deprecation policy in future. > > Yes, it seems reasonable, so from the struct above (plus @flags) we > could determine we have struct iommu_gpasid_bind_data as the payload > and read that using @version and @format as outlined. sure, thanks. > > c) how can vIOMMU emulator know that the vfio interface supports to config > > dual stage translation for vIOMMU? > > Yi: may do it via VFIO_IOMMU_GET_INFO. > > Yes please. got it. > > d) how can vIOMMU emulator know what @version and @format should be set > > in struct iommu_gpasid_bind_data? > > Yi: currently, we have two ways. First one, may do it via > > VFIO_IOMMU_GET_INFO. This is a natural idea as here @version and @format > > are used in vfio apis. It makes sense to let vfio to provide related info > > to vIOMMU emulator after checking with vendor specific iommu driver. Also, > > there is idea to do it via sysfs (/sys/class/iommu/dmar#) as we have plan > > to do IOMMU capability sync between vIOMMU and pIOMMU via sysfs. I have > > two concern on this option. Current iommu sysfs only provides vendor > > specific hardware infos. I'm not sure if it is good to expose infos > > defined in IOMMU generic layer via iommu sysfs. If this concern is not > > a big thing, I'm fine with both options. > > This seems like the same issue we had with IOMMU reserved regions, I'd > prefer that a user can figure out how to interact with the vfio > interface through the vfio interface. Forcing the user to poke around > in sysfs requires the user to have read permissions to sysfs in places > they otherwise wouldn't need. Thanks, thanks, let me prepare a new version. Regards, Yi Liu > Alex
