Re: [PATCH v4 04/13] iommu/vt-d: Add pasid replace helpers

[Yi Liu <yi.l.liu@xxxxxxxxx>]

On 2024/11/5 10:06, Baolu Lu wrote:
> On 11/4/24 21:18, Yi Liu wrote:
> > pasid replacement allows converting a present pasid entry to be FS, SS,
> > PT or nested, hence add helpers for such operations. This simplifies the
> > callers as well since the caller can switch the pasid to the new domain
> > by one-shot.
> >
> > Suggested-by: Lu Baolu<baolu.lu@xxxxxxxxxxxxxxx>
> > Signed-off-by: Yi Liu<yi.l.liu@xxxxxxxxx>
> > ---
> >   drivers/iommu/intel/pasid.c | 173 ++++++++++++++++++++++++++++++++++++
> >   drivers/iommu/intel/pasid.h |  12 +++
> >   2 files changed, 185 insertions(+)
>
> Reviewed-by: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>
>
> with a nit below
>
> > diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c
> > index 65fd2fee01b7..b7c2d65b8726 100644
> > --- a/drivers/iommu/intel/pasid.c
> > +++ b/drivers/iommu/intel/pasid.c
> > @@ -390,6 +390,40 @@ int intel_pasid_setup_first_level(struct intel_iommu 
> > *iommu,
> >       return 0;
> >   }
> > +int intel_pasid_replace_first_level(struct intel_iommu *iommu,
> > +                    struct device *dev, pgd_t *pgd,
> > +                    u32 pasid, u16 did, int flags)
> > +{
> > +    struct pasid_entry *pte;
> > +    u16 old_did;
> > +
> > +    if (!ecap_flts(iommu->ecap) ||
> > +        ((flags & PASID_FLAG_FL5LP) && !cap_fl5lp_support(iommu->cap)))
> > +        return -EINVAL;
> > +
> > +    spin_lock(&iommu->lock);
> > +    pte = intel_pasid_get_entry(dev, pasid);
> > +    if (!pte) {
> > +        spin_unlock(&iommu->lock);
> > +        return -ENODEV;
> > +    }
> > +
> > +    if (!pasid_pte_is_present(pte)) {
> > +        spin_unlock(&iommu->lock);
> > +        return -EINVAL;
> > +    }
> > +
> > +    old_did = pasid_get_domain_id(pte);
> > +
> > +    pasid_pte_config_first_level(iommu, pte, pgd, did, flags);
> > +    spin_unlock(&iommu->lock);
> > +
> > +    intel_pasid_flush_present(iommu, dev, pasid, old_did, pte);
> > +    intel_drain_pasid_prq(dev, pasid);
> > +
> > +    return 0;
> > +}
> > +
> >   /*
> >    * Skip top levels of page tables for iommu which has less agaw
> >    * than default. Unnecessary for PT mode.
> > @@ -483,6 +517,55 @@ int intel_pasid_setup_second_level(struct 
> > intel_iommu *iommu,
> >       return 0;
> >   }
> > +int intel_pasid_replace_second_level(struct intel_iommu *iommu,
> > +                     struct dmar_domain *domain,
> > +                     struct device *dev, u32 pasid)
> > +{
> > +    struct pasid_entry *pte;
> > +    struct dma_pte *pgd;
> > +    u16 did, old_did;
> > +    u64 pgd_val;
> > +    int agaw;
> > +
> > +    /*
> > +     * If hardware advertises no support for second level
> > +     * translation, return directly.
> > +     */
> > +    if (!ecap_slts(iommu->ecap))
> > +        return -EINVAL;
> > +
> > +    pgd = domain->pgd;
> > +    agaw = iommu_skip_agaw(domain, iommu, &pgd);
>
> iommu_skip_agaw() has been removed after domain_alloc_paging is
> supported in this driver. Perhaps you need a rebase if you have a new
> version.

yep.

> > +    if (agaw < 0)
> > +        return -EINVAL;
> > +
> > +    pgd_val = virt_to_phys(pgd);
> > +    did = domain_id_iommu(domain, iommu);
> > +
> > +    spin_lock(&iommu->lock);
> > +    pte = intel_pasid_get_entry(dev, pasid);
> > +    if (!pte) {
> > +        spin_unlock(&iommu->lock);
> > +        return -ENODEV;
> > +    }
> > +
> > +    if (!pasid_pte_is_present(pte)) {
> > +        spin_unlock(&iommu->lock);
> > +        return -EINVAL;
> > +    }
> > +
> > +    old_did = pasid_get_domain_id(pte);
> > +
> > +    pasid_pte_config_second_level(iommu, pte, pgd_val, agaw,
> > +                      did, domain->dirty_tracking);
> > +    spin_unlock(&iommu->lock);
> > +
> > +    intel_pasid_flush_present(iommu, dev, pasid, old_did, pte);
> > +    intel_drain_pasid_prq(dev, pasid);
> > +
> > +    return 0;
> > +}
>
> --
> baolu

--
Regards,
Yi Liu