All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] iommufd: Remove iommufd_hw_pagetable_has_group
@ 2023-01-28  2:29 Nicolin Chen
  2023-01-28  2:29 ` [PATCH 1/3] iommufd: Add devices_users to track the hw_pagetable usage by device Nicolin Chen
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Nicolin Chen @ 2023-01-28  2:29 UTC (permalink / raw)
  To: jgg, kevin.tian; +Cc: yi.l.liu, iommu, linux-kernel

The iommufd_hw_pagetable_has_group is not a device-centric API and has
been a bit of a hack. And it needs to keep tracking an attached device
list on the hw_pagetable, and a device lock to protect the device list.

However, the coming domain replacement series can overcomplicate this
list/lock solution, especially to handle nested hw_pagetable use cases.
So, as a preparatory series, remove the device list/lock and also fix
the iommufd_hw_pagetable_has_group hack.

The iommufd_hw_pagetable_has_group() using the device list could be
replaced with a domain-pointer comparison between the hwpt->domain and
iommu_get_domain_for_dev(). The piece of dependency on list_empty() of
the device list can be also replaced with a refcount. Yet, the removal
of the device lock might introduce a race condition, so the ioas mutex
can be moved as an alternative protection.

You can also find this series on Github:
https://github.com/nicolinc/iommufd/commits/remove_iommufd_hw_pagetable_has_group

Thanks
Nicolin Chen

Nicolin Chen (2):
  iommufd/device: Make hwpt_list list_add/del symmetric
  iommufd/device: Change iommufd_hw_pagetable_has_group to device
    centric

Yi Liu (1):
  iommufd: Add devices_users to track the hw_pagetable usage by device

 drivers/iommu/iommufd/device.c          | 72 ++++++++++---------------
 drivers/iommu/iommufd/hw_pagetable.c    | 16 ++++--
 drivers/iommu/iommufd/iommufd_private.h |  3 +-
 3 files changed, 40 insertions(+), 51 deletions(-)

-- 
2.39.1


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 1/3] iommufd: Add devices_users to track the hw_pagetable usage by device
  2023-01-28  2:29 [PATCH 0/3] iommufd: Remove iommufd_hw_pagetable_has_group Nicolin Chen
@ 2023-01-28  2:29 ` Nicolin Chen
  2023-01-28  2:29 ` [PATCH 2/3] iommufd/device: Make hwpt_list list_add/del symmetric Nicolin Chen
  2023-01-28  2:29 ` [PATCH 3/3] iommufd/device: Change iommufd_hw_pagetable_has_group to device centric Nicolin Chen
  2 siblings, 0 replies; 6+ messages in thread
From: Nicolin Chen @ 2023-01-28  2:29 UTC (permalink / raw)
  To: jgg, kevin.tian; +Cc: yi.l.liu, iommu, linux-kernel

From: Yi Liu <yi.l.liu@intel.com>

Currently, hw_pagetable tracks the attached devices using a device list.
When attaching the first device to the kernel-managed hw_pagetable, it
should be linked to IOAS. When detaching the last device from this hwpt,
the link with IOAS should be removed too. And this first-or-last device
check is done with list_empty(hwpt->devices).

However, with a nested configuration, when a device is attached to the
user-managed stage-1 hw_pagetable, it will be added to this user-managed
hwpt's device list instead of the kernel-managed stage-2 hwpt's one. And
this breaks the logic for a kernel-managed hw_pagetable link/disconnect
to/from IOAS/IOPT. e.g. the stage-2 hw_pagetable would be linked to IOAS
multiple times if multiple device is attached, but it will become empty
as soon as one device detached.

Add a devices_users in struct iommufd_hw_pagetable to track the users of
hw_pagetable by the attached devices. Make this field as a pointer, only
allocate for a stage-2 hw_pagetable. A stage-1 hw_pagetable should reuse
the stage-2 hw_pagetable's devices_users, because when a device attaches
to a stage-1 hw_pagetable, linking the stage-2 hwpt to the IOAS is still
required. So, with a nested configuration, increase the devices_users on
the stage-2 (parent) hwpt, no matter a device is attached to the stage-1
or the stage-2 hwpt.

Adding this devices_users also reduces the dependency on the device list,
so it helps the following patch to remove the device list completely.

