Re: [PATCH] iommu/iova: Sort out rbtree limit_pfn handling

[Aaron Sierra]

----- Original Message -----
> From: "Robin Murphy" <robin.murphy@xxxxxxx>
> Sent: Tuesday, May 16, 2017 6:26:48 AM

> When walking the rbtree, the fact that iovad->start_pfn and limit_pfn
> are both inclusive limits creates an ambiguity once limit_pfn reaches
> the bottom of the address space and they overlap. Commit 5016bdb796b3
> ("iommu/iova: Fix underflow bug in __alloc_and_insert_iova_range") fixed
> the worst side-effect of this, that of underflow wraparound leading to
> bogus allocations, but the remaining fallout is that any attempt to
> allocate start_pfn itself erroneously fails.
> 
> The cleanest way to resolve the ambiguity is to simply make limit_pfn an
> exclusive limit when inside the guts of the rbtree. Since we're working
> with PFNs, representing one past the top of the address space is always
> possible without fear of overflow, and elsewhere it just makes life a
> little more straightforward.
> 
> Reported-by: Aaron Sierra <asierra@xxxxxxxxxxx>
> Signed-off-by: Robin Murphy <robin.murphy@xxxxxxx>
> ---
> 
> I've now run this through some more targeted testing, and I'm
> confident that it works as intended - Aaron, can you confirm if
> this satisfies your tests as well?

Robin,
Thanks for giving this issue some consideration. I can confirm that
your patch passes all of the test cases where I'd previously observed
allocation failures.

FWIW, my testing consists of defining a fixed limit_pfn (0xfffff) and
iterating over domains with start_pfn values of 0, then all powers-of-two
up to half of limit_pfn (0x80000).

For each domain, I set a fixed allocation unit size, calculate how many
allocations I expect to succeed, alloc and save iova structs until
allocation fails, then compare expected to actual. I do this for
allocation unit sizes of 1, 2, 4, 8, 50% of alloc-able range, and 100%
of alloc-able range.

-Aaron S.