Re: [PATCH 4/7] iommu/vtd: Split intel_iommu_domain_alloc_paging_flags()

[Jason Gunthorpe <jgg@nvidia.com>]

On Tue, Jun 10, 2025 at 09:14:05AM +0000, Wang, Wei W wrote:
> > +static struct iommu_domain *
> > +intel_iommu_domain_alloc_paging_flags(struct device *dev, u32 flags,
> > +				      const struct iommu_user_data *user_data)
> > {
> > +	struct device_domain_info *info = dev_iommu_priv_get(dev);
> > +	struct intel_iommu *iommu = info->iommu;
> > +	struct iommu_domain *domain;
> > +
> > +	if (user_data)
> 
> Curious how is this user_data used. I check all the drivers (intel/amd/arm/selftest),
> seems nobody supports it.

It is a stand in for drivers to allow userspace to customize the page
table format. Only nesting domain types use it right now and since
they were split out the paging ones became disused.

> It seems a bit confusing to have it everywhere without
> real usages. I had a plan to remove it from all the interfaces (not sure if any
> misunderstanding)

Please just leave it, it will get used at some point

> > +
> > +	/* Prefer fist stage if possible by default. */
> 
> fist --> first

Yep

> > +	domain = intel_iommu_domain_alloc_first_stage(dev, iommu, flags);
> > +	if (domain != ERR_PTR(-EOPNOTSUPP))
> > +		return domain;
> 
> Should be "if (!IS_ERR(domain))" here?
> paging_domain_alloc() could also return ERR_PTR(-ENOMEM)

If alloc_first_stage() returns -ENOMEM then the first stage was
supported and we ran out of memory, the entire function should fail
and we don't try the second stage (since first was supported).

If we changed it as you propse then we'd never try the second stage.

EOPNOTSUPP means the first stage is not supported and the second stage
should be tried.

Jason