Re: [PATCH v3 19/19] iommu/intel: Access/Dirty bit support for SL domains

[Jason Gunthorpe <jgg@nvidia.com>]

On Tue, Oct 17, 2023 at 12:22:34PM +0100, Joao Martins wrote:
> On 17/10/2023 03:08, Baolu Lu wrote:
> > On 10/17/23 12:00 AM, Joao Martins wrote:
> >>>> 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.
> > 
> > It's okay since it serves a functional purpose. Can you please add some
> > comments around the code to explain the rationale.
> > 
> 
> I added this comment below:
> 
> +       /*
> +        * IOMMUFD core calls into a dirty tracking disabled domain without an
> +        * IOVA bitmap set in order to clean dirty bits in all PTEs that might
> +        * have occured when we stopped dirty tracking. This ensures that we
> +        * never inherit dirtied bits from a previous cycle.
> +        */
> 
> Also fixed an issue where I could theoretically clear the bit with
> IOMMU_NO_CLEAR. Essentially passed the read_and_clear_dirty flags and let
> dma_sl_pte_test_and_clear_dirty() to test and test-and-clear, similar to AMD:

How does all this work, does this leak into the uapi? Why would we
want to not clear the dirty bits upon enable/disable if dirty
tracking? I can understand that the driver needs help from the caller
due to the externalized locking, but do we leak this into the userspace?

Jason