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?
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
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?
struct iommufd_hwpt_paging *
iommufd_hwpt_paging_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
struct iommufd_device *idev, u32 flags,
bool immediate_attach,
const struct iommu_user_data *user_data)
{
...
if (ops->domain_alloc_user) {
hwpt->domain = ops->domain_alloc_user(idev->dev, flags, NULL,
user_data);
if (IS_ERR(hwpt->domain)) {
rc = PTR_ERR(hwpt->domain);
hwpt->domain = NULL;
goto out_abort;
}
hwpt->domain->owner = ops;
} else {
hwpt->domain = iommu_domain_alloc(idev->dev->bus);
if (!hwpt->domain) {
rc = -ENOMEM;
goto out_abort;
}
}
...
}
--
Regards,
Yi Liu
