On 2024/11/5 16:03, Baolu Lu wrote:
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 ...
aha, because the caller of this chunk would check hwpt->fault first.
- 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.
yep, no functional change is intended in this patch. But do let me know if
you find one. :)
--
Regards,
Yi Liu
