Re: [PATCH v6 05/10] iommufd: Derive iommufd_hwpt_paging from iommufd_hw_pagetable

Jason Gunthorpe
2023-10-24T16:31:39Z

On Tue, Oct 24, 2023 at 08:06:04AM -0700, Yi Liu wrote:

> static int iommufd_group_do_replace_paging(struct iommufd_group *igroup,
> - struct iommufd_hw_pagetable *hwpt)
> + struct iommufd_hwpt_paging *hwpt_paging)
> {
> struct iommufd_hw_pagetable *old_hwpt = igroup->hwpt;
> struct iommufd_device *cur;
> @@ -441,22 +447,23 @@ static int iommufd_group_do_replace_paging(struct iommufd_group *igroup,
>
> lockdep_assert_held(&igroup->lock);
>
> - if (hwpt_is_paging(old_hwpt) && hwpt->ioas != old_hwpt->ioas) {
> + if (hwpt_is_paging(old_hwpt) &&
> + hwpt_paging->ioas != to_hwpt_paging(old_hwpt)->ioas) {
> list_for_each_entry(cur, &igroup->device_list, group_item) {
> rc = iopt_table_enforce_dev_resv_regions(
> - &hwpt->ioas->iopt, cur->dev, NULL);
> + &hwpt_paging->ioas->iopt, cur->dev, NULL);
> if (rc)
> goto err_unresv;
> }
> }
>
> - rc = iommufd_group_setup_msi(igroup, hwpt);
> + rc = iommufd_group_setup_msi(igroup, hwpt_paging);
> if (rc)
> goto err_unresv;
> return 0;
>
> err_unresv:
> - iommufd_group_remove_reserved_iova(igroup, hwpt);
> + iommufd_group_remove_reserved_iova(igroup, hwpt_paging);
> return rc;
> }
>
> @@ -482,7 +489,8 @@ iommufd_device_do_replace(struct iommufd_device *idev,
> }
>
> if (hwpt_is_paging(hwpt)) {
> - rc = iommufd_group_do_replace_paging(igroup, hwpt);
> + rc = iommufd_group_do_replace_paging(igroup,
> + to_hwpt_paging(hwpt));
> if (rc)
> goto err_unlock;
> }
> @@ -493,8 +501,10 @@ iommufd_device_do_replace(struct iommufd_device *idev,
>
> old_hwpt = igroup->hwpt;
> if (hwpt_is_paging(old_hwpt) &&
> - (!hwpt_is_paging(hwpt) || hwpt->ioas != old_hwpt->ioas))
> - iommufd_group_remove_reserved_iova(igroup, old_hwpt);
> + (!hwpt_is_paging(hwpt) ||
> + to_hwpt_paging(hwpt)->ioas != to_hwpt_paging(old_hwpt)->ioas))
> + iommufd_group_remove_reserved_iova(igroup,
> + to_hwpt_paging(old_hwpt));
>
> igroup->hwpt = hwpt;
>
> @@ -513,7 +523,8 @@ iommufd_device_do_replace(struct iommufd_device *idev,
> return old_hwpt;
> err_unresv:
> if (hwpt_is_paging(hwpt))
> - iommufd_group_remove_reserved_iova(igroup, hwpt);
> + iommufd_group_remove_reserved_iova(igroup,
> + to_hwpt_paging(old_hwpt));

This gets a compiler warning:

../drivers/iommu/iommufd/device.c:527:25: warning: variable 'old_hwpt' is uninitialized when used here [-Wuninitialized]
to_hwpt_paging(old_hwpt));
^~~~~~~~
../drivers/iommu/iommufd/device.c:475:39: note: initialize the variable 'old_hwpt' to silence this warning
struct iommufd_hw_pagetable *old_hwpt;
^
= NULL

I fixed it with:

--- a/drivers/iommu/iommufd/device.c
+++ b/drivers/iommu/iommufd/device.c
@@ -488,6 +488,7 @@ iommufd_device_do_replace(struct iommufd_device *idev,
return NULL;
}

+ old_hwpt = igroup->hwpt;
if (hwpt_is_paging(hwpt)) {
rc = iommufd_group_do_replace_paging(igroup,
to_hwpt_paging(hwpt));
@@ -499,7 +500,6 @@ iommufd_device_do_replace(struct iommufd_device *idev,
if (rc)
goto err_unresv;

- old_hwpt = igroup->hwpt;
if (hwpt_is_paging(old_hwpt) &&
(!hwpt_is_paging(hwpt) ||
to_hwpt_paging(hwpt)->ioas != to_hwpt_paging(old_hwpt)->ioas))

Jason