Hi Kevin,

On 2022/3/29 16:42, Tian, Kevin wrote:
From: Lu Baolu <baolu...@linux.intel.com>
Sent: Tuesday, March 29, 2022 1:38 PM

Some of the interfaces in the IOMMU core require that only a single
kernel device driver controls the device in the IOMMU group. The
existing method is to check the device count in the IOMMU group in
the interfaces. This is unreliable because any device added to the
IOMMU group later breaks this assumption without notifying the driver
using the interface. This adds iommu_group_singleton_lockdown() that
checks the requirement and locks down the IOMMU group with only single
device driver bound.

Signed-off-by: Lu Baolu <baolu...@linux.intel.com>
---
  drivers/iommu/iommu.c | 30 ++++++++++++++++++------------
  1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 0c42ece25854..9fb8a5b4491e 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -48,6 +48,7 @@ struct iommu_group {
        struct list_head entry;
        unsigned int owner_cnt;
        void *owner;
+       bool singleton_lockdown;
  };

  struct group_device {
@@ -968,15 +969,16 @@ void iommu_group_remove_device(struct device
*dev)
  }
  EXPORT_SYMBOL_GPL(iommu_group_remove_device);

-static int iommu_group_device_count(struct iommu_group *group)
+/* Callers should hold the group->mutex. */
+static bool iommu_group_singleton_lockdown(struct iommu_group *group)
  {
-       struct group_device *entry;
-       int ret = 0;
-
-       list_for_each_entry(entry, &group->devices, list)
-               ret++;
+       if (group->owner_cnt != 1 ||
+           group->domain != group->default_domain ||
+           group->owner)
+               return false;

Curious why there will be a case where group uses default_domain
while still having a owner? I have the impression that owner is used
for userspace DMA where a different domain is used.

You are right. The default domain is automatically detached when a user
is claimed. As long as a user is claimed, the group could only use an
empty or user-specified domain.


+       group->singleton_lockdown = true;

-       return ret;
+       return true;
  }

btw I'm not sure whether this is what SVA requires. IIRC the problem with
SVA is because PASID TLP prefix is not counted in PCI packet routing thus
a DMA target address with PASID might be treated as P2P if the address
falls into the MMIO BAR of other devices in the group. This is why the
original code needs to strictly apply SVA in a group containing a single
device, instead of a group attached by a single driver, unless we want to
reserve those MMIO ranges in CPU VA space.

You are right. But I don't think the IOMMU core is able to guarantee
above in a platform/device-agnostic way. Or any suggestions?

I guess this should be somewhat off-loaded to the device driver which
knows details of the device. The device driver should know this and
guarantee it before calling
iommu_dev_enable_feature(dev, IOMMU_DEV_FEAT_SVA).

This patch itself just replaces the existing
"iommu_group_device_count(group) != 1" logic with a new one based on the
group ownership logistics. The former is obviously not friendly to
device hot joined afterward.


Jean can correct me if my memory is wrong.

Thanks
Kevin

Best regards,
baolu
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to