Re: [PATCH v2 3/6] iommu/vt-d: Make intel_iommu_set_dev_pasid() to handle domain replacement

[Yi Liu <yi.l.liu@intel.com>]

On 2024/9/13 10:17, Baolu Lu wrote:
> On 9/13/24 9:35 AM, Baolu Lu wrote:
>> On 9/12/24 9:04 PM, Yi Liu wrote:
>>> set_dev_pasid op is going to support domain replacement and keep the old
>>> hardware config if it fails. Make the Intel iommu driver be prepared for
>>> it.
>>>
>>> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
>>> ---
>>>   drivers/iommu/intel/iommu.c | 98 ++++++++++++++++++++++++-------------
>>>   1 file changed, 65 insertions(+), 33 deletions(-)
>>>
>>> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
>>> index 80b587de226d..6f5a8e549f3f 100644
>>> --- a/drivers/iommu/intel/iommu.c
>>> +++ b/drivers/iommu/intel/iommu.c
>>> @@ -4248,8 +4248,8 @@ static int intel_iommu_iotlb_sync_map(struct 
>>> iommu_domain *domain,
>>>       return 0;
>>>   }
>>> -static void intel_iommu_remove_dev_pasid(struct device *dev, ioasid_t 
>>> pasid,
>>> -                     struct iommu_domain *domain)
>>> +static void domain_remove_dev_pasid(struct iommu_domain *domain,
>>> +                    struct device *dev, ioasid_t pasid)
>>>   {
>>>       struct device_domain_info *info = dev_iommu_priv_get(dev);
>>>       struct dev_pasid_info *curr, *dev_pasid = NULL;
>>> @@ -4257,11 +4257,6 @@ static void intel_iommu_remove_dev_pasid(struct 
>>> device *dev, ioasid_t pasid,
>>>       struct dmar_domain *dmar_domain;
>>>       unsigned long flags;
>>> -    if (domain->type == IOMMU_DOMAIN_IDENTITY) {
>>> -        intel_pasid_tear_down_entry(iommu, dev, pasid, 0);
>>> -        return;
>>> -    }
>>> -
>>>       dmar_domain = to_dmar_domain(domain);
>>>       spin_lock_irqsave(&dmar_domain->lock, flags);
>>>       list_for_each_entry(curr, &dmar_domain->dev_pasids, link_domain) {
>>> @@ -4278,13 +4273,24 @@ static void intel_iommu_remove_dev_pasid(struct 
>>> device *dev, ioasid_t pasid,
>>>       domain_detach_iommu(dmar_domain, iommu);
>>>       intel_iommu_debugfs_remove_dev_pasid(dev_pasid);
>>>       kfree(dev_pasid);
>>> +}
>>> +
>>> +static void intel_iommu_remove_dev_pasid(struct device *dev, ioasid_t 
>>> pasid,
>>> +                     struct iommu_domain *domain)
>>> +{
>>> +    struct device_domain_info *info = dev_iommu_priv_get(dev);
>>> +    struct intel_iommu *iommu = info->iommu;
>>> +
>>>       intel_pasid_tear_down_entry(iommu, dev, pasid,
>>>                       INTEL_PASID_TEARDOWN_DRAIN_PRQ);
>>> +    if (domain->type == IOMMU_DOMAIN_IDENTITY)
>>> +        return;
>>
>> The static identity domain is not capable of handling page requests.
>> Therefore there is no need to drain PRQ for an identity domain removal.
>>
>> So it probably should be something like this:
>>
>>      if (domain->type == IOMMU_DOMAIN_IDENTITY) {
>>          intel_pasid_tear_down_entry(iommu, dev, pasid, 0);
>>          return;
>>      }
>>
>>      intel_pasid_tear_down_entry(iommu, dev, pasid,
>>                                      INTEL_PASID_TEARDOWN_DRAIN_PRQ);
> 
> Just revisited this. It seems that we just need to drain PRQ if the
> attached domain is iopf-capable. Therefore, how about making it like
> this?
> 
>      unsigned int flags = 0;
> 
>      if (domain->iopf_handler)
>          flags |= INTEL_PASID_TEARDOWN_DRAIN_PRQ;
> 
>      intel_pasid_tear_down_entry(iommu, dev, pasid, flags);
> 
>      /* Identity domain has no meta data for pasid. */
>      if (domain->type == IOMMU_DOMAIN_IDENTITY)
>          return;
> 
got it.

-- 
Regards,
Yi Liu