Received-SPF: pass (google.com: domain of linux-kernel+bounces-196342-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <f96d09a3-f1f5-4a73-88ae-50bf15b9e729@intel.com>
Date: Fri, 31 May 2024 14:24:41 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 00/20] iommu: Refactoring domain allocation interface
To: Baolu Lu <baolu.lu@linux.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>
CC: 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>
References: <20240529053250.91284-1-baolu.lu@linux.intel.com>
 <efd902f6-eafc-4a26-8057-bdd9d7d6e535@intel.com>
 <a1f2c08a-e92f-4080-b55e-8d6dbd94db78@linux.intel.com>
 <7af4fee2-1b37-4eb8-9d03-8b1a402ec00b@intel.com>
 <34d23852-ea68-414e-92ce-61dcfe6a0368@linux.intel.com>
Content-Language: en-US
From: Yi Liu <yi.l.liu@intel.com>
In-Reply-To: <34d23852-ea68-414e-92ce-61dcfe6a0368@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?ck1VVWtXd0wySHR5ZGs0Vk5UQ0wzdDYvN0pDclNheGcrWjVPcVE0MDFTK2F4?=
 =?utf-8?B?U2NUZDQrUldvK2VxaUJ1M1pjbVdlYmNqbWluZzRMUGZmb3BMQ3YyUWF3MnZ5?=
 =?utf-8?B?dlBraFdOeWUzNzhiRDErdVZidGpoMytId0szZFMxSlFTbXNkWXB5RUo3VFpK?=
 =?utf-8?B?cWV2SnVRY2g2NC9uaTI1YlRpM0xNbkQyeElPZzUwRFZhV2dFRmplREtBVlJ1?=
 =?utf-8?B?Z2Nadi90TlJHSUY2b1RKeTFlazNYdlRibWxJbytzeTg0eEQ1emVQNDlGU0l6?=
 =?utf-8?B?SHJBUUN6QlRHQ0ZqQmhHWS9pc25aUHZkdjFIblFsSnlIdkZxaDd3NXJ0L1BT?=
 =?utf-8?B?Q1AvczROaUg0SW1VMG9jeFZNdjh2Tmx5T3RhQUtIZTZCSXR3elIwckx2MzlK?=
 =?utf-8?B?QlNPR1RnMWJZTlhxM0I5cFIyR3hVbjd1WFlONWYwSTN1a1p5U0R4N0E3Tkpw?=
 =?utf-8?B?L2dwK2ptRjEvY2xtckZzYldUNGV4NlJHd2JvMHF5Nnh6UjdRS3BCMW1VbGVB?=
 =?utf-8?B?cUFNUjhzNmZkMXJDU3BPbUxJaWF3RkZtQXBKaWszU1U1M1VSSURGdjd6anN4?=
 =?utf-8?B?Y0VvRS9DaVBnVU0xMWdpcXNCZEtLTHYxQWpZRlRaMXVGUGRVT3NKSlR3NWp6?=
 =?utf-8?B?U2hLVlp4eTlWa0oxd3M2b2t0R1NaTGlHY002UmFVK21pV3hRMmYwd1dnWkpR?=
 =?utf-8?B?T25ETUgzSmFvYXJZbTc2SWdzL2RPMWlMbHdPM0pjakU1eUNWZ09yQmcvMkp4?=
 =?utf-8?B?UFNrRm9NT21tM3dvNC82S2lCQ0lHR203OTRBa2VORllLbHZrdnRIcXduc3B5?=
 =?utf-8?B?ajhSd1EyU1lqUkhRbkVMTkgzSzBqa0dwZ3cxS1FabUhFVVZNK2RpS2lIc1Fl?=
 =?utf-8?B?YWl1bVZlL3RXYXRoaGJ4UE5YaitwM2FvUlVRc0tqYit3WlcxVmZ2alk4eGNv?=
 =?utf-8?B?QTZ1Tlhla0lRL2dsQWQ3VUpLWitCVkxhdlBsb085SEdIeCtwN2FwMFZWQ2pS?=
 =?utf-8?B?d3kvY0xsdm0zQnN4T3VnbVNJYXQ3VVc1UVVUb2c0WFVIckJaTEQxdzZwUzBM?=
 =?utf-8?B?aitjZU5tbm1wZUV6RlJ0K21qUHFTMm1sR2hER1MvazY1TGQ2K2szQzVzcUtN?=
 =?utf-8?B?ajBUTTZjS096dVB2UVZLQm84RHFSOU4wenFJbzVLaU8xM1VRcTRCb0txcmZJ?=
 =?utf-8?B?QlVPWHo1aFlJeUlZTkVLQk5SSGNrRXZMNHVxY3F4MUxpYjdOZGFHTjVvZmlC?=
 =?utf-8?B?RFkrZjR5dkw4ZWZsMWlMSi84dkRMN280SEpZT0h5ZldWcWFZTXNybDNBbFdU?=
 =?utf-8?B?YTJjRkp5bjRlQ1lDQjQ1eWFrNEY0bWdsK0lFbGc0aTJEdkdLeTZnSWlsM1c0?=
 =?utf-8?B?eWNVM0RQWG9WWjNVUXcxck9hR00zWG01eUV0bUl1eGdUR0lMTlRIc1AvcjFt?=
 =?utf-8?B?dVhqdVQrMDB1NWV4ZWJpaTUrbDV6MHVUMGxVZU14aTVHdkFYTDFiRndyOFN4?=
 =?utf-8?B?d2ZzYmZ0YWM3NnQ4MitQcEEvSVc2d2ltUGdxTVZZWnoyV0puU1d0dEJTVlIw?=
 =?utf-8?B?KzIvYXVyMUJ0Vml1Uy8yVTJrUWdFN2RuWTg4MlpVNVUyWVJSMHd1Z3psYzhL?=
 =?utf-8?B?MDcvdTR2alV4QlVLR25vaVZxUkJJaXp2aXNyWlRDSGJpdk1HV0FkSkZjM1dJ?=
 =?utf-8?B?S1QrSWJjTlJicFhjLzNrS3Fidk0vYmUyYkxTa3lZQnBic28zMElXYTROZWdJ?=
 =?utf-8?B?UUNCNXBXU3d1UVNEek9EeTNRQmNHOTQwbVB0U1kwLzRDQ0FyL0Q0dEZiYklI?=
 =?utf-8?B?OXFRUzc0NVRTaUhXSTROS1hTZTl6V29Va05kR3ZwQUp3L1ZndmV6WjlBcWxZ?=
 =?utf-8?B?RGw2QUxiZlVMRzhnN1RjZmVwUFVrd1dLd1RLMklSOXVIRUIxaFRtd0l2TCts?=
 =?utf-8?B?aDl3cWVuYjNRM1BQZGdkejZqRmh5UnhlZ0tUeUtwWm9iZ2IwUHU1THpiNys1?=
 =?utf-8?B?MTIvZ1hXSWYvNk5vLzAwNzVPSmZaeVc2SnBwaVNZU3BRQmlLSkF0S3Bld3pk?=
 =?utf-8?B?U2RpcjNWcFBQdStlWkRSS1RJemRUeGxHeGN4UUpnZUJCUnJvMU44b1Q4SHgy?=
 =?utf-8?Q?Zpmc6/e78OmvxGNrlp3IsuyOz?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 60930d50-9c20-4bda-b80f-08dc8139d020
