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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2024/9/30 15:19, Tian, Kevin wrote:
From: Baolu Lu <baolu.lu@xxxxxxxxxxxxxxx>
Sent: Friday, September 13, 2024 10:17 AM

On 9/13/24 9:35 AM, Baolu Lu wrote:
On 9/12/24 9:04 PM, Yi Liu wrote:
+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;


this is the right thing to do, but also suggesting a bug in existing
code. intel_pasid_tear_down_entry() is not just for PRQ drain.
It's also about iotlb/devtlb invalidation. From device p.o.v it
has no idea about the translation mode in the IOMMU side and
always caches the valid mappings in devtlb when ATS is enabled.

Yes. You are right.

intel_pasid_tear_down_entry() takes care of iotlb/devtlb invalidation.
So it's fine as long as intel_pasid_tear_down_entry() is called for the
IDENTITY domain path, right?

Existing code skips all those housekeeping for identify domain
by early return before intel_pasid_tear_down_entry(). We need
a separate fix for it before this series?

Existing code doesn't skip intel_pasid_tear_down_entry().

        if (domain->type == IOMMU_DOMAIN_IDENTITY) {
                intel_pasid_tear_down_entry(iommu, dev, pasid, false);
                return;
        }

Or anything I overlooked?

Thanks,
baolu




[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux