[kvm-unit-tests PULL 00/12] CI-related fixes and improvements

[kvm-unit-tests PULL 00/12] CI-related fixes and improvements

[Thomas Huth]

 Hi Paolo,

the following changes since commit 4b74c718c57d1697e3228e2c699b0fe9c1d24e97:

  x86/pmu: Fix compilation on 32-bit hosts (2020-06-16 08:29:34 -0400)

are available in the Git repository at:

  https://gitlab.com/huth/kvm-unit-tests.git tags/pull-request-2020-06-16

for you to fetch changes up to 9d7f9a2f0b3427bbeda242c64f698d14e21d2a42:

  s390x: stsi: Make output tap13 compatible (2020-06-16 20:22:14 +0200)

----------------------------------------------------------------
* Lots of CI-related fixes and improvements
* Update the gitlab-CI to Fedora 32
* Test compilation with Clang
----------------------------------------------------------------

Andrew Jones (1):
      Fix out-of-tree builds

Bill Wendling (2):
      x86: use a non-negative number in shift
      x86: use inline asm to retrieve stack pointer

Janosch Frank (1):
      s390x: stsi: Make output tap13 compatible

Paolo Bonzini (2):
      x86: avoid multiple defined symbol
      x86: disable SSE on 32-bit hosts

Thomas Huth (6):
      Fixes for the umip test
      Always compile the kvm-unit-tests with -fno-common
      Fix powerpc issue with the linker from Fedora 32
      Update the gitlab-ci to Fedora 32
      vmx_tests: Silence warning from Clang
      Compile the kvm-unit-tests also with Clang

 .gitlab-ci.yml       | 17 +++++++++++++++--
 Makefile             |  2 +-
 configure            |  8 +++-----
 lib/auxinfo.h        |  3 +--
 lib/x86/fault_test.c |  2 +-
 lib/x86/usermode.c   |  2 +-
 powerpc/flat.lds     | 19 ++++++++++++++++---
 s390x/stsi.c         |  6 +++---
 x86/Makefile.common  |  1 +
 x86/Makefile.i386    |  1 +
 x86/svm_tests.c      |  2 +-
 x86/umip.c           |  6 ++++--
 x86/vmx_tests.c      | 10 +++++++---
 13 files changed, 55 insertions(+), 24 deletions(-)

[kvm-unit-tests PULL 01/12] Fix out-of-tree builds

[Thomas Huth]

From: Andrew Jones <drjones@redhat.com>

Since b16df9ee5f3b out-of-tree builds have been broken because we
started validating the newly user-configurable $erratatxt file
before linking it into the build dir. We fix this not by moving
the validation, but by removing the linking and instead using the
full path of the $erratatxt file. This allows one to keep that file
separate from the src and build dirs.

Fixes: b16df9ee5f3b ("arch-run: Add reserved variables to the default environ")
Reported-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Andrew Jones <drjones@redhat.com>
Message-Id: <20200511070641.23492-1-drjones@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 configure | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/configure b/configure
index 5d2cd90..f9d030f 100755
--- a/configure
+++ b/configure
@@ -17,7 +17,7 @@ environ_default=yes
 u32_long=
 vmm="qemu"
 errata_force=0
-erratatxt="errata.txt"
+erratatxt="$srcdir/errata.txt"
 
 usage() {
     cat <<-EOF
@@ -89,7 +89,8 @@ while [[ "$1" = -* ]]; do
 	    environ_default=no
 	    ;;
 	--erratatxt)
-	    erratatxt="$arg"
+	    erratatxt=
+	    [ "$arg" ] && erratatxt=$(eval realpath "$arg")
 	    ;;
 	--help)
 	    usage
@@ -169,9 +170,6 @@ if test ! -e Makefile; then
 
     echo "linking scripts..."
     ln -sf "$srcdir/scripts"
-
-    echo "linking errata.txt..."
-    ln -sf "$srcdir/errata.txt"
 fi
 
 # link lib/asm for the architecture
-- 
2.18.1

[kvm-unit-tests PULL 02/12] x86: avoid multiple defined symbol

[Thomas Huth]

From: Paolo Bonzini <pbonzini@redhat.com>

Fedora 32 croaks about a symbol that is defined twice, fix it.

