On Wed, May 08, 2024 at 09:26:47PM +0800, Yi Liu wrote: > On 2024/5/8 20:25, Jason Gunthorpe wrote: > > On Wed, May 08, 2024 at 02:10:05PM +0800, Yi Liu wrote: > > > On 2024/5/7 23:18, Jason Gunthorpe wrote: > > > > On Tue, May 07, 2024 at 10:28:34AM +0800, Yi Liu wrote: > > > > > > > We still need something to do before we can safely remove this check. > > > > > > > All the domain allocation interfaces should eventually have the device > > > > > > > pointer as the input, and all domain attributions could be initialized > > > > > > > during domain allocation. In the attach paths, it should return -EINVAL > > > > > > > directly if the domain is not compatible with the iommu for the device. > > > > > > > > > > > > Yes, and this is already true for PASID. > > > > > > > > > > I'm not quite get why it is already true for PASID. I think Baolu's remark > > > > > is general to domains attached to either RID or PASID. > > > > > > > > > > > I feel we could reasonably insist that domanis used with PASID are > > > > > > allocated with a non-NULL dev. > > > > > > > > > > Any special reason for this disclaim? > > > > > > > > If it makes the driver easier, why not? > > > > > > yep. > > > > > > > PASID is special since PASID is barely used, we could insist that > > > > new PASID users also use the new domian_alloc API. > > > > > > Ok. I have one doubt on how to make it in iommufd core. Shall the iommufd > > > core call ops->domain_alloc_paging() directly or still call > > > ops->domain_alloc_user() while ops->domain_alloc_user() flows into the > > > paging domain allocation with non-null dev? > > > > I mostly figured we'd need a new iommu_domain_alloc_dev() sort of > > thing? VFIO should be changed over too. > > Would this new iommu-domain_alloc_dev() have flags and user_data > input? No, it would be an in-kernel replacement for the existing API. > As below code snippet, the existing iommufd core uses domain_alloc_user > op to allocate the s2 domain (paging domain), and will fall back to > iommu_domain_alloc() only if the domain_alloc_user op does not exist. The Oh, right. Yeah we built it like that so that drivers would have consistency that iommufd always uses the _user version if it exists. > typical reason is to use domain_alloc_user op is to allocate a paging > domain with NESTED_PARENT flag. I suppose the new iommu_domain_alloc_dev() > shall allow allocating s2 domain with NESTED_PARENT as well. right? No, it is just a simple replacement for iommu_domain_alloc() that does exactly the same thing. We don't have any in-kernel use for anything more fancy than a simple domain right now. Jason
