Re: [PATCH v5 02/12] iommufd: Refactor __fault_domain_replace_dev() to be a wrapper of iommu_replace_group_handle()

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

 



On 2024/11/5 16:01, Yi Liu wrote:
On 2024/11/5 13:06, Baolu Lu wrote:
On 11/4/24 21:25, Yi Liu wrote:
There is a wrapper of iommu_attach_group_handle(), so making a wrapper for iommu_replace_group_handle() for further code refactor. No functional change
intended.

This patch is not a simple, non-functional refactoring. It allocates
attach_handle for all devices in domain attach/replace interfaces,
regardless of whether the domain is iopf-capable. Therefore, the commit
message should be rephrased to accurately reflect the patch's purpose
and rationale.

This patch splits the __fault_domain_replace_dev() a lot, the else branch of the below code was lifted to the iommufd_fault_domain_replace_dev().
While the new __fault_domain_replace_dev() will only be called when the
hwpt->fault is valid. So the iommu_attach_handle is still allocated only
for the iopf-capable path. When the hwpt->fault is invalid, the
iommufd_fault_domain_replace_dev() calls iommu_replace_group_handle() with
a null iommu_attach_handle. What you described is done in the patch 04 of
this series. 🙂

-    if (hwpt->fault) {
-        handle = kzalloc(sizeof(*handle), GFP_KERNEL);
-        if (!handle)
-            return -ENOMEM;
-
-        handle->idev = idev;
-        ret = iommu_replace_group_handle(idev->igroup->group,
-                         hwpt->domain, &handle->handle);
-    } else {
-        ret = iommu_replace_group_handle(idev->igroup->group,
-                         hwpt->domain, NULL);
-    }

Okay, I overlooked that part.

Below change caused me to think that attach handle is always allocated
in this patch no matter ...

-	if (hwpt->fault) {
-		handle = kzalloc(sizeof(*handle), GFP_KERNEL);
-		if (!handle)
-			return -ENOMEM;
-
-		handle->idev = idev;
-		ret = iommu_replace_group_handle(idev->igroup->group,
-						 hwpt->domain, &handle->handle);
-	} else {
-		ret = iommu_replace_group_handle(idev->igroup->group,
-						 hwpt->domain, NULL);
-	}
+	handle = kzalloc(sizeof(*handle), GFP_KERNEL);
+	if (!handle)
+		return -ENOMEM;

If no functional change, please just ignore this comment.

--
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