On 16/10/2023 12:26, Joao Martins wrote: > On 16/10/2023 03:07, Baolu Lu wrote: >> On 9/23/23 9:25 AM, Joao Martins wrote: >>> + >>> + if (!ad_enabled && dirty->bitmap) >>> + return -EINVAL; >> >> I don't understand above check of "dirty->bitmap". Isn't it always >> invalid to call this if dirty tracking is not enabled on the domain? >> > It is spurious (...) > I take this back; >> The iommu_dirty_bitmap is defined in iommu core. The iommu driver has no >> need to understand it and check its member anyway. >> > > (...) The iommu driver has no need to understand it. iommu_dirty_bitmap_record() > already makes those checks in case there's no iova_bitmap to set bits to. > This is all true but the reason I am checking iommu_dirty_bitmap::bitmap is to essentially not record anything in the iova bitmap and just clear the dirty bits from the IOPTEs, all when dirty tracking is technically disabled. This is done internally only when starting dirty tracking, and thus to ensure that we cleanup all dirty bits before we enable dirty tracking to have a consistent snapshot as opposed to inheriting dirties from the past. Some alternative ways to do this: 1) via the iommu_dirty_bitmap structure, where I add one field which if true then iommufd core is able to call into iommu driver on a "clear IOPTE" manner or 2) via the ::flags ... the thing is that ::flags values is UAPI, so it feels weird to use these flags for internal purposes. Joao
