On Mon, Jun 24, 2019 at 10:43:21AM +0200, Paolo Bonzini wrote: > On 24/06/19 10:06, Peter Xu wrote: > > Well, if with such an error we'd better fix it right away in this > > patch... :) > > > > Let me wait for some more comments, I'll touch that up too if I need a > > repost. > > Looks good to me, except for one minor issue in this patch. But do not > attribute this one to me, it's basically all code from you.
OK. > > > +static uint64_t vtd_get_next_mask(uint64_t start, uint64_t size, int gaw) > > +{ > > + /* Tries to find smallest mask from start first */ > > + uint64_t rmask = start & -start, max_mask = 1ULL << gaw; > > + > > + assert(size && gaw > 0 && gaw < 64); > > + > > + /* Zero start, or too big */ > > + if (!rmask || rmask > max_mask) { > > + rmask = max_mask; > > + } > > Perhaps simpler: > > uint64_t max_mask = 1ULL << gaw; > uint64_t alignment = start ? start & -start : max_mask; > (I'll add another "alignment = MIN(alignment, max_mask)" here if no one disagree...) > size = MIN(size, max_mask); > if (alignment <= size) { > /* Increase the alignment of start */ > return alignment; > } else { > /* Find the largest page mask from size */ > return 1ULL << (63 - clz64(size)); > } > > Also please rename it to get_naturally_aligned_size. Will do. Thanks, -- Peter Xu