Received-SPF: pass (google.com: domain of linux-kernel+bounces-203520-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Message-ID: <75d0e879-a807-420e-bbf6-2ba4238329cf@linux.intel.com>
Date: Thu, 6 Jun 2024 10:04:44 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Cc: baolu.lu@linux.intel.com, David Airlie <airlied@gmail.com>,
 Daniel Vetter <daniel@ffwll.ch>, Kalle Valo <kvalo@kernel.org>,
 Bjorn Andersson <andersson@kernel.org>,
 Mathieu Poirier <mathieu.poirier@linaro.org>,
 Alex Williamson <alex.williamson@redhat.com>, mst@redhat.com,
 Jason Wang <jasowang@redhat.com>, Thierry Reding <thierry.reding@gmail.com>,
 Jonathan Hunter <jonathanh@nvidia.com>,
 Mikko Perttunen <mperttunen@nvidia.com>, iommu@lists.linux.dev,
 dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 01/22] iommu: Add iommu_user_domain_alloc() interface
To: Yi Liu <yi.l.liu@intel.com>, Joerg Roedel <joro@8bytes.org>,
 Will Deacon <will@kernel.org>, Robin Murphy <robin.murphy@arm.com>,
 Jason Gunthorpe <jgg@ziepe.ca>, Kevin Tian <kevin.tian@intel.com>
References: <20240604015134.164206-1-baolu.lu@linux.intel.com>
 <20240604015134.164206-2-baolu.lu@linux.intel.com>
 <445fae9f-ea1e-4864-9f0e-f348c51146a1@intel.com>
 <91a42f46-060a-4430-a10a-7bfded67c4a9@linux.intel.com>
 <b205bcfb-9972-4e03-9994-62419117d76f@intel.com>
Content-Language: en-US
From: Baolu Lu <baolu.lu@linux.intel.com>
In-Reply-To: <b205bcfb-9972-4e03-9994-62419117d76f@intel.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit

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