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
