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