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

Yi Liu
2024-11-05T05:07:30Z

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@linux.intel.com>
>> Signed-off-by: Yi Liu<yi.l.liu@intel.com>
>> ---
>>   drivers/iommu/intel/pasid.c | 173 ++++++++++++++++++++++++++++++++++++
>>   drivers/iommu/intel/pasid.h |  12 +++
>>   2 files changed, 185 insertions(+)
>
> Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>
>
> 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