On Fri, 3 Nov 2023 17:14:47 +0100 Halil Pasic <pasic@xxxxxxxxxxxxx> wrote: >[...] > In our case min_align_mask == 0 and a) is thus not applicable, because b) and > c) we end up with iotlb_align_mask = 0x800. And because orig_add & 0x800 == > 0x800 but pool->start & 0x800 == 0 and the slot at index i is skipped over. The > slot 0 is skipped over because it is page aligned, when !!((1UL << PAGE_SHIFT) > & orig_addr) Wait. These mask values can quickly become confusing. Do you mean iotlb_align_mask == 0xfff? > Let us note that with the current implementation the min_align_size mask, that > is mechanism a) also controls the tlb_addr within the first slot so that: > tlb_addr & min_align_mask == orig_addr & min_align_mask. In that sense a) is > very unlike b) and c). It is silently assumed that PAGE_SIZE >= IO_TLB_SIZE, so if the buffer is page-aligned, the lower bits of the alignment inside the io tlb slot must be zero. If the same assumption is made about alloc_align_mask, it should be documented, but it is not. >[...] > In our opinion the first step towards getting this right is to figure out what > the different kinds of alignments are really supposed to mean. For each of the > mechanisms we need to understand and document, whether making sure that the > bounce buffer does not stretch over more of certain units of memory (like, > pages, iova granule size, whatever), or is it about preserving offset within a > certain unit of memory, and if yes to what extent (the least significant n-bits > of the orig_addr dictated by the respective mask, or something different). Seconded. I have also been struggling with the various alignment constraints. I have even written (but not yet submitted) a patch to calculate the combined alignment mask in swiotlb_tbl_map_single() and pass it down to all other functions, just to make it clear what alignment mask is used. My understanding is that buffer alignment may be required by: 1. hardware which cannot handle an unaligned base address (presumably because the chip performs a simple OR operation to get the addresses of individual fields); 2. isolation of untrusted devices, where no two bounce buffers should end up in the same iova granule; 3. allocation size; I could not find an explanation, so this might be merely an attempt at reducing SWIOTLB internal fragmentation. I hope other people on the Cc list can shed more light on the intended behaviour. Petr T
