Received-SPF: pass (google.com: domain of linux-kernel+bounces-200209-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <445fae9f-ea1e-4864-9f0e-f348c51146a1@intel.com>
Date: Tue, 4 Jun 2024 16:03:58 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v2 01/22] iommu: Add iommu_user_domain_alloc() interface
To: Lu Baolu <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: <20240604015134.164206-1-baolu.lu@linux.intel.com>
 <20240604015134.164206-2-baolu.lu@linux.intel.com>
Content-Language: en-US
From: Yi Liu <yi.l.liu@intel.com>
In-Reply-To: <20240604015134.164206-2-baolu.lu@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?aXJHQzMvNXo0Tkp2Z2d3a1lVc0ZzanNEOVRvUkNsVXVieE9iMVBRWUhaMzhv?=
 =?utf-8?B?NE9xQTAxWTBBcUFvaU5DSldQc0RKRnlJVjZLTjM0a0dCRjBpSnZBdGZ2ZUtC?=
 =?utf-8?B?OUpaMm42cGFnQ2NxOTdhL2NnL3VXT2h6enBVcjNEK3Ryb0NCa0s5UGtVNFVx?=
 =?utf-8?B?ekRhb2w5Q1pIbFdhbll5SVE1N0d5Qjk3aU5GVGwvUXVxTVBHOU5WbGh4K1Zv?=
 =?utf-8?B?OGlEUjM2WTN1aTRlR0ZnU2ZIczcwS0o4ajI1bjFSYnlkU2tnSytvZG9ZUTEy?=
 =?utf-8?B?MnpWUTlxeHBNVDZCVDdlRzdrUG9Lcm43UjdweFJ2UnNEVjFBenQ1OTUxZzd4?=
 =?utf-8?B?MHA1SE5tRWlGVTY1Z2hpaGpKSTZnNUQxMGlSOXNmMFA2bWlSYllUZitlM0wv?=
 =?utf-8?B?V1VJbFAxdG1GWXlUQWNsY1NpTUMxdVNQRGR2eU14eThOVlBHU3BIU0ZJbURK?=
 =?utf-8?B?SHRsUWNqcC90bnlhQlhVQWZhSXZEZjczbXFtQ2xXcFRmdGJVNFJpV1FPT0ZO?=
 =?utf-8?B?MHpFYVl0cXVNdlFycnp0UUc2b3liS0tBNDkvbG10ZkxUK2t5NXZvNk9GVjVD?=
 =?utf-8?B?YXViSURXQWJBVm1rOHVybkJBQ0hoMmp5N2dJclp3QXZLaklFSCtCTW5iQy9R?=
 =?utf-8?B?U0Q3UkVjMElVc0c5Um5JSndIRXkzeTFrVzlvTlBvRE1uSVpKa0k5bWE1SUlP?=
 =?utf-8?B?aVEwenorMU50N29kN1RsOWdOcnRoRktIRzRwTElzM2dFZUdKY1FPdHRFNndJ?=
 =?utf-8?B?NlgzaXNDRGJyVTJ5UzlTTnozUlhSYVJidHZ4MWNrWXlDeE9PbzBLdWhYQkNh?=
 =?utf-8?B?b2dUdlpJdWI4QXlDSTRNQnlQN29XZ2NPek9GYm1Td1NIN3FlWkxCMkVnckJq?=
 =?utf-8?B?MFhzbmI4SVpEODZ6eFBOLzB5ZW9GMy9XZElNUHdJU2wxbkoySjJ1SFE3WWdJ?=
 =?utf-8?B?U1ErZUhtM1dkZU90bDBoNVFpY25pRU5UbVBHQU81WmNlU2U2aUZCUzFCVVZX?=
 =?utf-8?B?WWpMSFhYWWRQOHJuL0tTd1ZHYmYyenNiMFRqMDNtb0FWWlIwOXRlODJReWpH?=
 =?utf-8?B?QlpyellPbXJJNU5VZ1AwV3JhbzY0SGM0VEpFRm5zWmtsT1VoOWJMeDdJa2JF?=
 =?utf-8?B?TTEzTjhhKzVBU3pHTjBEa2lEclliTVl2c3ZObTlnWmdlN1pTb0h4WW9QU3BN?=
 =?utf-8?B?dFBFbFJEemdLM2w3Q0dLWW05alF4clppNGlzRW8waVBVOGNDSzFvZ0lRQlQr?=
 =?utf-8?B?TmNVSGlLMTVxMk5PZ25RMTlsdFdnRS96R3NjaWVKeXQrZXFheXZPN1pyeUVv?=
 =?utf-8?B?MHF2bFovbXZtL1ROdlhUVzhHMWlXVXRKSHViZ3JSVzJZT0RqWHk5K2JDTXU3?=
 =?utf-8?B?LzFyd1NXZ2c3dW1mQU1YSmtEM2k0d2JRUm5qN29TcGlXUFRsNEh2Mmo3dHVJ?=
 =?utf-8?B?c1MvQXNpeTY3dFZZZjBFa2s5M05RY2dRTFFMQS9BSFNtWFZBbzd2MHM5Unc0?=
 =?utf-8?B?Q2FDaGNodlJ0L3RQTllTOCtOZC9NaldjSHBkUmhXV1lmWElMdHpZUDBNUEZl?=
 =?utf-8?B?bDIyRyt6V1N2SnF0ZHp1STZlWWU0MmlseWVZUkhEeE12LzVBRTNHWDlubW93?=
 =?utf-8?B?MnhHMmNaM0YyL25Jd2o1OVRVK0dEOUkyR0kxT2JRbFhUeklhTmFZZ2YxNU1W?=
 =?utf-8?B?RGZBcVovc0k4NHI4RHJLbkFUSzJUZ2ZDN0QxNkZYTFBRV1JQRFppMCtsK1pa?=
 =?utf-8?B?UEErZURpNnlYUFRHK0RYODdIT0JVZkFSd1daSi95dzdzQUZBQVA1WWNVMGkx?=
 =?utf-8?B?M0NnMWQ5bGs1d2g5bytkL1RwN0VVOTlRQ052Y1dtYUtpeTV0SVFiRGNqRW1M?=
 =?utf-8?B?aDhaVEJKWS9kRUtTeHZUMys3YWlDV1F1OCtDWnNMejZDdnpFQi9ndnVoUU40?=
 =?utf-8?B?NEhkRnRPaGdva3h6RmdxaDZwcDNvZVB0eUp5SGMzTEYxLzloSkZYZlVNU1Yy?=
 =?utf-8?B?QXZCYWN1RlNGMm9ndlUxeStDZ3hpdkZXNSt3RjdGcW84TkVCRHBGWW5qSVg3?=
 =?utf-8?B?NDZVTTJrYnNsdkd4SnF2eCsxTjRXUmVQMTJKS3o1M1grOXdwVm9JUndsdW9i?=
 =?utf-8?Q?X780RPU8C9pUV6ThWRYqTZW5o?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 39a8ab03-a3df-4770-49ce-08dc846c58dc
X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB7529.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Jun 2024 08:00:06.4962
 (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: QxZmtKOuj5RpwJtPXylCegX0fwH1VmuRqGKWv/Wi8D63cQEoZveZNiiiE0OaxzFGb6KFmtkdq/gOyFFxphpSxA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY5PR11MB6308
X-OriginatorOrg: intel.com

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