Re: [PATCH v2 iommu/next] iommu: Fix default domain setup

[Vasant Hegde <vasant.hegde@amd.com>]

Hi Baolu, Jason,


On 6/20/2023 5:12 PM, Jason Gunthorpe wrote:
> On Tue, Jun 20, 2023 at 02:31:43PM +0800, Baolu Lu wrote:
>>>   err_restore:
>>>   	if (old_dom) {
>>>   		__iommu_group_set_domain_internal(
>>>   			group, old_dom, IOMMU_SET_DOMAIN_MUST_SUCCEED);
>>> +		group->default_domain = old_dom;
>>>   		iommu_domain_free(dom);
>>>   		old_dom = NULL;
>>>   	}
>>
>> The err_restore branch doesn't work if old_dom is NULL. We have no means
>> to restore a group from a successful first-time attaching to NULL
>> attaching.
> 
> Yes, this is what I fixed in my alternative version

Not really. Even your version has

+err_restore_domain:
+	if (old_dom)


Only first time we will have old_dom is NULL and this functions returns error
code. iommu_probe_device()/bus_iommu_probe() handler error path properly and
frees resources. So I think this is fine.

@Jason,
  I'm fine with your version of patch as well. Are you going to send proper
patch -OR- do you want me to respin?

-Vasant