All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] kvm: selftests: aarch64: dirty_log_test: fix unaligned memslot size
@ 2019-05-23  9:34 Andrew Jones
  2019-05-23  9:48 ` Peter Xu
  2019-05-24 19:23 ` Paolo Bonzini
  0 siblings, 2 replies; 6+ messages in thread
From: Andrew Jones @ 2019-05-23  9:34 UTC (permalink / raw)
  To: kvm; +Cc: pbonzini, rkrcmar, peterx

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@redhat.com>

---
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;
 	host_page_size = getpagesize();
 	host_num_pages = (guest_num_pages * guest_page_size) / host_page_size +
 			 !!((guest_num_pages * guest_page_size) % host_page_size);
-- 
2.18.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] kvm: selftests: aarch64: dirty_log_test: fix unaligned memslot size
  2019-05-23  9:34 [PATCH] kvm: selftests: aarch64: dirty_log_test: fix unaligned memslot size Andrew Jones
@ 2019-05-23  9:48 ` Peter Xu
  2019-05-23 10:05   ` Andrew Jones
  2019-05-24 19:23 ` Paolo Bonzini
  1 sibling, 1 reply; 6+ messages in thread
From: Peter Xu @ 2019-05-23  9:48 UTC (permalink / raw)
  To: Andrew Jones; +Cc: kvm, pbonzini, rkrcmar

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@redhat.com>
> 
> ---
> 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...

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?

Thanks,

-- 
Peter Xu

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] kvm: selftests: aarch64: dirty_log_test: fix unaligned memslot size
  2019-05-23  9:48 ` Peter Xu
@ 2019-05-23 10:05   ` Andrew Jones
  2019-05-23 13:47     ` Peter Xu
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Jones @ 2019-05-23 10:05 UTC (permalink / raw)
  To: Peter Xu; +Cc: kvm, pbonzini, rkrcmar

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@redhat.com>
> > 
> > ---
> > 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] kvm: selftests: aarch64: dirty_log_test: fix unaligned memslot size
  2019-05-23 10:05   ` Andrew Jones
@ 2019-05-23 13:47     ` Peter Xu
  2019-05-23 17:15       ` Andrew Jones
  0 siblings, 1 reply; 6+ messages in thread
From: Peter Xu @ 2019-05-23 13:47 UTC (permalink / raw)
  To: Andrew Jones; +Cc: kvm, pbonzini, rkrcmar

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@redhat.com>
> > > 
> > > ---
> > > 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] kvm: selftests: aarch64: dirty_log_test: fix unaligned memslot size
  2019-05-23 13:47     ` Peter Xu
@ 2019-05-23 17:15       ` Andrew Jones
  0 siblings, 0 replies; 6+ messages in thread
From: Andrew Jones @ 2019-05-23 17:15 UTC (permalink / raw)
  To: Peter Xu; +Cc: kvm, pbonzini, rkrcmar

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@redhat.com>
> > > > 
> > > > ---
> > > > 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] kvm: selftests: aarch64: dirty_log_test: fix unaligned memslot size
  2019-05-23  9:34 [PATCH] kvm: selftests: aarch64: dirty_log_test: fix unaligned memslot size Andrew Jones
  2019-05-23  9:48 ` Peter Xu
@ 2019-05-24 19:23 ` Paolo Bonzini
  1 sibling, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2019-05-24 19:23 UTC (permalink / raw)
  To: Andrew Jones, kvm; +Cc: rkrcmar, peterx

On 23/05/19 11:34, 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@redhat.com>

Queued, thanks.

Paolo

> ---
> 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;
>  	host_page_size = getpagesize();
>  	host_num_pages = (guest_num_pages * guest_page_size) / host_page_size +
>  			 !!((guest_num_pages * guest_page_size) % host_page_size);
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-05-24 19:23 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-23  9:34 [PATCH] kvm: selftests: aarch64: dirty_log_test: fix unaligned memslot size Andrew Jones
2019-05-23  9:48 ` Peter Xu
2019-05-23 10:05   ` Andrew Jones
2019-05-23 13:47     ` Peter Xu
2019-05-23 17:15       ` Andrew Jones
2019-05-24 19:23 ` Paolo Bonzini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.