Re: [PATCH v3 19/19] iommu/intel: Access/Dirty bit support for SL domains

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

 



On 9/23/23 9:25 AM, Joao Martins wrote:
[...]
+static int intel_iommu_set_dirty_tracking(struct iommu_domain *domain,
+					  bool enable)
+{
+	struct dmar_domain *dmar_domain = to_dmar_domain(domain);
+	struct device_domain_info *info;
+	int ret = -EINVAL;
+
+	spin_lock(&dmar_domain->lock);
+	if (!(dmar_domain->dirty_tracking ^ enable) ||

Just out of curiosity, can we simply write

dmar_domain->dirty_tracking == enable

instead? I am not sure whether the compiler will be happy with this.

+	    list_empty(&dmar_domain->devices)) {

list_for_each_entry is no op if list is empty, so no need to check it.

+		spin_unlock(&dmar_domain->lock);
+		return 0;
+	}
+
+	list_for_each_entry(info, &dmar_domain->devices, link) {
+		/* First-level page table always enables dirty bit*/
+		if (dmar_domain->use_first_level) {

Since we leave out domain->use_first_level in the user_domain_alloc
function, we no longer need to check it here.

+			ret = 0;
+			break;
+		}
+
+		ret = intel_pasid_setup_dirty_tracking(info->iommu, info->domain,
+						     info->dev, IOMMU_NO_PASID,
+						     enable);
+		if (ret)
+			break;

We need to unwind to the previous status here. We cannot leave some
devices with status @enable while others do not.

+
+	}

The VT-d driver also support attaching domain to a pasid of a device. We
also need to enable dirty tracking on those devices.

+
+	if (!ret)
+		dmar_domain->dirty_tracking = enable;
+	spin_unlock(&dmar_domain->lock);
+
+	return ret;
+}

I have made some changes to the code based on my above comments. Please
let me know what you think.

static int intel_iommu_set_dirty_tracking(struct iommu_domain *domain,
                                          bool enable)
{
        struct dmar_domain *dmar_domain = to_dmar_domain(domain);
        struct dev_pasid_info *dev_pasid;
        struct device_domain_info *info;
        int ret;

        spin_lock(&dmar_domain->lock);
        if (!(dmar_domain->dirty_tracking ^ enable))
                goto out_unlock;

        list_for_each_entry(info, &dmar_domain->devices, link) {
ret = intel_pasid_setup_dirty_tracking(info->iommu, dmar_domain, info->dev, IOMMU_NO_PASID,
                                                       enable);
                if (ret)
                        goto err_unwind;
        }

list_for_each_entry(dev_pasid, &dmar_domain->dev_pasids, link_domain) {
                info = dev_iommu_priv_get(dev_pasid->dev);
ret = intel_pasid_setup_dirty_tracking(info->iommu, dmar_domain, info->dev, dev_pasid->pasid,
                                                       enable);
                if (ret)
                        goto err_unwind;
        }

        dmar_domain->dirty_tracking = enable;
out_unlock:
        spin_unlock(&dmar_domain->lock);

        return 0;

err_unwind:
        list_for_each_entry(info, &dmar_domain->devices, link)
intel_pasid_setup_dirty_tracking(info->iommu, dmar_domain, info->dev,
                                                 IOMMU_NO_PASID,

dmar_domain->dirty_tracking);
list_for_each_entry(dev_pasid, &dmar_domain->dev_pasids, link_domain) {
                info = dev_iommu_priv_get(dev_pasid->dev);
                intel_pasid_setup_dirty_tracking(info->iommu, dmar_domain,
info->dev, dev_pasid->pasid,

dmar_domain->dirty_tracking);
        }
        spin_unlock(&dmar_domain->lock);

        return ret;
}

Best regards,
baolu



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux