Re: [PATCH v2 01/22] iommu: Add iommu_user_domain_alloc() interface

Yi Liu
2024-06-04T08:00:12Z

On 2024/6/4 09:51, Lu Baolu wrote:
> Commit <909f4abd1097> ("iommu: Add new iommu op to create domains owned
> by userspace") added a dedicated iommu op to allocate a user domain.
> While IOMMUFD has already made use of this callback, other frameworks
> like vfio/type1 and vDPA still use the paging domain allocation interface.
>
> Add a new interface named iommu_user_domain_alloc(), which indicates the
> allocation of a domain for device DMA managed by user space driver. All
> device passthrough frameworks could use this interface for their domain
> allocation.
>
> Although it is expected that all iommu drivers could implement their own
> domain_alloc_user ops, most drivers haven't implemented it yet. Rollback
> to the paging domain allocation interface if the iommu driver hasn't
> implemented this op yet.
>
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
> include/linux/iommu.h | 6 ++++++
> drivers/iommu/iommu.c | 42 ++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 48 insertions(+)
>
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 7bc8dff7cf6d..6648b2415474 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -780,6 +780,7 @@ extern bool iommu_present(const struct bus_type *bus);
> extern bool device_iommu_capable(struct device *dev, enum iommu_cap cap);
> extern bool iommu_group_has_isolated_msi(struct iommu_group *group);
> extern struct iommu_domain *iommu_domain_alloc(const struct bus_type *bus);
> +struct iommu_domain *iommu_user_domain_alloc(struct device *dev, u32 flags);
> extern void iommu_domain_free(struct iommu_domain *domain);
> extern int iommu_attach_device(struct iommu_domain *domain,
> struct device *dev);
> @@ -1086,6 +1087,11 @@ static inline struct iommu_domain *iommu_domain_alloc(const struct bus_type *bus
> return NULL;
> }
>
> +static inline struct iommu_domain *iommu_user_domain_alloc(struct device *dev, u32 flags)
> +{
> + return ERR_PTR(-ENODEV);
> +}
> +
> static inline void iommu_domain_free(struct iommu_domain *domain)
> {
> }
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 9df7cc75c1bc..f1416892ef8e 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -2032,6 +2032,48 @@ struct iommu_domain *iommu_domain_alloc(const struct bus_type *bus)
> }
> EXPORT_SYMBOL_GPL(iommu_domain_alloc);
>
> +/**
> + * iommu_user_domain_alloc() - Allocate a user domain
> + * @dev: device for which the domain is allocated
> + * @flags: iommufd_hwpt_alloc_flags defined in uapi/linux/iommufd.h
> + *
> + * Allocate a user domain which will be managed by a userspace driver. Return
> + * allocated domain if successful, or a ERR pointer for failure.

do you want to mention that this is for paging domain allocation?

> + */
> +struct iommu_domain *iommu_user_domain_alloc(struct device *dev, u32 flags)
> +{
> + struct iommu_domain *domain;
> + const struct iommu_ops *ops;
> +
> + if (!dev_has_iommu(dev))
> + return ERR_PTR(-ENODEV);
> +
> + ops = dev_iommu_ops(dev);
> + if (ops->domain_alloc_user) {
> + domain = ops->domain_alloc_user(dev, flags, NULL, NULL);
> + if (IS_ERR(domain))
> + return domain;
> +
> + domain->type = IOMMU_DOMAIN_UNMANAGED;
> + domain->owner = ops;
> + domain->pgsize_bitmap = ops->pgsize_bitmap;

this seems to break the iommufd selftest as the mock driver sets extra
bits in the domain->pgsize_bitmap in allocation. Override it may fail
something in the testing. you may need to check if domain->pgsize_bitmap
is set or use &=.

static struct iommu_domain *mock_domain_alloc_paging(struct device *dev)
{
struct mock_dev *mdev = container_of(dev, struct mock_dev, dev);
struct mock_iommu_domain *mock;

mock = kzalloc(sizeof(*mock), GFP_KERNEL);
if (!mock)
return NULL;
mock->domain.geometry.aperture_start = MOCK_APERTURE_START;
mock->domain.geometry.aperture_end = MOCK_APERTURE_LAST;
mock->domain.pgsize_bitmap = MOCK_IO_PAGE_SIZE;
if (dev && mdev->flags & MOCK_FLAGS_DEVICE_HUGE_IOVA)
mock->domain.pgsize_bitmap |= MOCK_HUGE_PAGE_SIZE;
mock->domain.ops = mock_ops.default_domain_ops;
mock->domain.type = IOMMU_DOMAIN_UNMANAGED;
xa_init(&mock->pfns);
return &mock->domain;
}

> + domain->ops = ops->default_domain_ops;
> +
> + return domain;
> + }
> +
> + /*
> + * The iommu driver doesn't support domain_alloc_user callback.
> + * Rollback to a UNMANAGED paging domain which doesn't support
> + * the allocation flags.
> + */
> + if (flags)
> + return ERR_PTR(-EOPNOTSUPP);
> +
> + return __iommu_domain_alloc(ops, dev, IOMMU_DOMAIN_UNMANAGED);
> +}
> +EXPORT_SYMBOL_GPL(iommu_user_domain_alloc);
> +
> void iommu_domain_free(struct iommu_domain *domain)
> {
> if (domain->type == IOMMU_DOMAIN_SVA)

--
Regards,
Yi Liu