On Wed, Nov 16, 2022 at 12:09:52AM +0000, Tian, Kevin wrote:
0 is commonly used as an errant value for uninitialized things. We don't automatically map it into a process mm because it can cause security problems if we don't trap a bogus 0/NULL pointer reference.
The same logic applies here too, the allocator should not return 0 to reserve it as an unmapped IOVA page to catch bugs.
CPU doesn't reference IOVA. Where do such bugs exist?
SW is always buggy and SW programs the DMA address, so it could leave a 0 behind or something during the programming.
address 0 is never a bug in DMA to IOVA. if it is, it will be out of the aperture or in the reserved IOVA list.
It is a SW bug in the sense that 0 is commonly an uninitialized value or uninitialized memory.
DMA API is also a auto-iova scheme from driver p.o.v while it doesn't impose any restriction on address 0.
It probably shouldn't do that. It also allocates -1ULL which causes real bugs too. :(
if (!__alloc_iova_check_used(&allowed_span, length,
iova_alignment,
page_offset))
continue;
interval_tree_for_each_span(&area_span, &iopt-
area_itree,
allowed_span.start_used,
allowed_span.last_used) {
if (!__alloc_iova_check_hole(&area_span,
length,
iova_alignment,
page_offset))
continue;
interval_tree_for_each_span(&reserved_span,
&iopt-
reserved_itree,
area_span.start_used,
area_span.last_used) {
if (!__alloc_iova_check_hole(
&reserved_span, length,
iova_alignment,
page_offset))
continue;
this could be simplified by double span.
It is subtly not compatible, the double span looks for used areas. This is looking for a used area in the allowed_itree, a hole in the area_itree, and a hole in the reserved_itree.
the inner two loops can be replaced by double span, since both are skipping used areas.
The 2nd loop is looking for a used on allowed and the 3rd loop is looking for a hole in reserved. To fix it we'd have to invert allowed to work like reserved - which complicates the uAPI code.
The 1st loop finds an allowed range which can hold requested length
The 2nd loop finds an *unused* hole in the allowed range
The 3rd loop further looks for a hole in reserved.
last two both try to find a hole.
Ooh, OK, I read that in the wrong order, you know I looked at this many times to see if it could use the double span..
Ugh that is a pain, the double_span.h isn't setup for two .c files to use it.
Anyhow, so like this:
interval_tree_for_each_span(&allowed_span, &iopt->allowed_itree, PAGE_SIZE, ULONG_MAX - PAGE_SIZE) { if (RB_EMPTY_ROOT(&iopt->allowed_itree.rb_root)) { allowed_span.start_used = PAGE_SIZE; allowed_span.last_used = ULONG_MAX - PAGE_SIZE; allowed_span.is_hole = false; }
if (!__alloc_iova_check_used(&allowed_span, length, iova_alignment, page_offset)) continue;
interval_tree_for_each_double_span( &used_span, &iopt->reserved_itree, &iopt->area_itree, allowed_span.start_used, allowed_span.last_used) { if (!__alloc_iova_check_hole(&used_span, length, iova_alignment, page_offset)) continue;
*iova = used_span.start_hole; return 0; } }
This is the comment:
/*
- This is part of the VFIO compatibility support for VFIO_TYPE1_IOMMU.
That
- mode permits splitting a mapped area up, and then one of the splits is
- unmapped. Doing this normally would cause us to violate our invariant of
- pairing map/unmap. Thus, to support old VFIO compatibility disable
support
- for batching consecutive PFNs. All PFNs mapped into the iommu are done
in
- PAGE_SIZE units, not larger or smaller.
*/ static int batch_iommu_map_small(struct iommu_domain *domain, unsigned long iova, phys_addr_t paddr, size_t size, int prot)
I meant a comment in iopt_calculate_iova_alignment().
How about "see batch_iommu_map_small()" ?
Jason