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.
Thanks,
--
Peter Xu
