Re: [PATCH v1 0/9] iommufd/iova_bitmap: Bug fixes for IOMMU dirty tracking

[Joao Martins <joao.m.martins@oracle.com>]

On 06/02/2024 18:13, Jason Gunthorpe wrote:
> On Fri, Feb 02, 2024 at 01:34:06PM +0000, Joao Martins wrote:
> 
>> I have a separate set of changes, that proved also instrumental in
>> reproducing some of these corner cases in a more predictable manner but
>> with IOMMU drivers instead, rather than IOMMUFD mock IOMMU driver...
>> without relying on VF specific behaviour. Though, it relies on being
>> able to map IOVAs as dirty. This is not included in this series, as I
>> am not sure yet if such direction would make sense (outside of testing).
>> Comments, appreciated.
> 
> I would love to have a test suite that could drive real HW. I think
> that would make a big improvement to iommu driver quality.
>  

Ideally we would merge the selftests to use real hw domain rather than mock
domain, and skip what's not supported when hardware lacks. Specially as we add
more and more features (IOPF, DPT, nesting) and underlying testing, it seems the
most natural point of consolidation.

Dirty tracking is a little strange because one needs some sort of block/packet
I/O done to hit the path of having something allocated, which usually requires
some driver operating to mark things dirty plus page allocator luck in picking
the right pages. So the straightforward way to manipulate all this without any
of this complexity on real hardware is this is to "map as dirty" and check if
something is missing. It proved useful in these bugs as I couldn't reproduce
this by just using the thing (even sharing the same test as Avihai). In any
case, the gotcha is consuming a new perm bit.

>> The selftests in this series (a large majority of the series changes)
>> are meant to be kept in this order to be more clear in history that the
>> selftests exercise a given fix added earlier in the series.
>> The last patch is an efficiency improvement, but can act as workaround if
>> the IOVA ranges are smaller than 64G, which is why it is placed last.
>>
>> [I think this should be targetted for -rc rather than -next]
> 
> OK, I took it for -rc
> 
> It is bit unfortunate that the number of huge pages needed to run the
> test has increased but it seems necessary to reach all of those
> corners...
> 

One thing that I have here as TODO is to make sure we check if the necessary
amount of hugepages is set and instead of 'scarying' the user with 'test failed'
we SKIP("...") with some reporting of how many are missing for the test.