Messages in this thread | | | Date | Fri, 14 Jul 2023 13:47:18 +0800 | Subject | Re: [PATCH v10 5/7] iommu/vt-d: Make prq draining code generic | From | Baolu Lu <> |
| |
On 2023/7/14 11:49, Tian, Kevin wrote: >> From: Baolu Lu <baolu.lu@linux.intel.com> >> Sent: Friday, July 14, 2023 11:28 AM >> >> On 2023/7/13 15:49, Tian, Kevin wrote: >>>> From: Jacob Pan <jacob.jun.pan@linux.intel.com> >>>> Sent: Thursday, July 13, 2023 12:34 AM >>>> >>>> - /* Domain type specific cleanup: */ >>>> domain = iommu_get_domain_for_dev_pasid(dev, pasid, 0); >>>> - if (domain) { >>>> - switch (domain->type) { >>>> - case IOMMU_DOMAIN_SVA: >>>> - intel_svm_remove_dev_pasid(dev, pasid); >>>> - break; >>>> - default: >>>> - /* should never reach here */ >>>> - WARN_ON(1); >>>> - break; >>>> - } >>>> + if (!domain) >>>> + goto out_tear_down; >>> >>> WARN_ON() >> >> Why? >> >> My understanding is that remve_device_pasid could be call in any context >> including no domain attached. >> > > oh I'm not aware of that. Can you elaborate the usage which uses a pasid > w/o domain? pasid needs to point to a page table. Presumably every > page table should be wrapped by a iommu domain...
A case I can think of is error rewinding. A domain is being attached to multiple pasids. When one of them is failed, remove_device_pasid should be called on all pasids so that they are parked at a determinant state.
On the other hand, I don't want the remove_device_pasid to be the counterpart of attach_dev_pasid. remove_device_pasid simply denotes:
- The pasid will be parked in blocking state; - If any domain that has been attached to this pasid, stop reference to it any more. Otherwise, there might be use-after-free issues.
Hence, remove_device_pasid should never fail.
Best regards, baolu
| |