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

[Baolu Lu <baolu.lu@linux.intel.com>]

On 6/5/24 2:23 PM, Yi Liu wrote:
> On 2024/6/5 10:00, Baolu Lu wrote:
>> On 6/4/24 4:03 PM, Yi Liu wrote:
>>> 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?
>>
>> You are worrying about its confusion with nesting domain allocation,
>> right?
> 
> yes. As I replied in last version, user_domain is a bit confusing since
> nested domain is also a user_domain. That's why we introduced the
> alloc_domain_user op.

As I discussed with Jason in another reply, I will drop the
iommu_user_domain_alloc() interface as it is currently iommfd-specific
only and not generic for a common interface.

Best regards,
baolu