Re: [PATCH 1/5] iommu: Enhance domain allocation code to take additional flags

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

Jason,


On 8/26/2024 7:26 PM, Jason Gunthorpe wrote:
> On Mon, Aug 26, 2024 at 02:06:27PM +0530, Vasant Hegde wrote:
>>>> @Jason,
>>>>   Notice that I have added __iommu_paging_domain_alloc_flags() so that
>>>>   it can call iommu_domain_init() with appropriate domain type.
>>>
>>> It is okay, but also the type could be fixed in
>>> __iommu_group_alloc_default_domain() using something like:
>>>
>>> 		ret->type |= req_type;
>>
>> Sorry. I didn't get it. You mean domain->type? 
> 
> Sorry, yes. I mean the path that knows it is allocating for the DMA
> API can or in the DMA API flags after allocation succeeds and we don't
> have to muddle lower levels by passing them all around.

Ok.

> 
>>>>   I think instead of having separate function it may be better to
>>>>   enhance __iommu_domain_alloc() such that:
>>>>     - Keep below changes from this patch
>>>>       - iommu_domain_init()
>>>>       - iommu_get_dma_cookie call inside iommu_setup_default_domain()
>>>>     - modify __iommu_domain_alloc() to additional param (flags)
>>>>     - iommu_paging_domain_alloc_flags() will call __iommu_domain_alloc()
>>>
>>> My expectation was to basically remove iommu_domain_alloc() entirely
>>> once Lu's work is merged.
>>>
>>> Instead we'd have these direct APIs:
>>>   iommu_domain_alloc_paging_flags()
>>>   iommu_group_alloc_blocking_domain()
>>>   iommu_group_alloc_identity_domain()
>>
>> Sure (For AMD driver, we may have to tweak a bit to allocate_identity domain,
>> but doable).
> 
> Why does it need to allocate_identity ?

Ignore. I got mixed up with global identity domain and this one.

> 
>>> I think the series looks OK. The naming of domain_alloc_user() is now
>>> a little bit weird, it has now turned into
>>> domain_alloc_paging_extended().
>>
>> Ack. Do you want me to do it now -OR- May be wait until all changes settles down
>> (above described group() related changes, this series, Baolu's series, etc)?
> 
> I think if we rename things it will clash with the smmu nesting
> series, lets live with it for now.

Sure.

-Vasant