On 2/28/22 13:01, Joao Martins wrote: > On 2/25/22 20:44, Jason Gunthorpe wrote: >> On Fri, Feb 25, 2022 at 07:18:37PM +0000, Joao Martins wrote: >>> On 2/23/22 01:03, Jason Gunthorpe wrote: >>>> On Tue, Feb 22, 2022 at 11:55:55AM +0000, Joao Martins wrote: >>> I'll be simplifying the interface in the other type1 series I had and making it >>> a simple iommu_set_feature(domain, flag, value) behind an ioctl for iommufd that can >>> enable/disable over a domain. Perhaps same trick could be expanded to other >>> features to have a bit more control on what userspace is allowed to use. I think >>> this just needs to set/clear a feature bit in the domain, for VFIO or userspace >>> to have full control during the different stages of migration of dirty tracking. >>> In all of the IOMMU implementations/manuals I read it means setting a protection >>> domain descriptor flag: AMD is a 2-bit field in the DTE, on Intel likewise but on >>> the PASID table only for scalable-mode PTEs, on SMMUv3.2 there's an equivalent >>> (albeit past work had also it always-on). >>> >>> Provided the iommufd does /separately/ more finer granularity on what we can >>> do with page tables. Thus the VMM can demote/promote the ioptes to a lower page size >>> at will as separate operations, before and after migration respectivally. That logic >>> would probably be better to be in separate iommufd ioctls(), as that it's going to be >>> expensive. >> >> This all sounds right to me >> >> Questions I have: >> - Do we need ranges for some reason? You mentioned ARM SMMU wants >> ranges? how/what/why? >> > Ignore that. I got mislead by the implementation and when I read the SDM > I realized that the implementation was doing the same thing I was doing > i.e. enabling dirty-bit in the protection domain at start rather than > dynamic toggling. So ARM is similar to Intel/AMD in which you set CD.HD > bit in the context descriptor to enable dirty bits or the STE.S2HD in the > stream table entry for the stage2 equivalent. Nothing here is per-range > basis. And the ranges was only used by the implementation for the eager > splitting/merging of IO page table levels. > >> - What about the unmap and read dirty without races operation that >> vfio has? >> > I am afraid that might need a new unmap iommu op that reads the dirty bit > after clearing the page table entry. It's marshalling the bits from > iopte into a bitmap as opposed to some logic added on top. As far as I > looked for AMD this isn't difficult to add, (same for Intel) it can > *I think* reuse all of the unmap code. > OK, made some progress. It's a WIP (here be dragons!) and still missing things e.g. iommufd selftests, revising locking, bugs, and more -- works with my emulated qemu patches which is a good sign. But hopefully starts some sort of skeleton of what we were talking about in the above thread. The bigger TODO, though is the equivalent UAPI for IOMMUFD; I started with the vfio-compat one as it was easier provided there's existing userspace to work with (Qemu). To be fair the API is not that "far" from what would be IOMMUFD onto steering the dirty tracking, read-clear the dirty bits, unmap and get dirty. But as we discussed earlier, the one gap of VFIO dirty API is that it lacks controls for upgrading/downgrading area/IOPTE sizes which is where IOMMUFD would most likely shine. When that latter part is done we can probably adopt an eager-split approach inside vfio-compat. Additionally I also sort of want to skeleton ARM and Intel to see how it looks. Some of the commits made notes of some of research I made, so *I think* the APIs introduced capture h/w semantics for all the three IOMMUs supporting dirty tracking. I am storing my dirty-tracking bits here: https://github.com/jpemartins/linux iommufd It's a version of it rebased on top of your iommufd branch.
