[PATCH v6 02/25] iommu: Add IOMMU_DOMAIN_PLATFORM

[Baolu Lu]

On 2023/8/12 19:28, Jason Gunthorpe wrote:
> On Sat, Aug 12, 2023 at 09:36:33AM +0800, Baolu Lu wrote:
>>> @@ -290,6 +295,7 @@ struct iommu_ops {
>>>    	unsigned long pgsize_bitmap;
>>>    	struct module *owner;
>>>    	struct iommu_domain *identity_domain;
>>> +	struct iommu_domain *default_domain;
>>
>> I am imaging whether we can merge above two pointers into a single one.
>> It is either an IDENTITY or PLATFORM domain and the core will choose it
>> as the default domain of a group if iommu_group_alloc_default_domain()
>> fails to allocate one through the iommu dev_ops.
> 
> I think that would be the wrong direction..
> 
> identity_domain is a pointer that is always, ALWAYS an identity
> domain. It is the shortcut for drivers (and all drivers should do
> this) that implement a global static identity domain.

I see. I originally thought this was special for arm32.

> 
> default_domain is a shortcut to avoid implementing the entire flow
> around def_domain_type/domain_alloc for special cases. For this patch
> the specialc ase is the IOMMU_DOMAIN_PLATFORM.

I think this is special for drivers like s390. You don't want it to be
used beyond those special drivers, right?

If so, the naming of default_domain seems to be a bit generic. I can't
think of a better one, hence I am fine if you keep as it-is. After all,
the comment for this field has already explained it very clearly.

> We'll probably also get a blocking_domain pointer here too.

Yes.

> 
> All of this is removing the type multiplexor in alloc_domain so we can
> so  alloc_domain_paging()

Agreed with you. The dummy domains like identity and blocking could be
avoided from calling ops->domain_alloc.

>> Probably we could give it a more meaningful name? For example,
>> supplemental_domain or rescue_domain?
> 
> But that isn't what it is for, default_domain is the operational
> domain for attached drivers..

Best regards,
baolu