Re: [PATCH v6 08/25] iommu: Reorganize iommu_get_default_domain_type() to respect def_domain_type()

[Jason Gunthorpe <jgg@nvidia.com>]

On Tue, Aug 15, 2023 at 09:18:59AM +0800, Baolu Lu wrote:

> > 	/*
> > 	 * Try to recover, drivers are allowed to force IDENITY or DMA, IDENTITY
> > 	 * takes precedence.
> > 	 */
> > 	if (cur_type || type == IOMMU_DOMAIN_IDENTITY)
> > 		return IOMMU_DOMAIN_IDENTITY;
> 
> No need to check cur_type. It already returned if cur_type is 0.

Yep
 
> > 	return cur_type;
> > }
> > 
> > /*
> >   * A target_type of 0 will select the best domain type. 0 can be returned in
> >   * this case meaning the global default should be used.
> >   */
> > static int iommu_get_default_domain_type(struct iommu_group *group,
> > 					 int target_type)
> > {
> > 	struct device *untrusted = NULL;
> > 	struct group_device *gdev;
> > 	int driver_type = 0;
> > 
> > 	lockdep_assert_held(&group->mutex);
> > 
> > 	/*
> > 	 * ARM32 drivers supporting CONFIG_ARM_DMA_USE_IOMMU can declare an
> > 	 * identity_domain and it will automatically become their default
> > 	 * domain. Later on ARM_DMA_USE_IOMMU will install its UNMANAGED domain.
> > 	 * Override the selection to IDENTITY.
> > 	 */
> > 	if (IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)) {
> > 		static_assert(!(IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU) &&
> > 				IS_ENABLED(CONFIG_IOMMU_DMA)));
> 
> IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU) is duplicate with the condition in the
> if statement. So only
> 		static_assert(!IS_ENABLED(CONFIG_IOMMU_DMA));
> ?

static_assert doesn't work that way, it ignores its calling context
and always checks during compilation, so the duplication is required

> > 
> > 	for_each_group_device(group, gdev) {
> > 		driver_type = iommu_get_def_domain_type(group, gdev->dev,
> > 							driver_type);
> 
> No need to call this in the loop body?

Do need it, this only gets the def_domain_type of a single device so
we have to iterate over all the devices in the group to 'reduce' the
type for the group.

Thanks,
Jason