Re: [PATCH v2 12/14] iommu: Consolidate the default_domain setup to one function

[Robin Murphy <robin.murphy@arm.com>]

On 2023-03-30 16:42, Jason Gunthorpe wrote:
> On Thu, Mar 30, 2023 at 11:45:10AM -0300, Jason Gunthorpe wrote:
> 
>> IIRC the existing code had both orderings :( I'll add a note about
>> this justification
> 
> Robin, would this be helpful in the short term for ARM?

Yeah, something along these lines seems like the neatest compromise for now.

Cheers,
Robin.

> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index c31c5d6f913194..39c923e71e625a 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -1778,6 +1778,7 @@ static int iommu_setup_default_domain(struct iommu_group *group,
>   	struct bus_type *bus =
>   		list_first_entry(&group->devices, struct group_device, list)
>   			->dev->bus;
> +	bool direct_failed;
>   	int req_type;
>   	int ret;
>   
> @@ -1824,8 +1825,10 @@ static int iommu_setup_default_domain(struct iommu_group *group,
>   	 * mapped before their device is attached, in order to guarantee
>   	 * continuity with any FW activity
>   	 */
> +	direct_failed = false;
>   	for_each_group_device(group, gdev)
> -		iommu_create_device_direct_mappings(dom, gdev->dev);
> +		if (iommu_create_device_direct_mappings(dom, gdev->dev))
> +			direct_failed = true;
>   
>   	ret = __iommu_group_set_domain_internal(group, dom,
>   						IOMMU_SET_DOMAIN_WITH_DEFERRED);
> @@ -1845,6 +1848,16 @@ static int iommu_setup_default_domain(struct iommu_group *group,
>   		goto out_set;
>   	}
>   
> +	/*
> +	 * Driver's are supposed to allow mappings to be installed in a domain
> +	 * before device attachment, but some don't. Hack around this defect by
> +	 * trying again after.
> +	 */
> +	if (direct_failed) {
> +		for_each_group_device(group, gdev)
> +			iommu_create_device_direct_mappings(dom, gdev->dev);
> +	}
> +
>   out_set:
>   	if (group->default_domain)
>   		iommu_domain_free(group->default_domain);