On 19/05/2023 12:40, Jason Gunthorpe wrote: > On Thu, May 18, 2023 at 09:46:30PM +0100, Joao Martins wrote: >> Add to iommu domain operations a set of callbacks to perform dirty >> tracking, particulary to start and stop tracking and finally to read and >> clear the dirty data. >> >> Drivers are generally expected to dynamically change its translation >> structures to toggle the tracking and flush some form of control state >> structure that stands in the IOVA translation path. Though it's not >> mandatory, as drivers will be enable dirty tracking at boot, and just flush >> the IO pagetables when setting dirty tracking. For each of the newly added >> IOMMU core APIs: >> >> .supported_flags[IOMMU_DOMAIN_F_ENFORCE_DIRTY]: Introduce a set of flags >> that enforce certain restrictions in the iommu_domain object. For dirty >> tracking this means that when IOMMU_DOMAIN_F_ENFORCE_DIRTY is set via its >> helper iommu_domain_set_flags(...) devices attached via attach_dev will >> fail on devices that do *not* have dirty tracking supported. IOMMU drivers >> that support dirty tracking should advertise this flag, while enforcing >> that dirty tracking is supported by the device in its .attach_dev iommu op. >> >> iommu_cap::IOMMU_CAP_DIRTY: new device iommu_capable value when probing for >> capabilities of the device. >> >> .set_dirty_tracking(): an iommu driver is expected to change its >> translation structures and enable dirty tracking for the devices in the >> iommu_domain. For drivers making dirty tracking always-enabled, it should >> just return 0. >> >> .read_and_clear_dirty(): an iommu driver is expected to walk the iova range >> passed in and use iommu_dirty_bitmap_record() to record dirty info per >> IOVA. When detecting a given IOVA is dirty it should also clear its dirty >> state from the PTE, *unless* the flag IOMMU_DIRTY_NO_CLEAR is passed in -- >> flushing is steered from the caller of the domain_op via iotlb_gather. The >> iommu core APIs use the same data structure in use for dirty tracking for >> VFIO device dirty (struct iova_bitmap) abstracted by >> iommu_dirty_bitmap_record() helper function. >> >> Signed-off-by: Joao Martins <joao.m.martins@xxxxxxxxxx> >> --- >> drivers/iommu/iommu.c | 11 +++++++ >> include/linux/io-pgtable.h | 4 +++ >> include/linux/iommu.h | 67 ++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 82 insertions(+) >> >> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c >> index 2088caae5074..95acc543e8fb 100644 >> --- a/drivers/iommu/iommu.c >> +++ b/drivers/iommu/iommu.c >> @@ -2013,6 +2013,17 @@ struct iommu_domain *iommu_domain_alloc(const struct bus_type *bus) >> } >> EXPORT_SYMBOL_GPL(iommu_domain_alloc); >> >> +int iommu_domain_set_flags(struct iommu_domain *domain, >> + const struct bus_type *bus, unsigned long val) >> +{ > > Definately no bus argument. > > The supported_flags should be in the domain op not the bus op. > > But I think this is sort of the wrong direction, the dirty tracking > mode should be requested when the domain is created, not changed after > the fact. In practice it is done as soon after the domain is created but I understand what you mean that both should be together; I have this implemented like that as my first take as a domain_alloc passed flags, but I was a little undecided because we are adding another domain_alloc() op for the user-managed pagetable and after having another one we would end up with 3 ways of creating iommu domain -- but maybe that's not an issue