Reported-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20200511165959.42442-1-pbonzini@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 lib/x86/fault_test.c | 2 +-
 lib/x86/usermode.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/x86/fault_test.c b/lib/x86/fault_test.c
index 078dae3..e15a218 100644
--- a/lib/x86/fault_test.c
+++ b/lib/x86/fault_test.c
@@ -1,6 +1,6 @@
 #include "fault_test.h"
 
-jmp_buf jmpbuf;
+static jmp_buf jmpbuf;
 
 static void restore_exec_to_jmpbuf(void)
 {
diff --git a/lib/x86/usermode.c b/lib/x86/usermode.c
index f01ad9b..f032523 100644
--- a/lib/x86/usermode.c
+++ b/lib/x86/usermode.c
@@ -14,7 +14,7 @@
 #define USERMODE_STACK_SIZE	0x2000
 #define RET_TO_KERNEL_IRQ	0x20
 
-jmp_buf jmpbuf;
+static jmp_buf jmpbuf;
 
 static void restore_exec_to_jmpbuf(void)
 {
-- 
2.18.1

[kvm-unit-tests PULL 03/12] Fixes for the umip test

[Thomas Huth]

When compiling umip.c with -O2 instead of -O1, there are currently
two problems. First, the compiler complains:

 x86/umip.c: In function ‘do_ring3’:
 x86/umip.c:162:37: error: array subscript 4096 is above array bounds of
    ‘unsigned char[4096]’ [-Werror=array-bounds]
       [user_stack_top]"m"(user_stack[sizeof user_stack]),
                           ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~

This can be fixed by initializing the stack to point to one of the last
bytes of the array instead.

The second problem is that some tests are failing - and this is due
to the fact that the GP_ASM macro uses inline asm without the "volatile"
keyword - so that the compiler reorders this code in certain cases
where it should not. Fix it by adding "volatile" here.

Message-Id: <20200122160944.29750-1-thuth@redhat.com>
Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 x86/umip.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/x86/umip.c b/x86/umip.c
index afb373d..c5700b3 100644
--- a/x86/umip.c
+++ b/x86/umip.c
@@ -22,7 +22,8 @@ static void gp_handler(struct ex_regs *regs)
 
 
 #define GP_ASM(stmt, in, clobber)                  \
-     asm ("mov" W " $1f, %[expected_rip]\n\t"      \
+    asm volatile (                                 \
+          "mov" W " $1f, %[expected_rip]\n\t"      \
           "movl $2f-1f, %[skip_count]\n\t"         \
           "1: " stmt "\n\t"                        \
           "2: "                                    \
@@ -159,7 +160,8 @@ static int do_ring3(void (*fn)(const char *), const char *arg)
 		  : [ret] "=&a" (ret)
 		  : [user_ds] "i" (USER_DS),
 		    [user_cs] "i" (USER_CS),
-		    [user_stack_top]"m"(user_stack[sizeof user_stack]),
+		    [user_stack_top]"m"(user_stack[sizeof(user_stack) -
+						   sizeof(long)]),
 		    [fn]"r"(fn),
 		    [arg]"D"(arg),
 		    [kernel_ds]"i"(KERNEL_DS),
-- 
2.18.1

[kvm-unit-tests PULL 04/12] Always compile the kvm-unit-tests with -fno-common

[Thomas Huth]

The new GCC v10 uses -fno-common by default. To avoid that we commit
code that declares global variables twice and thus fails to link with
the latest version, we should also compile with -fno-common when using
older versions of the compiler. However, this now also means that we
can not play the trick with the common auxinfo struct anymore. Thus
declare it as extern in the header now and link auxinfo.c on x86, too.

Message-Id: <20200514192626.9950-6-thuth@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 Makefile            | 2 +-
 lib/auxinfo.h       | 3 +--
 x86/Makefile.common | 1 +
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 754ed65..3ff2f91 100644
--- a/Makefile
+++ b/Makefile
@@ -49,7 +49,7 @@ include $(SRCDIR)/$(TEST_DIR)/Makefile
 cc-option = $(shell if $(CC) -Werror $(1) -S -o /dev/null -xc /dev/null \
               > /dev/null 2>&1; then echo "$(1)"; else echo "$(2)"; fi ;)
 
-COMMON_CFLAGS += -g $(autodepend-flags) -fno-strict-aliasing
+COMMON_CFLAGS += -g $(autodepend-flags) -fno-strict-aliasing -fno-common
 COMMON_CFLAGS += -Wall -Wwrite-strings -Wempty-body -Wuninitialized
 COMMON_CFLAGS += -Wignored-qualifiers -Werror
 
diff --git a/lib/auxinfo.h b/lib/auxinfo.h
index 08b96f8..a46a1e6 100644
--- a/lib/auxinfo.h
+++ b/lib/auxinfo.h
@@ -13,7 +13,6 @@ struct auxinfo {
 	unsigned long flags;
 };
 
-/* No extern!  Define a common symbol.  */
-struct auxinfo auxinfo;
+extern struct auxinfo auxinfo;
 #endif
 #endif
diff --git a/x86/Makefile.common b/x86/Makefile.common
index ab67ca0..2ea9c9f 100644
--- a/x86/Makefile.common
+++ b/x86/Makefile.common
@@ -5,6 +5,7 @@ all: directories test_cases
 cflatobjs += lib/pci.o
 cflatobjs += lib/pci-edu.o
 cflatobjs += lib/alloc.o
+cflatobjs += lib/auxinfo.o
 cflatobjs += lib/vmalloc.o
 cflatobjs += lib/alloc_page.o
 cflatobjs += lib/alloc_phys.o
-- 
2.18.1

[kvm-unit-tests PULL 05/12] Fix powerpc issue with the linker from Fedora 32

[Thomas Huth]

The linker from Fedora 32 complains:

powerpc64-linux-gnu-ld: powerpc/selftest.elf: error: PHDR segment not
 covered by LOAD segment

Let's introduce some fake PHDRs to the linker script to get this
working again.

Message-Id: <20200514192626.9950-7-thuth@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 powerpc/flat.lds | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/powerpc/flat.lds b/powerpc/flat.lds
index 53221e8..5eed368 100644
--- a/powerpc/flat.lds
+++ b/powerpc/flat.lds
@@ -1,7 +1,17 @@
 
+PHDRS
+{
+    text PT_LOAD FLAGS(5);
+    data PT_LOAD FLAGS(6);
+}
+
 SECTIONS
 {
-    .text : { *(.init) *(.text) *(.text.*) }
+    .text : {
+        *(.init)
+        *(.text)
+        *(.text.*)
+    } :text
     . = ALIGN(64K);
     etext = .;
     .opd : { *(.opd) }
@@ -19,9 +29,12 @@ SECTIONS
     .data : {
         *(.data)
         *(.data.rel*)
-    }
+    } :data
     . = ALIGN(16);
-    .rodata : { *(.rodata) *(.rodata.*) }
+    .rodata : {
+        *(.rodata)
+        *(.rodata.*)
+    } :data
     . = ALIGN(16);
     .bss : { *(.bss) }
     . = ALIGN(256);
-- 
2.18.1

[kvm-unit-tests PULL 06/12] Update the gitlab-ci to Fedora 32

[Thomas Huth]

Fedora 30 is end of life, let's use the version 32 instead.

Unfortunately, we have to disable taskswitch2 in the gitlab-ci now.
It does not seem to work anymore with the latest version of gcc and/or
QEMU. We still check it in the travis-ci, though, so until somebody has
some spare time to debug this issue, it should be ok to disable it here.

Message-Id: <20200514192626.9950-8-thuth@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 .gitlab-ci.yml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 3093239..13e1a1f 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -1,4 +1,4 @@
-image: fedora:30
+image: fedora:32
 
 before_script:
  - dnf update -y
@@ -77,6 +77,6 @@ build-i386:
  - ./configure --arch=i386
  - make -j2
  - ACCEL=tcg ./run_tests.sh
-     cmpxchg8b eventinj port80 setjmp sieve tsc taskswitch taskswitch2 umip
+     cmpxchg8b eventinj port80 setjmp sieve tsc taskswitch umip
      | tee results.txt
  - if grep -q FAIL results.txt ; then exit 1 ; fi
-- 
2.18.1

[kvm-unit-tests PULL 07/12] x86: use a non-negative number in shift

[Thomas Huth]

From: Bill Wendling <morbo@google.com>

Shifting a negative number is undefined. Clang complains about it:

x86/svm.c:1131:38: error: shifting a negative signed value is undefined [-Werror,-Wshift-negative-value]
    test->vmcb->control.tsc_offset = TSC_OFFSET_VALUE;

Using "~0ull" results in identical asm code:

	before: movabsq $-281474976710656, %rsi
	after:  movabsq $-281474976710656, %rsi

Signed-off-by: Bill Wendling <morbo@google.com>
[thuth: Rebased to master - code is in svm_tests.c instead of svm.c now]
Message-Id: <20200514192626.9950-9-thuth@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 x86/svm_tests.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/x86/svm_tests.c b/x86/svm_tests.c
index c1abd55..a645d66 100644
--- a/x86/svm_tests.c
+++ b/x86/svm_tests.c
@@ -890,7 +890,7 @@ static bool npt_rw_l1mmio_check(struct svm_test *test)
 }
 
 #define TSC_ADJUST_VALUE    (1ll << 32)
-#define TSC_OFFSET_VALUE    (-1ll << 48)
+#define TSC_OFFSET_VALUE    (~0ull << 48)
 static bool ok;
 
 static void tsc_adjust_prepare(struct svm_test *test)
-- 
2.18.1

[kvm-unit-tests PULL 08/12] x86: use inline asm to retrieve stack pointer

[Thomas Huth]

From: Bill Wendling <morbo@google.com>

According to GCC's documentation, the only supported use for specifying
registers for local variables is "to specify registers for input and
output operands when calling Extended asm." Using it as a shortcut to
get the value in a register isn't guaranteed to work, and clang
complains that the variable is uninitialized.

Signed-off-by: Bill Wendling <morbo@google.com>
Message-Id: <20191030210419.213407-7-morbo@google.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 x86/vmx_tests.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index 68f93d3..fcd97a1 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -2253,7 +2253,9 @@ static void into_guest_main(void)
 		.offset = (uintptr_t)&&into,
 		.selector = KERNEL_CS32,
 	};
-	register uintptr_t rsp asm("rsp");
+	uintptr_t rsp;
+
+	asm volatile ("mov %%rsp, %0" : "=r"(rsp));
 
 	if (fp.offset != (uintptr_t)&&into) {
 		printf("Code address too high.\n");
@@ -3349,7 +3351,9 @@ static void try_compat_invvpid(void *unused)
 		.offset = (uintptr_t)&&invvpid,
 		.selector = KERNEL_CS32,
 	};
-	register uintptr_t rsp asm("rsp");
+	uintptr_t rsp;
+
+	asm volatile ("mov %%rsp, %0" : "=r"(rsp));
 
 	TEST_ASSERT_MSG(fp.offset == (uintptr_t)&&invvpid,
 			"Code address too high.");
-- 
2.18.1

[kvm-unit-tests PULL 09/12] vmx_tests: Silence warning from Clang

[Thomas Huth]

Clang complains:

x86/vmx_tests.c:8429:40: error: converting the result of '<<' to a boolean
 always evaluates to true [-Werror,-Wtautological-constant-compare]
         vmx_preemption_timer_zero_inject_db(1 << DB_VECTOR);
                                               ^

Looking at the code, the "1 << DB_VECTOR" is done within the function
vmx_preemption_timer_zero_inject_db() indeed:

	vmcs_write(EXC_BITMAP, intercept_db ? 1 << DB_VECTOR : 0);

... so using "true" as parameter for the function should be appropriate
here.

Message-Id: <20200514192626.9950-11-thuth@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 x86/vmx_tests.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index fcd97a1..36e94fa 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -8544,7 +8544,7 @@ static void vmx_preemption_timer_zero_test(void)
 	 * an event that you injected.
 	 */
 	vmx_set_test_stage(1);
-	vmx_preemption_timer_zero_inject_db(1 << DB_VECTOR);
+	vmx_preemption_timer_zero_inject_db(true);
 	vmx_preemption_timer_zero_expect_preempt_at_rip(db_fault_address);
 	vmx_preemption_timer_zero_advance_past_vmcall();
 
-- 
2.18.1

[kvm-unit-tests PULL 10/12] Compile the kvm-unit-tests also with Clang

[Thomas Huth]

To get some more test coverage, let's check compilation with Clang, too.

Message-Id: <20200514192626.9950-12-thuth@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 .gitlab-ci.yml | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 13e1a1f..3af53f0 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -80,3 +80,16 @@ build-i386:
      cmpxchg8b eventinj port80 setjmp sieve tsc taskswitch umip
      | tee results.txt
  - if grep -q FAIL results.txt ; then exit 1 ; fi
+
+build-clang:
+ script:
+ - dnf install -y qemu-system-x86 clang
+ - ./configure --arch=x86_64 --cc=clang
+ - make -j2
+ - ACCEL=tcg ./run_tests.sh
+     smptest smptest3 vmexit_cpuid vmexit_mov_from_cr8
+     vmexit_mov_to_cr8 vmexit_inl_pmtimer  vmexit_ipi vmexit_ipi_halt
+     vmexit_ple_round_robin vmexit_tscdeadline vmexit_tscdeadline_immed
+     eventinj msr port80 setjmp syscall tsc rmap_chain umip intel_iommu
+     | tee results.txt
+ - grep -q PASS results.txt && ! grep -q FAIL results.txt
-- 
2.18.1

[kvm-unit-tests PULL 11/12] x86: disable SSE on 32-bit hosts

[Thomas Huth]

From: Paolo Bonzini <pbonzini@redhat.com>

On 64-bit hosts we are disabling SSE and SSE2.  Depending on the
compiler however it may use movq instructions for 64-bit transfers
even when targeting 32-bit processors; when CR4.OSFXSR is not set,
this results in an undefined opcode exception, so tell the compiler
to avoid those instructions on 32-bit hosts as well.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20200616140217.104362-1-pbonzini@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 x86/Makefile.i386 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/x86/Makefile.i386 b/x86/Makefile.i386
index d801b80..be9d6bc 100644
--- a/x86/Makefile.i386
+++ b/x86/Makefile.i386
@@ -1,6 +1,7 @@
 cstart.o = $(TEST_DIR)/cstart.o
 bits = 32
 ldarch = elf32-i386
+COMMON_CFLAGS += -mno-sse -mno-sse2
 
 cflatobjs += lib/x86/setjmp32.o
 
-- 
2.18.1

[kvm-unit-tests PULL 12/12] s390x: stsi: Make output tap13 compatible

[Thomas Huth]

From: Janosch Frank <frankja@linux.ibm.com>

In tap13 output # is a special character and only "skip" and "todo"
are allowed to come after it. Let's appease our CI environment and
replace # with "count".

Signed-off-by: Janosch Frank <frankja@linux.ibm.com>
Message-Id: <20200525084340.1454-1-frankja@linux.ibm.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 s390x/stsi.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/s390x/stsi.c b/s390x/stsi.c
index 66b4257..b81cea7 100644
--- a/s390x/stsi.c
+++ b/s390x/stsi.c
@@ -129,11 +129,11 @@ static void test_3_2_2(void)
 	}
 
 	report(!memcmp(data->vm[0].uuid, uuid, sizeof(uuid)), "uuid");
-	report(data->vm[0].conf_cpus == smp_query_num_cpus(), "cpu # configured");
+	report(data->vm[0].conf_cpus == smp_query_num_cpus(), "cpu count configured");
 	report(data->vm[0].total_cpus ==
 	       data->vm[0].reserved_cpus + data->vm[0].conf_cpus,
-	       "cpu # total == conf + reserved");
-	report(data->vm[0].standby_cpus == 0, "cpu # standby");
+	       "cpu count total == conf + reserved");
+	report(data->vm[0].standby_cpus == 0, "cpu count standby");
 	report(!memcmp(data->vm[0].name, vm_name, sizeof(data->vm[0].name)),
 	       "VM name == kvm-unit-test");
 
-- 
2.18.1

Re: [kvm-unit-tests PULL 00/12] CI-related fixes and improvements

[Paolo Bonzini]

On 16/06/20 20:56, Thomas Huth wrote:
>   https://gitlab.com/huth/kvm-unit-tests.git tags/pull-request-2020-06-16

Pulled, thanks.

Paolo