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

quoted

	/*
	 * 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
 

quoted

	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

quoted

	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