* [PATCH 01/13] HACK: Ensure __NR_userfaultfd is defined
2020-02-14 14:59 [PATCH 00/13] KVM: selftests: Various fixes and cleanups Andrew Jones
@ 2020-02-14 14:59 ` Andrew Jones
2020-02-20 16:38 ` Paolo Bonzini
2020-02-14 14:59 ` [PATCH 02/13] fixup! KVM: selftests: Add support for vcpu_args_set to aarch64 and s390x Andrew Jones
` (13 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Andrew Jones @ 2020-02-14 14:59 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, bgardon, borntraeger, frankja, thuth, peterx
Without this hack kvm/queue kvm selftests don't compile for x86_64.
---
tools/testing/selftests/kvm/demand_paging_test.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
index df1fc38b4df1..ec8860b70129 100644
--- a/tools/testing/selftests/kvm/demand_paging_test.c
+++ b/tools/testing/selftests/kvm/demand_paging_test.c
@@ -20,6 +20,10 @@
#include <linux/bitops.h>
#include <linux/userfaultfd.h>
+#ifndef __NR_userfaultfd
+#define __NR_userfaultfd 282
+#endif
+
#include "test_util.h"
#include "kvm_util.h"
#include "processor.h"
--
2.21.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 01/13] HACK: Ensure __NR_userfaultfd is defined
2020-02-14 14:59 ` [PATCH 01/13] HACK: Ensure __NR_userfaultfd is defined Andrew Jones
@ 2020-02-20 16:38 ` Paolo Bonzini
0 siblings, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2020-02-20 16:38 UTC (permalink / raw)
To: Andrew Jones, kvm; +Cc: bgardon, borntraeger, frankja, thuth, peterx
On 14/02/20 15:59, Andrew Jones wrote:
> Without this hack kvm/queue kvm selftests don't compile for x86_64.
> ---
> tools/testing/selftests/kvm/demand_paging_test.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
> index df1fc38b4df1..ec8860b70129 100644
> --- a/tools/testing/selftests/kvm/demand_paging_test.c
> +++ b/tools/testing/selftests/kvm/demand_paging_test.c
> @@ -20,6 +20,10 @@
> #include <linux/bitops.h>
> #include <linux/userfaultfd.h>
>
> +#ifndef __NR_userfaultfd
> +#define __NR_userfaultfd 282
> +#endif
> +
> #include "test_util.h"
> #include "kvm_util.h"
> #include "processor.h"
>
This is because we're getting a limited version of
asm/unistd.h from tools/arch/x86/include/asm/unistd_64.h.
So:
------------ 8< ----------------
From: Paolo Bonzini <pbonzini@redhat.com>
Subject: [PATCH] fixup! KVM: selftests: Add demand paging content to the demand paging test
Without this, kvm selftests don't compile for x86_64 on old-enough
userspace.
diff --git a/tools/arch/x86/include/asm/unistd_64.h b/tools/arch/x86/include/asm/unistd_64.h
index cb52a3a8b8fc..4205ed4158bf 100644
--- a/tools/arch/x86/include/asm/unistd_64.h
+++ b/tools/arch/x86/include/asm/unistd_64.h
@@ -1,4 +1,7 @@
/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __NR_userfaultfd
+#define __NR_userfaultfd 282
+#endif
#ifndef __NR_perf_event_open
# define __NR_perf_event_open 298
#endif
diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
index df1fc38b4df1..dd6c5ee56201 100644
--- a/tools/testing/selftests/kvm/demand_paging_test.c
+++ b/tools/testing/selftests/kvm/demand_paging_test.c
@@ -13,6 +13,7 @@
#include <stdlib.h>
#include <sys/syscall.h>
#include <unistd.h>
+#include <asm/unistd.h>
#include <time.h>
#include <poll.h>
#include <pthread.h>
@@ -24,6 +25,8 @@
#include "kvm_util.h"
#include "processor.h"
+#ifdef __NR_userfaultfd
+
/* The memory slot index demand page */
#define TEST_MEM_SLOT_INDEX 1
@@ -678,3 +681,15 @@ int main(int argc, char *argv[])
return 0;
}
+
+#else /* __NR_userfaultfd */
+
+#warning "missing __NR_userfaultfd definition"
+
+int main(void)
+{
+ printf("skip: Skipping userfaultfd test (missing __NR_userfaultfd)\n");
+ return KSFT_SKIP;
+}
+
+#endif /* __NR_userfaultfd */
Paolo
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 02/13] fixup! KVM: selftests: Add support for vcpu_args_set to aarch64 and s390x
2020-02-14 14:59 [PATCH 00/13] KVM: selftests: Various fixes and cleanups Andrew Jones
2020-02-14 14:59 ` [PATCH 01/13] HACK: Ensure __NR_userfaultfd is defined Andrew Jones
@ 2020-02-14 14:59 ` Andrew Jones
2020-02-14 20:35 ` Christian Borntraeger
2020-02-18 17:30 ` Ben Gardon
2020-02-14 14:59 ` [PATCH 03/13] fixup! KVM: selftests: Support multiple vCPUs in demand paging test Andrew Jones
` (12 subsequent siblings)
14 siblings, 2 replies; 28+ messages in thread
From: Andrew Jones @ 2020-02-14 14:59 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, bgardon, borntraeger, frankja, thuth, peterx
[Fixed array index (num => i) and made some style changes.]
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
.../selftests/kvm/lib/aarch64/processor.c | 24 ++++---------------
1 file changed, 4 insertions(+), 20 deletions(-)
diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c
index 839a76c96f01..f7dffccea12c 100644
--- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
+++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
@@ -334,36 +334,20 @@ void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code)
aarch64_vcpu_add_default(vm, vcpuid, NULL, guest_code);
}
-/* VM VCPU Args Set
- *
- * Input Args:
- * vm - Virtual Machine
- * vcpuid - VCPU ID
- * num - number of arguments
- * ... - arguments, each of type uint64_t
- *
- * Output Args: None
- *
- * Return: None
- *
- * Sets the first num function input arguments to the values
- * given as variable args. Each of the variable args is expected to
- * be of type uint64_t. The registers set by this function are r0-r7.
- */
void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...)
{
va_list ap;
int i;
TEST_ASSERT(num >= 1 && num <= 8, "Unsupported number of args,\n"
- " num: %u\n",
- num);
+ " num: %u\n", num);
va_start(ap, num);
- for (i = 0; i < num; i++)
- set_reg(vm, vcpuid, ARM64_CORE_REG(regs.regs[num]),
+ for (i = 0; i < num; i++) {
+ set_reg(vm, vcpuid, ARM64_CORE_REG(regs.regs[i]),
va_arg(ap, uint64_t));
+ }
va_end(ap);
}
--
2.21.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 02/13] fixup! KVM: selftests: Add support for vcpu_args_set to aarch64 and s390x
2020-02-14 14:59 ` [PATCH 02/13] fixup! KVM: selftests: Add support for vcpu_args_set to aarch64 and s390x Andrew Jones
@ 2020-02-14 20:35 ` Christian Borntraeger
2020-02-15 7:04 ` Andrew Jones
2020-02-18 17:30 ` Ben Gardon
1 sibling, 1 reply; 28+ messages in thread
From: Christian Borntraeger @ 2020-02-14 20:35 UTC (permalink / raw)
To: Andrew Jones, kvm; +Cc: pbonzini, bgardon, frankja, thuth, peterx
On 14.02.20 15:59, Andrew Jones wrote:
> [Fixed array index (num => i) and made some style changes.]
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
> .../selftests/kvm/lib/aarch64/processor.c | 24 ++++---------------
subject says s390, the patch not.
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 02/13] fixup! KVM: selftests: Add support for vcpu_args_set to aarch64 and s390x
2020-02-14 20:35 ` Christian Borntraeger
@ 2020-02-15 7:04 ` Andrew Jones
0 siblings, 0 replies; 28+ messages in thread
From: Andrew Jones @ 2020-02-15 7:04 UTC (permalink / raw)
To: Christian Borntraeger; +Cc: kvm, pbonzini, bgardon, frankja, thuth, peterx
On Fri, Feb 14, 2020 at 09:35:06PM +0100, Christian Borntraeger wrote:
>
>
> On 14.02.20 15:59, Andrew Jones wrote:
> > [Fixed array index (num => i) and made some style changes.]
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> > .../selftests/kvm/lib/aarch64/processor.c | 24 ++++---------------
>
> subject says s390, the patch not.
>
It's a "fixup!" patch. The original subject has s390, so this one must as
well.
Thanks,
drew
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 02/13] fixup! KVM: selftests: Add support for vcpu_args_set to aarch64 and s390x
2020-02-14 14:59 ` [PATCH 02/13] fixup! KVM: selftests: Add support for vcpu_args_set to aarch64 and s390x Andrew Jones
2020-02-14 20:35 ` Christian Borntraeger
@ 2020-02-18 17:30 ` Ben Gardon
2020-02-18 17:38 ` Andrew Jones
1 sibling, 1 reply; 28+ messages in thread
From: Ben Gardon @ 2020-02-18 17:30 UTC (permalink / raw)
To: Andrew Jones; +Cc: kvm, Paolo Bonzini, borntraeger, frankja, thuth, Peter Xu
On Fri, Feb 14, 2020 at 6:59 AM Andrew Jones <drjones@redhat.com> wrote:
>
> [Fixed array index (num => i) and made some style changes.]
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
> .../selftests/kvm/lib/aarch64/processor.c | 24 ++++---------------
> 1 file changed, 4 insertions(+), 20 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c
> index 839a76c96f01..f7dffccea12c 100644
> --- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
> +++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
> @@ -334,36 +334,20 @@ void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code)
> aarch64_vcpu_add_default(vm, vcpuid, NULL, guest_code);
> }
>
> -/* VM VCPU Args Set
> - *
> - * Input Args:
> - * vm - Virtual Machine
> - * vcpuid - VCPU ID
> - * num - number of arguments
> - * ... - arguments, each of type uint64_t
> - *
> - * Output Args: None
> - *
> - * Return: None
> - *
> - * Sets the first num function input arguments to the values
> - * given as variable args. Each of the variable args is expected to
> - * be of type uint64_t. The registers set by this function are r0-r7.
> - */
I'm sad to see this comment go. I realize it might be more verbose
than necessary, but calling out that the args will all be interpreted
as uint_64s and which registers are set feels like useful context to
have here.
> void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...)
> {
> va_list ap;
> int i;
>
> TEST_ASSERT(num >= 1 && num <= 8, "Unsupported number of args,\n"
> - " num: %u\n",
> - num);
> + " num: %u\n", num);
>
> va_start(ap, num);
>
> - for (i = 0; i < num; i++)
> - set_reg(vm, vcpuid, ARM64_CORE_REG(regs.regs[num]),
> + for (i = 0; i < num; i++) {
> + set_reg(vm, vcpuid, ARM64_CORE_REG(regs.regs[i]),
> va_arg(ap, uint64_t));
> + }
Woops, I should have caught this in the original demand paging test
series, but didn't notice because this function was only ever called
with one argument.
Thank you for fixing this.
>
> va_end(ap);
> }
> --
> 2.21.1
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 02/13] fixup! KVM: selftests: Add support for vcpu_args_set to aarch64 and s390x
2020-02-18 17:30 ` Ben Gardon
@ 2020-02-18 17:38 ` Andrew Jones
2020-02-20 16:40 ` Paolo Bonzini
0 siblings, 1 reply; 28+ messages in thread
From: Andrew Jones @ 2020-02-18 17:38 UTC (permalink / raw)
To: Ben Gardon; +Cc: kvm, Paolo Bonzini, borntraeger, frankja, thuth, Peter Xu
On Tue, Feb 18, 2020 at 09:30:25AM -0800, Ben Gardon wrote:
> On Fri, Feb 14, 2020 at 6:59 AM Andrew Jones <drjones@redhat.com> wrote:
> >
> > [Fixed array index (num => i) and made some style changes.]
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> > .../selftests/kvm/lib/aarch64/processor.c | 24 ++++---------------
> > 1 file changed, 4 insertions(+), 20 deletions(-)
> >
> > diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c
> > index 839a76c96f01..f7dffccea12c 100644
> > --- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
> > +++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
> > @@ -334,36 +334,20 @@ void vm_vcpu_add_default(struct kvm_vm *vm, uint32_t vcpuid, void *guest_code)
> > aarch64_vcpu_add_default(vm, vcpuid, NULL, guest_code);
> > }
> >
> > -/* VM VCPU Args Set
> > - *
> > - * Input Args:
> > - * vm - Virtual Machine
> > - * vcpuid - VCPU ID
> > - * num - number of arguments
> > - * ... - arguments, each of type uint64_t
> > - *
> > - * Output Args: None
> > - *
> > - * Return: None
> > - *
> > - * Sets the first num function input arguments to the values
> > - * given as variable args. Each of the variable args is expected to
> > - * be of type uint64_t. The registers set by this function are r0-r7.
> > - */
> I'm sad to see this comment go. I realize it might be more verbose
> than necessary, but calling out that the args will all be interpreted
> as uint_64s and which registers are set feels like useful context to
> have here.
For me the code makes that super obvious, and I prefer not to describe what
code does. Also, I'd put these type of comment blocks, written more
generally, in the header files if they're functions that are implemented
by all architectures, rather than duplicating them in each source file.
>
> > void vcpu_args_set(struct kvm_vm *vm, uint32_t vcpuid, unsigned int num, ...)
> > {
> > va_list ap;
> > int i;
> >
> > TEST_ASSERT(num >= 1 && num <= 8, "Unsupported number of args,\n"
> > - " num: %u\n",
> > - num);
> > + " num: %u\n", num);
> >
> > va_start(ap, num);
> >
> > - for (i = 0; i < num; i++)
> > - set_reg(vm, vcpuid, ARM64_CORE_REG(regs.regs[num]),
> > + for (i = 0; i < num; i++) {
> > + set_reg(vm, vcpuid, ARM64_CORE_REG(regs.regs[i]),
> > va_arg(ap, uint64_t));
> > + }
> Woops, I should have caught this in the original demand paging test
> series, but didn't notice because this function was only ever called
> with one argument.
> Thank you for fixing this.
>
> >
> > va_end(ap);
> > }
> > --
> > 2.21.1
> >
>
Thanks,
drew
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 02/13] fixup! KVM: selftests: Add support for vcpu_args_set to aarch64 and s390x
2020-02-18 17:38 ` Andrew Jones
@ 2020-02-20 16:40 ` Paolo Bonzini
0 siblings, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2020-02-20 16:40 UTC (permalink / raw)
To: Andrew Jones, Ben Gardon; +Cc: kvm, borntraeger, frankja, thuth, Peter Xu
On 18/02/20 18:38, Andrew Jones wrote:
> For me the code makes that super obvious, and I prefer not to describe what
> code does. Also, I'd put these type of comment blocks, written more
> generally, in the header files if they're functions that are implemented
> by all architectures, rather than duplicating them in each source file.
This makes sense. For now I've restored the comment, but moving them to
the .h files is a good idea.
Paolo
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 03/13] fixup! KVM: selftests: Support multiple vCPUs in demand paging test
2020-02-14 14:59 [PATCH 00/13] KVM: selftests: Various fixes and cleanups Andrew Jones
2020-02-14 14:59 ` [PATCH 01/13] HACK: Ensure __NR_userfaultfd is defined Andrew Jones
2020-02-14 14:59 ` [PATCH 02/13] fixup! KVM: selftests: Add support for vcpu_args_set to aarch64 and s390x Andrew Jones
@ 2020-02-14 14:59 ` Andrew Jones
2020-02-18 17:39 ` Ben Gardon
2020-02-14 14:59 ` [PATCH 04/13] fixup! KVM: selftests: Add memory size parameter to the " Andrew Jones
` (11 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Andrew Jones @ 2020-02-14 14:59 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, bgardon, borntraeger, frankja, thuth, peterx
[guest_code() can't return, use GUEST_ASSERT(). Ensure the number
of guests pages is compatible with the host.]
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
tools/testing/selftests/kvm/demand_paging_test.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
index ec8860b70129..2e6e3db8418a 100644
--- a/tools/testing/selftests/kvm/demand_paging_test.c
+++ b/tools/testing/selftests/kvm/demand_paging_test.c
@@ -115,9 +115,8 @@ static void guest_code(uint32_t vcpu_id)
uint64_t pages;
int i;
- /* Return to signal error if vCPU args data structure is courrupt. */
- if (vcpu_args[vcpu_id].vcpu_id != vcpu_id)
- return;
+ /* Make sure vCPU args data structure is not corrupt. */
+ GUEST_ASSERT(vcpu_args[vcpu_id].vcpu_id == vcpu_id);
gva = vcpu_args[vcpu_id].gva;
pages = vcpu_args[vcpu_id].pages;
@@ -186,6 +185,12 @@ static struct kvm_vm *create_vm(enum vm_guest_mode mode, int vcpus,
pages += ((2 * vcpus * vcpu_memory_bytes) >> PAGE_SHIFT_4K) /
PTES_PER_4K_PT;
+ /*
+ * If the host is uing 64K pages, then we need the number of 4K
+ * guest pages to be a multiple of 16.
+ */
+ pages += 16 - pages % 16;
+
vm = _vm_create(mode, pages, O_RDWR);
kvm_vm_elf_load(vm, program_invocation_name, 0, 0);
#ifdef __x86_64__
--
2.21.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 03/13] fixup! KVM: selftests: Support multiple vCPUs in demand paging test
2020-02-14 14:59 ` [PATCH 03/13] fixup! KVM: selftests: Support multiple vCPUs in demand paging test Andrew Jones
@ 2020-02-18 17:39 ` Ben Gardon
0 siblings, 0 replies; 28+ messages in thread
From: Ben Gardon @ 2020-02-18 17:39 UTC (permalink / raw)
To: Andrew Jones; +Cc: kvm, Paolo Bonzini, borntraeger, frankja, thuth, Peter Xu
On Fri, Feb 14, 2020 at 6:59 AM Andrew Jones <drjones@redhat.com> wrote:
>
> [guest_code() can't return, use GUEST_ASSERT(). Ensure the number
> of guests pages is compatible with the host.]
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
> tools/testing/selftests/kvm/demand_paging_test.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
> index ec8860b70129..2e6e3db8418a 100644
> --- a/tools/testing/selftests/kvm/demand_paging_test.c
> +++ b/tools/testing/selftests/kvm/demand_paging_test.c
> @@ -115,9 +115,8 @@ static void guest_code(uint32_t vcpu_id)
> uint64_t pages;
> int i;
>
> - /* Return to signal error if vCPU args data structure is courrupt. */
> - if (vcpu_args[vcpu_id].vcpu_id != vcpu_id)
> - return;
> + /* Make sure vCPU args data structure is not corrupt. */
> + GUEST_ASSERT(vcpu_args[vcpu_id].vcpu_id == vcpu_id);
>
> gva = vcpu_args[vcpu_id].gva;
> pages = vcpu_args[vcpu_id].pages;
> @@ -186,6 +185,12 @@ static struct kvm_vm *create_vm(enum vm_guest_mode mode, int vcpus,
> pages += ((2 * vcpus * vcpu_memory_bytes) >> PAGE_SHIFT_4K) /
> PTES_PER_4K_PT;
>
> + /*
> + * If the host is uing 64K pages, then we need the number of 4K
s/uing/using
> + * guest pages to be a multiple of 16.
> + */
> + pages += 16 - pages % 16;
> +
Could we use some derivative of getpagesize() here instead?
e.g.
ASSERT(getpagesize() >= (1 << PAGE_SHIFT_4K));
ASSERT(!(getpagesize() % (1 << PAGE_SHIFT_4K)));
pages_4k_per_host_page = getpagesize() / (1 << PAGE_SHIFT_4K);
pages += pages_4k_per_host_page - pages % pages_4k_per_host_page;
> vm = _vm_create(mode, pages, O_RDWR);
> kvm_vm_elf_load(vm, program_invocation_name, 0, 0);
> #ifdef __x86_64__
> --
> 2.21.1
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 04/13] fixup! KVM: selftests: Add memory size parameter to the demand paging test
2020-02-14 14:59 [PATCH 00/13] KVM: selftests: Various fixes and cleanups Andrew Jones
` (2 preceding siblings ...)
2020-02-14 14:59 ` [PATCH 03/13] fixup! KVM: selftests: Support multiple vCPUs in demand paging test Andrew Jones
@ 2020-02-14 14:59 ` Andrew Jones
2020-02-18 17:43 ` Ben Gardon
2020-02-14 14:59 ` [PATCH 05/13] fixup! KVM: selftests: Time guest demand paging Andrew Jones
` (10 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Andrew Jones @ 2020-02-14 14:59 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, bgardon, borntraeger, frankja, thuth, peterx
[Rewrote parse_size() to simplify and provide user more flexibility as
to how sizes are input. Also fixed size overflow assert.]
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
tools/testing/selftests/kvm/lib/test_util.c | 76 +++++++++------------
1 file changed, 33 insertions(+), 43 deletions(-)
diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
index 706e0f963a44..cbd7f51b07a1 100644
--- a/tools/testing/selftests/kvm/lib/test_util.c
+++ b/tools/testing/selftests/kvm/lib/test_util.c
@@ -4,58 +4,48 @@
*
* Copyright (C) 2020, Google LLC.
*/
-
-#include "test_util.h"
-
+#include <stdlib.h>
#include <ctype.h>
+#include <limits.h>
+#include "test_util.h"
/*
* Parses "[0-9]+[kmgt]?".
*/
size_t parse_size(const char *size)
{
- size_t len = strlen(size);
- size_t i;
- size_t scale_shift = 0;
size_t base;
-
- TEST_ASSERT(len > 0, "Need at least 1 digit in '%s'", size);
-
- /* Find the first letter in the string, indicating scale. */
- for (i = 0; i < len; i++) {
- if (!isdigit(size[i])) {
- TEST_ASSERT(i > 0, "Need at least 1 digit in '%s'",
- size);
- TEST_ASSERT(i == len - 1,
- "Expected letter at the end in '%s'.",
- size);
- switch (tolower(size[i])) {
- case 't':
- scale_shift = 40;
- break;
- case 'g':
- scale_shift = 30;
- break;
- case 'm':
- scale_shift = 20;
- break;
- case 'k':
- scale_shift = 10;
- break;
- default:
- TEST_ASSERT(false, "Unknown size letter %c",
- size[i]);
- }
- }
+ char *scale;
+ int shift = 0;
+
+ TEST_ASSERT(size && isdigit(size[0]), "Need at least one digit in '%s'", size);
+
+ base = strtoull(size, &scale, 0);
+
+ TEST_ASSERT(base != ULLONG_MAX, "Overflow parsing size!");
+
+ switch (tolower(*scale)) {
+ case 't':
+ shift = 40;
+ break;
+ case 'g':
+ shift = 30;
+ break;
+ case 'm':
+ shift = 20;
+ break;
+ case 'k':
+ shift = 10;
+ break;
+ case 'b':
+ case '\0':
+ shift = 0;
+ break;
+ default:
+ TEST_ASSERT(false, "Unknown size letter %c", *scale);
}
- TEST_ASSERT(scale_shift < 8 * sizeof(size_t),
- "Overflow parsing scale!");
-
- base = atoi(size);
-
- TEST_ASSERT(!(base & ~((1 << (sizeof(size_t) - scale_shift)) - 1)),
- "Overflow parsing size!");
+ TEST_ASSERT((base << shift) >> shift == base, "Overflow scaling size!");
- return base << scale_shift;
+ return base << shift;
}
--
2.21.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 04/13] fixup! KVM: selftests: Add memory size parameter to the demand paging test
2020-02-14 14:59 ` [PATCH 04/13] fixup! KVM: selftests: Add memory size parameter to the " Andrew Jones
@ 2020-02-18 17:43 ` Ben Gardon
0 siblings, 0 replies; 28+ messages in thread
From: Ben Gardon @ 2020-02-18 17:43 UTC (permalink / raw)
To: Andrew Jones; +Cc: kvm, Paolo Bonzini, borntraeger, frankja, thuth, Peter Xu
On Fri, Feb 14, 2020 at 6:59 AM Andrew Jones <drjones@redhat.com> wrote:
>
> [Rewrote parse_size() to simplify and provide user more flexibility as
> to how sizes are input. Also fixed size overflow assert.]
> Signed-off-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Ben Gardon <bgardon@google.com>
Great cleanup and fix, thank you.
> ---
> tools/testing/selftests/kvm/lib/test_util.c | 76 +++++++++------------
> 1 file changed, 33 insertions(+), 43 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
> index 706e0f963a44..cbd7f51b07a1 100644
> --- a/tools/testing/selftests/kvm/lib/test_util.c
> +++ b/tools/testing/selftests/kvm/lib/test_util.c
> @@ -4,58 +4,48 @@
> *
> * Copyright (C) 2020, Google LLC.
> */
> -
> -#include "test_util.h"
> -
> +#include <stdlib.h>
> #include <ctype.h>
> +#include <limits.h>
> +#include "test_util.h"
>
> /*
> * Parses "[0-9]+[kmgt]?".
> */
> size_t parse_size(const char *size)
> {
> - size_t len = strlen(size);
> - size_t i;
> - size_t scale_shift = 0;
> size_t base;
> -
> - TEST_ASSERT(len > 0, "Need at least 1 digit in '%s'", size);
> -
> - /* Find the first letter in the string, indicating scale. */
> - for (i = 0; i < len; i++) {
> - if (!isdigit(size[i])) {
> - TEST_ASSERT(i > 0, "Need at least 1 digit in '%s'",
> - size);
> - TEST_ASSERT(i == len - 1,
> - "Expected letter at the end in '%s'.",
> - size);
> - switch (tolower(size[i])) {
> - case 't':
> - scale_shift = 40;
> - break;
> - case 'g':
> - scale_shift = 30;
> - break;
> - case 'm':
> - scale_shift = 20;
> - break;
> - case 'k':
> - scale_shift = 10;
> - break;
> - default:
> - TEST_ASSERT(false, "Unknown size letter %c",
> - size[i]);
> - }
> - }
> + char *scale;
> + int shift = 0;
> +
> + TEST_ASSERT(size && isdigit(size[0]), "Need at least one digit in '%s'", size);
> +
> + base = strtoull(size, &scale, 0);
> +
> + TEST_ASSERT(base != ULLONG_MAX, "Overflow parsing size!");
> +
> + switch (tolower(*scale)) {
> + case 't':
> + shift = 40;
> + break;
> + case 'g':
> + shift = 30;
> + break;
> + case 'm':
> + shift = 20;
> + break;
> + case 'k':
> + shift = 10;
> + break;
> + case 'b':
> + case '\0':
> + shift = 0;
> + break;
> + default:
> + TEST_ASSERT(false, "Unknown size letter %c", *scale);
> }
>
> - TEST_ASSERT(scale_shift < 8 * sizeof(size_t),
> - "Overflow parsing scale!");
> -
> - base = atoi(size);
> -
> - TEST_ASSERT(!(base & ~((1 << (sizeof(size_t) - scale_shift)) - 1)),
> - "Overflow parsing size!");
> + TEST_ASSERT((base << shift) >> shift == base, "Overflow scaling size!");
>
> - return base << scale_shift;
> + return base << shift;
> }
> --
> 2.21.1
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 05/13] fixup! KVM: selftests: Time guest demand paging
2020-02-14 14:59 [PATCH 00/13] KVM: selftests: Various fixes and cleanups Andrew Jones
` (3 preceding siblings ...)
2020-02-14 14:59 ` [PATCH 04/13] fixup! KVM: selftests: Add memory size parameter to the " Andrew Jones
@ 2020-02-14 14:59 ` Andrew Jones
2020-02-14 14:59 ` [PATCH 06/13] KVM: selftests: Remove unnecessary defines Andrew Jones
` (9 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Andrew Jones @ 2020-02-14 14:59 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, bgardon, borntraeger, frankja, thuth, peterx
[Move timespec-diff to test_util.h]
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
.../selftests/kvm/demand_paging_test.c | 42 +++++--------------
.../testing/selftests/kvm/include/test_util.h | 3 ++
tools/testing/selftests/kvm/lib/test_util.c | 20 +++++++++
3 files changed, 34 insertions(+), 31 deletions(-)
diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
index 2e6e3db8418a..22a3011df62f 100644
--- a/tools/testing/selftests/kvm/demand_paging_test.c
+++ b/tools/testing/selftests/kvm/demand_paging_test.c
@@ -74,26 +74,6 @@ static uint64_t guest_test_phys_mem;
*/
static uint64_t guest_test_virt_mem = DEFAULT_GUEST_TEST_MEM;
-int64_t to_ns(struct timespec ts)
-{
- return (int64_t)ts.tv_nsec + 1000000000LL * (int64_t)ts.tv_sec;
-}
-
-struct timespec diff(struct timespec start, struct timespec end)
-{
- struct timespec temp;
-
- if ((end.tv_nsec-start.tv_nsec) < 0) {
- temp.tv_sec = end.tv_sec - start.tv_sec - 1;
- temp.tv_nsec = 1000000000 + end.tv_nsec - start.tv_nsec;
- } else {
- temp.tv_sec = end.tv_sec - start.tv_sec;
- temp.tv_nsec = end.tv_nsec - start.tv_nsec;
- }
-
- return temp;
-}
-
struct vcpu_args {
uint64_t gva;
uint64_t pages;
@@ -157,8 +137,8 @@ static void *vcpu_worker(void *data)
clock_gettime(CLOCK_MONOTONIC, &end);
PER_VCPU_DEBUG("vCPU %d execution time: %lld.%.9lds\n", vcpu_id,
- (long long)(diff(start, end).tv_sec),
- diff(start, end).tv_nsec);
+ (long long)(timespec_diff(start, end).tv_sec),
+ timespec_diff(start, end).tv_nsec);
return NULL;
}
@@ -226,7 +206,7 @@ static int handle_uffd_page_request(int uffd, uint64_t addr)
clock_gettime(CLOCK_MONOTONIC, &end);
PER_PAGE_DEBUG("UFFDIO_COPY %d \t%lld ns\n", tid,
- (long long)to_ns(diff(start, end)));
+ (long long)timespec_to_ns(timespec_diff(start, end)));
PER_PAGE_DEBUG("Paged in %ld bytes at 0x%lx from thread %d\n",
host_page_size, addr, tid);
@@ -321,10 +301,10 @@ static void *uffd_handler_thread_fn(void *arg)
clock_gettime(CLOCK_MONOTONIC, &end);
PER_VCPU_DEBUG("userfaulted %ld pages over %lld.%.9lds. (%f/sec)\n",
- pages, (long long)(diff(start, end).tv_sec),
- diff(start, end).tv_nsec, pages /
- ((double)diff(start, end).tv_sec +
- (double)diff(start, end).tv_nsec / 100000000.0));
+ pages, (long long)(timespec_diff(start, end).tv_sec),
+ timespec_diff(start, end).tv_nsec, pages /
+ ((double)timespec_diff(start, end).tv_sec +
+ (double)timespec_diff(start, end).tv_nsec / 100000000.0));
return NULL;
}
@@ -432,7 +412,6 @@ static void run_test(enum vm_guest_mode mode, bool use_uffd,
DEBUG("guest physical test memory offset: 0x%lx\n",
guest_test_phys_mem);
-
/* Add an extra memory slot for testing demand paging */
vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS,
guest_test_phys_mem,
@@ -548,10 +527,11 @@ static void run_test(enum vm_guest_mode mode, bool use_uffd,
}
DEBUG("Total guest execution time: %lld.%.9lds\n",
- (long long)(diff(start, end).tv_sec), diff(start, end).tv_nsec);
+ (long long)(timespec_diff(start, end).tv_sec),
+ timespec_diff(start, end).tv_nsec);
DEBUG("Overall demand paging rate: %f pgs/sec\n",
- guest_num_pages / ((double)diff(start, end).tv_sec +
- (double)diff(start, end).tv_nsec / 100000000.0));
+ guest_num_pages / ((double)timespec_diff(start, end).tv_sec +
+ (double)timespec_diff(start, end).tv_nsec / 100000000.0));
ucall_uninit(vm);
kvm_vm_free(vm);
diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
index e696c8219d69..920328ca5f7e 100644
--- a/tools/testing/selftests/kvm/include/test_util.h
+++ b/tools/testing/selftests/kvm/include/test_util.h
@@ -41,4 +41,7 @@ void test_assert(bool exp, const char *exp_str,
size_t parse_size(const char *size);
+int64_t timespec_to_ns(struct timespec ts);
+struct timespec timespec_diff(struct timespec start, struct timespec end);
+
#endif /* SELFTEST_KVM_TEST_UTIL_H */
diff --git a/tools/testing/selftests/kvm/lib/test_util.c b/tools/testing/selftests/kvm/lib/test_util.c
index cbd7f51b07a1..1c0d45afdf36 100644
--- a/tools/testing/selftests/kvm/lib/test_util.c
+++ b/tools/testing/selftests/kvm/lib/test_util.c
@@ -49,3 +49,23 @@ size_t parse_size(const char *size)
return base << shift;
}
+
+int64_t timespec_to_ns(struct timespec ts)
+{
+ return (int64_t)ts.tv_nsec + 1000000000LL * (int64_t)ts.tv_sec;
+}
+
+struct timespec timespec_diff(struct timespec start, struct timespec end)
+{
+ struct timespec temp;
+
+ if ((end.tv_nsec - start.tv_nsec) < 0) {
+ temp.tv_sec = end.tv_sec - start.tv_sec - 1;
+ temp.tv_nsec = 1000000000LL + end.tv_nsec - start.tv_nsec;
+ } else {
+ temp.tv_sec = end.tv_sec - start.tv_sec;
+ temp.tv_nsec = end.tv_nsec - start.tv_nsec;
+ }
+
+ return temp;
+}
--
2.21.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 06/13] KVM: selftests: Remove unnecessary defines
2020-02-14 14:59 [PATCH 00/13] KVM: selftests: Various fixes and cleanups Andrew Jones
` (4 preceding siblings ...)
2020-02-14 14:59 ` [PATCH 05/13] fixup! KVM: selftests: Time guest demand paging Andrew Jones
@ 2020-02-14 14:59 ` Andrew Jones
2020-02-14 14:59 ` [PATCH 07/13] KVM: selftests: aarch64: Remove unnecessary ifdefs Andrew Jones
` (8 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Andrew Jones @ 2020-02-14 14:59 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, bgardon, borntraeger, frankja, thuth, peterx
BITS_PER_LONG and friends are provided by linux/bitops.h
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
tools/testing/selftests/kvm/lib/kvm_util_internal.h | 11 -----------
1 file changed, 11 deletions(-)
diff --git a/tools/testing/selftests/kvm/lib/kvm_util_internal.h b/tools/testing/selftests/kvm/lib/kvm_util_internal.h
index ac50c42750cf..2fce6750b8b3 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util_internal.h
+++ b/tools/testing/selftests/kvm/lib/kvm_util_internal.h
@@ -12,17 +12,6 @@
#define KVM_DEV_PATH "/dev/kvm"
-#ifndef BITS_PER_BYTE
-#define BITS_PER_BYTE 8
-#endif
-
-#ifndef BITS_PER_LONG
-#define BITS_PER_LONG (BITS_PER_BYTE * sizeof(long))
-#endif
-
-#define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
-#define BITS_TO_LONGS(nr) DIV_ROUND_UP(nr, BITS_PER_LONG)
-
struct userspace_mem_region {
struct userspace_mem_region *next, *prev;
struct kvm_userspace_memory_region region;
--
2.21.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 07/13] KVM: selftests: aarch64: Remove unnecessary ifdefs
2020-02-14 14:59 [PATCH 00/13] KVM: selftests: Various fixes and cleanups Andrew Jones
` (5 preceding siblings ...)
2020-02-14 14:59 ` [PATCH 06/13] KVM: selftests: Remove unnecessary defines Andrew Jones
@ 2020-02-14 14:59 ` Andrew Jones
2020-02-14 14:59 ` [PATCH 08/13] KVM: selftests: aarch64: Use stream when given Andrew Jones
` (7 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Andrew Jones @ 2020-02-14 14:59 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, bgardon, borntraeger, frankja, thuth, peterx
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
.../selftests/kvm/demand_paging_test.c | 21 ++++++++-----------
tools/testing/selftests/kvm/dirty_log_test.c | 21 ++++++++-----------
2 files changed, 18 insertions(+), 24 deletions(-)
diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
index 22a3011df62f..f20aa9f0a227 100644
--- a/tools/testing/selftests/kvm/demand_paging_test.c
+++ b/tools/testing/selftests/kvm/demand_paging_test.c
@@ -422,9 +422,7 @@ static void run_test(enum vm_guest_mode mode, bool use_uffd,
virt_map(vm, guest_test_virt_mem, guest_test_phys_mem,
guest_num_pages * guest_page_size, 0);
-#ifdef __aarch64__
ucall_init(vm, NULL);
-#endif
guest_data_prototype = malloc(host_page_size);
TEST_ASSERT(guest_data_prototype,
@@ -594,9 +592,6 @@ int main(int argc, char *argv[])
int opt, i;
bool use_uffd = false;
useconds_t uffd_delay = 0;
-#ifdef __aarch64__
- unsigned int host_ipa_limit;
-#endif
#ifdef __x86_64__
vm_guest_mode_params_init(VM_MODE_PXXV48_4K, true, true);
@@ -604,13 +599,15 @@ int main(int argc, char *argv[])
#ifdef __aarch64__
vm_guest_mode_params_init(VM_MODE_P40V48_4K, true, true);
vm_guest_mode_params_init(VM_MODE_P40V48_64K, true, true);
-
- host_ipa_limit = kvm_check_cap(KVM_CAP_ARM_VM_IPA_SIZE);
- if (host_ipa_limit >= 52)
- vm_guest_mode_params_init(VM_MODE_P52V48_64K, true, true);
- if (host_ipa_limit >= 48) {
- vm_guest_mode_params_init(VM_MODE_P48V48_4K, true, true);
- vm_guest_mode_params_init(VM_MODE_P48V48_64K, true, true);
+ {
+ unsigned int limit = kvm_check_cap(KVM_CAP_ARM_VM_IPA_SIZE);
+
+ if (limit >= 52)
+ vm_guest_mode_params_init(VM_MODE_P52V48_64K, true, true);
+ if (limit >= 48) {
+ vm_guest_mode_params_init(VM_MODE_P48V48_4K, true, true);
+ vm_guest_mode_params_init(VM_MODE_P48V48_64K, true, true);
+ }
}
#endif
#ifdef __s390x__
diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
index 5614222a6628..3146302ac563 100644
--- a/tools/testing/selftests/kvm/dirty_log_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_test.c
@@ -341,9 +341,7 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
#ifdef __x86_64__
vcpu_set_cpuid(vm, VCPU_ID, kvm_get_supported_cpuid());
#endif
-#ifdef __aarch64__
ucall_init(vm, NULL);
-#endif
/* Export the shared variables to the guest */
sync_global_to_guest(vm, host_page_size);
@@ -433,9 +431,6 @@ int main(int argc, char *argv[])
uint64_t phys_offset = 0;
unsigned int mode;
int opt, i;
-#ifdef __aarch64__
- unsigned int host_ipa_limit;
-#endif
#ifdef USE_CLEAR_DIRTY_LOG
if (!kvm_check_cap(KVM_CAP_MANUAL_DIRTY_LOG_PROTECT2)) {
@@ -450,13 +445,15 @@ int main(int argc, char *argv[])
#ifdef __aarch64__
vm_guest_mode_params_init(VM_MODE_P40V48_4K, true, true);
vm_guest_mode_params_init(VM_MODE_P40V48_64K, true, true);
-
- host_ipa_limit = kvm_check_cap(KVM_CAP_ARM_VM_IPA_SIZE);
- if (host_ipa_limit >= 52)
- vm_guest_mode_params_init(VM_MODE_P52V48_64K, true, true);
- if (host_ipa_limit >= 48) {
- vm_guest_mode_params_init(VM_MODE_P48V48_4K, true, true);
- vm_guest_mode_params_init(VM_MODE_P48V48_64K, true, true);
+ {
+ unsigned int limit = kvm_check_cap(KVM_CAP_ARM_VM_IPA_SIZE);
+
+ if (limit >= 52)
+ vm_guest_mode_params_init(VM_MODE_P52V48_64K, true, true);
+ if (limit >= 48) {
+ vm_guest_mode_params_init(VM_MODE_P48V48_4K, true, true);
+ vm_guest_mode_params_init(VM_MODE_P48V48_64K, true, true);
+ }
}
#endif
#ifdef __s390x__
--
2.21.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 08/13] KVM: selftests: aarch64: Use stream when given
2020-02-14 14:59 [PATCH 00/13] KVM: selftests: Various fixes and cleanups Andrew Jones
` (6 preceding siblings ...)
2020-02-14 14:59 ` [PATCH 07/13] KVM: selftests: aarch64: Remove unnecessary ifdefs Andrew Jones
@ 2020-02-14 14:59 ` Andrew Jones
2020-02-14 14:59 ` [PATCH 09/13] KVM: selftests: Rework debug message printing Andrew Jones
` (6 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Andrew Jones @ 2020-02-14 14:59 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, bgardon, borntraeger, frankja, thuth, peterx
I'm not sure how we ended up using printf instead of fprintf in
virt_dump(). Fix it.
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
tools/testing/selftests/kvm/lib/aarch64/processor.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c
index f7dffccea12c..053e1c940e7c 100644
--- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
+++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
@@ -197,7 +197,7 @@ static void pte_dump(FILE *stream, struct kvm_vm *vm, uint8_t indent, uint64_t p
ptep = addr_gpa2hva(vm, pte);
if (!*ptep)
continue;
- printf("%*s%s: %lx: %lx at %p\n", indent, "", type[level], pte, *ptep, ptep);
+ fprintf(stream, "%*s%s: %lx: %lx at %p\n", indent, "", type[level], pte, *ptep, ptep);
pte_dump(stream, vm, indent + 1, pte_addr(vm, *ptep), level + 1);
}
#endif
@@ -215,7 +215,7 @@ void virt_dump(FILE *stream, struct kvm_vm *vm, uint8_t indent)
ptep = addr_gpa2hva(vm, pgd);
if (!*ptep)
continue;
- printf("%*spgd: %lx: %lx at %p\n", indent, "", pgd, *ptep, ptep);
+ fprintf(stream, "%*spgd: %lx: %lx at %p\n", indent, "", pgd, *ptep, ptep);
pte_dump(stream, vm, indent + 1, pte_addr(vm, *ptep), level);
}
}
--
2.21.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 09/13] KVM: selftests: Rework debug message printing
2020-02-14 14:59 [PATCH 00/13] KVM: selftests: Various fixes and cleanups Andrew Jones
` (7 preceding siblings ...)
2020-02-14 14:59 ` [PATCH 08/13] KVM: selftests: aarch64: Use stream when given Andrew Jones
@ 2020-02-14 14:59 ` Andrew Jones
2020-02-14 14:59 ` [PATCH 10/13] KVM: selftests: Convert some printf's to pr_info's Andrew Jones
` (5 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Andrew Jones @ 2020-02-14 14:59 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, bgardon, borntraeger, frankja, thuth, peterx
There were a few problems with the way we output "debug" messages.
The first is that we used DEBUG() which is defined when NDEBUG is
not defined, but NDEBUG will never be defined for kselftests
because it relies too much on assert(). The next is that most
of the DEBUG() messages were actually "info" messages, which
users may want to turn off if they just want a silent test that
either completes or asserts. Finally, a debug message output from
a library function, and thus for all tests, was annoying when its
information wasn't interesting for a test.
Rework these messages so debug messages only output when DEBUG
is defined and info messages output unless QUIET is defined.
Also name the functions pr_debug and pr_info and make sure that
when they're disabled we eat all the inputs. The later avoids
unused variable warnings when the variables were only defined
for the purpose of printing.
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
.../selftests/kvm/demand_paging_test.c | 54 +++++++++----------
tools/testing/selftests/kvm/dirty_log_test.c | 16 +++---
.../testing/selftests/kvm/include/kvm_util.h | 6 ---
.../testing/selftests/kvm/include/test_util.h | 13 +++++
.../selftests/kvm/lib/aarch64/processor.c | 2 +-
tools/testing/selftests/kvm/lib/kvm_util.c | 7 +--
6 files changed, 54 insertions(+), 44 deletions(-)
diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
index f20aa9f0a227..5aae166c2817 100644
--- a/tools/testing/selftests/kvm/demand_paging_test.c
+++ b/tools/testing/selftests/kvm/demand_paging_test.c
@@ -37,15 +37,15 @@
#define DEFAULT_GUEST_TEST_MEM_SIZE (1 << 30) /* 1G */
#ifdef PRINT_PER_PAGE_UPDATES
-#define PER_PAGE_DEBUG(...) DEBUG(__VA_ARGS__)
+#define PER_PAGE_DEBUG(...) printf(__VA_ARGS__)
#else
-#define PER_PAGE_DEBUG(...)
+#define PER_PAGE_DEBUG(...) _no_printf(__VA_ARGS__)
#endif
#ifdef PRINT_PER_VCPU_UPDATES
-#define PER_VCPU_DEBUG(...) DEBUG(__VA_ARGS__)
+#define PER_VCPU_DEBUG(...) printf(__VA_ARGS__)
#else
-#define PER_VCPU_DEBUG(...)
+#define PER_VCPU_DEBUG(...) _no_printf(__VA_ARGS__)
#endif
#define MAX_VCPUS 512
@@ -171,6 +171,8 @@ static struct kvm_vm *create_vm(enum vm_guest_mode mode, int vcpus,
*/
pages += 16 - pages % 16;
+ pr_info("Testing guest mode: %s\n", vm_guest_mode_string(mode));
+
vm = _vm_create(mode, pages, O_RDWR);
kvm_vm_elf_load(vm, program_invocation_name, 0, 0);
#ifdef __x86_64__
@@ -198,8 +200,8 @@ static int handle_uffd_page_request(int uffd, uint64_t addr)
r = ioctl(uffd, UFFDIO_COPY, ©);
if (r == -1) {
- DEBUG("Failed Paged in 0x%lx from thread %d with errno: %d\n",
- addr, tid, errno);
+ pr_info("Failed Paged in 0x%lx from thread %d with errno: %d\n",
+ addr, tid, errno);
return r;
}
@@ -247,19 +249,19 @@ static void *uffd_handler_thread_fn(void *arg)
r = poll(pollfd, 2, -1);
switch (r) {
case -1:
- DEBUG("poll err");
+ pr_info("poll err");
continue;
case 0:
continue;
case 1:
break;
default:
- DEBUG("Polling uffd returned %d", r);
+ pr_info("Polling uffd returned %d", r);
return NULL;
}
if (pollfd[0].revents & POLLERR) {
- DEBUG("uffd revents has POLLERR");
+ pr_info("uffd revents has POLLERR");
return NULL;
}
@@ -277,13 +279,12 @@ static void *uffd_handler_thread_fn(void *arg)
if (r == -1) {
if (errno == EAGAIN)
continue;
- DEBUG("Read of uffd gor errno %d", errno);
+ pr_info("Read of uffd gor errno %d", errno);
return NULL;
}
if (r != sizeof(msg)) {
- DEBUG("Read on uffd returned unexpected size: %d bytes",
- r);
+ pr_info("Read on uffd returned unexpected size: %d bytes", r);
return NULL;
}
@@ -321,14 +322,14 @@ static int setup_demand_paging(struct kvm_vm *vm,
uffd = syscall(__NR_userfaultfd, O_CLOEXEC | O_NONBLOCK);
if (uffd == -1) {
- DEBUG("uffd creation failed\n");
+ pr_info("uffd creation failed\n");
return -1;
}
uffdio_api.api = UFFD_API;
uffdio_api.features = 0;
if (ioctl(uffd, UFFDIO_API, &uffdio_api) == -1) {
- DEBUG("ioctl uffdio_api failed\n");
+ pr_info("ioctl uffdio_api failed\n");
return -1;
}
@@ -336,13 +337,13 @@ static int setup_demand_paging(struct kvm_vm *vm,
uffdio_register.range.len = len;
uffdio_register.mode = UFFDIO_REGISTER_MODE_MISSING;
if (ioctl(uffd, UFFDIO_REGISTER, &uffdio_register) == -1) {
- DEBUG("ioctl uffdio_register failed\n");
+ pr_info("ioctl uffdio_register failed\n");
return -1;
}
if ((uffdio_register.ioctls & UFFD_API_RANGE_IOCTLS) !=
UFFD_API_RANGE_IOCTLS) {
- DEBUG("unexpected userfaultfd ioctl set\n");
+ pr_info("unexpected userfaultfd ioctl set\n");
return -1;
}
@@ -409,8 +410,7 @@ static void run_test(enum vm_guest_mode mode, bool use_uffd,
guest_test_phys_mem &= ~((1 << 20) - 1);
#endif
- DEBUG("guest physical test memory offset: 0x%lx\n",
- guest_test_phys_mem);
+ pr_info("guest physical test memory offset: 0x%lx\n", guest_test_phys_mem);
/* Add an extra memory slot for testing demand paging */
vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS,
@@ -491,7 +491,7 @@ static void run_test(enum vm_guest_mode mode, bool use_uffd,
sync_global_to_guest(vm, guest_page_size);
sync_global_to_guest(vm, vcpu_args);
- DEBUG("Finished creating vCPUs and starting uffd threads\n");
+ pr_info("Finished creating vCPUs and starting uffd threads\n");
clock_gettime(CLOCK_MONOTONIC, &start);
@@ -500,7 +500,7 @@ static void run_test(enum vm_guest_mode mode, bool use_uffd,
&vcpu_args[vcpu_id]);
}
- DEBUG("Started all vCPUs\n");
+ pr_info("Started all vCPUs\n");
/* Wait for the vcpu threads to quit */
for (vcpu_id = 0; vcpu_id < vcpus; vcpu_id++) {
@@ -508,7 +508,7 @@ static void run_test(enum vm_guest_mode mode, bool use_uffd,
PER_VCPU_DEBUG("Joined thread for vCPU %d\n", vcpu_id);
}
- DEBUG("All vCPU threads joined\n");
+ pr_info("All vCPU threads joined\n");
clock_gettime(CLOCK_MONOTONIC, &end);
@@ -524,12 +524,12 @@ static void run_test(enum vm_guest_mode mode, bool use_uffd,
}
}
- DEBUG("Total guest execution time: %lld.%.9lds\n",
- (long long)(timespec_diff(start, end).tv_sec),
- timespec_diff(start, end).tv_nsec);
- DEBUG("Overall demand paging rate: %f pgs/sec\n",
- guest_num_pages / ((double)timespec_diff(start, end).tv_sec +
- (double)timespec_diff(start, end).tv_nsec / 100000000.0));
+ pr_info("Total guest execution time: %lld.%.9lds\n",
+ (long long)(timespec_diff(start, end).tv_sec),
+ timespec_diff(start, end).tv_nsec);
+ pr_info("Overall demand paging rate: %f pgs/sec\n",
+ guest_num_pages / ((double)timespec_diff(start, end).tv_sec +
+ (double)timespec_diff(start, end).tv_nsec / 100000000.0));
ucall_uninit(vm);
kvm_vm_free(vm);
diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
index 3146302ac563..587edf40cc32 100644
--- a/tools/testing/selftests/kvm/dirty_log_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_test.c
@@ -173,7 +173,7 @@ static void *vcpu_worker(void *data)
}
}
- DEBUG("Dirtied %"PRIu64" pages\n", pages_count);
+ pr_info("Dirtied %"PRIu64" pages\n", pages_count);
return NULL;
}
@@ -252,6 +252,8 @@ static struct kvm_vm *create_vm(enum vm_guest_mode mode, uint32_t vcpuid,
struct kvm_vm *vm;
uint64_t extra_pg_pages = extra_mem_pages / 512 * 2;
+ pr_info("Testing guest mode: %s\n", vm_guest_mode_string(mode));
+
vm = _vm_create(mode, DEFAULT_GUEST_PHY_PAGES + extra_pg_pages, O_RDWR);
kvm_vm_elf_load(vm, program_invocation_name, 0, 0);
#ifdef __x86_64__
@@ -311,7 +313,7 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
guest_test_phys_mem &= ~((1 << 20) - 1);
#endif
- DEBUG("guest physical test memory offset: 0x%lx\n", guest_test_phys_mem);
+ pr_info("guest physical test memory offset: 0x%lx\n", guest_test_phys_mem);
bmap = bitmap_alloc(host_num_pages);
host_bmap_track = bitmap_alloc(host_num_pages);
@@ -376,9 +378,9 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
host_quit = true;
pthread_join(vcpu_thread, NULL);
- DEBUG("Total bits checked: dirty (%"PRIu64"), clear (%"PRIu64"), "
- "track_next (%"PRIu64")\n", host_dirty_count, host_clear_count,
- host_track_next_count);
+ pr_info("Total bits checked: dirty (%"PRIu64"), clear (%"PRIu64"), "
+ "track_next (%"PRIu64")\n", host_dirty_count, host_clear_count,
+ host_track_next_count);
free(bmap);
free(host_bmap_track);
@@ -492,8 +494,8 @@ int main(int argc, char *argv[])
TEST_ASSERT(iterations > 2, "Iterations must be greater than two");
TEST_ASSERT(interval > 0, "Interval must be greater than zero");
- DEBUG("Test iterations: %"PRIu64", interval: %"PRIu64" (ms)\n",
- iterations, interval);
+ pr_info("Test iterations: %"PRIu64", interval: %"PRIu64" (ms)\n",
+ iterations, interval);
srandom(time(0));
diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index ae0d14c2540a..45c6c7ea24c5 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -24,12 +24,6 @@ struct kvm_vm;
typedef uint64_t vm_paddr_t; /* Virtual Machine (Guest) physical address */
typedef uint64_t vm_vaddr_t; /* Virtual Machine (Guest) virtual address */
-#ifndef NDEBUG
-#define DEBUG(...) printf(__VA_ARGS__);
-#else
-#define DEBUG(...)
-#endif
-
/* Minimum allocated guest virtual and physical addresses */
#define KVM_UTIL_MIN_VADDR 0x2000
diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
index 920328ca5f7e..c921ea719ae0 100644
--- a/tools/testing/selftests/kvm/include/test_util.h
+++ b/tools/testing/selftests/kvm/include/test_util.h
@@ -19,6 +19,19 @@
#include <fcntl.h>
#include "kselftest.h"
+static inline int _no_printf(const char *format, ...) { return 0; }
+
+#ifdef DEBUG
+#define pr_debug(...) printf(__VA_ARGS__)
+#else
+#define pr_debug(...) _no_printf(__VA_ARGS__)
+#endif
+#ifndef QUIET
+#define pr_info(...) printf(__VA_ARGS__)
+#else
+#define pr_info(...) _no_printf(__VA_ARGS__)
+#endif
+
ssize_t test_write(int fd, const void *buf, size_t count);
ssize_t test_read(int fd, void *buf, size_t count);
int test_seq_read(const char *path, char **bufp, size_t *sizep);
diff --git a/tools/testing/selftests/kvm/lib/aarch64/processor.c b/tools/testing/selftests/kvm/lib/aarch64/processor.c
index 053e1c940e7c..f84270f0e32c 100644
--- a/tools/testing/selftests/kvm/lib/aarch64/processor.c
+++ b/tools/testing/selftests/kvm/lib/aarch64/processor.c
@@ -186,7 +186,7 @@ vm_paddr_t addr_gva2gpa(struct kvm_vm *vm, vm_vaddr_t gva)
static void pte_dump(FILE *stream, struct kvm_vm *vm, uint8_t indent, uint64_t page, int level)
{
-#ifdef DEBUG_VM
+#ifdef DEBUG
static const char * const type[] = { "", "pud", "pmd", "pte" };
uint64_t pte, *ptep;
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index fc597d7d6f84..c8a7ed338bed 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -138,7 +138,8 @@ struct kvm_vm *_vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm)
{
struct kvm_vm *vm;
- DEBUG("Testing guest mode: %s\n", vm_guest_mode_string(mode));
+ pr_debug("%s: mode='%s' pages='%ld' perm='%d'\n", __func__,
+ vm_guest_mode_string(mode), phy_pages, perm);
vm = calloc(1, sizeof(*vm));
TEST_ASSERT(vm != NULL, "Insufficient Memory");
@@ -198,8 +199,8 @@ struct kvm_vm *_vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm)
vm->pgtable_levels = 4;
vm->page_size = 0x1000;
vm->page_shift = 12;
- DEBUG("Guest physical address width detected: %d\n",
- vm->pa_bits);
+ pr_debug("Guest physical address width detected: %d\n",
+ vm->pa_bits);
#else
TEST_ASSERT(false, "VM_MODE_PXXV48_4K not supported on "
"non-x86 platforms");
--
2.21.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 10/13] KVM: selftests: Convert some printf's to pr_info's
2020-02-14 14:59 [PATCH 00/13] KVM: selftests: Various fixes and cleanups Andrew Jones
` (8 preceding siblings ...)
2020-02-14 14:59 ` [PATCH 09/13] KVM: selftests: Rework debug message printing Andrew Jones
@ 2020-02-14 14:59 ` Andrew Jones
2020-02-14 14:59 ` [PATCH 11/13] KVM: selftests: Rename vm_guest_mode_params Andrew Jones
` (4 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Andrew Jones @ 2020-02-14 14:59 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, bgardon, borntraeger, frankja, thuth, peterx
We leave some printf's because they inform the user the test is being
skipped. QUIET should not disable those. We also leave the printf's
used for help text.
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
tools/testing/selftests/kvm/kvm_create_max_vcpus.c | 8 ++++----
tools/testing/selftests/kvm/s390x/resets.c | 6 +++---
tools/testing/selftests/kvm/x86_64/mmio_warning_test.c | 2 +-
tools/testing/selftests/kvm/x86_64/smm_test.c | 2 +-
tools/testing/selftests/kvm/x86_64/state_test.c | 2 +-
tools/testing/selftests/kvm/x86_64/vmx_tsc_adjust_test.c | 4 ++--
tools/testing/selftests/kvm/x86_64/xss_msr_test.c | 2 +-
7 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/tools/testing/selftests/kvm/kvm_create_max_vcpus.c b/tools/testing/selftests/kvm/kvm_create_max_vcpus.c
index 6f38c3dc0d56..0299cd81b8ba 100644
--- a/tools/testing/selftests/kvm/kvm_create_max_vcpus.c
+++ b/tools/testing/selftests/kvm/kvm_create_max_vcpus.c
@@ -24,8 +24,8 @@ void test_vcpu_creation(int first_vcpu_id, int num_vcpus)
struct kvm_vm *vm;
int i;
- printf("Testing creating %d vCPUs, with IDs %d...%d.\n",
- num_vcpus, first_vcpu_id, first_vcpu_id + num_vcpus - 1);
+ pr_info("Testing creating %d vCPUs, with IDs %d...%d.\n",
+ num_vcpus, first_vcpu_id, first_vcpu_id + num_vcpus - 1);
vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES, O_RDWR);
@@ -41,8 +41,8 @@ int main(int argc, char *argv[])
int kvm_max_vcpu_id = kvm_check_cap(KVM_CAP_MAX_VCPU_ID);
int kvm_max_vcpus = kvm_check_cap(KVM_CAP_MAX_VCPUS);
- printf("KVM_CAP_MAX_VCPU_ID: %d\n", kvm_max_vcpu_id);
- printf("KVM_CAP_MAX_VCPUS: %d\n", kvm_max_vcpus);
+ pr_info("KVM_CAP_MAX_VCPU_ID: %d\n", kvm_max_vcpu_id);
+ pr_info("KVM_CAP_MAX_VCPUS: %d\n", kvm_max_vcpus);
/*
* Upstream KVM prior to 4.8 does not support KVM_CAP_MAX_VCPU_ID.
diff --git a/tools/testing/selftests/kvm/s390x/resets.c b/tools/testing/selftests/kvm/s390x/resets.c
index 1485bc6c8999..c59db2c95e9e 100644
--- a/tools/testing/selftests/kvm/s390x/resets.c
+++ b/tools/testing/selftests/kvm/s390x/resets.c
@@ -134,7 +134,7 @@ static void inject_irq(int cpu_id)
static void test_normal(void)
{
- printf("Testing normal reset\n");
+ pr_info("Testing normal reset\n");
/* Create VM */
vm = vm_create_default(VCPU_ID, 0, guest_code_initial);
run = vcpu_state(vm, VCPU_ID);
@@ -151,7 +151,7 @@ static void test_normal(void)
static void test_initial(void)
{
- printf("Testing initial reset\n");
+ pr_info("Testing initial reset\n");
vm = vm_create_default(VCPU_ID, 0, guest_code_initial);
run = vcpu_state(vm, VCPU_ID);
regs = &run->s.regs;
@@ -168,7 +168,7 @@ static void test_initial(void)
static void test_clear(void)
{
- printf("Testing clear reset\n");
+ pr_info("Testing clear reset\n");
vm = vm_create_default(VCPU_ID, 0, guest_code_initial);
run = vcpu_state(vm, VCPU_ID);
regs = &run->s.regs;
diff --git a/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c b/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c
index 00bb97d76000..5350c2d6f736 100644
--- a/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c
+++ b/tools/testing/selftests/kvm/x86_64/mmio_warning_test.c
@@ -44,7 +44,7 @@ void *thr(void *arg)
struct kvm_run *run = tc->run;
res = ioctl(kvmcpu, KVM_RUN, 0);
- printf("ret1=%d exit_reason=%d suberror=%d\n",
+ pr_info("ret1=%d exit_reason=%d suberror=%d\n",
res, run->exit_reason, run->internal.suberror);
return 0;
diff --git a/tools/testing/selftests/kvm/x86_64/smm_test.c b/tools/testing/selftests/kvm/x86_64/smm_test.c
index 8c063646f2a0..8230b6bc6b8f 100644
--- a/tools/testing/selftests/kvm/x86_64/smm_test.c
+++ b/tools/testing/selftests/kvm/x86_64/smm_test.c
@@ -117,7 +117,7 @@ int main(int argc, char *argv[])
vcpu_alloc_vmx(vm, &vmx_pages_gva);
vcpu_args_set(vm, VCPU_ID, 1, vmx_pages_gva);
} else {
- printf("will skip SMM test with VMX enabled\n");
+ pr_info("will skip SMM test with VMX enabled\n");
vcpu_args_set(vm, VCPU_ID, 1, 0);
}
diff --git a/tools/testing/selftests/kvm/x86_64/state_test.c b/tools/testing/selftests/kvm/x86_64/state_test.c
index 3ab5ec3da9f4..9d2daffd6110 100644
--- a/tools/testing/selftests/kvm/x86_64/state_test.c
+++ b/tools/testing/selftests/kvm/x86_64/state_test.c
@@ -139,7 +139,7 @@ int main(int argc, char *argv[])
vcpu_alloc_vmx(vm, &vmx_pages_gva);
vcpu_args_set(vm, VCPU_ID, 1, vmx_pages_gva);
} else {
- printf("will skip nested state checks\n");
+ pr_info("will skip nested state checks\n");
vcpu_args_set(vm, VCPU_ID, 1, 0);
}
diff --git a/tools/testing/selftests/kvm/x86_64/vmx_tsc_adjust_test.c b/tools/testing/selftests/kvm/x86_64/vmx_tsc_adjust_test.c
index 69e482a95c47..64f7cb81f28d 100644
--- a/tools/testing/selftests/kvm/x86_64/vmx_tsc_adjust_test.c
+++ b/tools/testing/selftests/kvm/x86_64/vmx_tsc_adjust_test.c
@@ -121,8 +121,8 @@ static void l1_guest_code(struct vmx_pages *vmx_pages)
static void report(int64_t val)
{
- printf("IA32_TSC_ADJUST is %ld (%lld * TSC_ADJUST_VALUE + %lld).\n",
- val, val / TSC_ADJUST_VALUE, val % TSC_ADJUST_VALUE);
+ pr_info("IA32_TSC_ADJUST is %ld (%lld * TSC_ADJUST_VALUE + %lld).\n",
+ val, val / TSC_ADJUST_VALUE, val % TSC_ADJUST_VALUE);
}
int main(int argc, char *argv[])
diff --git a/tools/testing/selftests/kvm/x86_64/xss_msr_test.c b/tools/testing/selftests/kvm/x86_64/xss_msr_test.c
index 851ea81b9d9f..fc8328d8d5b0 100644
--- a/tools/testing/selftests/kvm/x86_64/xss_msr_test.c
+++ b/tools/testing/selftests/kvm/x86_64/xss_msr_test.c
@@ -51,7 +51,7 @@ int main(int argc, char *argv[])
xss_supported = entry && !!(entry->eax & X86_FEATURE_XSAVES);
}
if (!xss_supported) {
- printf("IA32_XSS is not supported by the vCPU.\n");
+ printf("IA32_XSS is not supported by the vCPU, skipping test\n");
exit(KSFT_SKIP);
}
--
2.21.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 11/13] KVM: selftests: Rename vm_guest_mode_params
2020-02-14 14:59 [PATCH 00/13] KVM: selftests: Various fixes and cleanups Andrew Jones
` (9 preceding siblings ...)
2020-02-14 14:59 ` [PATCH 10/13] KVM: selftests: Convert some printf's to pr_info's Andrew Jones
@ 2020-02-14 14:59 ` Andrew Jones
2020-02-14 14:59 ` [PATCH 12/13] KVM: selftests: Introduce vm_guest_mode_params Andrew Jones
` (3 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Andrew Jones @ 2020-02-14 14:59 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, bgardon, borntraeger, frankja, thuth, peterx
We're going to want this name in the library code, so use a shorter
name in the tests.
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
.../selftests/kvm/demand_paging_test.c | 34 +++++++++----------
tools/testing/selftests/kvm/dirty_log_test.c | 34 +++++++++----------
2 files changed, 33 insertions(+), 35 deletions(-)
diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
index 5aae166c2817..a5e57bd63e78 100644
--- a/tools/testing/selftests/kvm/demand_paging_test.c
+++ b/tools/testing/selftests/kvm/demand_paging_test.c
@@ -543,16 +543,14 @@ static void run_test(enum vm_guest_mode mode, bool use_uffd,
}
}
-struct vm_guest_mode_params {
+struct guest_mode {
bool supported;
bool enabled;
};
-struct vm_guest_mode_params vm_guest_mode_params[NUM_VM_MODES];
+static struct guest_mode guest_modes[NUM_VM_MODES];
-#define vm_guest_mode_params_init(mode, supported, enabled) \
-({ \
- vm_guest_mode_params[mode] = \
- (struct vm_guest_mode_params){ supported, enabled }; \
+#define guest_mode_init(mode, supported, enabled) ({ \
+ guest_modes[mode] = (struct guest_mode){ supported, enabled }; \
})
static void help(char *name)
@@ -568,7 +566,7 @@ static void help(char *name)
" Guest mode IDs:\n");
for (i = 0; i < NUM_VM_MODES; ++i) {
printf(" %d: %s%s\n", i, vm_guest_mode_string(i),
- vm_guest_mode_params[i].supported ? " (supported)" : "");
+ guest_modes[i].supported ? " (supported)" : "");
}
printf(" -u: use User Fault FD to handle vCPU page\n"
" faults.\n");
@@ -594,24 +592,24 @@ int main(int argc, char *argv[])
useconds_t uffd_delay = 0;
#ifdef __x86_64__
- vm_guest_mode_params_init(VM_MODE_PXXV48_4K, true, true);
+ guest_mode_init(VM_MODE_PXXV48_4K, true, true);
#endif
#ifdef __aarch64__
- vm_guest_mode_params_init(VM_MODE_P40V48_4K, true, true);
- vm_guest_mode_params_init(VM_MODE_P40V48_64K, true, true);
+ guest_mode_init(VM_MODE_P40V48_4K, true, true);
+ guest_mode_init(VM_MODE_P40V48_64K, true, true);
{
unsigned int limit = kvm_check_cap(KVM_CAP_ARM_VM_IPA_SIZE);
if (limit >= 52)
- vm_guest_mode_params_init(VM_MODE_P52V48_64K, true, true);
+ guest_mode_init(VM_MODE_P52V48_64K, true, true);
if (limit >= 48) {
- vm_guest_mode_params_init(VM_MODE_P48V48_4K, true, true);
- vm_guest_mode_params_init(VM_MODE_P48V48_64K, true, true);
+ guest_mode_init(VM_MODE_P48V48_4K, true, true);
+ guest_mode_init(VM_MODE_P48V48_64K, true, true);
}
}
#endif
#ifdef __s390x__
- vm_guest_mode_params_init(VM_MODE_P40V48_4K, true, true);
+ guest_mode_init(VM_MODE_P40V48_4K, true, true);
#endif
while ((opt = getopt(argc, argv, "hm:ud:b:v:")) != -1) {
@@ -619,13 +617,13 @@ int main(int argc, char *argv[])
case 'm':
if (!mode_selected) {
for (i = 0; i < NUM_VM_MODES; ++i)
- vm_guest_mode_params[i].enabled = false;
+ guest_modes[i].enabled = false;
mode_selected = true;
}
mode = strtoul(optarg, NULL, 10);
TEST_ASSERT(mode < NUM_VM_MODES,
"Guest mode ID %d too big", mode);
- vm_guest_mode_params[mode].enabled = true;
+ guest_modes[mode].enabled = true;
break;
case 'u':
use_uffd = true;
@@ -654,9 +652,9 @@ int main(int argc, char *argv[])
}
for (i = 0; i < NUM_VM_MODES; ++i) {
- if (!vm_guest_mode_params[i].enabled)
+ if (!guest_modes[i].enabled)
continue;
- TEST_ASSERT(vm_guest_mode_params[i].supported,
+ TEST_ASSERT(guest_modes[i].supported,
"Guest mode ID %d (%s) not supported.",
i, vm_guest_mode_string(i));
run_test(i, use_uffd, uffd_delay, vcpus, vcpu_memory_bytes);
diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
index 587edf40cc32..12acf90826c1 100644
--- a/tools/testing/selftests/kvm/dirty_log_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_test.c
@@ -388,15 +388,14 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
kvm_vm_free(vm);
}
-struct vm_guest_mode_params {
+struct guest_mode {
bool supported;
bool enabled;
};
-struct vm_guest_mode_params vm_guest_mode_params[NUM_VM_MODES];
+static struct guest_mode guest_modes[NUM_VM_MODES];
-#define vm_guest_mode_params_init(mode, supported, enabled) \
-({ \
- vm_guest_mode_params[mode] = (struct vm_guest_mode_params){ supported, enabled }; \
+#define guest_mode_init(mode, supported, enabled) ({ \
+ guest_modes[mode] = (struct guest_mode){ supported, enabled }; \
})
static void help(char *name)
@@ -419,7 +418,7 @@ static void help(char *name)
" Guest mode IDs:\n");
for (i = 0; i < NUM_VM_MODES; ++i) {
printf(" %d: %s%s\n", i, vm_guest_mode_string(i),
- vm_guest_mode_params[i].supported ? " (supported)" : "");
+ guest_modes[i].supported ? " (supported)" : "");
}
puts("");
exit(0);
@@ -442,24 +441,25 @@ int main(int argc, char *argv[])
#endif
#ifdef __x86_64__
- vm_guest_mode_params_init(VM_MODE_PXXV48_4K, true, true);
+ guest_mode_init(VM_MODE_PXXV48_4K, true, true);
#endif
#ifdef __aarch64__
- vm_guest_mode_params_init(VM_MODE_P40V48_4K, true, true);
- vm_guest_mode_params_init(VM_MODE_P40V48_64K, true, true);
+ guest_mode_init(VM_MODE_P40V48_4K, true, true);
+ guest_mode_init(VM_MODE_P40V48_64K, true, true);
+
{
unsigned int limit = kvm_check_cap(KVM_CAP_ARM_VM_IPA_SIZE);
if (limit >= 52)
- vm_guest_mode_params_init(VM_MODE_P52V48_64K, true, true);
+ guest_mode_init(VM_MODE_P52V48_64K, true, true);
if (limit >= 48) {
- vm_guest_mode_params_init(VM_MODE_P48V48_4K, true, true);
- vm_guest_mode_params_init(VM_MODE_P48V48_64K, true, true);
+ guest_mode_init(VM_MODE_P48V48_4K, true, true);
+ guest_mode_init(VM_MODE_P48V48_64K, true, true);
}
}
#endif
#ifdef __s390x__
- vm_guest_mode_params_init(VM_MODE_P40V48_4K, true, true);
+ guest_mode_init(VM_MODE_P40V48_4K, true, true);
#endif
while ((opt = getopt(argc, argv, "hi:I:p:m:")) != -1) {
@@ -476,13 +476,13 @@ int main(int argc, char *argv[])
case 'm':
if (!mode_selected) {
for (i = 0; i < NUM_VM_MODES; ++i)
- vm_guest_mode_params[i].enabled = false;
+ guest_modes[i].enabled = false;
mode_selected = true;
}
mode = strtoul(optarg, NULL, 10);
TEST_ASSERT(mode < NUM_VM_MODES,
"Guest mode ID %d too big", mode);
- vm_guest_mode_params[mode].enabled = true;
+ guest_modes[mode].enabled = true;
break;
case 'h':
default:
@@ -500,9 +500,9 @@ int main(int argc, char *argv[])
srandom(time(0));
for (i = 0; i < NUM_VM_MODES; ++i) {
- if (!vm_guest_mode_params[i].enabled)
+ if (!guest_modes[i].enabled)
continue;
- TEST_ASSERT(vm_guest_mode_params[i].supported,
+ TEST_ASSERT(guest_modes[i].supported,
"Guest mode ID %d (%s) not supported.",
i, vm_guest_mode_string(i));
run_test(i, iterations, interval, phys_offset);
--
2.21.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 12/13] KVM: selftests: Introduce vm_guest_mode_params
2020-02-14 14:59 [PATCH 00/13] KVM: selftests: Various fixes and cleanups Andrew Jones
` (10 preceding siblings ...)
2020-02-14 14:59 ` [PATCH 11/13] KVM: selftests: Rename vm_guest_mode_params Andrew Jones
@ 2020-02-14 14:59 ` Andrew Jones
2020-02-14 14:59 ` [PATCH 13/13] KVM: selftests: Introduce num-pages conversion utilities Andrew Jones
` (2 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Andrew Jones @ 2020-02-14 14:59 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, bgardon, borntraeger, frankja, thuth, peterx
This array will allow us to easily translate modes to their parameter
values.
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
tools/testing/selftests/kvm/lib/kvm_util.c | 52 +++++++++++-----------
1 file changed, 25 insertions(+), 27 deletions(-)
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index c8a7ed338bed..5e26e24bd609 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -113,6 +113,25 @@ const char * const vm_guest_mode_string[] = {
_Static_assert(sizeof(vm_guest_mode_string)/sizeof(char *) == NUM_VM_MODES,
"Missing new mode strings?");
+struct vm_guest_mode_params {
+ unsigned int pa_bits;
+ unsigned int va_bits;
+ unsigned int page_size;
+ unsigned int page_shift;
+};
+
+static const struct vm_guest_mode_params vm_guest_mode_params[] = {
+ { 52, 48, 0x1000, 12 },
+ { 52, 48, 0x10000, 16 },
+ { 48, 48, 0x1000, 12 },
+ { 48, 48, 0x10000, 16 },
+ { 40, 48, 0x1000, 12 },
+ { 40, 48, 0x10000, 16 },
+ { 0, 0, 0x1000, 12 },
+};
+_Static_assert(sizeof(vm_guest_mode_params)/sizeof(struct vm_guest_mode_params) == NUM_VM_MODES,
+ "Missing new mode params?");
+
#define KVM_INTERNAL_MEMSLOTS_START_PADDR 0xfe000000ULL
#define KVM_INTERNAL_MEMSLOTS_END_PADDR (4ULL << 30)
/*
@@ -147,60 +166,39 @@ struct kvm_vm *_vm_create(enum vm_guest_mode mode, uint64_t phy_pages, int perm)
vm->mode = mode;
vm->type = 0;
+ vm->pa_bits = vm_guest_mode_params[mode].pa_bits;
+ vm->va_bits = vm_guest_mode_params[mode].va_bits;
+ vm->page_size = vm_guest_mode_params[mode].page_size;
+ vm->page_shift = vm_guest_mode_params[mode].page_shift;
+
/* Setup mode specific traits. */
switch (vm->mode) {
case VM_MODE_P52V48_4K:
vm->pgtable_levels = 4;
- vm->pa_bits = 52;
- vm->va_bits = 48;
- vm->page_size = 0x1000;
- vm->page_shift = 12;
break;
case VM_MODE_P52V48_64K:
vm->pgtable_levels = 3;
- vm->pa_bits = 52;
- vm->va_bits = 48;
- vm->page_size = 0x10000;
- vm->page_shift = 16;
break;
case VM_MODE_P48V48_4K:
vm->pgtable_levels = 4;
- vm->pa_bits = 48;
- vm->va_bits = 48;
- vm->page_size = 0x1000;
- vm->page_shift = 12;
break;
case VM_MODE_P48V48_64K:
vm->pgtable_levels = 3;
- vm->pa_bits = 48;
- vm->va_bits = 48;
- vm->page_size = 0x10000;
- vm->page_shift = 16;
break;
case VM_MODE_P40V48_4K:
vm->pgtable_levels = 4;
- vm->pa_bits = 40;
- vm->va_bits = 48;
- vm->page_size = 0x1000;
- vm->page_shift = 12;
break;
case VM_MODE_P40V48_64K:
vm->pgtable_levels = 3;
- vm->pa_bits = 40;
- vm->va_bits = 48;
- vm->page_size = 0x10000;
- vm->page_shift = 16;
break;
case VM_MODE_PXXV48_4K:
#ifdef __x86_64__
kvm_get_cpu_address_width(&vm->pa_bits, &vm->va_bits);
TEST_ASSERT(vm->va_bits == 48, "Linear address width "
"(%d bits) not supported", vm->va_bits);
- vm->pgtable_levels = 4;
- vm->page_size = 0x1000;
- vm->page_shift = 12;
pr_debug("Guest physical address width detected: %d\n",
vm->pa_bits);
+ vm->pgtable_levels = 4;
#else
TEST_ASSERT(false, "VM_MODE_PXXV48_4K not supported on "
"non-x86 platforms");
--
2.21.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 13/13] KVM: selftests: Introduce num-pages conversion utilities
2020-02-14 14:59 [PATCH 00/13] KVM: selftests: Various fixes and cleanups Andrew Jones
` (11 preceding siblings ...)
2020-02-14 14:59 ` [PATCH 12/13] KVM: selftests: Introduce vm_guest_mode_params Andrew Jones
@ 2020-02-14 14:59 ` Andrew Jones
2020-02-20 16:46 ` Paolo Bonzini
2020-02-14 15:23 ` [PATCH 00/13] KVM: selftests: Various fixes and cleanups Paolo Bonzini
2020-02-14 22:26 ` Peter Xu
14 siblings, 1 reply; 28+ messages in thread
From: Andrew Jones @ 2020-02-14 14:59 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, bgardon, borntraeger, frankja, thuth, peterx
Guests and hosts don't have to have the same page size. This means
calculations are necessary when selecting the number of guest pages
to allocate in order to ensure the number is compatible with the
host. Provide utilities to help with those calculations and apply
them where appropriate.
We also revert commit bffed38d4fb5 ("kvm: selftests: aarch64:
dirty_log_test: fix unaligned memslot size") and then use
vm_adjust_num_guest_pages() there instead.
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
.../selftests/kvm/demand_paging_test.c | 8 ++---
tools/testing/selftests/kvm/dirty_log_test.c | 13 ++++----
.../testing/selftests/kvm/include/kvm_util.h | 8 +++++
.../testing/selftests/kvm/include/test_util.h | 2 ++
tools/testing/selftests/kvm/lib/kvm_util.c | 32 +++++++++++++++++++
5 files changed, 50 insertions(+), 13 deletions(-)
diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
index a5e57bd63e78..a9289a9386c0 100644
--- a/tools/testing/selftests/kvm/demand_paging_test.c
+++ b/tools/testing/selftests/kvm/demand_paging_test.c
@@ -164,12 +164,7 @@ static struct kvm_vm *create_vm(enum vm_guest_mode mode, int vcpus,
pages += (2 * pages) / PTES_PER_4K_PT;
pages += ((2 * vcpus * vcpu_memory_bytes) >> PAGE_SHIFT_4K) /
PTES_PER_4K_PT;
-
- /*
- * If the host is uing 64K pages, then we need the number of 4K
- * guest pages to be a multiple of 16.
- */
- pages += 16 - pages % 16;
+ pages = vm_adjust_num_guest_pages(mode, pages);
pr_info("Testing guest mode: %s\n", vm_guest_mode_string(mode));
@@ -382,6 +377,7 @@ static void run_test(enum vm_guest_mode mode, bool use_uffd,
"Guest memory size is not guest page size aligned.");
guest_num_pages = (vcpus * vcpu_memory_bytes) / guest_page_size;
+ guest_num_pages = vm_adjust_num_guest_pages(mode, guest_num_pages);
#ifdef __s390x__
/* Round up to multiple of 1M (segment size) */
diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
index 12acf90826c1..a723333b138a 100644
--- a/tools/testing/selftests/kvm/dirty_log_test.c
+++ b/tools/testing/selftests/kvm/dirty_log_test.c
@@ -178,12 +178,11 @@ static void *vcpu_worker(void *data)
return NULL;
}
-static void vm_dirty_log_verify(unsigned long *bmap)
+static void vm_dirty_log_verify(enum vm_guest_mode mode, unsigned long *bmap)
{
+ uint64_t step = vm_num_host_pages(mode, 1);
uint64_t page;
uint64_t *value_ptr;
- uint64_t step = host_page_size >= guest_page_size ? 1 :
- guest_page_size / host_page_size;
for (page = 0; page < host_num_pages; page += step) {
value_ptr = host_test_mem + page * host_page_size;
@@ -291,14 +290,14 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
* case where the size is not aligned to 64 pages.
*/
guest_num_pages = (1ul << (DIRTY_MEM_BITS -
- vm_get_page_shift(vm))) + 16;
+ vm_get_page_shift(vm))) + 3;
+ guest_num_pages = vm_adjust_num_guest_pages(mode, guest_num_pages);
#ifdef __s390x__
/* Round up to multiple of 1M (segment size) */
guest_num_pages = (guest_num_pages + 0xff) & ~0xffUL;
#endif
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);
+ host_num_pages = vm_num_host_pages(mode, guest_num_pages);
if (!phys_offset) {
guest_test_phys_mem = (vm_get_max_gfn(vm) -
@@ -369,7 +368,7 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
kvm_vm_clear_dirty_log(vm, TEST_MEM_SLOT_INDEX, bmap, 0,
host_num_pages);
#endif
- vm_dirty_log_verify(bmap);
+ vm_dirty_log_verify(mode, bmap);
iteration++;
sync_global_to_guest(vm, iteration);
}
diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
index 45c6c7ea24c5..bc7c67913fe0 100644
--- a/tools/testing/selftests/kvm/include/kvm_util.h
+++ b/tools/testing/selftests/kvm/include/kvm_util.h
@@ -158,6 +158,14 @@ unsigned int vm_get_page_size(struct kvm_vm *vm);
unsigned int vm_get_page_shift(struct kvm_vm *vm);
unsigned int vm_get_max_gfn(struct kvm_vm *vm);
+unsigned int vm_num_host_pages(enum vm_guest_mode mode, unsigned int num_guest_pages);
+unsigned int vm_num_guest_pages(enum vm_guest_mode mode, unsigned int num_host_pages);
+static inline unsigned int
+vm_adjust_num_guest_pages(enum vm_guest_mode mode, unsigned int num_guest_pages)
+{
+ return vm_num_guest_pages(mode, vm_num_host_pages(mode, num_guest_pages));
+}
+
struct kvm_userspace_memory_region *
kvm_userspace_memory_region_find(struct kvm_vm *vm, uint64_t start,
uint64_t end);
diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
index c921ea719ae0..a60cf4ffcc3b 100644
--- a/tools/testing/selftests/kvm/include/test_util.h
+++ b/tools/testing/selftests/kvm/include/test_util.h
@@ -19,6 +19,8 @@
#include <fcntl.h>
#include "kselftest.h"
+#define getpageshift() (__builtin_ffs(getpagesize()) - 1)
+
static inline int _no_printf(const char *format, ...) { return 0; }
#ifdef DEBUG
diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
index 5e26e24bd609..44f1ef064085 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -583,6 +583,10 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
size_t huge_page_size = KVM_UTIL_PGS_PER_HUGEPG * vm->page_size;
size_t alignment;
+ TEST_ASSERT(vm_adjust_num_guest_pages(vm->mode, npages) == npages,
+ "Number of guest pages is not compatible with the host. "
+ "Try npages=%d", vm_adjust_num_guest_pages(vm->mode, npages));
+
TEST_ASSERT((guest_paddr % vm->page_size) == 0, "Guest physical "
"address not on a page boundary.\n"
" guest_paddr: 0x%lx vm->page_size: 0x%x",
@@ -1718,3 +1722,31 @@ unsigned int vm_get_max_gfn(struct kvm_vm *vm)
{
return vm->max_gfn;
}
+
+static unsigned int vm_calc_num_pages(unsigned int num_pages,
+ unsigned int page_shift,
+ unsigned int new_page_shift,
+ bool ceil)
+{
+ unsigned int n = 1 << (new_page_shift - page_shift);
+
+ if (page_shift >= new_page_shift)
+ return num_pages * (1 << (page_shift - new_page_shift));
+
+ return num_pages / n + !!(ceil && num_pages % n);
+}
+
+unsigned int
+vm_num_host_pages(enum vm_guest_mode mode, unsigned int num_guest_pages)
+{
+ return vm_calc_num_pages(num_guest_pages,
+ vm_guest_mode_params[mode].page_shift,
+ getpageshift(), true);
+}
+
+unsigned int
+vm_num_guest_pages(enum vm_guest_mode mode, unsigned int num_host_pages)
+{
+ return vm_calc_num_pages(num_host_pages, getpageshift(),
+ vm_guest_mode_params[mode].page_shift, false);
+}
--
2.21.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 13/13] KVM: selftests: Introduce num-pages conversion utilities
2020-02-14 14:59 ` [PATCH 13/13] KVM: selftests: Introduce num-pages conversion utilities Andrew Jones
@ 2020-02-20 16:46 ` Paolo Bonzini
0 siblings, 0 replies; 28+ messages in thread
From: Paolo Bonzini @ 2020-02-20 16:46 UTC (permalink / raw)
To: Andrew Jones, kvm; +Cc: bgardon, borntraeger, frankja, thuth, peterx
On 14/02/20 15:59, Andrew Jones wrote:
> Guests and hosts don't have to have the same page size. This means
> calculations are necessary when selecting the number of guest pages
> to allocate in order to ensure the number is compatible with the
> host. Provide utilities to help with those calculations and apply
> them where appropriate.
>
> We also revert commit bffed38d4fb5 ("kvm: selftests: aarch64:
> dirty_log_test: fix unaligned memslot size") and then use
> vm_adjust_num_guest_pages() there instead.
>
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
> .../selftests/kvm/demand_paging_test.c | 8 ++---
> tools/testing/selftests/kvm/dirty_log_test.c | 13 ++++----
> .../testing/selftests/kvm/include/kvm_util.h | 8 +++++
> .../testing/selftests/kvm/include/test_util.h | 2 ++
> tools/testing/selftests/kvm/lib/kvm_util.c | 32 +++++++++++++++++++
> 5 files changed, 50 insertions(+), 13 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/demand_paging_test.c b/tools/testing/selftests/kvm/demand_paging_test.c
> index a5e57bd63e78..a9289a9386c0 100644
> --- a/tools/testing/selftests/kvm/demand_paging_test.c
> +++ b/tools/testing/selftests/kvm/demand_paging_test.c
> @@ -164,12 +164,7 @@ static struct kvm_vm *create_vm(enum vm_guest_mode mode, int vcpus,
> pages += (2 * pages) / PTES_PER_4K_PT;
> pages += ((2 * vcpus * vcpu_memory_bytes) >> PAGE_SHIFT_4K) /
> PTES_PER_4K_PT;
> -
> - /*
> - * If the host is uing 64K pages, then we need the number of 4K
> - * guest pages to be a multiple of 16.
> - */
> - pages += 16 - pages % 16;
> + pages = vm_adjust_num_guest_pages(mode, pages);
>
> pr_info("Testing guest mode: %s\n", vm_guest_mode_string(mode));
>
> @@ -382,6 +377,7 @@ static void run_test(enum vm_guest_mode mode, bool use_uffd,
> "Guest memory size is not guest page size aligned.");
>
> guest_num_pages = (vcpus * vcpu_memory_bytes) / guest_page_size;
> + guest_num_pages = vm_adjust_num_guest_pages(mode, guest_num_pages);
>
> #ifdef __s390x__
> /* Round up to multiple of 1M (segment size) */
Moved this part to "fixup! KVM: selftests: Support multiple vCPUs in
demand paging test".
Paolo
> diff --git a/tools/testing/selftests/kvm/dirty_log_test.c b/tools/testing/selftests/kvm/dirty_log_test.c
> index 12acf90826c1..a723333b138a 100644
> --- a/tools/testing/selftests/kvm/dirty_log_test.c
> +++ b/tools/testing/selftests/kvm/dirty_log_test.c
> @@ -178,12 +178,11 @@ static void *vcpu_worker(void *data)
> return NULL;
> }
>
> -static void vm_dirty_log_verify(unsigned long *bmap)
> +static void vm_dirty_log_verify(enum vm_guest_mode mode, unsigned long *bmap)
> {
> + uint64_t step = vm_num_host_pages(mode, 1);
> uint64_t page;
> uint64_t *value_ptr;
> - uint64_t step = host_page_size >= guest_page_size ? 1 :
> - guest_page_size / host_page_size;
>
> for (page = 0; page < host_num_pages; page += step) {
> value_ptr = host_test_mem + page * host_page_size;
> @@ -291,14 +290,14 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
> * case where the size is not aligned to 64 pages.
> */
> guest_num_pages = (1ul << (DIRTY_MEM_BITS -
> - vm_get_page_shift(vm))) + 16;
> + vm_get_page_shift(vm))) + 3;
> + guest_num_pages = vm_adjust_num_guest_pages(mode, guest_num_pages);
> #ifdef __s390x__
> /* Round up to multiple of 1M (segment size) */
> guest_num_pages = (guest_num_pages + 0xff) & ~0xffUL;
> #endif
> 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);
> + host_num_pages = vm_num_host_pages(mode, guest_num_pages);
>
> if (!phys_offset) {
> guest_test_phys_mem = (vm_get_max_gfn(vm) -
> @@ -369,7 +368,7 @@ static void run_test(enum vm_guest_mode mode, unsigned long iterations,
> kvm_vm_clear_dirty_log(vm, TEST_MEM_SLOT_INDEX, bmap, 0,
> host_num_pages);
> #endif
> - vm_dirty_log_verify(bmap);
> + vm_dirty_log_verify(mode, bmap);
> iteration++;
> sync_global_to_guest(vm, iteration);
> }
> diff --git a/tools/testing/selftests/kvm/include/kvm_util.h b/tools/testing/selftests/kvm/include/kvm_util.h
> index 45c6c7ea24c5..bc7c67913fe0 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util.h
> @@ -158,6 +158,14 @@ unsigned int vm_get_page_size(struct kvm_vm *vm);
> unsigned int vm_get_page_shift(struct kvm_vm *vm);
> unsigned int vm_get_max_gfn(struct kvm_vm *vm);
>
> +unsigned int vm_num_host_pages(enum vm_guest_mode mode, unsigned int num_guest_pages);
> +unsigned int vm_num_guest_pages(enum vm_guest_mode mode, unsigned int num_host_pages);
> +static inline unsigned int
> +vm_adjust_num_guest_pages(enum vm_guest_mode mode, unsigned int num_guest_pages)
> +{
> + return vm_num_guest_pages(mode, vm_num_host_pages(mode, num_guest_pages));
> +}
> +
> struct kvm_userspace_memory_region *
> kvm_userspace_memory_region_find(struct kvm_vm *vm, uint64_t start,
> uint64_t end);
> diff --git a/tools/testing/selftests/kvm/include/test_util.h b/tools/testing/selftests/kvm/include/test_util.h
> index c921ea719ae0..a60cf4ffcc3b 100644
> --- a/tools/testing/selftests/kvm/include/test_util.h
> +++ b/tools/testing/selftests/kvm/include/test_util.h
> @@ -19,6 +19,8 @@
> #include <fcntl.h>
> #include "kselftest.h"
>
> +#define getpageshift() (__builtin_ffs(getpagesize()) - 1)
> +
> static inline int _no_printf(const char *format, ...) { return 0; }
>
> #ifdef DEBUG
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index 5e26e24bd609..44f1ef064085 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -583,6 +583,10 @@ void vm_userspace_mem_region_add(struct kvm_vm *vm,
> size_t huge_page_size = KVM_UTIL_PGS_PER_HUGEPG * vm->page_size;
> size_t alignment;
>
> + TEST_ASSERT(vm_adjust_num_guest_pages(vm->mode, npages) == npages,
> + "Number of guest pages is not compatible with the host. "
> + "Try npages=%d", vm_adjust_num_guest_pages(vm->mode, npages));
> +
> TEST_ASSERT((guest_paddr % vm->page_size) == 0, "Guest physical "
> "address not on a page boundary.\n"
> " guest_paddr: 0x%lx vm->page_size: 0x%x",
> @@ -1718,3 +1722,31 @@ unsigned int vm_get_max_gfn(struct kvm_vm *vm)
> {
> return vm->max_gfn;
> }
> +
> +static unsigned int vm_calc_num_pages(unsigned int num_pages,
> + unsigned int page_shift,
> + unsigned int new_page_shift,
> + bool ceil)
> +{
> + unsigned int n = 1 << (new_page_shift - page_shift);
> +
> + if (page_shift >= new_page_shift)
> + return num_pages * (1 << (page_shift - new_page_shift));
> +
> + return num_pages / n + !!(ceil && num_pages % n);
> +}
> +
> +unsigned int
> +vm_num_host_pages(enum vm_guest_mode mode, unsigned int num_guest_pages)
> +{
> + return vm_calc_num_pages(num_guest_pages,
> + vm_guest_mode_params[mode].page_shift,
> + getpageshift(), true);
> +}
> +
> +unsigned int
> +vm_num_guest_pages(enum vm_guest_mode mode, unsigned int num_host_pages)
> +{
> + return vm_calc_num_pages(num_host_pages, getpageshift(),
> + vm_guest_mode_params[mode].page_shift, false);
> +}
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 00/13] KVM: selftests: Various fixes and cleanups
2020-02-14 14:59 [PATCH 00/13] KVM: selftests: Various fixes and cleanups Andrew Jones
` (12 preceding siblings ...)
2020-02-14 14:59 ` [PATCH 13/13] KVM: selftests: Introduce num-pages conversion utilities Andrew Jones
@ 2020-02-14 15:23 ` Paolo Bonzini
2020-02-15 7:04 ` Andrew Jones
2020-02-14 22:26 ` Peter Xu
14 siblings, 1 reply; 28+ messages in thread
From: Paolo Bonzini @ 2020-02-14 15:23 UTC (permalink / raw)
To: Andrew Jones, kvm; +Cc: bgardon, borntraeger, frankja, thuth, peterx
On 14/02/20 15:59, Andrew Jones wrote:
> This series has several parts:
>
> * First, a hack to get x86 to compile. The missing __NR_userfaultfd
> define should be fixed a better way.
>
> * Then, fixups for several commits in kvm/queue. These fixups correspond
> to review comments that didn't have a chance to get addressed before
> the commits were applied.
Right, that's why they're in kvm/queue only. :) Did you test this
series on aarch64?
Paolo
>
> * Next, a few unnecessary #define/#ifdef deletions.
>
> * Then, a rework of debug and info message printing.
>
> * Finally, an addition to the API, num-pages conversion utilities,
> which cleans up all the num-pages calculations.
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 00/13] KVM: selftests: Various fixes and cleanups
2020-02-14 15:23 ` [PATCH 00/13] KVM: selftests: Various fixes and cleanups Paolo Bonzini
@ 2020-02-15 7:04 ` Andrew Jones
0 siblings, 0 replies; 28+ messages in thread
From: Andrew Jones @ 2020-02-15 7:04 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm, bgardon, borntraeger, frankja, thuth, peterx
On Fri, Feb 14, 2020 at 04:23:56PM +0100, Paolo Bonzini wrote:
> On 14/02/20 15:59, Andrew Jones wrote:
> > This series has several parts:
> >
> > * First, a hack to get x86 to compile. The missing __NR_userfaultfd
> > define should be fixed a better way.
> >
> > * Then, fixups for several commits in kvm/queue. These fixups correspond
> > to review comments that didn't have a chance to get addressed before
> > the commits were applied.
>
> Right, that's why they're in kvm/queue only. :) Did you test this
> series on aarch64?
Yup
>
> Paolo
>
> >
> > * Next, a few unnecessary #define/#ifdef deletions.
> >
> > * Then, a rework of debug and info message printing.
> >
> > * Finally, an addition to the API, num-pages conversion utilities,
> > which cleans up all the num-pages calculations.
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 00/13] KVM: selftests: Various fixes and cleanups
2020-02-14 14:59 [PATCH 00/13] KVM: selftests: Various fixes and cleanups Andrew Jones
` (13 preceding siblings ...)
2020-02-14 15:23 ` [PATCH 00/13] KVM: selftests: Various fixes and cleanups Paolo Bonzini
@ 2020-02-14 22:26 ` Peter Xu
2020-02-15 7:07 ` Andrew Jones
14 siblings, 1 reply; 28+ messages in thread
From: Peter Xu @ 2020-02-14 22:26 UTC (permalink / raw)
To: Andrew Jones; +Cc: kvm, pbonzini, bgardon, borntraeger, frankja, thuth
On Fri, Feb 14, 2020 at 03:59:07PM +0100, Andrew Jones wrote:
> This series has several parts:
>
> * First, a hack to get x86 to compile. The missing __NR_userfaultfd
> define should be fixed a better way.
Yeh otherwise I think it will only compile on x86_64.
My gut feeling is we've got an artificial unistd_{32|64}.h under tools
that is included rather than the real one that we should include
(which should locate under $LINUX_ROOT/usr/include/asm/). Below patch
worked for me, but I'm not 100% sure whether I fixed all the current
users of that artifact header just in case I'll break some (what I saw
is only this evsel.c and another setns.c, while that setns.c has
syscall.h included correct so it seems fine):
diff --git a/tools/arch/x86/include/asm/unistd_32.h b/tools/arch/x86/include/asm/unistd_32.h
deleted file mode 100644
index 60a89dba01b6..000000000000
--- a/tools/arch/x86/include/asm/unistd_32.h
+++ /dev/null
@@ -1,16 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef __NR_perf_event_open
-# define __NR_perf_event_open 336
-#endif
-#ifndef __NR_futex
-# define __NR_futex 240
-#endif
-#ifndef __NR_gettid
-# define __NR_gettid 224
-#endif
-#ifndef __NR_getcpu
-# define __NR_getcpu 318
-#endif
-#ifndef __NR_setns
-# define __NR_setns 346
-#endif
diff --git a/tools/arch/x86/include/asm/unistd_64.h b/tools/arch/x86/include/asm/unistd_64.h
deleted file mode 100644
index cb52a3a8b8fc..000000000000
--- a/tools/arch/x86/include/asm/unistd_64.h
+++ /dev/null
@@ -1,16 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef __NR_perf_event_open
-# define __NR_perf_event_open 298
-#endif
-#ifndef __NR_futex
-# define __NR_futex 202
-#endif
-#ifndef __NR_gettid
-# define __NR_gettid 186
-#endif
-#ifndef __NR_getcpu
-# define __NR_getcpu 309
-#endif
-#ifndef __NR_setns
-#define __NR_setns 308
-#endif
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index a69e64236120..f4075392dcb6 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -21,6 +21,7 @@
#include <sys/ioctl.h>
#include <sys/resource.h>
#include <sys/types.h>
+#include <sys/syscall.h>
#include <dirent.h>
#include <stdlib.h>
#include <perf/evsel.h>
--
Peter Xu
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 00/13] KVM: selftests: Various fixes and cleanups
2020-02-14 22:26 ` Peter Xu
@ 2020-02-15 7:07 ` Andrew Jones
2020-02-15 19:11 ` Peter Xu
0 siblings, 1 reply; 28+ messages in thread
From: Andrew Jones @ 2020-02-15 7:07 UTC (permalink / raw)
To: Peter Xu; +Cc: kvm, pbonzini, bgardon, borntraeger, frankja, thuth
On Fri, Feb 14, 2020 at 05:26:39PM -0500, Peter Xu wrote:
> On Fri, Feb 14, 2020 at 03:59:07PM +0100, Andrew Jones wrote:
> > This series has several parts:
> >
> > * First, a hack to get x86 to compile. The missing __NR_userfaultfd
> > define should be fixed a better way.
>
> Yeh otherwise I think it will only compile on x86_64.
The opposite for me. I could compile on AArch64 without this hack, but on
x86 (my Fedora 30 laptop) I could not.
>
> My gut feeling is we've got an artificial unistd_{32|64}.h under tools
> that is included rather than the real one that we should include
> (which should locate under $LINUX_ROOT/usr/include/asm/). Below patch
> worked for me, but I'm not 100% sure whether I fixed all the current
> users of that artifact header just in case I'll break some (what I saw
> is only this evsel.c and another setns.c, while that setns.c has
> syscall.h included correct so it seems fine):
Yeah, there's something strange about it because I saw the definition in
the tools includes.
Thanks,
drew
>
> diff --git a/tools/arch/x86/include/asm/unistd_32.h b/tools/arch/x86/include/asm/unistd_32.h
> deleted file mode 100644
> index 60a89dba01b6..000000000000
> --- a/tools/arch/x86/include/asm/unistd_32.h
> +++ /dev/null
> @@ -1,16 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -#ifndef __NR_perf_event_open
> -# define __NR_perf_event_open 336
> -#endif
> -#ifndef __NR_futex
> -# define __NR_futex 240
> -#endif
> -#ifndef __NR_gettid
> -# define __NR_gettid 224
> -#endif
> -#ifndef __NR_getcpu
> -# define __NR_getcpu 318
> -#endif
> -#ifndef __NR_setns
> -# define __NR_setns 346
> -#endif
> diff --git a/tools/arch/x86/include/asm/unistd_64.h b/tools/arch/x86/include/asm/unistd_64.h
> deleted file mode 100644
> index cb52a3a8b8fc..000000000000
> --- a/tools/arch/x86/include/asm/unistd_64.h
> +++ /dev/null
> @@ -1,16 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0 */
> -#ifndef __NR_perf_event_open
> -# define __NR_perf_event_open 298
> -#endif
> -#ifndef __NR_futex
> -# define __NR_futex 202
> -#endif
> -#ifndef __NR_gettid
> -# define __NR_gettid 186
> -#endif
> -#ifndef __NR_getcpu
> -# define __NR_getcpu 309
> -#endif
> -#ifndef __NR_setns
> -#define __NR_setns 308
> -#endif
> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index a69e64236120..f4075392dcb6 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -21,6 +21,7 @@
> #include <sys/ioctl.h>
> #include <sys/resource.h>
> #include <sys/types.h>
> +#include <sys/syscall.h>
> #include <dirent.h>
> #include <stdlib.h>
> #include <perf/evsel.h>
>
> --
> Peter Xu
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 00/13] KVM: selftests: Various fixes and cleanups
2020-02-15 7:07 ` Andrew Jones
@ 2020-02-15 19:11 ` Peter Xu
0 siblings, 0 replies; 28+ messages in thread
From: Peter Xu @ 2020-02-15 19:11 UTC (permalink / raw)
To: Andrew Jones; +Cc: kvm, pbonzini, bgardon, borntraeger, frankja, thuth
On Sat, Feb 15, 2020 at 08:07:52AM +0100, Andrew Jones wrote:
> On Fri, Feb 14, 2020 at 05:26:39PM -0500, Peter Xu wrote:
> > On Fri, Feb 14, 2020 at 03:59:07PM +0100, Andrew Jones wrote:
> > > This series has several parts:
> > >
> > > * First, a hack to get x86 to compile. The missing __NR_userfaultfd
> > > define should be fixed a better way.
> >
> > Yeh otherwise I think it will only compile on x86_64.
>
> The opposite for me. I could compile on AArch64 without this hack, but on
> x86 (my Fedora 30 laptop) I could not.
Ah, then probably because ARM does not have that artificial unisted*.h
defined (tools/arch/x86/include/asm/unistd_{32|64}.h for x86), then
<sys/syscall.h> can find the correct headers.
And I have said it wrong above... with patch 1 compilation should
always work, but IIUC the syscall number will be wrong on 32bit
systems. Maybe we still need the other solution to make the test
runnable on all platforms by removing those two artificial headers.
Thanks,
--
Peter Xu
^ permalink raw reply [flat|nested] 28+ messages in thread