On Thu, May 23, 2019 at 05:48:59PM +0800, Peter Xu wrote:
> On Thu, May 23, 2019 at 11:34:05AM +0200, Andrew Jones wrote:
> > The memory slot size must be aligned to the host's page size. When
> > testing a guest with a 4k page size on a host with a 64k page size,
> > then 3 guest pages are not host page size aligned. Since we just need
> > a nearly arbitrary number of extra pages to ensure the memslot is not
> > aligned to a 64 host-page boundary for this test, then we can use
> > 16, as that's 64k aligned, but not 64 * 64k aligned.
> >
> > Fixes: 76d58e0f07ec ("KVM: fix KVM_CLEAR_DIRTY_LOG for memory slots of unaligned size", 2019-04-17)
> > Signed-off-by: Andrew Jones <drjones@xxxxxxxxxx>
> >
> > ---
> > Note, the commit "KVM: fix KVM_CLEAR_DIRTY_LOG for memory slots of
> > unaligned size" was somehow committed twice. 76d58e0f07ec is the
> > first instance.
> >
> > tools/testing/selftests/kvm/dirty_log_test.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
> > index f50a15c38f9b..bf85afbf1b5f 100644
> > --- a/tools/testing/selftests/kvm/dirty_log_test.c
> > +++ b/tools/testing/selftests/kvm/dirty_log_test.c
> > @@ -292,7 +292,7 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
> > * A little more than 1G of guest page sized pages. Cover the
> > * case where the size is not aligned to 64 pages.
> > */
> > - guest_num_pages = (1ul << (30 - guest_page_shift)) + 3;
> > + guest_num_pages = (1ul << (30 - guest_page_shift)) + 16;
>
> Hi, Drew,
>
> Could you help explain what's the error on ARM? Since I still cannot
> understand how it failed from the first glance...
The KVM_SET_USER_MEMORY_REGION ioctl will fail because of
if (mem->memory_size & (PAGE_SIZE - 1))
goto out;
in __kvm_set_memory_region(). And that's because PAGE_SIZE == 64k
on the host (kvm), but we're attempting to allocate a size of 3*4k.
>
> Also, even if we want to have the alignment, shall we do the math
> using known host/guest page size rather than another adhoc number or
> could it still break with some other combinations of host/guest page
> sizes?
I don't think we need to worry too much about > 64k pages being a
thing any time soon and I'd rather not change the number of pages
allocated based on the page sizes, so other than maybe doing something
like
/*
* Comment stating why we have this.
*/
#define GUEST_EXTRA_PAGES 16
then I think we're already fine.
Thanks,
drew
