Re: [PATCH v3 03/19] iommu: Add iommu_domain ops for dirty tracking

Joao Martins
2023-10-13T16:30:17Z

On 13/10/2023 17:05, Jason Gunthorpe wrote:
> On Sat, Sep 23, 2023 at 02:24:55AM +0100, Joao Martins wrote:
>
>> +/**
>> + * struct iommu_dirty_bitmap - Dirty IOVA bitmap state
>> + * @bitmap: IOVA bitmap
>> + * @gather: Range information for a pending IOTLB flush
>> + */
>> +struct iommu_dirty_bitmap {
>> + struct iova_bitmap *bitmap;
>> + struct iommu_iotlb_gather *gather;
>> +};
>
> Why the struct ?
>

...

>> +
>> +/**
>> + * struct iommu_dirty_ops - domain specific dirty tracking operations
>> + * @set_dirty_tracking: Enable or Disable dirty tracking on the iommu domain
>> + * @read_and_clear_dirty: Walk IOMMU page tables for dirtied PTEs marshalled
>> + * into a bitmap, with a bit represented as a page.
>> + * Reads the dirty PTE bits and clears it from IO
>> + * pagetables.
>> + */
>> +struct iommu_dirty_ops {
>> + int (*set_dirty_tracking)(struct iommu_domain *domain, bool enabled);
>> + int (*read_and_clear_dirty)(struct iommu_domain *domain,
>> + unsigned long iova, size_t size,
>> + unsigned long flags,
>> + struct iommu_dirty_bitmap *dirty);
>> +};
>
> vs 1 more parameter here?
>
.. I was just trying to avoid one parameter, and I wanted to abstract the
iotlb_gather from the iommu driver, to simplify it for the iommu driver.

But honestly I was quite undecided with 5 args vs 6 args sounded like stretching
to the max, and then the other simplification sort of made sense to consolidate.
Is there one you go at preferably?

> vs putting more stuff in the struct?
>
This I sort of disagree as iova-bitmap has no knowledge of IOTLB -- it's
serializing bits into the bitmap

> Jason