Re: [PATCH 1/2] iommu: Add ops->domain_alloc_nested()

[Jason Gunthorpe <jgg@nvidia.com>]

On Fri, Nov 15, 2024 at 11:19:03AM +0800, Baolu Lu wrote:

> > diff --git a/drivers/iommu/intel/nested.c b/drivers/iommu/intel/nested.c
> > index 42c4533a6ea21d..aba92c00b42740 100644
> > --- a/drivers/iommu/intel/nested.c
> > +++ b/drivers/iommu/intel/nested.c
> > @@ -186,14 +186,21 @@ static const struct iommu_domain_ops intel_nested_domain_ops = {
> >   	.cache_invalidate_user	= intel_nested_cache_invalidate_user,
> >   };
> > -struct iommu_domain *intel_nested_domain_alloc(struct iommu_domain *parent,
> > -					       const struct iommu_user_data *user_data)
> > +struct iommu_domain *
> > +intel_iommu_domain_alloc_nested(struct device *dev, struct iommu_domain *parent,
> > +				u32 flags,
> > +				const struct iommu_user_data *user_data)
> >   {
> > +	struct device_domain_info *info = dev_iommu_priv_get(dev);
> >   	struct dmar_domain *s2_domain = to_dmar_domain(parent);
> > +	struct intel_iommu *iommu = info->iommu;
> >   	struct iommu_hwpt_vtd_s1 vtd;
> >   	struct dmar_domain *domain;
> >   	int ret;
> > +	if (!nested_supported(iommu) || flags)
> > +		return ERR_PTR(-EOPNOTSUPP);
> 
> How about making it like
> 
>         if (!nested_supported(iommu) || (flags &
> ~IOMMU_HWPT_FAULT_ID_VALID))
>                 return ERR_PTR(-EOPNOTSUPP);
> 
>         if ((flags & IOMMU_HWPT_FAULT_ID_VALID) && !info->pri_supported)
>                 return ERR_PTR(-EOPNOTSUPP);

I think that is possibly a good idea for a followup, but right now it
was left like this:

+       hwpt->domain = ops->domain_alloc_nested(
+               idev->dev, parent->common.domain,
+               flags & ~IOMMU_HWPT_FAULT_ID_VALID, user_data);

So you'd also want to remove the FAUlT_ID_VALID masking there to move
it to the driver.

I'm also wondering if we should put the pri_supported concept into
core code flags and have less boilerplate in drivers.

Thanks,
Jason