Signed-off-by: Yi Liu <yi.l.liu@intel.com>
Co-developed-by: Nicolin Chen <nicolinc@nvidia.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/iommufd/device.c          |  8 +++++---
 drivers/iommu/iommufd/hw_pagetable.c    | 11 +++++++++++
 drivers/iommu/iommufd/iommufd_private.h |  1 +
 3 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
index 9f3b9674d72e..208757c39c90 100644
--- a/drivers/iommu/iommufd/device.c
+++ b/drivers/iommu/iommufd/device.c
@@ -212,7 +212,7 @@ static int iommufd_device_do_attach(struct iommufd_device *idev,
 				hwpt->domain->ops->enforce_cache_coherency(
 					hwpt->domain);
 		if (!hwpt->enforce_cache_coherency) {
-			WARN_ON(list_empty(&hwpt->devices));
+			WARN_ON(refcount_read(hwpt->devices_users) == 1);
 			rc = -EINVAL;
 			goto out_unlock;
 		}
@@ -236,7 +236,7 @@ static int iommufd_device_do_attach(struct iommufd_device *idev,
 		if (rc)
 			goto out_iova;
 
-		if (list_empty(&hwpt->devices)) {
+		if (refcount_read(hwpt->devices_users) == 1) {
 			rc = iopt_table_add_domain(&hwpt->ioas->iopt,
 						   hwpt->domain);
 			if (rc)
@@ -246,6 +246,7 @@ static int iommufd_device_do_attach(struct iommufd_device *idev,
 
 	idev->hwpt = hwpt;
 	refcount_inc(&hwpt->obj.users);
+	refcount_inc(hwpt->devices_users);
 	list_add(&idev->devices_item, &hwpt->devices);
 	mutex_unlock(&hwpt->devices_lock);
 	return 0;
@@ -387,9 +388,10 @@ void iommufd_device_detach(struct iommufd_device *idev)
 
 	mutex_lock(&hwpt->ioas->mutex);
 	mutex_lock(&hwpt->devices_lock);
+	refcount_dec(hwpt->devices_users);
 	list_del(&idev->devices_item);
 	if (!iommufd_hw_pagetable_has_group(hwpt, idev->group)) {
-		if (list_empty(&hwpt->devices)) {
+		if (refcount_read(hwpt->devices_users) == 1) {
 			iopt_table_remove_domain(&hwpt->ioas->iopt,
 						 hwpt->domain);
 			list_del(&hwpt->hwpt_item);
diff --git a/drivers/iommu/iommufd/hw_pagetable.c b/drivers/iommu/iommufd/hw_pagetable.c
index 43d473989a06..910e759ffeac 100644
--- a/drivers/iommu/iommufd/hw_pagetable.c
+++ b/drivers/iommu/iommufd/hw_pagetable.c
@@ -16,6 +16,8 @@ void iommufd_hw_pagetable_destroy(struct iommufd_object *obj)
 	iommu_domain_free(hwpt->domain);
 	refcount_dec(&hwpt->ioas->obj.users);
 	mutex_destroy(&hwpt->devices_lock);
+	WARN_ON(!refcount_dec_if_one(hwpt->devices_users));
+	kfree(hwpt->devices_users);
 }
 
 /**
@@ -46,11 +48,20 @@ iommufd_hw_pagetable_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
 	INIT_LIST_HEAD(&hwpt->devices);
 	INIT_LIST_HEAD(&hwpt->hwpt_item);
 	mutex_init(&hwpt->devices_lock);
+	hwpt->devices_users = kzalloc(sizeof(*hwpt->devices_users), GFP_KERNEL);
+	if (!hwpt->devices_users) {
+		rc = -ENOMEM;
+		goto out_free_domain;
+	}
+	refcount_set(hwpt->devices_users, 1);
+
 	/* Pairs with iommufd_hw_pagetable_destroy() */
 	refcount_inc(&ioas->obj.users);
 	hwpt->ioas = ioas;
 	return hwpt;
 
+out_free_domain:
+	iommu_domain_free(hwpt->domain);
 out_abort:
 	iommufd_object_abort(ictx, &hwpt->obj);
 	return ERR_PTR(rc);
diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h
index 222e86591f8a..f128d77fb076 100644
--- a/drivers/iommu/iommufd/iommufd_private.h
+++ b/drivers/iommu/iommufd/iommufd_private.h
@@ -247,6 +247,7 @@ struct iommufd_hw_pagetable {
 	/* Head at iommufd_ioas::hwpt_list */
 	struct list_head hwpt_item;
 	struct mutex devices_lock;
+	refcount_t *devices_users;
 	struct list_head devices;
 };
 
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 2/3] iommufd/device: Make hwpt_list list_add/del symmetric
  2023-01-28  2:29 [PATCH 0/3] iommufd: Remove iommufd_hw_pagetable_has_group Nicolin Chen
  2023-01-28  2:29 ` [PATCH 1/3] iommufd: Add devices_users to track the hw_pagetable usage by device Nicolin Chen
@ 2023-01-28  2:29 ` Nicolin Chen
  2023-01-28 11:52   ` kernel test robot
  2023-01-28  2:29 ` [PATCH 3/3] iommufd/device: Change iommufd_hw_pagetable_has_group to device centric Nicolin Chen
  2 siblings, 1 reply; 6+ messages in thread
From: Nicolin Chen @ 2023-01-28  2:29 UTC (permalink / raw)
  To: jgg, kevin.tian; +Cc: yi.l.liu, iommu, linux-kernel

Since the list_del() of hwpt_item is done in iommufd_device_detach(), move
its list_add_tail() to a similar place in iommufd_device_do_attach().

Also move and place the mutex outside the iommufd_device_auto_get_domain()
and iommufd_device_do_attach() calls, to serialize attach/detach routines.
This adds an additional locking protection so that the following patch can
safely remove devices_lock.

Co-developed-by: Yi Liu <yi.l.liu@intel.com>
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/iommufd/device.c | 26 ++++++++++++--------------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
index 208757c39c90..0248073e3169 100644
--- a/drivers/iommu/iommufd/device.c
+++ b/drivers/iommu/iommufd/device.c
@@ -198,6 +198,8 @@ static int iommufd_device_do_attach(struct iommufd_device *idev,
 	phys_addr_t sw_msi_start = PHYS_ADDR_MAX;
 	int rc;
 
+	lockdep_assert_held(&hwpt->ioas->mutex);
+
 	mutex_lock(&hwpt->devices_lock);
 
 	/*
@@ -241,6 +243,7 @@ static int iommufd_device_do_attach(struct iommufd_device *idev,
 						   hwpt->domain);
 			if (rc)
 				goto out_detach;
+			list_add_tail(&hwpt->hwpt_item, &hwpt->ioas->hwpt_list);
 		}
 	}
 
@@ -271,12 +274,13 @@ static int iommufd_device_auto_get_domain(struct iommufd_device *idev,
 	struct iommufd_hw_pagetable *hwpt;
 	int rc;
 
+	lockdep_assert_held(&hwpt->ioas->mutex);
+
 	/*
 	 * There is no differentiation when domains are allocated, so any domain
 	 * that is willing to attach to the device is interchangeable with any
 	 * other.
 	 */
-	mutex_lock(&ioas->mutex);
 	list_for_each_entry(hwpt, &ioas->hwpt_list, hwpt_item) {
 		if (!hwpt->auto_domain)
 			continue;
@@ -290,29 +294,23 @@ static int iommufd_device_auto_get_domain(struct iommufd_device *idev,
 		 */
 		if (rc == -EINVAL)
 			continue;
-		goto out_unlock;
+		return rc;
 	}
 
 	hwpt = iommufd_hw_pagetable_alloc(idev->ictx, ioas, idev->dev);
-	if (IS_ERR(hwpt)) {
-		rc = PTR_ERR(hwpt);
-		goto out_unlock;
-	}
+	if (IS_ERR(hwpt))
+		return PTR_ERR(hwpt);
 	hwpt->auto_domain = true;
 
 	rc = iommufd_device_do_attach(idev, hwpt);
 	if (rc)
 		goto out_abort;
-	list_add_tail(&hwpt->hwpt_item, &ioas->hwpt_list);
 
-	mutex_unlock(&ioas->mutex);
 	iommufd_object_finalize(idev->ictx, &hwpt->obj);
 	return 0;
 
 out_abort:
 	iommufd_object_abort_and_destroy(idev->ictx, &hwpt->obj);
-out_unlock:
-	mutex_unlock(&ioas->mutex);
 	return rc;
 }
 
@@ -342,20 +340,20 @@ int iommufd_device_attach(struct iommufd_device *idev, u32 *pt_id)
 		struct iommufd_hw_pagetable *hwpt =
 			container_of(pt_obj, struct iommufd_hw_pagetable, obj);
 
+		mutex_lock(&hwpt->ioas->mutex);
 		rc = iommufd_device_do_attach(idev, hwpt);
+		mutex_unlock(&hwpt->ioas->mutex);
 		if (rc)
 			goto out_put_pt_obj;
-
-		mutex_lock(&hwpt->ioas->mutex);
-		list_add_tail(&hwpt->hwpt_item, &hwpt->ioas->hwpt_list);
-		mutex_unlock(&hwpt->ioas->mutex);
 		break;
 	}
 	case IOMMUFD_OBJ_IOAS: {
 		struct iommufd_ioas *ioas =
 			container_of(pt_obj, struct iommufd_ioas, obj);
 
+		mutex_lock(&ioas->mutex);
 		rc = iommufd_device_auto_get_domain(idev, ioas);
+		mutex_unlock(&ioas->mutex);
 		if (rc)
 			goto out_put_pt_obj;
 		break;
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 3/3] iommufd/device: Change iommufd_hw_pagetable_has_group to device centric
  2023-01-28  2:29 [PATCH 0/3] iommufd: Remove iommufd_hw_pagetable_has_group Nicolin Chen
  2023-01-28  2:29 ` [PATCH 1/3] iommufd: Add devices_users to track the hw_pagetable usage by device Nicolin Chen
  2023-01-28  2:29 ` [PATCH 2/3] iommufd/device: Make hwpt_list list_add/del symmetric Nicolin Chen
@ 2023-01-28  2:29 ` Nicolin Chen
  2 siblings, 0 replies; 6+ messages in thread
From: Nicolin Chen @ 2023-01-28  2:29 UTC (permalink / raw)
  To: jgg, kevin.tian; +Cc: yi.l.liu, iommu, linux-kernel

The iommufd_hw_pagetable_has_group() is a little hack to check whether
a hw_pagetable has an idev->group attached. This isn't device-centric
firstly, but also requires the hw_pagetable to maintain a device list
with a devices_lock, which gets overcomplicated among the routines for
different use cases, upcoming nested hwpts especially.

Since we can compare the iommu_group->domain and the hwpt->domain to
serve the same purpose while being device centric, change it and drop
the device list with the devices_lock accordingly.

Note that, in the detach routine, it previously checked !has_group as
there was a list_del on the device list. But now, the device is still
being attached to the hwpt, so the if logic gets inverted.

Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
---
 drivers/iommu/iommufd/device.c          | 40 ++++++++-----------------
 drivers/iommu/iommufd/hw_pagetable.c    |  5 ----
 drivers/iommu/iommufd/iommufd_private.h |  2 --
 3 files changed, 12 insertions(+), 35 deletions(-)

diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
index 0248073e3169..5a5dfc0c4ca0 100644
--- a/drivers/iommu/iommufd/device.c
+++ b/drivers/iommu/iommufd/device.c
@@ -24,8 +24,6 @@ struct iommufd_device {
 	struct iommufd_object obj;
 	struct iommufd_ctx *ictx;
 	struct iommufd_hw_pagetable *hwpt;
-	/* Head at iommufd_hw_pagetable::devices */
-	struct list_head devices_item;
 	/* always the physical device */
 	struct device *dev;
 	struct iommu_group *group;
@@ -181,15 +179,15 @@ static int iommufd_device_setup_msi(struct iommufd_device *idev,
 	return 0;
 }
 
-static bool iommufd_hw_pagetable_has_group(struct iommufd_hw_pagetable *hwpt,
-					   struct iommu_group *group)
+static bool iommufd_hw_pagetable_has_device(struct iommufd_hw_pagetable *hwpt,
+					    struct device *dev)
 {
-	struct iommufd_device *cur_dev;
-
-	list_for_each_entry(cur_dev, &hwpt->devices, devices_item)
-		if (cur_dev->group == group)
-			return true;
-	return false;
+	/*
+	 * iommu_get_domain_for_dev() returns an iommu_group->domain ptr, if it
+	 * is the same domain as the hwpt->domain, it means that this hwpt has
+	 * the iommu_group/device.
+	 */
+	return hwpt->domain == iommu_get_domain_for_dev(dev);
 }
 
 static int iommufd_device_do_attach(struct iommufd_device *idev,
@@ -200,8 +198,6 @@ static int iommufd_device_do_attach(struct iommufd_device *idev,
 
 	lockdep_assert_held(&hwpt->ioas->mutex);
 
-	mutex_lock(&hwpt->devices_lock);
-
 	/*
 	 * Try to upgrade the domain we have, it is an iommu driver bug to
 	 * report IOMMU_CAP_ENFORCE_CACHE_COHERENCY but fail
@@ -215,25 +211,20 @@ static int iommufd_device_do_attach(struct iommufd_device *idev,
 					hwpt->domain);
 		if (!hwpt->enforce_cache_coherency) {
 			WARN_ON(refcount_read(hwpt->devices_users) == 1);
-			rc = -EINVAL;
-			goto out_unlock;
+			return -EINVAL;
 		}
 	}
 
 	rc = iopt_table_enforce_group_resv_regions(&hwpt->ioas->iopt, idev->dev,
 						   idev->group, &sw_msi_start);
 	if (rc)
-		goto out_unlock;
+		return rc;
 
 	rc = iommufd_device_setup_msi(idev, hwpt, sw_msi_start);
 	if (rc)
 		goto out_iova;
 
-	/*
-	 * FIXME: Hack around missing a device-centric iommu api, only attach to
-	 * the group once for the first device that is in the group.
-	 */
-	if (!iommufd_hw_pagetable_has_group(hwpt, idev->group)) {
+	if (!iommufd_hw_pagetable_has_device(hwpt, idev->dev)) {
 		rc = iommu_attach_group(hwpt->domain, idev->group);
 		if (rc)
 			goto out_iova;
@@ -250,16 +241,12 @@ static int iommufd_device_do_attach(struct iommufd_device *idev,
 	idev->hwpt = hwpt;
 	refcount_inc(&hwpt->obj.users);
 	refcount_inc(hwpt->devices_users);
-	list_add(&idev->devices_item, &hwpt->devices);
-	mutex_unlock(&hwpt->devices_lock);
 	return 0;
 
 out_detach:
 	iommu_detach_group(hwpt->domain, idev->group);
 out_iova:
 	iopt_remove_reserved_iova(&hwpt->ioas->iopt, idev->dev);
-out_unlock:
-	mutex_unlock(&hwpt->devices_lock);
 	return rc;
 }
 
@@ -385,10 +372,8 @@ void iommufd_device_detach(struct iommufd_device *idev)
 	struct iommufd_hw_pagetable *hwpt = idev->hwpt;
 
 	mutex_lock(&hwpt->ioas->mutex);
-	mutex_lock(&hwpt->devices_lock);
 	refcount_dec(hwpt->devices_users);
-	list_del(&idev->devices_item);
-	if (!iommufd_hw_pagetable_has_group(hwpt, idev->group)) {
+	if (iommufd_hw_pagetable_has_device(hwpt, idev->dev)) {
 		if (refcount_read(hwpt->devices_users) == 1) {
 			iopt_table_remove_domain(&hwpt->ioas->iopt,
 						 hwpt->domain);
@@ -397,7 +382,6 @@ void iommufd_device_detach(struct iommufd_device *idev)
 		iommu_detach_group(hwpt->domain, idev->group);
 	}
 	iopt_remove_reserved_iova(&hwpt->ioas->iopt, idev->dev);
-	mutex_unlock(&hwpt->devices_lock);
 	mutex_unlock(&hwpt->ioas->mutex);
 
 	if (hwpt->auto_domain)
diff --git a/drivers/iommu/iommufd/hw_pagetable.c b/drivers/iommu/iommufd/hw_pagetable.c
index 910e759ffeac..868a126ff37d 100644
--- a/drivers/iommu/iommufd/hw_pagetable.c
+++ b/drivers/iommu/iommufd/hw_pagetable.c
@@ -11,11 +11,8 @@ void iommufd_hw_pagetable_destroy(struct iommufd_object *obj)
 	struct iommufd_hw_pagetable *hwpt =
 		container_of(obj, struct iommufd_hw_pagetable, obj);
 
-	WARN_ON(!list_empty(&hwpt->devices));
-
 	iommu_domain_free(hwpt->domain);
 	refcount_dec(&hwpt->ioas->obj.users);
-	mutex_destroy(&hwpt->devices_lock);
 	WARN_ON(!refcount_dec_if_one(hwpt->devices_users));
 	kfree(hwpt->devices_users);
 }
@@ -45,9 +42,7 @@ iommufd_hw_pagetable_alloc(struct iommufd_ctx *ictx, struct iommufd_ioas *ioas,
 		goto out_abort;
 	}
 
-	INIT_LIST_HEAD(&hwpt->devices);
 	INIT_LIST_HEAD(&hwpt->hwpt_item);
-	mutex_init(&hwpt->devices_lock);
 	hwpt->devices_users = kzalloc(sizeof(*hwpt->devices_users), GFP_KERNEL);
 	if (!hwpt->devices_users) {
 		rc = -ENOMEM;
diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h
index f128d77fb076..1c8e59b37f46 100644
--- a/drivers/iommu/iommufd/iommufd_private.h
+++ b/drivers/iommu/iommufd/iommufd_private.h
@@ -246,9 +246,7 @@ struct iommufd_hw_pagetable {
 	bool msi_cookie : 1;
 	/* Head at iommufd_ioas::hwpt_list */
 	struct list_head hwpt_item;
-	struct mutex devices_lock;
 	refcount_t *devices_users;
-	struct list_head devices;
 };
 
 struct iommufd_hw_pagetable *
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] iommufd/device: Make hwpt_list list_add/del symmetric
  2023-01-28  2:29 ` [PATCH 2/3] iommufd/device: Make hwpt_list list_add/del symmetric Nicolin Chen
@ 2023-01-28 11:52   ` kernel test robot
  2023-01-28 20:27     ` Nicolin Chen
  0 siblings, 1 reply; 6+ messages in thread
From: kernel test robot @ 2023-01-28 11:52 UTC (permalink / raw)
  To: Nicolin Chen, jgg, kevin.tian
  Cc: llvm, oe-kbuild-all, yi.l.liu, iommu, linux-kernel

Hi Nicolin,

I love your patch! Perhaps something to improve:

[auto build test WARNING on v6.2-rc5]
[also build test WARNING on linus/master next-20230127]
[cannot apply to joro-iommu/next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Nicolin-Chen/iommufd-Add-devices_users-to-track-the-hw_pagetable-usage-by-device/20230128-150429
patch link:    https://lore.kernel.org/r/25b3b85b03fc2a7968c476b0533f451acecdfd13.1674872884.git.nicolinc%40nvidia.com
patch subject: [PATCH 2/3] iommufd/device: Make hwpt_list list_add/del symmetric
config: x86_64-randconfig-a011-20230123 (https://download.01.org/0day-ci/archive/20230128/202301281943.Jdvwci2p-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/4d48fdd86375f6d888bbcf830a10c48720008955
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Nicolin-Chen/iommufd-Add-devices_users-to-track-the-hw_pagetable-usage-by-device/20230128-150429
        git checkout 4d48fdd86375f6d888bbcf830a10c48720008955
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/iommu/iommufd/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/iommu/iommufd/device.c:279:23: warning: variable 'hwpt' is uninitialized when used here [-Wuninitialized]
           lockdep_assert_held(&hwpt->ioas->mutex);
                                ^~~~
   include/linux/lockdep.h:319:33: note: expanded from macro 'lockdep_assert_held'
           lockdep_assert(lockdep_is_held(l) != LOCK_STATE_NOT_HELD)
                                          ^
   include/linux/lockdep.h:286:47: note: expanded from macro 'lockdep_is_held'
   #define lockdep_is_held(lock)           lock_is_held(&(lock)->dep_map)
                                                          ^~~~
   include/linux/lockdep.h:313:32: note: expanded from macro 'lockdep_assert'
           do { WARN_ON(debug_locks && !(cond)); } while (0)
                                         ^~~~
   include/asm-generic/bug.h:122:25: note: expanded from macro 'WARN_ON'
           int __ret_warn_on = !!(condition);                              \
                                  ^~~~~~~~~
   drivers/iommu/iommufd/device.c:276:35: note: initialize the variable 'hwpt' to silence this warning
           struct iommufd_hw_pagetable *hwpt;
                                            ^
                                             = NULL
   1 warning generated.


vim +/hwpt +279 drivers/iommu/iommufd/device.c

   267	
   268	/*
   269	 * When automatically managing the domains we search for a compatible domain in
   270	 * the iopt and if one is found use it, otherwise create a new domain.
   271	 * Automatic domain selection will never pick a manually created domain.
   272	 */
   273	static int iommufd_device_auto_get_domain(struct iommufd_device *idev,
   274						  struct iommufd_ioas *ioas)
   275	{
   276		struct iommufd_hw_pagetable *hwpt;
   277		int rc;
   278	
 > 279		lockdep_assert_held(&hwpt->ioas->mutex);
   280	
   281		/*
   282		 * There is no differentiation when domains are allocated, so any domain
   283		 * that is willing to attach to the device is interchangeable with any
   284		 * other.
   285		 */
   286		list_for_each_entry(hwpt, &ioas->hwpt_list, hwpt_item) {
   287			if (!hwpt->auto_domain)
   288				continue;
   289	
   290			rc = iommufd_device_do_attach(idev, hwpt);
   291	
   292			/*
   293			 * -EINVAL means the domain is incompatible with the device.
   294			 * Other error codes should propagate to userspace as failure.
   295			 * Success means the domain is attached.
   296			 */
   297			if (rc == -EINVAL)
   298				continue;
   299			return rc;
   300		}
   301	
   302		hwpt = iommufd_hw_pagetable_alloc(idev->ictx, ioas, idev->dev);
   303		if (IS_ERR(hwpt))
   304			return PTR_ERR(hwpt);
   305		hwpt->auto_domain = true;
   306	
   307		rc = iommufd_device_do_attach(idev, hwpt);
   308		if (rc)
   309			goto out_abort;
   310	
   311		iommufd_object_finalize(idev->ictx, &hwpt->obj);
   312		return 0;
   313	
   314	out_abort:
   315		iommufd_object_abort_and_destroy(idev->ictx, &hwpt->obj);
   316		return rc;
   317	}
   318	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] iommufd/device: Make hwpt_list list_add/del symmetric
  2023-01-28 11:52   ` kernel test robot
@ 2023-01-28 20:27     ` Nicolin Chen
  0 siblings, 0 replies; 6+ messages in thread
From: Nicolin Chen @ 2023-01-28 20:27 UTC (permalink / raw)
  To: jgg, kevin.tian, kernel test robot
  Cc: llvm, oe-kbuild-all, yi.l.liu, iommu, linux-kernel

On Sat, Jan 28, 2023 at 07:52:54PM +0800, kernel test robot wrote:

> >> drivers/iommu/iommufd/device.c:279:23: warning: variable 'hwpt' is uninitialized when used here [-Wuninitialized]
>            lockdep_assert_held(&hwpt->ioas->mutex);
>                                 ^~~~

A mistake here...will fix with a v2:
            lockdep_assert_held(&ioas->mutex);

Thanks
Nicolin

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-01-28 20:27 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-28  2:29 [PATCH 0/3] iommufd: Remove iommufd_hw_pagetable_has_group Nicolin Chen
2023-01-28  2:29 ` [PATCH 1/3] iommufd: Add devices_users to track the hw_pagetable usage by device Nicolin Chen
2023-01-28  2:29 ` [PATCH 2/3] iommufd/device: Make hwpt_list list_add/del symmetric Nicolin Chen
2023-01-28 11:52   ` kernel test robot
2023-01-28 20:27     ` Nicolin Chen
2023-01-28  2:29 ` [PATCH 3/3] iommufd/device: Change iommufd_hw_pagetable_has_group to device centric Nicolin Chen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.