On 2020/12/10 22:50, Peter Xu wrote: > On Thu, Dec 10, 2020 at 10:53:23AM +0800, zhukeqian wrote: >> >> >> On 2020/12/10 10:08, Peter Xu wrote: >>> Keqian, >>> >>> On Thu, Dec 10, 2020 at 09:46:06AM +0800, zhukeqian wrote: >>>> Hi, >>>> >>>> I see that if start or size is not PAGE aligned, it also clears areas >>>> which beyond caller's expectation, so do we also need to consider this? >>> >>> Could you elaborate? >>> >>> If start_delta != 0, kvm_log_clear_one_slot() should already go the slow >>> path. >>> >>> Thanks, >>> >> >> Hi Peter, >> >> start_delta /= psize; >> >> If start is not PAGE aligned, then start_delta is not PAGE aligned. >> so I think the above code will implicitly extend our start to be PAGE >> aligned. >> >> I suggest that we should shrink the start and (start + size) to be PAGE >> aligned >> at beginning of this function. > > Callers should be with TARGET_PAGE_SIZE aligned on the size, so at least > x86_64 > should be pretty safe since host/guest page sizes match. > > Though indeed I must confess I don't know how it worked in general when host > page size != target page size, at least for migration. For example, I believe > kvm dirty logging is host page size based, though migration should be > migrating > pages in guest page size granule when it spots a dirty bit set. > Hi,
Indeed, we handle target_page_size aligned @start and @size in general. Maybe we'd better add explicit function comments about alignment requirement, and explicit alignment assert on @start and @size? Keqian.
