| Message ID | 20200214145920.30792-1-drjones@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | KVM: selftests: Various fixes and cleanups | expand |
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.
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>
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. >
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 >
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,
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. * 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. Andrew Jones (13): HACK: Ensure __NR_userfaultfd is defined fixup! KVM: selftests: Add support for vcpu_args_set to aarch64 and s390x fixup! KVM: selftests: Support multiple vCPUs in demand paging test fixup! KVM: selftests: Add memory size parameter to the demand paging test fixup! KVM: selftests: Time guest demand paging KVM: selftests: Remove unnecessary defines KVM: selftests: aarch64: Remove unnecessary ifdefs KVM: selftests: aarch64: Use stream when given KVM: selftests: Rework debug message printing KVM: selftests: Convert some printf's to pr_info's KVM: selftests: Rename vm_guest_mode_params KVM: selftests: Introduce vm_guest_mode_params KVM: selftests: Introduce num-pages conversion utilities .../selftests/kvm/demand_paging_test.c | 148 ++++++++---------- tools/testing/selftests/kvm/dirty_log_test.c | 78 +++++---- .../testing/selftests/kvm/include/kvm_util.h | 14 +- .../testing/selftests/kvm/include/test_util.h | 18 +++ .../selftests/kvm/kvm_create_max_vcpus.c | 8 +- .../selftests/kvm/lib/aarch64/processor.c | 30 +--- tools/testing/selftests/kvm/lib/kvm_util.c | 89 +++++++---- .../selftests/kvm/lib/kvm_util_internal.h | 11 -- tools/testing/selftests/kvm/lib/test_util.c | 90 ++++++----- tools/testing/selftests/kvm/s390x/resets.c | 6 +- .../selftests/kvm/x86_64/mmio_warning_test.c | 2 +- tools/testing/selftests/kvm/x86_64/smm_test.c | 2 +- .../testing/selftests/kvm/x86_64/state_test.c | 2 +- .../kvm/x86_64/vmx_tsc_adjust_test.c | 4 +- .../selftests/kvm/x86_64/xss_msr_test.c | 2 +- 15 files changed, 258 insertions(+), 246 deletions(-)