Re: [RFC PATCH v4 01/13] iommu: Introduce dirty log tracking framework

From: Lu Baolu
Date: Sun May 09 2021 - 21:11:20 EST


Hi Keqian,

On 5/8/21 3:35 PM, Keqian Zhu wrote:
Hi Baolu,

On 2021/5/8 11:46, Lu Baolu wrote:
Hi Keqian,

On 5/7/21 6:21 PM, Keqian Zhu wrote:
Some types of IOMMU are capable of tracking DMA dirty log, such as
ARM SMMU with HTTU or Intel IOMMU with SLADE. This introduces the
dirty log tracking framework in the IOMMU base layer.

Four new essential interfaces are added, and we maintaince the status
of dirty log tracking in iommu_domain.
1. iommu_support_dirty_log: Check whether domain supports dirty log tracking
2. iommu_switch_dirty_log: Perform actions to start|stop dirty log tracking
3. iommu_sync_dirty_log: Sync dirty log from IOMMU into a dirty bitmap
4. iommu_clear_dirty_log: Clear dirty log of IOMMU by a mask bitmap

Note: Don't concurrently call these interfaces with other ops that
access underlying page table.

Signed-off-by: Keqian Zhu<zhukeqian1@xxxxxxxxxx>
Signed-off-by: Kunkun Jiang<jiangkunkun@xxxxxxxxxx>
---
drivers/iommu/iommu.c | 201 +++++++++++++++++++++++++++++++++++
include/linux/iommu.h | 63 +++++++++++
include/trace/events/iommu.h | 63 +++++++++++
3 files changed, 327 insertions(+)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 808ab70d5df5..0d15620d1e90 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1940,6 +1940,7 @@ static struct iommu_domain *__iommu_domain_alloc(struct bus_type *bus,
domain->type = type;
/* Assume all sizes by default; the driver may override this later */
domain->pgsize_bitmap = bus->iommu_ops->pgsize_bitmap;
+ mutex_init(&domain->switch_log_lock);
return domain;
}
@@ -2703,6 +2704,206 @@ int iommu_set_pgtable_quirks(struct iommu_domain *domain,
}
EXPORT_SYMBOL_GPL(iommu_set_pgtable_quirks);
+bool iommu_support_dirty_log(struct iommu_domain *domain)
+{
+ const struct iommu_ops *ops = domain->ops;
+
+ return ops->support_dirty_log && ops->support_dirty_log(domain);
+}
+EXPORT_SYMBOL_GPL(iommu_support_dirty_log);
I suppose this interface is to ask the vendor IOMMU driver to check
whether each device/iommu in the domain supports dirty bit tracking.
But what will happen if new devices with different tracking capability
are added afterward?
Yep, this is considered in the vfio part. We will query again after attaching or
detaching devices from the domain. When the domain becomes capable, we enable
dirty log for it. When it becomes not capable, we disable dirty log for it.

If that's the case, why not putting this logic in the iommu subsystem so
that it doesn't need to be duplicate in different upper layers?

For example, add something like dirty_page_trackable in the struct of
iommu_domain and ask the vendor iommu driver to update it once any
device is added/removed to/from the domain. It's also better to disallow
any domain attach/detach once the dirty page tracking is on.


To make things simple, is it possible to support this tracking only when
all underlying IOMMUs support dirty bit tracking?
IIUC, all underlying IOMMUs you refer is of system wide. I think this idea may has
two issues. 1) The target domain may just contains part of system IOMMUs. 2) The
dirty tracking capability can be related to the capability of devices. For example,
we can track dirty log based on IOPF, which needs the capability of devices. That's
to say, we can make this framework more common.

Yes. Fair enough. Thanks for sharing.


Or, the more crazy idea is that we don't need to check this capability
at all. If dirty bit tracking is not supported by hardware, just mark
all pages dirty?
Yeah, I think this idea is nice:).

Still one concern is that we may have other dirty tracking methods in the future,
if we can't track dirty through iommu, we can still try other methods.

If there is no interface to check this capability, we have no chance to try
other methods. What do you think?


Agreed.

Best regards,
baolu