On Thu, May 23, 2019 at 09:47:09PM +0800, Peter Xu wrote:
> On Thu, May 23, 2019 at 12:05:27PM +0200, Andrew Jones wrote:
> > 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.
>
> Oops yes. I merely forgot we've got two memory regions for the test,
> sorry.
>
> >
> > >
> > > 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.
>
> IMHO it would be as simple as replacing 3 with "3 * host_size /
> guest_size", but both work for me.
As I said, I'd rather not change the number of pages allocated based
on the pages sizes. With this you get 3 4k or 64k pages when both host
and guest are the same, 48 4k pages when the host is 64 and guest is 4,
and when the host has 4k and the guest has 64k, we would get 0, which
we don't want.
Thanks,
drew