X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB7529.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 May 2024 06:20:48.7749
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: JIB/BGNcwg5Ebq6chmyTaomhAhxV71xX8ibb5XWWvjRczDHZhtG3JeH5zwTEBl0G3e75uzKyN1YOBNu8nWnwGg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB6312
X-OriginatorOrg: intel.com

On 2024/5/31 14:00, Baolu Lu wrote:
> On 5/31/24 11:16 AM, Yi Liu wrote:
>> On 2024/5/29 20:02, Baolu Lu wrote:
>>> On 2024/5/29 17:03, Yi Liu wrote:
>>>> On 2024/5/29 13:32, Lu Baolu wrote:
>>>>> The IOMMU subsystem has undergone some changes, including the removal
>>>>> of iommu_ops from the bus structure. Consequently, the existing domain
>>>>> allocation interface, which relies on a bus type argument, is no longer
>>>>> relevant:
>>>>>
>>>>>      struct iommu_domain *iommu_domain_alloc(struct bus_type *bus)
>>>>>
>>>>> This series is designed to refactor the use of this interface. It
>>>>> proposes two new interfaces to replace iommu_domain_alloc():
>>>>>
>>>>> - iommu_user_domain_alloc(): This interface is intended for allocating
>>>>>    iommu domains managed by userspace for device passthrough scenarios,
>>>>>    such as those used by iommufd, vfio, and vdpa. It clearly indicates
>>>>>    that the domain is for user-managed device DMA.
>>>>
>>>> user paging domain? It looks to me user domain includes the nested domains
>>>> as well.
>>>
>>> Yes, nested domain is a user domain. The iommu driver should implement
>>> iommu_ops->domain_alloc_user for nested domain allocation.
>>
>> will it be more clear to name iommu_user_domain_alloc() be
>> iommu_user_paging_domain_alloc() as it is mainly for paging domain
>> allocation?
> 
> That might be better; let's wait and see if there's another option.
> 
>>
>>>>
>>>>>    If an IOMMU driver does not implement iommu_ops->domain_alloc_user,
>>>>>    this interface will rollback to the generic paging domain allocation.
>>>>>
>>>>> - iommu_paging_domain_alloc(): This interface is for allocating iommu
>>>>>    domains managed by kernel drivers for kernel DMA purposes. It takes a
>>>>>    device pointer as a parameter, which better reflects the current
>>>>>    design of the IOMMU subsystem.
>>>>>
>>>>> The majority of device drivers currently using iommu_domain_alloc() do
>>>>> so to allocate a domain for a specific device and then attach that
>>>>> domain to the device. These cases can be straightforwardly migrated to
>>>>> the new interfaces.
>>>>>
>>>>> However, there are some drivers with more complex use cases that do
>>>>> not fit neatly into this new scheme. For example:
>>>>>
>>>>> $ git grep "= iommu_domain_alloc"
>>>>> arch/arm/mm/dma-mapping.c:      mapping->domain = 
>>>>> iommu_domain_alloc(bus);
>>>>> drivers/gpu/drm/rockchip/rockchip_drm_drv.c:    private->domain = 
>>>>> iommu_domain_alloc(private->iommu_dev->bus);
>>>>> drivers/gpu/drm/tegra/drm.c:            tegra->domain = 
>>>>> iommu_domain_alloc(&platform_bus_type);
>>>>> drivers/infiniband/hw/usnic/usnic_uiom.c:       pd->domain = domain = 
>>>>> iommu_domain_alloc(dev->bus);
>>>>>
>>>>> This series leave those cases unchanged and keep iommu_domain_alloc()
>>>>> for their usage. But new drivers should not use it anymore.
>>>>
>>>> does it mean there is still domains allocated via iommu_domain_alloc()
>>>> on VT-d platform?
>>>
>>> I think the drivers mentioned above do not run on x86 platforms, or do
>>> they?
>>
>> cool. BTW. I know out-of-tree drivers are not counted in upstream review.
>> Just out of curious, is there a formal way to let such drivers know it is
>> no longer allowed to use iommu_domain_alloc() on VT-d?
> 
> As Robin suggested, we should try to remove iommu_domain_alloc() from
> the tree in this series.

If it's completely dropped, that's fine. OOT drivers should fail in the
time of compiling.

-- 
Regards,
Yi Liu