Re: [PATCH v1 2/2] vfio/iova_bitmap: Fix PAGE_SIZE unaligned bitmaps

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

On 25/11/2022 12:42, Jason Gunthorpe wrote:
> On Fri, Nov 25, 2022 at 10:37:39AM +0000, Joao Martins wrote:
> 
>>> Yes, when we move this to iommufd the test suite should be included,
>>> either as integrated using the mock domain and the selftests or
>>> otherwise.
>>
>> So in iommufd counterpart I have already tests which exercise this. But not as
>> extensive.
> 
> We are getting to the point where we should start posting the iommufd
> dirty tracking stuff. Do you have time to work on it for the next
> cycle? Meaning get it largely sorted out in the next 3 weeks for review?
>

I'll post it for the next cycle -- It has been a bit on crazy on my end this
past month or so.

>>> void iova_bitmap_set(struct iova_bitmap *bitmap,
>>> 		     unsigned long iova, size_t length)
>>> {
>>> 	struct iova_bitmap_map *mapped = &bitmap->mapped;
>>> 	unsigned cur_bit =
>>> 		((iova - mapped->iova) >> mapped->pgshift) + mapped->pgoff * 8;
>>> 	unsigned long last_bit =
>>> 		(((iova + length - 1) - mapped->iova) >> mapped->pgshift) +
>>> 		mapped->pgoff * 8;
>>>
>>> 	do {
>>> 		unsigned int page_idx = cur_bit / BITS_PER_PAGE;
>>> 		unsigned int nbits =
>>> 			min(BITS_PER_PAGE - cur_bit, last_bit - cur_bit + 1);
> 
> min(BITS_PER_PAGE - (cur_bit % BITS_PER_PAGE), ...)
> 

I actually had this already in my changeset :) as the earlier snip wasn't
passing my tests. Plus I need to account for less indexes with pgoff, contrary
to what I said earlier in the remaining() function calculation.

>> Not sure if the vfio tree is a rebasing tree (or not?) and can just send a new
>> version, 
> 
> It isn't, you should just post a new patch on top of Alex's current
> tree "rework iova_bitmap_et to handle all page crossings" and along
> the way revert the first bit

OK -- makes sense. The other fix wasn't incorrect either (as we need to account
for pgoff on the same function), this one though fixes the real issue of
iova_bitmap_set().

Also, I'll add your Signed-off-by+Co-developed-by -- let me know otherwise if I
should not.

	Joao