* [kvm-unit-tests PATCH v2 01/10] x86: Makefile: Allow division on x86_64-elf binutils
2020-09-01 8:50 [kvm-unit-tests PATCH v2 00/10] Add support for generic ELF cross-compiler Roman Bolshakov
@ 2020-09-01 8:50 ` Roman Bolshakov
2020-09-03 15:19 ` Thomas Huth
2020-09-01 8:50 ` [kvm-unit-tests PATCH v2 02/10] x86: Replace instruction prefixes with spaces Roman Bolshakov
` (8 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Roman Bolshakov @ 2020-09-01 8:50 UTC (permalink / raw)
To: kvm; +Cc: Thomas Huth, Paolo Bonzini, Roman Bolshakov, Cameron Esfahani
For compatibility with other SVR4 assemblers, '/' starts a comment on
*-elf binutils target and thus division operator is not allowed [1][2].
That breaks cstart64.S build:
x86/cstart64.S: Assembler messages:
x86/cstart64.S:294: Error: unbalanced parenthesis in operand 1.
configure should detect if --divide needs to be passed to assembler by
compiling a small snippet where division is used inside parentheses.
1. https://sourceware.org/binutils/docs/as/i386_002dChars.html
2. https://sourceware.org/binutils/docs/as/i386_002dOptions.html#index-_002d_002ddivide-option_002c-i386
Cc: Cameron Esfahani <dirty@apple.com>
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
configure | 12 ++++++++++++
x86/Makefile.common | 3 +++
2 files changed, 15 insertions(+)
diff --git a/configure b/configure
index f9d030f..4eb504f 100755
--- a/configure
+++ b/configure
@@ -15,6 +15,7 @@ endian=""
pretty_print_stacks=yes
environ_default=yes
u32_long=
+wa_divide=
vmm="qemu"
errata_force=0
erratatxt="$srcdir/errata.txt"
@@ -156,6 +157,16 @@ EOF
u32_long=$("$cross_prefix$cc" -E lib-test.c | grep -v '^#' | grep -q long && echo yes)
rm -f lib-test.c
+# check if slash can be used for division
+if [ "$arch" = "i386" ] || [ "$arch" = "x86_64" ]; then
+ cat << EOF > lib-test.S
+foo:
+ movl (8 / 2), %eax
+EOF
+ wa_divide=$("$cross_prefix$cc" -c lib-test.S >/dev/null 2>&1 || echo yes)
+ rm -f lib-test.{o,S}
+fi
+
# Are we in a separate build tree? If so, link the Makefile
# and shared stuff so that 'make' and run_tests.sh work.
if test ! -e Makefile; then
@@ -205,6 +216,7 @@ PRETTY_PRINT_STACKS=$pretty_print_stacks
ENVIRON_DEFAULT=$environ_default
ERRATATXT=$erratatxt
U32_LONG_FMT=$u32_long
+WA_DIVIDE=$wa_divide
EOF
cat <<EOF > lib/config.h
diff --git a/x86/Makefile.common b/x86/Makefile.common
index 2ea9c9f..c3f7dc4 100644
--- a/x86/Makefile.common
+++ b/x86/Makefile.common
@@ -29,6 +29,9 @@ $(libcflat): LDFLAGS += -nostdlib
$(libcflat): CFLAGS += -ffreestanding -I $(SRCDIR)/lib -I lib
COMMON_CFLAGS += -m$(bits)
+ifneq ($(WA_DIVIDE),)
+COMMON_CFLAGS += -Wa,--divide
+endif
COMMON_CFLAGS += -O1
# stack.o relies on frame pointers.
--
2.28.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [kvm-unit-tests PATCH v2 02/10] x86: Replace instruction prefixes with spaces
2020-09-01 8:50 [kvm-unit-tests PATCH v2 00/10] Add support for generic ELF cross-compiler Roman Bolshakov
2020-09-01 8:50 ` [kvm-unit-tests PATCH v2 01/10] x86: Makefile: Allow division on x86_64-elf binutils Roman Bolshakov
@ 2020-09-01 8:50 ` Roman Bolshakov
2020-09-01 8:50 ` [kvm-unit-tests PATCH v2 03/10] x86: Makefile: Fix linkage of realmode on x86_64-elf binutils Roman Bolshakov
` (7 subsequent siblings)
9 siblings, 0 replies; 27+ messages in thread
From: Roman Bolshakov @ 2020-09-01 8:50 UTC (permalink / raw)
To: kvm; +Cc: Thomas Huth, Paolo Bonzini, Roman Bolshakov, Cameron Esfahani
There are three kinds of x86 prefix delimiters in GNU binutils:
'/', '\\' and a space.
The first works on Linux and few other platforms. The second one is
SVR-4 compatible and works on the generic elf target. The last kind is
universal and works everywhere, it's also used in the GAS manual [1].
Space delimiters fix the build errors on x86_64-elf binutils:
x86/cstart64.S:217: Error: invalid character '/' in mnemonic
x86/cstart64.S:313: Error: invalid character '/' in mnemonic
1. https://sourceware.org/binutils/docs/as/i386_002dPrefixes.html
Cc: Cameron Esfahani <dirty@apple.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
x86/cstart.S | 4 ++--
x86/cstart64.S | 4 ++--
x86/emulator.c | 38 +++++++++++++++++++-------------------
3 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/x86/cstart.S b/x86/cstart.S
index c0efc5f..489c561 100644
--- a/x86/cstart.S
+++ b/x86/cstart.S
@@ -149,7 +149,7 @@ save_id:
ap_start32:
setup_segments
mov $-4096, %esp
- lock/xaddl %esp, smp_stacktop
+ lock xaddl %esp, smp_stacktop
setup_percpu_area
call prepare_32
call reset_apic
@@ -206,7 +206,7 @@ ap_init:
lea sipi_entry, %esi
xor %edi, %edi
mov $(sipi_end - sipi_entry), %ecx
- rep/movsb
+ rep movsb
mov $APIC_DEFAULT_PHYS_BASE, %eax
movl $(APIC_DEST_ALLBUT | APIC_DEST_PHYSICAL | APIC_DM_INIT | APIC_INT_ASSERT), APIC_ICR(%eax)
movl $(APIC_DEST_ALLBUT | APIC_DEST_PHYSICAL | APIC_DM_STARTUP), APIC_ICR(%eax)
diff --git a/x86/cstart64.S b/x86/cstart64.S
index 2d16688..25a296c 100644
--- a/x86/cstart64.S
+++ b/x86/cstart64.S
@@ -226,7 +226,7 @@ sipi_end:
ap_start32:
setup_segments
mov $-4096, %esp
- lock/xaddl %esp, smp_stacktop
+ lock xaddl %esp, smp_stacktop
setup_percpu_area
call prepare_64
ljmpl $8, $ap_start64
@@ -323,7 +323,7 @@ ap_init:
lea sipi_entry, %rsi
xor %rdi, %rdi
mov $(sipi_end - sipi_entry), %rcx
- rep/movsb
+ rep movsb
mov $APIC_DEFAULT_PHYS_BASE, %eax
movl $(APIC_DEST_ALLBUT | APIC_DEST_PHYSICAL | APIC_DM_INIT | APIC_INT_ASSERT), APIC_ICR(%rax)
movl $(APIC_DEST_ALLBUT | APIC_DEST_PHYSICAL | APIC_DM_STARTUP), APIC_ICR(%rax)
diff --git a/x86/emulator.c b/x86/emulator.c
index 98743d1..e46d97e 100644
--- a/x86/emulator.c
+++ b/x86/emulator.c
@@ -61,71 +61,71 @@ static void test_cmps_one(unsigned char *m1, unsigned char *m3)
rsi = m1; rdi = m3; rcx = 30;
asm volatile("xor %[tmp], %[tmp] \n\t"
- "repe/cmpsb"
+ "repe cmpsb"
: "+S"(rsi), "+D"(rdi), "+c"(rcx), [tmp]"=&r"(tmp)
: : "cc");
report(rcx == 0 && rsi == m1 + 30 && rdi == m3 + 30, "repe/cmpsb (1)");
rsi = m1; rdi = m3; rcx = 30;
asm volatile("or $1, %[tmp]\n\t" // clear ZF
- "repe/cmpsb"
+ "repe cmpsb"
: "+S"(rsi), "+D"(rdi), "+c"(rcx), [tmp]"=&r"(tmp)
: : "cc");
report(rcx == 0 && rsi == m1 + 30 && rdi == m3 + 30,
- "repe/cmpsb (1.zf)");
+ "repe cmpsb (1.zf)");
rsi = m1; rdi = m3; rcx = 15;
asm volatile("xor %[tmp], %[tmp] \n\t"
- "repe/cmpsw"
+ "repe cmpsw"
: "+S"(rsi), "+D"(rdi), "+c"(rcx), [tmp]"=&r"(tmp)
: : "cc");
- report(rcx == 0 && rsi == m1 + 30 && rdi == m3 + 30, "repe/cmpsw (1)");
+ report(rcx == 0 && rsi == m1 + 30 && rdi == m3 + 30, "repe cmpsw (1)");
rsi = m1; rdi = m3; rcx = 7;
asm volatile("xor %[tmp], %[tmp] \n\t"
- "repe/cmpsl"
+ "repe cmpsl"
: "+S"(rsi), "+D"(rdi), "+c"(rcx), [tmp]"=&r"(tmp)
: : "cc");
- report(rcx == 0 && rsi == m1 + 28 && rdi == m3 + 28, "repe/cmpll (1)");
+ report(rcx == 0 && rsi == m1 + 28 && rdi == m3 + 28, "repe cmpll (1)");
rsi = m1; rdi = m3; rcx = 4;
asm volatile("xor %[tmp], %[tmp] \n\t"
- "repe/cmpsq"
+ "repe cmpsq"
: "+S"(rsi), "+D"(rdi), "+c"(rcx), [tmp]"=&r"(tmp)
: : "cc");
- report(rcx == 0 && rsi == m1 + 32 && rdi == m3 + 32, "repe/cmpsq (1)");
+ report(rcx == 0 && rsi == m1 + 32 && rdi == m3 + 32, "repe cmpsq (1)");
rsi = m1; rdi = m3; rcx = 130;
asm volatile("xor %[tmp], %[tmp] \n\t"
- "repe/cmpsb"
+ "repe cmpsb"
: "+S"(rsi), "+D"(rdi), "+c"(rcx), [tmp]"=&r"(tmp)
: : "cc");
report(rcx == 29 && rsi == m1 + 101 && rdi == m3 + 101,
- "repe/cmpsb (2)");
+ "repe cmpsb (2)");
rsi = m1; rdi = m3; rcx = 65;
asm volatile("xor %[tmp], %[tmp] \n\t"
- "repe/cmpsw"
+ "repe cmpsw"
: "+S"(rsi), "+D"(rdi), "+c"(rcx), [tmp]"=&r"(tmp)
: : "cc");
report(rcx == 14 && rsi == m1 + 102 && rdi == m3 + 102,
- "repe/cmpsw (2)");
+ "repe cmpsw (2)");
rsi = m1; rdi = m3; rcx = 32;
asm volatile("xor %[tmp], %[tmp] \n\t"
- "repe/cmpsl"
+ "repe cmpsl"
: "+S"(rsi), "+D"(rdi), "+c"(rcx), [tmp]"=&r"(tmp)
: : "cc");
report(rcx == 6 && rsi == m1 + 104 && rdi == m3 + 104,
- "repe/cmpll (2)");
+ "repe cmpll (2)");
rsi = m1; rdi = m3; rcx = 16;
asm volatile("xor %[tmp], %[tmp] \n\t"
- "repe/cmpsq"
+ "repe cmpsq"
: "+S"(rsi), "+D"(rdi), "+c"(rcx), [tmp]"=&r"(tmp)
: : "cc");
report(rcx == 3 && rsi == m1 + 104 && rdi == m3 + 104,
- "repe/cmpsq (2)");
+ "repe cmpsq (2)");
}
@@ -304,8 +304,8 @@ static void test_ljmp(void *mem)
volatile int res = 1;
*(unsigned long**)m = &&jmpf;
- asm volatile ("data16/mov %%cs, %0":"=m"(*(m + sizeof(unsigned long))));
- asm volatile ("rex64/ljmp *%0"::"m"(*m));
+ asm volatile ("data16 mov %%cs, %0":"=m"(*(m + sizeof(unsigned long))));
+ asm volatile ("rex64 ljmp *%0"::"m"(*m));
res = 0;
jmpf:
report(res, "ljmp");
--
2.28.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [kvm-unit-tests PATCH v2 03/10] x86: Makefile: Fix linkage of realmode on x86_64-elf binutils
2020-09-01 8:50 [kvm-unit-tests PATCH v2 00/10] Add support for generic ELF cross-compiler Roman Bolshakov
2020-09-01 8:50 ` [kvm-unit-tests PATCH v2 01/10] x86: Makefile: Allow division on x86_64-elf binutils Roman Bolshakov
2020-09-01 8:50 ` [kvm-unit-tests PATCH v2 02/10] x86: Replace instruction prefixes with spaces Roman Bolshakov
@ 2020-09-01 8:50 ` Roman Bolshakov
2020-09-01 8:50 ` [kvm-unit-tests PATCH v2 04/10] lib: Bundle debugreg.h from the kernel Roman Bolshakov
` (6 subsequent siblings)
9 siblings, 0 replies; 27+ messages in thread
From: Roman Bolshakov @ 2020-09-01 8:50 UTC (permalink / raw)
To: kvm; +Cc: Thomas Huth, Paolo Bonzini, Roman Bolshakov, Cameron Esfahani
link spec [1][2] is empty on x86_64-elf-gcc, i.e. -m32 is not propogated
to the linker as "-m elf_i386" and that causes the error:
/usr/local/opt/x86_64-elf-binutils/bin/x86_64-elf-ld: i386 architecture
of input file `x86/realmode.o' is incompatible with i386:x86-64 output
1. https://gcc.gnu.org/onlinedocs/gcc/Spec-Files.html
2. https://gcc.gnu.org/onlinedocs/gccint/Driver.html
Cc: Cameron Esfahani <dirty@apple.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
x86/Makefile.common | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/x86/Makefile.common b/x86/Makefile.common
index c3f7dc4..090ce22 100644
--- a/x86/Makefile.common
+++ b/x86/Makefile.common
@@ -69,7 +69,8 @@ test_cases: $(tests-common) $(tests)
$(TEST_DIR)/%.o: CFLAGS += -std=gnu99 -ffreestanding -I $(SRCDIR)/lib -I $(SRCDIR)/lib/x86 -I lib
$(TEST_DIR)/realmode.elf: $(TEST_DIR)/realmode.o
- $(CC) -m32 -nostdlib -o $@ -Wl,-T,$(SRCDIR)/$(TEST_DIR)/realmode.lds $^
+ $(CC) -m32 -nostdlib -o $@ -Wl,-m,elf_i386 \
+ -Wl,-T,$(SRCDIR)/$(TEST_DIR)/realmode.lds $^
$(TEST_DIR)/realmode.o: bits = 32
--
2.28.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [kvm-unit-tests PATCH v2 04/10] lib: Bundle debugreg.h from the kernel
2020-09-01 8:50 [kvm-unit-tests PATCH v2 00/10] Add support for generic ELF cross-compiler Roman Bolshakov
` (2 preceding siblings ...)
2020-09-01 8:50 ` [kvm-unit-tests PATCH v2 03/10] x86: Makefile: Fix linkage of realmode on x86_64-elf binutils Roman Bolshakov
@ 2020-09-01 8:50 ` Roman Bolshakov
2020-09-01 8:50 ` [kvm-unit-tests PATCH v2 05/10] lib: x86: Use portable format macros for uint32_t Roman Bolshakov
` (5 subsequent siblings)
9 siblings, 0 replies; 27+ messages in thread
From: Roman Bolshakov @ 2020-09-01 8:50 UTC (permalink / raw)
To: kvm
Cc: Thomas Huth, Paolo Bonzini, Roman Bolshakov, Jim Mattson,
Peter Shier, Cameron Esfahani
x86/vmx_tests.c depends on the kernel header and can't be compiled
otherwise on x86_64-elf gcc on macOS.
Cc: Jim Mattson <jmattson@google.com>
Cc: Peter Shier <pshier@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Cameron Esfahani <dirty@apple.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
lib/x86/asm/debugreg.h | 81 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 81 insertions(+)
create mode 100644 lib/x86/asm/debugreg.h
diff --git a/lib/x86/asm/debugreg.h b/lib/x86/asm/debugreg.h
new file mode 100644
index 0000000..d95d080
--- /dev/null
+++ b/lib/x86/asm/debugreg.h
@@ -0,0 +1,81 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI_ASM_X86_DEBUGREG_H
+#define _UAPI_ASM_X86_DEBUGREG_H
+
+
+/* Indicate the register numbers for a number of the specific
+ debug registers. Registers 0-3 contain the addresses we wish to trap on */
+#define DR_FIRSTADDR 0 /* u_debugreg[DR_FIRSTADDR] */
+#define DR_LASTADDR 3 /* u_debugreg[DR_LASTADDR] */
+
+#define DR_STATUS 6 /* u_debugreg[DR_STATUS] */
+#define DR_CONTROL 7 /* u_debugreg[DR_CONTROL] */
+
+/* Define a few things for the status register. We can use this to determine
+ which debugging register was responsible for the trap. The other bits
+ are either reserved or not of interest to us. */
+
+/* Define reserved bits in DR6 which are always set to 1 */
+#define DR6_RESERVED (0xFFFF0FF0)
+
+#define DR_TRAP0 (0x1) /* db0 */
+#define DR_TRAP1 (0x2) /* db1 */
+#define DR_TRAP2 (0x4) /* db2 */
+#define DR_TRAP3 (0x8) /* db3 */
+#define DR_TRAP_BITS (DR_TRAP0|DR_TRAP1|DR_TRAP2|DR_TRAP3)
+
+#define DR_STEP (0x4000) /* single-step */
+#define DR_SWITCH (0x8000) /* task switch */
+
+/* Now define a bunch of things for manipulating the control register.
+ The top two bytes of the control register consist of 4 fields of 4
+ bits - each field corresponds to one of the four debug registers,
+ and indicates what types of access we trap on, and how large the data
+ field is that we are looking at */
+
+#define DR_CONTROL_SHIFT 16 /* Skip this many bits in ctl register */
+#define DR_CONTROL_SIZE 4 /* 4 control bits per register */
+
+#define DR_RW_EXECUTE (0x0) /* Settings for the access types to trap on */
+#define DR_RW_WRITE (0x1)
+#define DR_RW_READ (0x3)
+
+#define DR_LEN_1 (0x0) /* Settings for data length to trap on */
+#define DR_LEN_2 (0x4)
+#define DR_LEN_4 (0xC)
+#define DR_LEN_8 (0x8)
+
+/* The low byte to the control register determine which registers are
+ enabled. There are 4 fields of two bits. One bit is "local", meaning
+ that the processor will reset the bit after a task switch and the other
+ is global meaning that we have to explicitly reset the bit. With linux,
+ you can use either one, since we explicitly zero the register when we enter
+ kernel mode. */
+
+#define DR_LOCAL_ENABLE_SHIFT 0 /* Extra shift to the local enable bit */
+#define DR_GLOBAL_ENABLE_SHIFT 1 /* Extra shift to the global enable bit */
+#define DR_LOCAL_ENABLE (0x1) /* Local enable for reg 0 */
+#define DR_GLOBAL_ENABLE (0x2) /* Global enable for reg 0 */
+#define DR_ENABLE_SIZE 2 /* 2 enable bits per register */
+
+#define DR_LOCAL_ENABLE_MASK (0x55) /* Set local bits for all 4 regs */
+#define DR_GLOBAL_ENABLE_MASK (0xAA) /* Set global bits for all 4 regs */
+
+/* The second byte to the control register has a few special things.
+ We can slow the instruction pipeline for instructions coming via the
+ gdt or the ldt if we want to. I am not sure why this is an advantage */
+
+#ifdef __i386__
+#define DR_CONTROL_RESERVED (0xFC00) /* Reserved by Intel */
+#else
+#define DR_CONTROL_RESERVED (0xFFFFFFFF0000FC00UL) /* Reserved */
+#endif
+
+#define DR_LOCAL_SLOWDOWN (0x100) /* Local slow the pipeline */
+#define DR_GLOBAL_SLOWDOWN (0x200) /* Global slow the pipeline */
+
+/*
+ * HW breakpoint additions
+ */
+
+#endif /* _UAPI_ASM_X86_DEBUGREG_H */
--
2.28.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [kvm-unit-tests PATCH v2 05/10] lib: x86: Use portable format macros for uint32_t
2020-09-01 8:50 [kvm-unit-tests PATCH v2 00/10] Add support for generic ELF cross-compiler Roman Bolshakov
` (3 preceding siblings ...)
2020-09-01 8:50 ` [kvm-unit-tests PATCH v2 04/10] lib: Bundle debugreg.h from the kernel Roman Bolshakov
@ 2020-09-01 8:50 ` Roman Bolshakov
2020-09-04 13:47 ` Thomas Huth
2020-09-01 8:50 ` [kvm-unit-tests PATCH v2 06/10] configure: Add an option to specify getopt Roman Bolshakov
` (4 subsequent siblings)
9 siblings, 1 reply; 27+ messages in thread
From: Roman Bolshakov @ 2020-09-01 8:50 UTC (permalink / raw)
To: kvm
Cc: Thomas Huth, Paolo Bonzini, Roman Bolshakov, Alex Bennée,
Andrew Jones, Cameron Esfahani, Radim Krčmář
Compilation of the files fails on ARCH=i386 with i686-elf gcc because
they use "%x" or "%d" format specifier that does not match the actual
size of uint32_t:
x86/s3.c: In function ‘main’:
x86/s3.c:53:35: error: format ‘%x’ expects argument of type ‘unsigned int’, but argument 2 has type ‘u32’ {aka ‘long unsigned int’}
[-Werror=format=]
53 | printf("PM1a event registers at %x\n", fadt->pm1a_evt_blk);
| ~^ ~~~~~~~~~~~~~~~~~~
| | |
| | u32 {aka long unsigned int}
| unsigned int
| %lx
Use PRIx32 instead of "x" and PRId32 instead of "d" to take into account
u32_long case.
Cc: Alex Bennée <alex.bennee@linaro.org>
Cc: Andrew Jones <drjones@redhat.com>
Cc: Cameron Esfahani <dirty@apple.com>
Cc: Radim Krčmář <rkrcmar@redhat.com>
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
lib/pci.c | 2 +-
x86/asyncpf.c | 2 +-
x86/msr.c | 3 ++-
x86/s3.c | 2 +-
4 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/lib/pci.c b/lib/pci.c
index daa33e1..175caf0 100644
--- a/lib/pci.c
+++ b/lib/pci.c
@@ -248,7 +248,7 @@ void pci_bar_print(struct pci_dev *dev, int bar_num)
printf("BAR#%d,%d [%" PRIx64 "-%" PRIx64 " ",
bar_num, bar_num + 1, start, end);
} else {
- printf("BAR#%d [%02x-%02x ",
+ printf("BAR#%d [%02" PRIx32 "-%02" PRIx32 " ",
bar_num, (uint32_t)start, (uint32_t)end);
}
diff --git a/x86/asyncpf.c b/x86/asyncpf.c
index 305a923..8239e16 100644
--- a/x86/asyncpf.c
+++ b/x86/asyncpf.c
@@ -78,7 +78,7 @@ static void pf_isr(struct ex_regs *r)
phys = 0;
break;
default:
- report(false, "unexpected async pf reason %d", reason);
+ report(false, "unexpected async pf reason %" PRId32, reason);
break;
}
}
diff --git a/x86/msr.c b/x86/msr.c
index f7539c3..ce5dabe 100644
--- a/x86/msr.c
+++ b/x86/msr.c
@@ -89,7 +89,8 @@ static void test_msr_rw(int msr_index, unsigned long long input, unsigned long l
wrmsr(msr_index, input);
r = rdmsr(msr_index);
if (expected != r) {
- printf("testing %s: output = %#x:%#x expected = %#x:%#x\n", sptr,
+ printf("testing %s: output = %#" PRIx32 ":%#" PRIx32
+ " expected = %#" PRIx32 ":%#" PRIx32 "\n", sptr,
(u32)(r >> 32), (u32)r, (u32)(expected >> 32), (u32)expected);
}
report(expected == r, "%s", sptr);
diff --git a/x86/s3.c b/x86/s3.c
index da2d00c..6e41d0c 100644
--- a/x86/s3.c
+++ b/x86/s3.c
@@ -50,7 +50,7 @@ int main(int argc, char **argv)
*resume_vec++ = *addr;
printf("copy resume code from %p\n", &resume_start);
- printf("PM1a event registers at %x\n", fadt->pm1a_evt_blk);
+ printf("PM1a event registers at %" PRIx32 "\n", fadt->pm1a_evt_blk);
outw(0x400, fadt->pm1a_evt_blk + 2);
/* Setup RTC alarm to wake up on the next second. */
--
2.28.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [kvm-unit-tests PATCH v2 05/10] lib: x86: Use portable format macros for uint32_t
2020-09-01 8:50 ` [kvm-unit-tests PATCH v2 05/10] lib: x86: Use portable format macros for uint32_t Roman Bolshakov
@ 2020-09-04 13:47 ` Thomas Huth
0 siblings, 0 replies; 27+ messages in thread
From: Thomas Huth @ 2020-09-04 13:47 UTC (permalink / raw)
To: Roman Bolshakov, kvm
Cc: Paolo Bonzini, Alex Bennée, Andrew Jones, Cameron Esfahani
On 01/09/2020 10.50, Roman Bolshakov wrote:
> Compilation of the files fails on ARCH=i386 with i686-elf gcc because
> they use "%x" or "%d" format specifier that does not match the actual
> size of uint32_t:
>
> x86/s3.c: In function ‘main’:
> x86/s3.c:53:35: error: format ‘%x’ expects argument of type ‘unsigned int’, but argument 2 has type ‘u32’ {aka ‘long unsigned int’}
> [-Werror=format=]
> 53 | printf("PM1a event registers at %x\n", fadt->pm1a_evt_blk);
> | ~^ ~~~~~~~~~~~~~~~~~~
> | | |
> | | u32 {aka long unsigned int}
> | unsigned int
> | %lx
>
> Use PRIx32 instead of "x" and PRId32 instead of "d" to take into account
> u32_long case.
>
> Cc: Alex Bennée <alex.bennee@linaro.org>
> Cc: Andrew Jones <drjones@redhat.com>
> Cc: Cameron Esfahani <dirty@apple.com>
> Cc: Radim Krčmář <rkrcmar@redhat.com>
Radim does not work at redhat.com anymore, so please remove that line in
case you respin the patch.
> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> ---
> lib/pci.c | 2 +-
> x86/asyncpf.c | 2 +-
> x86/msr.c | 3 ++-
> x86/s3.c | 2 +-
> 4 files changed, 5 insertions(+), 4 deletions(-)
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [kvm-unit-tests PATCH v2 06/10] configure: Add an option to specify getopt
2020-09-01 8:50 [kvm-unit-tests PATCH v2 00/10] Add support for generic ELF cross-compiler Roman Bolshakov
` (4 preceding siblings ...)
2020-09-01 8:50 ` [kvm-unit-tests PATCH v2 05/10] lib: x86: Use portable format macros for uint32_t Roman Bolshakov
@ 2020-09-01 8:50 ` Roman Bolshakov
2020-09-04 13:50 ` Thomas Huth
2020-09-22 13:53 ` Paolo Bonzini
2020-09-01 8:50 ` [kvm-unit-tests PATCH v2 07/10] README: Update build instructions for macOS Roman Bolshakov
` (3 subsequent siblings)
9 siblings, 2 replies; 27+ messages in thread
From: Roman Bolshakov @ 2020-09-01 8:50 UTC (permalink / raw)
To: kvm; +Cc: Thomas Huth, Paolo Bonzini, Roman Bolshakov, Cameron Esfahani
macOS is shipped with an old non-enhanced version of getopt and it
doesn't support options used by run_tests.sh. Proper version of getopt
is available from homebrew but it has to be added to PATH before invoking
run_tests.sh. It's not convenient because it has to be done in each
shell instance and there could be many if a multiplexor is used.
The change provides a way to override getopt and halts ./configure if
enhanced getopt can't be found.
Cc: Cameron Esfahani <dirty@apple.com>
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
configure | 13 +++++++++++++
run_tests.sh | 2 +-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/configure b/configure
index 4eb504f..b85420b 100755
--- a/configure
+++ b/configure
@@ -8,6 +8,7 @@ objcopy=objcopy
objdump=objdump
ar=ar
addr2line=addr2line
+getopt=getopt
arch=`uname -m | sed -e 's/i.86/i386/;s/arm.*/arm/;s/ppc64.*/ppc64/'`
host=$arch
cross_prefix=
@@ -32,6 +33,7 @@ usage() {
--cross-prefix=PREFIX cross compiler prefix
--cc=CC c compiler to use ($cc)
--ld=LD ld linker to use ($ld)
+ --getopt=GETOPT enhanced getopt to use ($getopt)
--prefix=PREFIX where to install things ($prefix)
--endian=ENDIAN endianness to compile for (little or big, ppc64 only)
--[enable|disable]-pretty-print-stacks
@@ -77,6 +79,9 @@ while [[ "$1" = -* ]]; do
--ld)
ld="$arg"
;;
+ --getopt)
+ getopt="$arg"
+ ;;
--enable-pretty-print-stacks)
pretty_print_stacks=yes
;;
@@ -167,6 +172,13 @@ EOF
rm -f lib-test.{o,S}
fi
+# require enhanced getopt
+$getopt -T > /dev/null
+if [ $? -ne 4 ]; then
+ echo "Enchanced getopt is not available"
+ exit 1
+fi
+
# Are we in a separate build tree? If so, link the Makefile
# and shared stuff so that 'make' and run_tests.sh work.
if test ! -e Makefile; then
@@ -209,6 +221,7 @@ OBJCOPY=$cross_prefix$objcopy
OBJDUMP=$cross_prefix$objdump
AR=$cross_prefix$ar
ADDR2LINE=$cross_prefix$addr2line
+GETOPT=$getopt
TEST_DIR=$testdir
FIRMWARE=$firmware
ENDIAN=$endian
diff --git a/run_tests.sh b/run_tests.sh
index 01e36dc..c4f436b 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -35,7 +35,7 @@ RUNTIME_arch_run="./$TEST_DIR/run"
source scripts/runtime.bash
only_tests=""
-args=`getopt -u -o ag:htj:v -l all,group:,help,tap13,parallel:,verbose -- $*`
+args=`$GETOPT -u -o ag:htj:v -l all,group:,help,tap13,parallel:,verbose -- $*`
[ $? -ne 0 ] && exit 2;
set -- $args;
while [ $# -gt 0 ]; do
--
2.28.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [kvm-unit-tests PATCH v2 06/10] configure: Add an option to specify getopt
2020-09-01 8:50 ` [kvm-unit-tests PATCH v2 06/10] configure: Add an option to specify getopt Roman Bolshakov
@ 2020-09-04 13:50 ` Thomas Huth
2020-09-22 13:53 ` Paolo Bonzini
1 sibling, 0 replies; 27+ messages in thread
From: Thomas Huth @ 2020-09-04 13:50 UTC (permalink / raw)
To: Roman Bolshakov, kvm; +Cc: Paolo Bonzini, Cameron Esfahani
On 01/09/2020 10.50, Roman Bolshakov wrote:
> macOS is shipped with an old non-enhanced version of getopt and it
> doesn't support options used by run_tests.sh. Proper version of getopt
> is available from homebrew but it has to be added to PATH before invoking
> run_tests.sh. It's not convenient because it has to be done in each
> shell instance and there could be many if a multiplexor is used.
>
> The change provides a way to override getopt and halts ./configure if
> enhanced getopt can't be found.
>
> Cc: Cameron Esfahani <dirty@apple.com>
> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> ---
> configure | 13 +++++++++++++
> run_tests.sh | 2 +-
> 2 files changed, 14 insertions(+), 1 deletion(-)
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [kvm-unit-tests PATCH v2 06/10] configure: Add an option to specify getopt
2020-09-01 8:50 ` [kvm-unit-tests PATCH v2 06/10] configure: Add an option to specify getopt Roman Bolshakov
2020-09-04 13:50 ` Thomas Huth
@ 2020-09-22 13:53 ` Paolo Bonzini
2020-09-22 21:51 ` Roman Bolshakov
1 sibling, 1 reply; 27+ messages in thread
From: Paolo Bonzini @ 2020-09-22 13:53 UTC (permalink / raw)
To: Roman Bolshakov, kvm; +Cc: Thomas Huth, Cameron Esfahani
On 01/09/20 10:50, Roman Bolshakov wrote:
> macOS is shipped with an old non-enhanced version of getopt and it
> doesn't support options used by run_tests.sh. Proper version of getopt
> is available from homebrew but it has to be added to PATH before invoking
> run_tests.sh. It's not convenient because it has to be done in each
> shell instance and there could be many if a multiplexor is used.
>
> The change provides a way to override getopt and halts ./configure if
> enhanced getopt can't be found.
>
> Cc: Cameron Esfahani <dirty@apple.com>
> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
I don't like the extra option very much, generally people are just expected
to stick homebrew in their path somehow. Reporting a better error is a
good idea though, what about this:
diff --git a/configure b/configure
index 4eb504f..3293634 100755
--- a/configure
+++ b/configure
@@ -167,6 +167,13 @@ EOF
rm -f lib-test.{o,S}
fi
+# require enhanced getopt
+getopt -T > /dev/null
+if [ $? -ne 4 ]; then
+ echo "Enhanced getopt is not available, add it to your PATH?"
+ exit 1
+fi
+
# Are we in a separate build tree? If so, link the Makefile
# and shared stuff so that 'make' and run_tests.sh work.
if test ! -e Makefile; then
diff --git a/run_tests.sh b/run_tests.sh
index 01e36dc..70d012c 100755
--- a/run_tests.sh
+++ b/run_tests.sh
@@ -34,6 +34,13 @@ EOF
RUNTIME_arch_run="./$TEST_DIR/run"
source scripts/runtime.bash
+# require enhanced getopt
+getopt -T > /dev/null
+if [ $? -ne 4 ]; then
+ echo "Enhanced getopt is not available, add it to your PATH?"
+ exit 1
+fi
+
only_tests=""
args=`getopt -u -o ag:htj:v -l all,group:,help,tap13,parallel:,verbose -- $*`
[ $? -ne 0 ] && exit 2;
Paolo
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [kvm-unit-tests PATCH v2 06/10] configure: Add an option to specify getopt
2020-09-22 13:53 ` Paolo Bonzini
@ 2020-09-22 21:51 ` Roman Bolshakov
2020-09-23 2:41 ` Paolo Bonzini
0 siblings, 1 reply; 27+ messages in thread
From: Roman Bolshakov @ 2020-09-22 21:51 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm, Thomas Huth, Cameron Esfahani
On Tue, Sep 22, 2020 at 03:53:27PM +0200, Paolo Bonzini wrote:
> On 01/09/20 10:50, Roman Bolshakov wrote:
> > macOS is shipped with an old non-enhanced version of getopt and it
> > doesn't support options used by run_tests.sh. Proper version of getopt
> > is available from homebrew but it has to be added to PATH before invoking
> > run_tests.sh. It's not convenient because it has to be done in each
> > shell instance and there could be many if a multiplexor is used.
> >
> > The change provides a way to override getopt and halts ./configure if
> > enhanced getopt can't be found.
> >
> > Cc: Cameron Esfahani <dirty@apple.com>
> > Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
>
> I don't like the extra option very much, generally people are just expected
> to stick homebrew in their path somehow. Reporting a better error is a
> good idea though, what about this:
>
Homebrew doesn't shadow system tools unlike macports. That's why the
patch is helpful and the error below can be corrected automatically
without human intervention. The error in the proposed below patch would
still cause frustrating:
"ugh. I again forgot to set PATH for this tmux window..."
May be I'm exaggarating the issue, but I don't set PATH on a per-project
basis unless I'm doing something extremely rare or something weird :)
The original patch also shouldn't have an impact on most modern Linux
systems. It would help only a few who build kvm-unit-tests on mac.
Hopefully, it eases contribution and testing of QEMU without an access
to Linux box.
Thanks,
Roman
> diff --git a/configure b/configure
> index 4eb504f..3293634 100755
> --- a/configure
> +++ b/configure
> @@ -167,6 +167,13 @@ EOF
> rm -f lib-test.{o,S}
> fi
>
> +# require enhanced getopt
> +getopt -T > /dev/null
> +if [ $? -ne 4 ]; then
> + echo "Enhanced getopt is not available, add it to your PATH?"
> + exit 1
> +fi
> +
> # Are we in a separate build tree? If so, link the Makefile
> # and shared stuff so that 'make' and run_tests.sh work.
> if test ! -e Makefile; then
> diff --git a/run_tests.sh b/run_tests.sh
> index 01e36dc..70d012c 100755
> --- a/run_tests.sh
> +++ b/run_tests.sh
> @@ -34,6 +34,13 @@ EOF
> RUNTIME_arch_run="./$TEST_DIR/run"
> source scripts/runtime.bash
>
> +# require enhanced getopt
> +getopt -T > /dev/null
> +if [ $? -ne 4 ]; then
> + echo "Enhanced getopt is not available, add it to your PATH?"
> + exit 1
> +fi
> +
> only_tests=""
> args=`getopt -u -o ag:htj:v -l all,group:,help,tap13,parallel:,verbose -- $*`
> [ $? -ne 0 ] && exit 2;
>
> Paolo
>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [kvm-unit-tests PATCH v2 06/10] configure: Add an option to specify getopt
2020-09-22 21:51 ` Roman Bolshakov
@ 2020-09-23 2:41 ` Paolo Bonzini
2020-09-23 5:14 ` Thomas Huth
0 siblings, 1 reply; 27+ messages in thread
From: Paolo Bonzini @ 2020-09-23 2:41 UTC (permalink / raw)
To: Roman Bolshakov; +Cc: kvm, Thomas Huth, Cameron Esfahani
On 22/09/20 23:51, Roman Bolshakov wrote:
> Homebrew doesn't shadow system tools unlike macports. That's why the
> patch is helpful and the error below can be corrected automatically
> without human intervention. The error in the proposed below patch would
> still cause frustrating:
>
> "ugh. I again forgot to set PATH for this tmux window..."
>
> May be I'm exaggarating the issue, but I don't set PATH on a per-project
> basis unless I'm doing something extremely rare or something weird :)
When I was using a Mac (with Fink... it was a few years ago :)) I used
to set PATH, COMPILER_PATH, MANPATH etc. in my .bashrc file. Isn't it
the same?
Paolo
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [kvm-unit-tests PATCH v2 06/10] configure: Add an option to specify getopt
2020-09-23 2:41 ` Paolo Bonzini
@ 2020-09-23 5:14 ` Thomas Huth
0 siblings, 0 replies; 27+ messages in thread
From: Thomas Huth @ 2020-09-23 5:14 UTC (permalink / raw)
To: Paolo Bonzini, Roman Bolshakov; +Cc: kvm, Cameron Esfahani
On 23/09/2020 04.41, Paolo Bonzini wrote:
> On 22/09/20 23:51, Roman Bolshakov wrote:
>> Homebrew doesn't shadow system tools unlike macports. That's why the
>> patch is helpful and the error below can be corrected automatically
>> without human intervention. The error in the proposed below patch would
>> still cause frustrating:
>>
>> "ugh. I again forgot to set PATH for this tmux window..."
>>
>> May be I'm exaggarating the issue, but I don't set PATH on a per-project
>> basis unless I'm doing something extremely rare or something weird :)
>
> When I was using a Mac (with Fink... it was a few years ago :)) I used
> to set PATH, COMPILER_PATH, MANPATH etc. in my .bashrc file. Isn't it
> the same?
You should at least have tweaked the .travis.yml file, too. It still
uses the now non-existing --getopt parameter, so the travis builds are
failing now.
Thomas
^ permalink raw reply [flat|nested] 27+ messages in thread
* [kvm-unit-tests PATCH v2 07/10] README: Update build instructions for macOS
2020-09-01 8:50 [kvm-unit-tests PATCH v2 00/10] Add support for generic ELF cross-compiler Roman Bolshakov
` (5 preceding siblings ...)
2020-09-01 8:50 ` [kvm-unit-tests PATCH v2 06/10] configure: Add an option to specify getopt Roman Bolshakov
@ 2020-09-01 8:50 ` Roman Bolshakov
2020-09-01 8:50 ` [kvm-unit-tests PATCH v2 08/10] travis.yml: Add CI " Roman Bolshakov
` (2 subsequent siblings)
9 siblings, 0 replies; 27+ messages in thread
From: Roman Bolshakov @ 2020-09-01 8:50 UTC (permalink / raw)
To: kvm; +Cc: Thomas Huth, Paolo Bonzini, Roman Bolshakov, Cameron Esfahani
Pre-built cross-compilers for x86 are available in homebrew and can be
used to build the tests.
Cc: Cameron Esfahani <dirty@apple.com>
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
README.macOS.md | 71 +++++++++++++++++++++++++++++++++++--------------
1 file changed, 51 insertions(+), 20 deletions(-)
diff --git a/README.macOS.md b/README.macOS.md
index de46d5f..4ca5a57 100644
--- a/README.macOS.md
+++ b/README.macOS.md
@@ -1,14 +1,54 @@
# kvm-unit-tests on macOS
-Cross-compiler with ELF support is required for build of kvm-unit-tests on
-macOS.
+The tests can be used to validate TCG or HVF accel on macOS.
-## Building cross-compiler from source
+## Prerequisites
-A cross-compiler toolchain can be built from source using crosstool-ng. The
-latest released version of
+GNU getopt and coreutils should be installed prior to building and running the
+tests. They're available in [homebrew](https://brew.sh):
+```
+$ brew install coreutils
+$ brew install gnu-getopt
+```
+
+A cross-compiler with ELF support is required to build kvm-unit-tests on macOS.
+
+### Pre-built cross-compiler
+
+Binary packages of ELF cross-compilers for i386 and x86_64 target can be
+installed from homebrew:
+```
+$ brew install i686-elf-gcc
+$ brew install x86_64-elf-gcc
+```
+
+32-bit x86 tests can be built like that:
+```
+$ ./configure \
+ --getopt=/usr/local/opt/gnu-getopt/bin/getopt \
+ --arch=i386 \
+ --cross-prefix=i686-elf-
+$ make -j $(nproc)
+```
+
+64-bit x86 tests can be built likewise:
+```
+$ ./configure \
+ --getopt=/usr/local/opt/gnu-getopt/bin/getopt \
+ --arch=x86_64 \
+ --cross-prefix=x86_64-elf-
+$ make -j $(nproc)
+```
+
+Out-of-tree build can be used to make tests for both architectures
+simultaneously in separate build directories.
+
+### Building cross-compiler from source
+
+An alternative is to build cross-compiler toolchain from source using
+crosstool-ng. The latest released version of
[crosstool-ng](https://github.com/crosstool-ng/crosstool-ng) can be installed
-using [homebrew](https://brew.sh)
+using homebrew:
```
$ brew install crosstool-ng
```
@@ -30,18 +70,9 @@ $ ct-ng -C $X_BUILD_DIR build CT_PREFIX=$X_INSTALL_DIR
Once compiled, the cross-compiler can be used to build the tests:
```
-$ ./configure --cross-prefix=$X_INSTALL_DIR/x86_64-unknown-linux-gnu/bin/x86_64-unknown-linux-gnu-
-$ make
-```
-
-## Pre-built cross-compiler
-
-x86_64-elf-gcc package in Homebrew provides pre-built cross-compiler but it
-fails to compile kvm-unit-tests.
-
-## Running the tests
-
-GNU coreutils should be installed prior to running the tests:
-```
-$ brew install coreutils
+$ ./configure \
+ --getopt=/usr/local/opt/gnu-getopt/bin/getopt \
+ --arch=x86_64 \
+ --cross-prefix=$X_INSTALL_DIR/x86_64-unknown-linux-gnu/bin/x86_64-unknown-linux-gnu-
+$ make -j $(nproc)
```
--
2.28.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [kvm-unit-tests PATCH v2 08/10] travis.yml: Add CI for macOS
2020-09-01 8:50 [kvm-unit-tests PATCH v2 00/10] Add support for generic ELF cross-compiler Roman Bolshakov
` (6 preceding siblings ...)
2020-09-01 8:50 ` [kvm-unit-tests PATCH v2 07/10] README: Update build instructions for macOS Roman Bolshakov
@ 2020-09-01 8:50 ` Roman Bolshakov
2020-09-04 13:53 ` Thomas Huth
2020-09-01 8:50 ` [kvm-unit-tests PATCH v2 09/10] travis.yml: Change matrix keyword to jobs Roman Bolshakov
2020-09-01 8:50 ` [kvm-unit-tests PATCH v2 10/10] travis.yml: Add x86 build with clang 10 Roman Bolshakov
9 siblings, 1 reply; 27+ messages in thread
From: Roman Bolshakov @ 2020-09-01 8:50 UTC (permalink / raw)
To: kvm; +Cc: Thomas Huth, Paolo Bonzini, Roman Bolshakov
Build the tests on macOS and test TCG. HVF doesn't work in travis.
sieve tests pass but they might timeout in travis, they were left out
because of that.
Suggested-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
.travis.yml | 42 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/.travis.yml b/.travis.yml
index f0cfc82..7bd0205 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -108,6 +108,48 @@ matrix:
- TESTS="sieve"
- ACCEL="tcg,firmware=s390x/run"
+ - os: osx
+ osx_image: xcode11.6
+ addons:
+ homebrew:
+ packages:
+ - bash
+ - coreutils
+ - gnu-getopt
+ - qemu
+ - x86_64-elf-gcc
+ env:
+ - CONFIG="--cross-prefix=x86_64-elf-
+ --getopt=/usr/local/opt/gnu-getopt/bin/getopt"
+ - BUILD_DIR="build"
+ - TESTS="ioapic-split 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"
+ - ACCEL="tcg"
+
+ - os: osx
+ osx_image: xcode11.6
+ addons:
+ homebrew:
+ packages:
+ - bash
+ - coreutils
+ - gnu-getopt
+ - qemu
+ - i686-elf-gcc
+ env:
+ - CONFIG="--arch=i386 --cross-prefix=i686-elf-
+ --getopt=/usr/local/opt/gnu-getopt/bin/getopt"
+ - BUILD_DIR="build"
+ - TESTS="cmpxchg8b 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 port80 setjmp tsc
+ taskswitch umip"
+ - ACCEL="tcg"
+
before_script:
- if [ "$ACCEL" = "kvm" ]; then
sudo chgrp kvm /usr/bin/qemu-system-* ;
--
2.28.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [kvm-unit-tests PATCH v2 08/10] travis.yml: Add CI for macOS
2020-09-01 8:50 ` [kvm-unit-tests PATCH v2 08/10] travis.yml: Add CI " Roman Bolshakov
@ 2020-09-04 13:53 ` Thomas Huth
0 siblings, 0 replies; 27+ messages in thread
From: Thomas Huth @ 2020-09-04 13:53 UTC (permalink / raw)
To: Roman Bolshakov, kvm; +Cc: Paolo Bonzini
On 01/09/2020 10.50, Roman Bolshakov wrote:
> Build the tests on macOS and test TCG. HVF doesn't work in travis.
>
> sieve tests pass but they might timeout in travis, they were left out
> because of that.
>
> Suggested-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> ---
> .travis.yml | 42 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [kvm-unit-tests PATCH v2 09/10] travis.yml: Change matrix keyword to jobs
2020-09-01 8:50 [kvm-unit-tests PATCH v2 00/10] Add support for generic ELF cross-compiler Roman Bolshakov
` (7 preceding siblings ...)
2020-09-01 8:50 ` [kvm-unit-tests PATCH v2 08/10] travis.yml: Add CI " Roman Bolshakov
@ 2020-09-01 8:50 ` Roman Bolshakov
2020-09-04 14:20 ` Thomas Huth
2020-09-01 8:50 ` [kvm-unit-tests PATCH v2 10/10] travis.yml: Add x86 build with clang 10 Roman Bolshakov
9 siblings, 1 reply; 27+ messages in thread
From: Roman Bolshakov @ 2020-09-01 8:50 UTC (permalink / raw)
To: kvm; +Cc: Thomas Huth, Paolo Bonzini, Roman Bolshakov
jobs keyword is used throughout Travis documentation and matrix is an
alias for it (according to Travis config validation):
root: key matrix is an alias for jobs, using jobs
At first glance it's not clear if they're the same and if jobs
documentation applies to matrix. Changing keyword name should make it
obvious.
While at it, fix the Travis config warning:
root: deprecated key sudo (The key `sudo` has no effect anymore.)
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
.travis.yml | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/.travis.yml b/.travis.yml
index 7bd0205..f3a8899 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -1,11 +1,10 @@
-sudo: true
dist: bionic
language: c
cache: ccache
git:
submodules: false
-matrix:
+jobs:
include:
- addons:
--
2.28.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [kvm-unit-tests PATCH v2 09/10] travis.yml: Change matrix keyword to jobs
2020-09-01 8:50 ` [kvm-unit-tests PATCH v2 09/10] travis.yml: Change matrix keyword to jobs Roman Bolshakov
@ 2020-09-04 14:20 ` Thomas Huth
0 siblings, 0 replies; 27+ messages in thread
From: Thomas Huth @ 2020-09-04 14:20 UTC (permalink / raw)
To: Roman Bolshakov, kvm; +Cc: Paolo Bonzini
On 01/09/2020 10.50, Roman Bolshakov wrote:
> jobs keyword is used throughout Travis documentation and matrix is an
> alias for it (according to Travis config validation):
>
> root: key matrix is an alias for jobs, using jobs
>
> At first glance it's not clear if they're the same and if jobs
> documentation applies to matrix. Changing keyword name should make it
> obvious.
>
> While at it, fix the Travis config warning:
>
> root: deprecated key sudo (The key `sudo` has no effect anymore.)
>
> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> ---
> .travis.yml | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/.travis.yml b/.travis.yml
> index 7bd0205..f3a8899 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -1,11 +1,10 @@
> -sudo: true
> dist: bionic
> language: c
> cache: ccache
> git:
> submodules: false
>
> -matrix:
> +jobs:
> include:
>
> - addons:
>
Seems to work.
Tested-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 27+ messages in thread
* [kvm-unit-tests PATCH v2 10/10] travis.yml: Add x86 build with clang 10
2020-09-01 8:50 [kvm-unit-tests PATCH v2 00/10] Add support for generic ELF cross-compiler Roman Bolshakov
` (8 preceding siblings ...)
2020-09-01 8:50 ` [kvm-unit-tests PATCH v2 09/10] travis.yml: Change matrix keyword to jobs Roman Bolshakov
@ 2020-09-01 8:50 ` Roman Bolshakov
2020-09-04 14:31 ` Thomas Huth
9 siblings, 1 reply; 27+ messages in thread
From: Roman Bolshakov @ 2020-09-01 8:50 UTC (permalink / raw)
To: kvm; +Cc: Thomas Huth, Paolo Bonzini, Roman Bolshakov
.gitlab-ci.yml already has a job to build the tests with clang but it's
not clear how to set it up on a personal github repo.
NB, realmode test is disabled because it fails immediately after start
if compiled with clang-10.
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
.travis.yml | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/.travis.yml b/.travis.yml
index f3a8899..ae4ed08 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -17,6 +17,16 @@ jobs:
kvmclock_test msr pcid rdpru realmode rmap_chain s3 setjmp umip"
- ACCEL="kvm"
+ - addons:
+ apt_packages: clang-10 qemu-system-x86
+ env:
+ - CONFIG="--cc=clang-10"
+ - BUILD_DIR="."
+ - TESTS="access asyncpf debug emulator ept hypercall hyperv_stimer
+ hyperv_synic idt_test intel_iommu ioapic ioapic-split
+ kvmclock_test msr pcid rdpru rmap_chain s3 setjmp umip"
+ - ACCEL="kvm"
+
- addons:
apt_packages: gcc qemu-system-x86
env:
--
2.28.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [kvm-unit-tests PATCH v2 10/10] travis.yml: Add x86 build with clang 10
2020-09-01 8:50 ` [kvm-unit-tests PATCH v2 10/10] travis.yml: Add x86 build with clang 10 Roman Bolshakov
@ 2020-09-04 14:31 ` Thomas Huth
2020-09-14 14:45 ` Roman Bolshakov
0 siblings, 1 reply; 27+ messages in thread
From: Thomas Huth @ 2020-09-04 14:31 UTC (permalink / raw)
To: Roman Bolshakov, kvm; +Cc: Paolo Bonzini
On 01/09/2020 10.50, Roman Bolshakov wrote:
> .gitlab-ci.yml already has a job to build the tests with clang but it's
> not clear how to set it up on a personal github repo.
You can't use gitlab-ci from a github repo, it's a separate git forge
system.
> NB, realmode test is disabled because it fails immediately after start
> if compiled with clang-10.
>
> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> ---
> .travis.yml | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/.travis.yml b/.travis.yml
> index f3a8899..ae4ed08 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -17,6 +17,16 @@ jobs:
> kvmclock_test msr pcid rdpru realmode rmap_chain s3 setjmp umip"
> - ACCEL="kvm"
>
> + - addons:
> + apt_packages: clang-10 qemu-system-x86
> + env:
> + - CONFIG="--cc=clang-10"
> + - BUILD_DIR="."
> + - TESTS="access asyncpf debug emulator ept hypercall hyperv_stimer
> + hyperv_synic idt_test intel_iommu ioapic ioapic-split
> + kvmclock_test msr pcid rdpru rmap_chain s3 setjmp umip"
> + - ACCEL="kvm"
We already have two jobs for compiling on x86, one for testing in-tree
builds and one for testing out-of-tree builds ... I wonder whether we
should simply switch one of those two jobs to use clang-10 instead of
gcc (since the in/out-of-tree stuff should be hopefully independent of
the compiler type)? Since Travis limits the amount of jobs that run at
the same time, that would not increase the total testing time, I think.
Thomas
PS: Maybe we could update from bionic to focal now, too, and see whether
some more tests are working with the newer version of QEMU there...
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [kvm-unit-tests PATCH v2 10/10] travis.yml: Add x86 build with clang 10
2020-09-04 14:31 ` Thomas Huth
@ 2020-09-14 14:45 ` Roman Bolshakov
2020-09-14 16:37 ` Thomas Huth
0 siblings, 1 reply; 27+ messages in thread
From: Roman Bolshakov @ 2020-09-14 14:45 UTC (permalink / raw)
To: Thomas Huth; +Cc: kvm, Paolo Bonzini
On Fri, Sep 04, 2020 at 04:31:03PM +0200, Thomas Huth wrote:
> On 01/09/2020 10.50, Roman Bolshakov wrote:
> > .gitlab-ci.yml already has a job to build the tests with clang but it's
> > not clear how to set it up on a personal github repo.
>
> You can't use gitlab-ci from a github repo, it's a separate git forge
> system.
>
> > NB, realmode test is disabled because it fails immediately after start
> > if compiled with clang-10.
> >
> > Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> > ---
> > .travis.yml | 10 ++++++++++
> > 1 file changed, 10 insertions(+)
> >
> > diff --git a/.travis.yml b/.travis.yml
> > index f3a8899..ae4ed08 100644
> > --- a/.travis.yml
> > +++ b/.travis.yml
> > @@ -17,6 +17,16 @@ jobs:
> > kvmclock_test msr pcid rdpru realmode rmap_chain s3 setjmp umip"
> > - ACCEL="kvm"
> >
> > + - addons:
> > + apt_packages: clang-10 qemu-system-x86
> > + env:
> > + - CONFIG="--cc=clang-10"
> > + - BUILD_DIR="."
> > + - TESTS="access asyncpf debug emulator ept hypercall hyperv_stimer
> > + hyperv_synic idt_test intel_iommu ioapic ioapic-split
> > + kvmclock_test msr pcid rdpru rmap_chain s3 setjmp umip"
> > + - ACCEL="kvm"
>
> We already have two jobs for compiling on x86, one for testing in-tree
> builds and one for testing out-of-tree builds ... I wonder whether we
> should simply switch one of those two jobs to use clang-10 instead of
> gcc (since the in/out-of-tree stuff should be hopefully independent of
> the compiler type)? Since Travis limits the amount of jobs that run at
> the same time, that would not increase the total testing time, I think.
>
Hi Thomas,
sure, that works for me.
> Thomas
>
>
> PS: Maybe we could update from bionic to focal now, too, and see whether
> some more tests are working with the newer version of QEMU there...
>
no problem, here're results for focal/kvm on IBM x3500 M3 (Nehalem) if
the tests are built with clang:
PASS apic-split (53 tests)
PASS ioapic-split (19 tests)
PASS apic (53 tests)
PASS ioapic (26 tests)
SKIP cmpxchg8b (i386 only)
PASS smptest (1 tests)
PASS smptest3 (1 tests)
PASS vmexit_cpuid
PASS vmexit_vmcall
PASS vmexit_mov_from_cr8
PASS vmexit_mov_to_cr8
PASS vmexit_inl_pmtimer
PASS vmexit_ipi
PASS vmexit_ipi_halt
PASS vmexit_ple_round_robin
PASS vmexit_tscdeadline
PASS vmexit_tscdeadline_immed
PASS access
SKIP smap (0 tests)
SKIP pku (0 tests)
PASS asyncpf (1 tests)
PASS emulator (125 tests, 2 skipped)
PASS eventinj (13 tests)
PASS hypercall (2 tests)
PASS idt_test (4 tests)
PASS memory (8 tests)
PASS msr (12 tests)
SKIP pmu (/proc/sys/kernel/nmi_watchdog not equal to 0)
SKIP vmware_backdoors (/sys/module/kvm/parameters/enable_vmware_backdoor
not equal to Y)
PASS port80
FAIL realmode
PASS s3
PASS setjmp (10 tests)
PASS sieve
PASS syscall (2 tests)
PASS tsc (3 tests)
PASS tsc_adjust (5 tests)
PASS xsave (4 tests)
PASS rmap_chain
SKIP svm (0 tests)
SKIP taskswitch (i386 only)
SKIP taskswitch2 (i386 only)
PASS kvmclock_test
PASS pcid (3 tests)
PASS pcid-disabled (3 tests)
PASS rdpru (1 tests)
PASS umip (21 tests)
FAIL vmx
PASS ept (8636 tests)
SKIP vmx_eoi_bitmap_ioapic_scan (1 tests, 1 skipped)
SKIP vmx_hlt_with_rvi_test (1 tests, 1 skipped)
SKIP vmx_apicv_test (2 tests, 2 skipped)
PASS vmx_apic_passthrough_thread (8 tests)
FAIL vmx_init_signal_test (8 tests, 1 unexpected failures)
FAIL vmx_apic_passthrough_tpr_threshold_test (timeout; duration=10)
PASS vmx_vmcs_shadow_test (142218 tests)
PASS debug (11 tests)
PASS hyperv_synic (1 tests)
PASS hyperv_connections (7 tests)
PASS hyperv_stimer (12 tests)
PASS hyperv_clock
PASS intel_iommu (11 tests)
PASS tsx-ctrl
Here are results for the same server if tests are built with gcc:
SureSS apic-split (53 tests)
PASS ioapic-split (19 tests)
PASS apic (53 tests)
PASS ioapic (26 tests)
SKIP cmpxchg8b (i386 only)
PASS smptest (1 tests)
PASS smptest3 (1 tests)
PASS vmexit_cpuid
PASS vmexit_vmcall
PASS vmexit_mov_from_cr8
PASS vmexit_mov_to_cr8
PASS vmexit_inl_pmtimer
PASS vmexit_ipi
PASS vmexit_ipi_halt
PASS vmexit_ple_round_robin
PASS vmexit_tscdeadline
PASS vmexit_tscdeadline_immed
PASS access
SKIP smap (0 tests)
SKIP pku (0 tests)
PASS asyncpf (1 tests)
PASS emulator (125 tests, 2 skipped)
PASS eventinj (13 tests)
PASS hypercall (2 tests)
PASS idt_test (4 tests)
PASS memory (8 tests)
PASS msr (12 tests)
SKIP pmu (/proc/sys/kernel/nmi_watchdog not equal to 0)
SKIP vmware_backdoors (/sys/module/kvm/parameters/enable_vmware_backdoor
not equal to Y)
PASS port80
PASS realmode
PASS s3
PASS setjmp (10 tests)
PASS sieve
PASS syscall (2 tests)
PASS tsc (3 tests)
PASS tsc_adjust (5 tests)
PASS xsave (4 tests)
PASS rmap_chain
SKIP svm (0 tests)
SKIP taskswitch (i386 only)
SKIP taskswitch2 (i386 only)
PASS kvmclock_test
PASS pcid (3 tests)
PASS pcid-disabled (3 tests)
PASS rdpru (1 tests)
PASS umip (21 tests)
FAIL vmx
PASS ept (8636 tests)
SKIP vmx_eoi_bitmap_ioapic_scan (1 tests, 1 skipped)
SKIP vmx_hlt_with_rvi_test (1 tests, 1 skipped)
SKIP vmx_apicv_test (2 tests, 2 skipped)
PASS vmx_apic_passthrough_thread (8 tests)
FAIL vmx_init_signal_test (8 tests, 1 unexpected failures)
FAIL vmx_apic_passthrough_tpr_threshold_test (timeout; duration=10)
PASS vmx_vmcs_shadow_test (142218 tests)
PASS debug (11 tests)
PASS hyperv_synic (1 tests)
PASS hyperv_connections (7 tests)
PASS hyperv_stimer (12 tests)
PASS hyperv_clock
PASS intel_iommu (11 tests)
PASS tsx-ctrl
The difference is only realmode test which doesn't work if built by
clang.
Thanks,
Roman
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [kvm-unit-tests PATCH v2 10/10] travis.yml: Add x86 build with clang 10
2020-09-14 14:45 ` Roman Bolshakov
@ 2020-09-14 16:37 ` Thomas Huth
2020-09-15 15:59 ` Roman Bolshakov
0 siblings, 1 reply; 27+ messages in thread
From: Thomas Huth @ 2020-09-14 16:37 UTC (permalink / raw)
To: Roman Bolshakov; +Cc: kvm, Paolo Bonzini
On 14/09/2020 16.45, Roman Bolshakov wrote:
> On Fri, Sep 04, 2020 at 04:31:03PM +0200, Thomas Huth wrote:
>> On 01/09/2020 10.50, Roman Bolshakov wrote:
>>> .gitlab-ci.yml already has a job to build the tests with clang but it's
>>> not clear how to set it up on a personal github repo.
>>
>> You can't use gitlab-ci from a github repo, it's a separate git forge
>> system.
>>
>>> NB, realmode test is disabled because it fails immediately after start
>>> if compiled with clang-10.
>>>
>>> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
>>> ---
>>> .travis.yml | 10 ++++++++++
>>> 1 file changed, 10 insertions(+)
>>>
>>> diff --git a/.travis.yml b/.travis.yml
>>> index f3a8899..ae4ed08 100644
>>> --- a/.travis.yml
>>> +++ b/.travis.yml
>>> @@ -17,6 +17,16 @@ jobs:
>>> kvmclock_test msr pcid rdpru realmode rmap_chain s3 setjmp umip"
>>> - ACCEL="kvm"
>>>
>>> + - addons:
>>> + apt_packages: clang-10 qemu-system-x86
>>> + env:
>>> + - CONFIG="--cc=clang-10"
>>> + - BUILD_DIR="."
>>> + - TESTS="access asyncpf debug emulator ept hypercall hyperv_stimer
>>> + hyperv_synic idt_test intel_iommu ioapic ioapic-split
>>> + kvmclock_test msr pcid rdpru rmap_chain s3 setjmp umip"
>>> + - ACCEL="kvm"
>>
>> We already have two jobs for compiling on x86, one for testing in-tree
>> builds and one for testing out-of-tree builds ... I wonder whether we
>> should simply switch one of those two jobs to use clang-10 instead of
>> gcc (since the in/out-of-tree stuff should be hopefully independent of
>> the compiler type)? Since Travis limits the amount of jobs that run at
>> the same time, that would not increase the total testing time, I think.
>>
>
> Hi Thomas,
>
> sure, that works for me.
>
>> Thomas
>>
>>
>> PS: Maybe we could update from bionic to focal now, too, and see whether
>> some more tests are working with the newer version of QEMU there...
>>
>
> no problem, here're results for focal/kvm on IBM x3500 M3 (Nehalem) if
> the tests are built with clang:
[...]
Thanks for checking, that looks promising!
> The difference is only realmode test which doesn't work if built by
> clang.
Hmm, if you got some spare minutes, could you check if it works when
replacing the asm() statements there with asm volatile() ?
(Otherwise I'll check it if I got some spare time again ... so likely
not this week ;-))
Thomas
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [kvm-unit-tests PATCH v2 10/10] travis.yml: Add x86 build with clang 10
2020-09-14 16:37 ` Thomas Huth
@ 2020-09-15 15:59 ` Roman Bolshakov
2020-09-22 14:51 ` Paolo Bonzini
0 siblings, 1 reply; 27+ messages in thread
From: Roman Bolshakov @ 2020-09-15 15:59 UTC (permalink / raw)
To: Thomas Huth; +Cc: kvm, Paolo Bonzini
On Mon, Sep 14, 2020 at 06:37:33PM +0200, Thomas Huth wrote:
> On 14/09/2020 16.45, Roman Bolshakov wrote:
> > The difference is only realmode test which doesn't work if built by
> > clang.
>
> Hmm, if you got some spare minutes, could you check if it works when
> replacing the asm() statements there with asm volatile() ?
> (Otherwise I'll check it if I got some spare time again ... so likely
> not this week ;-))
>
Hi Thomas,
Sure. There are only two places where volatile is missed (inside functions) and
would make sense to add it. Unfortunately it doesn't help much:
diff --git a/x86/realmode.c b/x86/realmode.c
index 7c2d776..30691bc 100644
--- a/x86/realmode.c
+++ b/x86/realmode.c
@@ -271,7 +271,7 @@ static void report(const char *name, u16 regs_ignore, _Bool ok)
}
#define MK_INSN(name, str) \
- asm ( \
+ asm volatile ( \
".pushsection .data.insn \n\t" \
"insn_" #name ": \n\t" \
".word 1001f, 1002f - 1001f \n\t" \
@@ -1448,7 +1448,7 @@ static void test_cpuid(void)
eax = inregs.eax;
ecx = inregs.ecx;
- asm("cpuid" : "+a"(eax), "=b"(ebx), "+c"(ecx), "=d"(edx));
+ asm volatile("cpuid" : "+a"(eax), "=b"(ebx), "+c"(ecx), "=d"(edx));
exec_in_big_real_mode(&insn_cpuid);
report("cpuid", R_AX|R_BX|R_CX|R_DX,
outregs.eax == eax && outregs.ebx == ebx
So, I looked further and noticed that multiboot header is missed in the flat
binary:
".section .init \n\t"
".code32 \n\t"
"mb_magic = 0x1BADB002 \n\t"
"mb_flags = 0x0 \n\t"
"# multiboot header \n\t"
".long mb_magic, mb_flags, 0 - (mb_magic + mb_flags) \n\t"
But I can see it in the object file. So, I believe linker didn't place .init
section at 0x1000 despite realmode.lds instruction:
SECTIONS
{
. = 16K;
stext = .;
.text : { *(.init) *(.text) }
. = ALIGN(4K);
.data : { *(.data) *(.rodata*) }
. = ALIGN(16);
.bss : { *(.bss) }
edata = .;
}
ENTRY(start)
If I read it correctly, given the segment in realmode.elf:
Program Headers:
Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align
LOAD 0x001000 0x00004000 0x00004000 0x03c90 0x03c90 R E 0x1000
Here's multiboot header in GCC-compiled binary:
00001000: 02b0ad1b 00000000 fe4f52e4 66b83412 .........OR.f.4.
Here's the same location in clang-compiled binary:
00001000: 04000000 14000000 03000000 474e5500 ............GNU.
Here's verbose invocation of linker by clang:
$ clang-10 -v -m32 -nostdlib -o x86/realmode.elf -Wl,-m,elf_i386 -Wl,-T,/home/roolebo
/dev/kvm-unit-tests/x86/realmode.lds x86/realmode.o
clang version 10.0.0-4ubuntu1
Target: i386-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin
Found candidate GCC installation: /usr/bin/../lib/gcc/x86_64-linux-gnu/9
Found candidate GCC installation: /usr/lib/gcc/x86_64-linux-gnu/9
"/usr/bin/ld" -z relro --hash-style=gnu --build-id --eh-frame-hdr -m elf_i386 -dynamic-linker /lib/ld-linux.so.2 -o x86/realmode.elf -L/lib/../lib32 -L/usr/lib/../lib32 -L/usr/lib/llvm-10/bin/../lib -L/lib -L/usr/lib -m elf_i386 -T /home/roolebo/dev/kvm-unit-tests/x86/realmode.lds x86/realmode.o
(BTW, I'm surprised to see -dynamic-linker being set, it has little sense for
the tests but that's likely because -static is not passed explicitly)
And if I pass --print-map option to the linker I can see that GNU build id is
placed at 0x4000:
$ clang-10 -m32 -nostdlib -o x86/realmode.elf -Wl,-M -Wl,-m,elf_i386 -Wl,-T,/home/roo
lebo/dev/kvm-unit-tests/x86/realmode.lds x86/realmode.o
Discarded input sections
.llvm_addrsig 0x0000000000000000 0x122 x86/realmode.o
Memory Configuration
Name Origin Length Attributes
*default* 0x0000000000000000 0xffffffffffffffff
Linker script and memory map
0x0000000000004000 . = 0x4000
0x0000000000004000 stext = .
.note.gnu.build-id
0x0000000000004000 0x24
.note.gnu.build-id
0x0000000000004000 0x24 x86/realmode.o
.text 0x0000000000004030 0x290c
*(.init)
.init 0x0000000000004030 0xc x86/realmode.o
*(.text)
*fill* 0x000000000000403c 0x4
.text 0x0000000000004040 0x28fc x86/realmode.o
0x000000000000404c start
0x00000000000040a0 realmode_start
.text.insn 0x000000000000693c 0x40e
.text.insn 0x000000000000693c 0x40e x86/realmode.o
.iplt 0x0000000000006d4a 0x0
.iplt 0x0000000000006d4a 0x0 x86/realmode.o
.rel.dyn 0x0000000000006d4c 0x0
.rel.got 0x0000000000006d4c 0x0 x86/realmode.o
.rel.iplt 0x0000000000006d4c 0x0 x86/realmode.o
0x0000000000007000 . = ALIGN (0x1000)
.data 0x0000000000007000 0x2538
*(.data)
.data 0x0000000000007000 0x1064 x86/realmode.o
0x0000000000008000 r_gdt
0x0000000000008018 r_gdt_descr
0x000000000000801e r_idt_descr
*(.rodata*)
.rodata.str1.1
0x0000000000008064 0x4d3 x86/realmode.o
0x4f9 (size before relaxing)
*fill* 0x0000000000008537 0x1
.rodata 0x0000000000008538 0x1000 x86/realmode.o
.data.insn 0x0000000000009538 0x21c
.data.insn 0x0000000000009538 0x21c x86/realmode.o
.got 0x0000000000009754 0x0
.got 0x0000000000009754 0x0 x86/realmode.o
.got.plt 0x0000000000009754 0x0
.got.plt 0x0000000000009754 0x0 x86/realmode.o
.igot.plt 0x0000000000009754 0x0
.igot.plt 0x0000000000009754 0x0 x86/realmode.o
0x0000000000009760 . = ALIGN (0x10)
.bss 0x0000000000009760 0x284
*(.bss)
.bss 0x0000000000009760 0x284 x86/realmode.o
0x0000000000009764 tmp_stack
0x00000000000099e4 edata = .
LOAD x86/realmode.o
OUTPUT(x86/realmode.elf elf32-i386)
.debug_str 0x0000000000000000 0x509
.debug_str 0x0000000000000000 0x509 x86/realmode.o
0x59f (size before relaxing)
.debug_loc 0x0000000000000000 0x484
.debug_loc 0x0000000000000000 0x484 x86/realmode.o
.debug_abbrev 0x0000000000000000 0x212
.debug_abbrev 0x0000000000000000 0x212 x86/realmode.o
.debug_info 0x0000000000000000 0x1935
.debug_info 0x0000000000000000 0x1935 x86/realmode.o
.comment 0x0000000000000000 0x1f
.comment 0x0000000000000000 0x1f x86/realmode.o
0x20 (size before relaxing)
.note.GNU-stack
0x0000000000000000 0x0
.note.GNU-stack
0x0000000000000000 0x0 x86/realmode.o
.debug_frame 0x0000000000000000 0x824
.debug_frame 0x0000000000000000 0x824 x86/realmode.o
.debug_line 0x0000000000000000 0xc06
.debug_line 0x0000000000000000 0xc06 x86/realmode.o
So, a workaround for that could be adding '-Wl,--build-id=none' to the
makefile rule for realmode.elf. Then multiboot magic is placed properly
at 0x4000 instead of 0x4030. Unfortunately it doesn't help with the
test :-)
-Roman
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [kvm-unit-tests PATCH v2 10/10] travis.yml: Add x86 build with clang 10
2020-09-15 15:59 ` Roman Bolshakov
@ 2020-09-22 14:51 ` Paolo Bonzini
2020-09-22 21:25 ` Roman Bolshakov
0 siblings, 1 reply; 27+ messages in thread
From: Paolo Bonzini @ 2020-09-22 14:51 UTC (permalink / raw)
To: Roman Bolshakov, Thomas Huth; +Cc: kvm
On 15/09/20 17:59, Roman Bolshakov wrote:
> So, a workaround for that could be adding '-Wl,--build-id=none' to the
> makefile rule for realmode.elf. Then multiboot magic is placed properly
> at 0x4000 instead of 0x4030. Unfortunately it doesn't help with the
> test :-)
Heh, weird. I also tried adding
/DISCARD/ : { *(.note.gnu.build-id) }
to the linker script and I got a very helpful (not) linker warning:
/usr/bin/ld: warning: .note.gnu.build-id section discarded, --build-id ignored.
... except that the --build-id was placed not by me but rather by gcc.
So we should probably simplify things doing this:
diff --git a/x86/Makefile.common b/x86/Makefile.common
index 090ce22..10c8a42 100644
--- a/x86/Makefile.common
+++ b/x86/Makefile.common
@@ -69,8 +69,8 @@ test_cases: $(tests-common) $(tests)
$(TEST_DIR)/%.o: CFLAGS += -std=gnu99 -ffreestanding -I $(SRCDIR)/lib -I $(SRCDIR)/lib/x86 -I lib
$(TEST_DIR)/realmode.elf: $(TEST_DIR)/realmode.o
- $(CC) -m32 -nostdlib -o $@ -Wl,-m,elf_i386 \
- -Wl,-T,$(SRCDIR)/$(TEST_DIR)/realmode.lds $^
+ $(LD) -o $@ -m elf_i386 \
+ -T $(SRCDIR)/$(TEST_DIR)/realmode.lds $^
$(TEST_DIR)/realmode.o: bits = 32
diff --git a/x86/realmode.lds b/x86/realmode.lds
index 0ed3063..3220c19 100644
--- a/x86/realmode.lds
+++ b/x86/realmode.lds
@@ -1,5 +1,6 @@
SECTIONS
{
+ /DISCARD/ : { *(.note.gnu.build-id) }
. = 16K;
stext = .;
.text : { *(.init) *(.text) }
which I will squash in your patch 3.
But the main issue is that clang does not support .code16gcc so it
writes 32-bit code that is run in 16-bit mode. It'd be a start to
use -m16 instead of -m32, but then I think it still miscompiles the
(32-bit) code between "start" and the .code16gcc label.
Paolo
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [kvm-unit-tests PATCH v2 10/10] travis.yml: Add x86 build with clang 10
2020-09-22 14:51 ` Paolo Bonzini
@ 2020-09-22 21:25 ` Roman Bolshakov
2020-09-23 2:37 ` Paolo Bonzini
0 siblings, 1 reply; 27+ messages in thread
From: Roman Bolshakov @ 2020-09-22 21:25 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: Thomas Huth, kvm
On Tue, Sep 22, 2020 at 04:51:18PM +0200, Paolo Bonzini wrote:
> On 15/09/20 17:59, Roman Bolshakov wrote:
> > So, a workaround for that could be adding '-Wl,--build-id=none' to the
> > makefile rule for realmode.elf. Then multiboot magic is placed properly
> > at 0x4000 instead of 0x4030. Unfortunately it doesn't help with the
> > test :-)
>
> Heh, weird. I also tried adding
>
> /DISCARD/ : { *(.note.gnu.build-id) }
>
> to the linker script and I got a very helpful (not) linker warning:
>
> /usr/bin/ld: warning: .note.gnu.build-id section discarded, --build-id ignored.
>
> ... except that the --build-id was placed not by me but rather by gcc.
> So we should probably simplify things doing this:
>
> diff --git a/x86/Makefile.common b/x86/Makefile.common
> index 090ce22..10c8a42 100644
> --- a/x86/Makefile.common
> +++ b/x86/Makefile.common
> @@ -69,8 +69,8 @@ test_cases: $(tests-common) $(tests)
> $(TEST_DIR)/%.o: CFLAGS += -std=gnu99 -ffreestanding -I $(SRCDIR)/lib -I $(SRCDIR)/lib/x86 -I lib
>
> $(TEST_DIR)/realmode.elf: $(TEST_DIR)/realmode.o
> - $(CC) -m32 -nostdlib -o $@ -Wl,-m,elf_i386 \
> - -Wl,-T,$(SRCDIR)/$(TEST_DIR)/realmode.lds $^
> + $(LD) -o $@ -m elf_i386 \
> + -T $(SRCDIR)/$(TEST_DIR)/realmode.lds $^
>
Agreed, in the case it's better to tell linker directly what is needed
rather than fighting with compiler's way of invoking the linker.
> $(TEST_DIR)/realmode.o: bits = 32
>
> diff --git a/x86/realmode.lds b/x86/realmode.lds
> index 0ed3063..3220c19 100644
> --- a/x86/realmode.lds
> +++ b/x86/realmode.lds
> @@ -1,5 +1,6 @@
> SECTIONS
> {
> + /DISCARD/ : { *(.note.gnu.build-id) }
> . = 16K;
> stext = .;
> .text : { *(.init) *(.text) }
>
> which I will squash in your patch 3.
>
Thanks!
There's another difference right after multiboot header.
Here's how GCC binary looks:
00004000 <stext>:
4000: 02 b0 ad 1b 00 00 add 0x1bad(%eax),%dh
4006: 00 00 add %al,(%eax)
4008: fe 4f 52 decb 0x52(%edi)
400b: e4 .byte 0xe4
0000400c <test_function>:
400c: 66 b8 34 12 mov $0x1234,%ax
4010: 00 00 add %al,(%eax)
4012: 66 c3 retw
Here's clang:
00004000 <stext>:
4000: 02 b0 ad 1b 00 00 add 0x1bad(%eax),%dh
4006: 00 00 add %al,(%eax)
4008: fe 4f 52 decb 0x52(%edi)
400b: e4 66 in $0x66,%al
400d: 90 nop
400e: 66 90 xchg %ax,%ax
00004010 <test_function>:
4010: 66 b8 34 12 mov $0x1234,%ax
4014: 00 00 add %al,(%eax)
4016: 66 c3 retw
So, clang pads stext with two NOPs after 400b until it's quad-aligned.
I'm not sure how we can ask it to stop doing that.
The assembly (clang-10 -S) doesn't show an alignment requirement:
.set mb_magic, 464367618
.set mb_flags, 0
# multiboot header
.long 464367618
.long 0
.long -464367618
.p2align 0, 0x90
.globl start
".p2align 0, 0x90" behaves like ".p2align 4, 0x90", sounds like a bug?
But it doesn't introduce an issue as it turned out later
> But the main issue is that clang does not support .code16gcc so it
> writes 32-bit code that is run in 16-bit mode.
I had impression that it does support .code16gcc from the PR (and
included since LLVM 4.0):
https://reviews.llvm.org/D20109
https://github.com/llvm/llvm-project/commit/6477ce2697bf1d9afd2bcc0cf0c16c7cf08713be
Then another changes register allocation since LLVM 5.0 but I don't
know if it breaks anything (I'm not familiar with LLVM TBH).
https://github.com/llvm/llvm-project/commit/f5f593b674ed031f3f5aa2c44ac705547532d5cb
> It'd be a start to use -m16 instead of -m32, but then I think it still
> miscompiles the (32-bit) code between "start" and the .code16gcc
> label.
>
Bingo! Changing target variable "bits = 32" to "bits = 16" helps, it
proceeds properly until "iret 1" (insn_code_iret32) test and then it
hangs.
Inline assembly:
MK_INSN(iret32, "pushf\n\t"
"pushl %cs\n\t"
"call 1f\n\t" /* a near call will push eip onto the stack */
"jmp 2f\n\t"
"1: iretl\n\t"
"2:\n\t"
);
GCC:
00006c25 <insn_code_iret32>:
6c25: 66 9c pushfw
6c27: 66 0e pushw %cs
6c29: 66 e8 02 00 callw 6c2f <insn_code_iret32+0xa>
6c2d: 00 00 add %al,(%eax)
6c2f: eb 02 jmp 6c33 <insn_code_iret16>
6c31: 66 cf iretw
Clang saves 16-bit registers but restores 32-bit in `iret` and `call`
doesn't have an operand-size prefix:
00007547 <insn_code_iret32>:
7547: 9c pushf
7548: 66 0e pushw %cs
754a: e8 02 00 eb 02 call 2eb7551 <edata+0x2eacb6d>
754f: 66 cf iretw
So, this fixes the test and makes "iret 3" pass (otherwise it hangs):
diff --git a/x86/realmode.c b/x86/realmode.c
index 7c2d776..0ae5186 100644
--- a/x86/realmode.c
+++ b/x86/realmode.c
@@ -761,9 +761,9 @@ static void test_pusha_popa(void)
static void test_iret(void)
{
- MK_INSN(iret32, "pushf\n\t"
+ MK_INSN(iret32, "pushfl\n\t"
"pushl %cs\n\t"
- "call 1f\n\t" /* a near call will push eip onto the stack */
+ "calll 1f\n\t" /* a near call will push eip onto the stack */
"jmp 2f\n\t"
"1: iretl\n\t"
"2:\n\t"
@@ -782,7 +782,7 @@ static void test_iret(void)
"orl $0xffc18028, %eax\n\t"
"pushl %eax\n\t"
"pushl %cs\n\t"
- "call 1f\n\t"
+ "calll 1f\n\t"
"jmp 2f\n\t"
"1: iretl\n\t"
"2:\n\t");
With the above change I get the following machine code for iret32 which
is equivalent to GCC:
00007547 <insn_code_iret32>:
7547: 66 9c pushfw
7549: 66 0e pushw %cs
754b: 66 e8 02 00 callw 7551 <insn_code_iret32+0xa>
754f: 00 00 add %al,(%eax)
7551: eb 02 jmp 7555 <insn_code_iret16>
7553: 66 cf iretw
Still, there're a few more failed tests but realmode doesn't hang anymore:
FAIL: pusha/popa 1
FAIL: pusha/popa 1
FAIL: jmp far 1
And explicit instruction suffixes fix them too:
@@ -639,7 +639,7 @@ static void test_jcc_near(void)
static void test_long_jmp(void)
{
- MK_INSN(long_jmp, "call 1f\n\t"
+ MK_INSN(long_jmp, "calll 1f\n\t"
"jmp 2f\n\t"
"1: jmp $0, $test_function\n\t"
"2:\n\t");
@@ -728,26 +728,26 @@ static void test_null(void)
static void test_pusha_popa(void)
{
- MK_INSN(pusha, "pusha\n\t"
- "pop %edi\n\t"
- "pop %esi\n\t"
- "pop %ebp\n\t"
- "add $4, %esp\n\t"
- "pop %ebx\n\t"
- "pop %edx\n\t"
- "pop %ecx\n\t"
- "pop %eax\n\t"
+ MK_INSN(pusha, "pushal\n\t"
+ "popl %edi\n\t"
+ "popl %esi\n\t"
+ "popl %ebp\n\t"
+ "addl $4, %esp\n\t"
+ "popl %ebx\n\t"
+ "popl %edx\n\t"
+ "popl %ecx\n\t"
+ "popl %eax\n\t"
);
- MK_INSN(popa, "push %eax\n\t"
- "push %ecx\n\t"
- "push %edx\n\t"
- "push %ebx\n\t"
- "push %esp\n\t"
- "push %ebp\n\t"
- "push %esi\n\t"
- "push %edi\n\t"
- "popa\n\t"
+ MK_INSN(popa, "pushl %eax\n\t"
+ "pushl %ecx\n\t"
+ "pushl %edx\n\t"
+ "pushl %ebx\n\t"
+ "pushl %esp\n\t"
+ "pushl %ebp\n\t"
+ "pushl %esi\n\t"
+ "pushl %edi\n\t"
+ "popal\n\t"
);
init_inregs(&(struct regs){ .eax = 0, .ebx = 1, .ecx = 2, .edx = 3, .esi = 4, .edi = 5, .ebp = 6 });
Then everything passes in realmode test:
$ ./x86-run x86/realmode.flat | grep FAIL
qemu-system-x86_64: warning: host doesn't support requested feature:
CPUID.80000001H:ECX.svm [bit 2]
$
Perhaps it's worth to respin the series.
Thanks,
Roman
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [kvm-unit-tests PATCH v2 10/10] travis.yml: Add x86 build with clang 10
2020-09-22 21:25 ` Roman Bolshakov
@ 2020-09-23 2:37 ` Paolo Bonzini
0 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2020-09-23 2:37 UTC (permalink / raw)
To: Roman Bolshakov; +Cc: Thomas Huth, kvm
On 22/09/20 23:25, Roman Bolshakov wrote:
>
> Then everything passes in realmode test:
> $ ./x86-run x86/realmode.flat | grep FAIL
> qemu-system-x86_64: warning: host doesn't support requested feature:
> CPUID.80000001H:ECX.svm [bit 2]
> $
>
>
> Perhaps it's worth to respin the series.
No, just send stuff on top. I've now pushed everything to master.
Paolo
^ permalink raw reply [flat|nested] 27+ messages in thread