> From: Baolu Lu <baolu.lu@xxxxxxxxxxxxxxx>
> Sent: Saturday, October 21, 2023 11:08 AM
>
> On 10/20/23 5:32 PM, Yi Liu wrote:
> > From: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
> >
> > This adds the support for IOMMU_HWPT_DATA_VTD_S1 type.
> >
> > Signed-off-by: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
> > Signed-off-by: Yi Liu <yi.l.liu@xxxxxxxxx>
> > ---
> > drivers/iommu/intel/iommu.c | 39 ++++++++++++++++++++++++++-----------
> > 1 file changed, 28 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> > index b47025fbdea4..c7704e7efd4a 100644
> > --- a/drivers/iommu/intel/iommu.c
> > +++ b/drivers/iommu/intel/iommu.c
> > @@ -4076,7 +4076,7 @@ intel_iommu_domain_alloc_user(struct device *dev, u32
> flags,
> > struct iommu_domain *parent,
> > const struct iommu_user_data *user_data)
> > {
> > - struct iommu_domain *domain;
> > + bool request_nest_parent = flags & IOMMU_HWPT_ALLOC_NEST_PARENT;
> > struct intel_iommu *iommu;
> >
> > if (flags & (~IOMMU_HWPT_ALLOC_NEST_PARENT))
> > @@ -4086,18 +4086,35 @@ intel_iommu_domain_alloc_user(struct device *dev, u32
> flags,
> > if (!iommu)
> > return ERR_PTR(-ENODEV);
> >
> > - if ((flags & IOMMU_HWPT_ALLOC_NEST_PARENT) && !ecap_nest(iommu-
> >ecap))
> > + if (!user_data) { /* Must be PAGING domain */
> > + struct iommu_domain *domain;
> > +
> > + if (request_nest_parent && !ecap_nest(iommu->ecap))
>
> Hardware capability is not sufficient. How about adding below helper:
Yes. But the sm_supported() seems a bit confusing. Is it? actually, it's
a combined check of the sm capability and software choice (use sm or
not). How about renaming it to be sm_enabled()? Although the best
enable status check is to read the SM bit in root address register. No hurry
in this circle, but may be nice to have.
> diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
> index b5f33a7c1973..b04bbabcd696 100644
> --- a/drivers/iommu/intel/iommu.h
> +++ b/drivers/iommu/intel/iommu.h
> @@ -540,6 +540,8 @@ enum {
> #define sm_supported(iommu) (intel_iommu_sm &&
> ecap_smts((iommu)->ecap))
> #define pasid_supported(iommu) (sm_supported(iommu) && \
> ecap_pasid((iommu)->ecap))
> +#define nested_supported(iommu) (sm_supported(iommu) &&
> \
> + ecap_nest((iommu)->ecap))
>
> struct pasid_entry;
> struct pasid_state_entry;
>
> And, use nested_supported() here and bleow
>
> > + return ERR_PTR(-EOPNOTSUPP);
> > + if (parent)
> > + return ERR_PTR(-EINVAL);
> > + /*
> > + * domain_alloc_user op needs to fully initialize a domain
> > + * before return, so uses iommu_domain_alloc() here for
> > + * simple.
> > + */
> > + domain = iommu_domain_alloc(dev->bus);
> > + if (!domain)
> > + return ERR_PTR(-ENOMEM);
> > + return domain;
> > + }
> > +
> > + /* Must be nested domain */
> > + if (!ecap_nest(iommu->ecap))
>
> ...here.
>
> > + return ERR_PTR(-EOPNOTSUPP);
> > + if (user_data->type != IOMMU_HWPT_DATA_VTD_S1)
> > return ERR_PTR(-EOPNOTSUPP);
> > + if (!parent || parent->ops != intel_iommu_ops.default_domain_ops)
> > + return ERR_PTR(-EINVAL);
> > + if (request_nest_parent)
> > + return ERR_PTR(-EINVAL);
> >
> > - /*
> > - * domain_alloc_user op needs to fully initialize a domain
> > - * before return, so uses iommu_domain_alloc() here for
> > - * simple.
> > - */
> > - domain = iommu_domain_alloc(dev->bus);
> > - if (!domain)
> > - domain = ERR_PTR(-ENOMEM);
> > - return domain;
> > + return intel_nested_domain_alloc(parent, user_data);
> > }
> >
> > static void intel_iommu_domain_free(struct iommu_domain *domain)
>
> Best regards,
> baolu
