[PATCH v3 2/4] iommu/io-pgtable-arm: Add read_and_clear_dirty() support

Joao Martins joao.m.martins at oracle.com
Mon Jun 3 11:50:17 PDT 2024


On 01/06/2024 19:55, Jason Gunthorpe wrote:
> On Mon, May 27, 2024 at 01:21:14AM +0000, Tian, Kevin wrote:
> 
>> emmm there could be multiple domains under a iopt while the dirty
>> tracking toggling must be done per domain.
> 
> For some reason I thought we had made this per ioas, so yeah it is not
> right like I showed.
> 

Something like below instead?

diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index 721ede5a1faf..9756ff18b5f2 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -2645,18 +2645,10 @@ static int amd_iommu_read_and_clear_dirty(struct
iommu_domain *domain,
 {
        struct protection_domain *pdomain = to_pdomain(domain);
        struct io_pgtable_ops *ops = &pdomain->iop.iop.ops;
-       unsigned long lflags;

        if (!ops || !ops->read_and_clear_dirty)
                return -EOPNOTSUPP;

-       spin_lock_irqsave(&pdomain->lock, lflags);
-       if (!pdomain->dirty_tracking && dirty->bitmap) {
-               spin_unlock_irqrestore(&pdomain->lock, lflags);
-               return -EINVAL;
-       }
-       spin_unlock_irqrestore(&pdomain->lock, lflags);
-
        return ops->read_and_clear_dirty(ops, iova, size, flags, dirty);
 }

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index 15411d8d1b3d..6db733f73922 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -4799,15 +4799,6 @@ static int intel_iommu_read_and_clear_dirty(struct
iommu_domain *domain,
        unsigned long end = iova + size - 1;
        unsigned long pgsize;

-       /*
-        * IOMMUFD core calls into a dirty tracking disabled domain without an
-        * IOVA bitmap set in order to clean dirty bits in all PTEs that might
-        * have occurred when we stopped dirty tracking. This ensures that we
-        * never inherit dirtied bits from a previous cycle.
-        */
-       if (!dmar_domain->dirty_tracking && dirty->bitmap)
-               return -EINVAL;
-
        do {
                struct dma_pte *pte;
                int lvl = 0;
diff --git a/drivers/iommu/iommufd/hw_pagetable.c
b/drivers/iommu/iommufd/hw_pagetable.c
index 33d142f8057d..de3064ee849e 100644
--- a/drivers/iommu/iommufd/hw_pagetable.c
+++ b/drivers/iommu/iommufd/hw_pagetable.c
@@ -331,7 +331,9 @@ int iommufd_hwpt_set_dirty_tracking(struct iommufd_ucmd *ucmd)
 {
        struct iommu_hwpt_set_dirty_tracking *cmd = ucmd->cmd;
        struct iommufd_hwpt_paging *hwpt_paging;
-       struct iommufd_ioas *ioas;
+       const struct iommu_dirty_ops *ops;
+       struct iommu_domain *domain;
+       struct io_pagetable *iopt;
        int rc = -EOPNOTSUPP;
        bool enable;

@@ -342,12 +344,35 @@ int iommufd_hwpt_set_dirty_tracking(struct iommufd_ucmd *ucmd)
        if (IS_ERR(hwpt_paging))
                return PTR_ERR(hwpt_paging);

-       ioas = hwpt_paging->ioas;
+       iopt = &hwpt_paging->ioas->iopt;
        enable = cmd->flags & IOMMU_HWPT_DIRTY_TRACKING_ENABLE;
+       domain = hwpt_paging->common.domain;

-       rc = iopt_set_dirty_tracking(&ioas->iopt, hwpt_paging->common.domain,
-                                    enable);
+       ops = domain->dirty_ops;
+       if (!ops) {
+               rc = -EOPNOTSUPP;
+               goto out_unlock;
+       }
+
+       down_write(&iopt->iova_rwsem);
+       if (hwpt_paging->dirty_tracking == enable) {
+               rc = 0;
+               goto out_unlock;
+       }
+
+       /* Clear dirty bits from PTEs to ensure a clean snapshot */
+       if (enable) {
+               rc = iopt_clear_dirty_data(iopt, domain);
+               if (rc)
+                       goto out_unlock;
+       }

+       rc = ops->set_dirty_tracking(domain, enable);
+       if (rc)
+               goto out_unlock;
+       hwpt_paging->dirty_tracking = enable;
+out_unlock:
+       up_write(&iopt->iova_rwsem);
        iommufd_put_object(ucmd->ictx, &hwpt_paging->common.obj);
        return rc;
 }
@@ -356,7 +381,7 @@ int iommufd_hwpt_get_dirty_bitmap(struct iommufd_ucmd *ucmd)
 {
        struct iommu_hwpt_get_dirty_bitmap *cmd = ucmd->cmd;
        struct iommufd_hwpt_paging *hwpt_paging;
-       struct iommufd_ioas *ioas;
+       struct io_pagetable *iopt;
        int rc = -EOPNOTSUPP;

        if ((cmd->flags & ~(IOMMU_HWPT_GET_DIRTY_BITMAP_NO_CLEAR)) ||
@@ -367,10 +392,19 @@ int iommufd_hwpt_get_dirty_bitmap(struct iommufd_ucmd *ucmd)
        if (IS_ERR(hwpt_paging))
                return PTR_ERR(hwpt_paging);

-       ioas = hwpt_paging->ioas;
+       iopt = &hwpt_paging->ioas->iopt;
+
+       down_read(&iopt->iova_rwsem);
+       if (!hwpt_paging->dirty_tracking) {
+               rc = -EINVAL;
+               goto out_put_hwpt;
+       }
+
        rc = iopt_read_and_clear_dirty_data(
-               &ioas->iopt, hwpt_paging->common.domain, cmd->flags, cmd);
+               iopt, hwpt_paging->common.domain, cmd->flags, cmd);

+out_put_hwpt:
+       up_read(&iopt->iova_rwsem);
        iommufd_put_object(ucmd->ictx, &hwpt_paging->common.obj);
        return rc;
 }
diff --git a/drivers/iommu/iommufd/io_pagetable.c
b/drivers/iommu/iommufd/io_pagetable.c
index b40eab6ebe4c..25547d79f294 100644
--- a/drivers/iommu/iommufd/io_pagetable.c
+++ b/drivers/iommu/iommufd/io_pagetable.c
@@ -532,19 +532,17 @@ int iopt_read_and_clear_dirty_data(struct io_pagetable *iopt,
 {
        int ret;

+       lockdep_assert_held_read(&iopt->iova_rwsem);
+
        ret = iommufd_check_iova_range(iopt, bitmap);
        if (ret)
                return ret;

-       down_read(&iopt->iova_rwsem);
-       ret = iommu_read_and_clear_dirty(domain, iopt, flags, bitmap);
-       up_read(&iopt->iova_rwsem);
-
-       return ret;
+       return iommu_read_and_clear_dirty(domain, iopt, flags, bitmap);
 }

-static int iopt_clear_dirty_data(struct io_pagetable *iopt,
-                                struct iommu_domain *domain)
+int iopt_clear_dirty_data(struct io_pagetable *iopt,
+                         struct iommu_domain *domain)
 {
        const struct iommu_dirty_ops *ops = domain->dirty_ops;
        struct iommu_iotlb_gather gather;
@@ -576,31 +574,6 @@ static int iopt_clear_dirty_data(struct io_pagetable *iopt,
        return ret;
 }

-int iopt_set_dirty_tracking(struct io_pagetable *iopt,
-                           struct iommu_domain *domain, bool enable)
-{
-       const struct iommu_dirty_ops *ops = domain->dirty_ops;
-       int ret = 0;
-
-       if (!ops)
-               return -EOPNOTSUPP;
-
-       down_read(&iopt->iova_rwsem);
-
-       /* Clear dirty bits from PTEs to ensure a clean snapshot */
-       if (enable) {
-               ret = iopt_clear_dirty_data(iopt, domain);
-               if (ret)
-                       goto out_unlock;
-       }
-
-       ret = ops->set_dirty_tracking(domain, enable);
-
-out_unlock:
-       up_read(&iopt->iova_rwsem);
-       return ret;
-}
-
 int iopt_get_pages(struct io_pagetable *iopt, unsigned long iova,
                   unsigned long length, struct list_head *pages_list)
 {
diff --git a/drivers/iommu/iommufd/iommufd_private.h
b/drivers/iommu/iommufd/iommufd_private.h
index 991f864d1f9b..9f946df4ca77 100644
--- a/drivers/iommu/iommufd/iommufd_private.h
+++ b/drivers/iommu/iommufd/iommufd_private.h
@@ -78,8 +78,8 @@ int iopt_read_and_clear_dirty_data(struct io_pagetable *iopt,
                                   struct iommu_domain *domain,
                                   unsigned long flags,
                                   struct iommu_hwpt_get_dirty_bitmap *bitmap);
-int iopt_set_dirty_tracking(struct io_pagetable *iopt,
-                           struct iommu_domain *domain, bool enable);
+int iopt_clear_dirty_data(struct io_pagetable *iopt,
+                         struct iommu_domain *domain);

 void iommufd_access_notify_unmap(struct io_pagetable *iopt, unsigned long iova,
                                 unsigned long length);
@@ -301,6 +301,7 @@ struct iommufd_hwpt_paging {
        bool enforce_cache_coherency : 1;
        bool msi_cookie : 1;
        bool nest_parent : 1;
+       bool dirty_tracking : 1;
        /* Head at iommufd_ioas::hwpt_list */
        struct list_head hwpt_item;
 };





More information about the linux-arm-kernel mailing list