All of lore.kernel.org
 help / color / mirror / Atom feed
* [kvm-unit-tests PATCH 0/7] Add support for generic ELF cross-compiler
@ 2020-08-10 13:06 Roman Bolshakov
  2020-08-10 13:06 ` [kvm-unit-tests PATCH 1/7] x86: Makefile: Allow division on x86_64-elf binutils Roman Bolshakov
                   ` (7 more replies)
  0 siblings, 8 replies; 24+ messages in thread
From: Roman Bolshakov @ 2020-08-10 13:06 UTC (permalink / raw)
  To: kvm; +Cc: Roman Bolshakov

The series introduces a way to build the tests with generic i686-pc-elf
and x86_64-pc-elf GCC target. It also fixes build on macOS and
introduces a way to specify enhanced getopt. Build instructions for macOS
have been updated to reflect the changes.

Roman Bolshakov (7):
  x86: Makefile: Allow division on x86_64-elf binutils
  x86: Replace instruction prefixes with spaces
  x86: Makefile: Fix linkage of realmode on x86_64-elf binutils
  lib: Bundle debugreg.h from the kernel
  lib: x86: Use portable format macros for uint32_t
  configure: Add an option to specify getopt
  README: Update build instructions for macOS

 README.macOS.md        | 71 +++++++++++++++++++++++++-----------
 configure              | 13 +++++++
 lib/pci.c              |  2 +-
 lib/x86/asm/debugreg.h | 81 ++++++++++++++++++++++++++++++++++++++++++
 run_tests.sh           |  2 +-
 x86/Makefile           |  2 ++
 x86/Makefile.common    |  3 +-
 x86/asyncpf.c          |  2 +-
 x86/cstart.S           |  4 +--
 x86/cstart64.S         |  4 +--
 x86/emulator.c         | 38 ++++++++++----------
 x86/msr.c              |  3 +-
 x86/s3.c               |  2 +-
 13 files changed, 178 insertions(+), 49 deletions(-)
 create mode 100644 lib/x86/asm/debugreg.h

-- 
2.26.1


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

* [kvm-unit-tests PATCH 1/7] x86: Makefile: Allow division on x86_64-elf binutils
  2020-08-10 13:06 [kvm-unit-tests PATCH 0/7] Add support for generic ELF cross-compiler Roman Bolshakov
@ 2020-08-10 13:06 ` Roman Bolshakov
  2020-08-28  5:00   ` Thomas Huth
  2020-08-10 13:06 ` [kvm-unit-tests PATCH 2/7] x86: Replace instruction prefixes with spaces Roman Bolshakov
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 24+ messages in thread
From: Roman Bolshakov @ 2020-08-10 13:06 UTC (permalink / raw)
  To: kvm; +Cc: 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.

The option is ignored on the Linux target of GNU binutils.

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>
---
 x86/Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/x86/Makefile b/x86/Makefile
index 8a007ab..22afbb9 100644
--- a/x86/Makefile
+++ b/x86/Makefile
@@ -1 +1,3 @@
 include $(SRCDIR)/$(TEST_DIR)/Makefile.$(ARCH)
+
+COMMON_CFLAGS += -Wa,--divide
-- 
2.26.1


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

* [kvm-unit-tests PATCH 2/7] x86: Replace instruction prefixes with spaces
  2020-08-10 13:06 [kvm-unit-tests PATCH 0/7] Add support for generic ELF cross-compiler Roman Bolshakov
  2020-08-10 13:06 ` [kvm-unit-tests PATCH 1/7] x86: Makefile: Allow division on x86_64-elf binutils Roman Bolshakov
@ 2020-08-10 13:06 ` Roman Bolshakov
  2020-08-28  4:34   ` Thomas Huth
  2020-08-10 13:06 ` [kvm-unit-tests PATCH 3/7] x86: Makefile: Fix linkage of realmode on x86_64-elf binutils Roman Bolshakov
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 24+ messages in thread
From: Roman Bolshakov @ 2020-08-10 13:06 UTC (permalink / raw)
  To: kvm; +Cc: 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>
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.26.1


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

* [kvm-unit-tests PATCH 3/7] x86: Makefile: Fix linkage of realmode on x86_64-elf binutils
  2020-08-10 13:06 [kvm-unit-tests PATCH 0/7] Add support for generic ELF cross-compiler Roman Bolshakov
  2020-08-10 13:06 ` [kvm-unit-tests PATCH 1/7] x86: Makefile: Allow division on x86_64-elf binutils Roman Bolshakov
  2020-08-10 13:06 ` [kvm-unit-tests PATCH 2/7] x86: Replace instruction prefixes with spaces Roman Bolshakov
@ 2020-08-10 13:06 ` Roman Bolshakov
  2020-08-28  5:44   ` Thomas Huth
  2020-08-10 13:06 ` [kvm-unit-tests PATCH 4/7] lib: Bundle debugreg.h from the kernel Roman Bolshakov
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 24+ messages in thread
From: Roman Bolshakov @ 2020-08-10 13:06 UTC (permalink / raw)
  To: kvm; +Cc: 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>
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 2ea9c9f..8230ac0 100644
--- a/x86/Makefile.common
+++ b/x86/Makefile.common
@@ -66,7 +66,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.26.1


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

* [kvm-unit-tests PATCH 4/7] lib: Bundle debugreg.h from the kernel
  2020-08-10 13:06 [kvm-unit-tests PATCH 0/7] Add support for generic ELF cross-compiler Roman Bolshakov
                   ` (2 preceding siblings ...)
  2020-08-10 13:06 ` [kvm-unit-tests PATCH 3/7] x86: Makefile: Fix linkage of realmode on x86_64-elf binutils Roman Bolshakov
@ 2020-08-10 13:06 ` Roman Bolshakov
  2020-08-28  4:56   ` Thomas Huth
  2020-08-10 13:06 ` [kvm-unit-tests PATCH 5/7] lib: x86: Use portable format macros for uint32_t Roman Bolshakov
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 24+ messages in thread
From: Roman Bolshakov @ 2020-08-10 13:06 UTC (permalink / raw)
  To: kvm
  Cc: Roman Bolshakov, Jim Mattson, Peter Shier, Paolo Bonzini,
	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>
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.26.1


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

* [kvm-unit-tests PATCH 5/7] lib: x86: Use portable format macros for uint32_t
  2020-08-10 13:06 [kvm-unit-tests PATCH 0/7] Add support for generic ELF cross-compiler Roman Bolshakov
                   ` (3 preceding siblings ...)
  2020-08-10 13:06 ` [kvm-unit-tests PATCH 4/7] lib: Bundle debugreg.h from the kernel Roman Bolshakov
@ 2020-08-10 13:06 ` Roman Bolshakov
  2020-08-28  5:49   ` Thomas Huth
  2020-08-10 13:06 ` [kvm-unit-tests PATCH 6/7] configure: Add an option to specify getopt Roman Bolshakov
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 24+ messages in thread
From: Roman Bolshakov @ 2020-08-10 13:06 UTC (permalink / raw)
  To: kvm
  Cc: 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.26.1


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

* [kvm-unit-tests PATCH 6/7] configure: Add an option to specify getopt
  2020-08-10 13:06 [kvm-unit-tests PATCH 0/7] Add support for generic ELF cross-compiler Roman Bolshakov
                   ` (4 preceding siblings ...)
  2020-08-10 13:06 ` [kvm-unit-tests PATCH 5/7] lib: x86: Use portable format macros for uint32_t Roman Bolshakov
@ 2020-08-10 13:06 ` Roman Bolshakov
  2020-08-28  5:55   ` Thomas Huth
  2020-08-10 13:06 ` [kvm-unit-tests PATCH 7/7] README: Update build instructions for macOS Roman Bolshakov
  2020-08-26 16:52 ` [kvm-unit-tests PATCH 0/7] Add support for generic ELF cross-compiler Roman Bolshakov
  7 siblings, 1 reply; 24+ messages in thread
From: Roman Bolshakov @ 2020-08-10 13:06 UTC (permalink / raw)
  To: kvm; +Cc: 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 f9d030f..556ff13 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=
@@ -31,6 +32,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
@@ -76,6 +78,9 @@ while [[ "$1" = -* ]]; do
 	--ld)
 	    ld="$arg"
 	    ;;
+	--getopt)
+	    getopt="$arg"
+	    ;;
 	--enable-pretty-print-stacks)
 	    pretty_print_stacks=yes
 	    ;;
@@ -156,6 +161,13 @@ EOF
 u32_long=$("$cross_prefix$cc" -E lib-test.c | grep -v '^#' | grep -q long && echo yes)
 rm -f lib-test.c
 
+# 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
@@ -198,6 +210,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.26.1


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

* [kvm-unit-tests PATCH 7/7] README: Update build instructions for macOS
  2020-08-10 13:06 [kvm-unit-tests PATCH 0/7] Add support for generic ELF cross-compiler Roman Bolshakov
                   ` (5 preceding siblings ...)
  2020-08-10 13:06 ` [kvm-unit-tests PATCH 6/7] configure: Add an option to specify getopt Roman Bolshakov
@ 2020-08-10 13:06 ` Roman Bolshakov
  2020-08-26 16:52 ` [kvm-unit-tests PATCH 0/7] Add support for generic ELF cross-compiler Roman Bolshakov
  7 siblings, 0 replies; 24+ messages in thread
From: Roman Bolshakov @ 2020-08-10 13:06 UTC (permalink / raw)
  To: kvm; +Cc: 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.26.1


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

* Re: [kvm-unit-tests PATCH 0/7] Add support for generic ELF cross-compiler
  2020-08-10 13:06 [kvm-unit-tests PATCH 0/7] Add support for generic ELF cross-compiler Roman Bolshakov
                   ` (6 preceding siblings ...)
  2020-08-10 13:06 ` [kvm-unit-tests PATCH 7/7] README: Update build instructions for macOS Roman Bolshakov
@ 2020-08-26 16:52 ` Roman Bolshakov
  7 siblings, 0 replies; 24+ messages in thread
From: Roman Bolshakov @ 2020-08-26 16:52 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: kvm

On Mon, Aug 10, 2020 at 04:06:11PM +0300, Roman Bolshakov wrote:
> The series introduces a way to build the tests with generic i686-pc-elf
> and x86_64-pc-elf GCC target. It also fixes build on macOS and
> introduces a way to specify enhanced getopt. Build instructions for macOS
> have been updated to reflect the changes.
> 
> Roman Bolshakov (7):
>   x86: Makefile: Allow division on x86_64-elf binutils
>   x86: Replace instruction prefixes with spaces
>   x86: Makefile: Fix linkage of realmode on x86_64-elf binutils
>   lib: Bundle debugreg.h from the kernel
>   lib: x86: Use portable format macros for uint32_t
>   configure: Add an option to specify getopt
>   README: Update build instructions for macOS
> 
>  README.macOS.md        | 71 +++++++++++++++++++++++++-----------
>  configure              | 13 +++++++
>  lib/pci.c              |  2 +-
>  lib/x86/asm/debugreg.h | 81 ++++++++++++++++++++++++++++++++++++++++++
>  run_tests.sh           |  2 +-
>  x86/Makefile           |  2 ++
>  x86/Makefile.common    |  3 +-
>  x86/asyncpf.c          |  2 +-
>  x86/cstart.S           |  4 +--
>  x86/cstart64.S         |  4 +--
>  x86/emulator.c         | 38 ++++++++++----------
>  x86/msr.c              |  3 +-
>  x86/s3.c               |  2 +-
>  13 files changed, 178 insertions(+), 49 deletions(-)
>  create mode 100644 lib/x86/asm/debugreg.h
> 
> -- 
> 2.26.1
> 

Hi Paolo,

could you please take a look?

Best Regards,
Roman

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

* Re: [kvm-unit-tests PATCH 2/7] x86: Replace instruction prefixes with spaces
  2020-08-10 13:06 ` [kvm-unit-tests PATCH 2/7] x86: Replace instruction prefixes with spaces Roman Bolshakov
@ 2020-08-28  4:34   ` Thomas Huth
  0 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2020-08-28  4:34 UTC (permalink / raw)
  To: Roman Bolshakov, kvm; +Cc: Cameron Esfahani

On 10/08/2020 15.06, Roman Bolshakov wrote:
> 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>
> 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");
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>


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

* Re: [kvm-unit-tests PATCH 4/7] lib: Bundle debugreg.h from the kernel
  2020-08-10 13:06 ` [kvm-unit-tests PATCH 4/7] lib: Bundle debugreg.h from the kernel Roman Bolshakov
@ 2020-08-28  4:56   ` Thomas Huth
  0 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2020-08-28  4:56 UTC (permalink / raw)
  To: Roman Bolshakov, kvm
  Cc: Jim Mattson, Peter Shier, Paolo Bonzini, Cameron Esfahani

On 10/08/2020 15.06, Roman Bolshakov wrote:
> 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>
> 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

Reviewed-by: Thomas Huth <thuth@redhat.com>


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

* Re: [kvm-unit-tests PATCH 1/7] x86: Makefile: Allow division on x86_64-elf binutils
  2020-08-10 13:06 ` [kvm-unit-tests PATCH 1/7] x86: Makefile: Allow division on x86_64-elf binutils Roman Bolshakov
@ 2020-08-28  5:00   ` Thomas Huth
  2020-08-28  6:54     ` Roman Bolshakov
  2020-08-31 17:30     ` Roman Bolshakov
  0 siblings, 2 replies; 24+ messages in thread
From: Thomas Huth @ 2020-08-28  5:00 UTC (permalink / raw)
  To: Roman Bolshakov, kvm; +Cc: Cameron Esfahani

On 10/08/2020 15.06, Roman Bolshakov wrote:
> 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.
> 
> The option is ignored on the Linux target of GNU binutils.
> 
> 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>
> ---
>  x86/Makefile | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/x86/Makefile b/x86/Makefile
> index 8a007ab..22afbb9 100644
> --- a/x86/Makefile
> +++ b/x86/Makefile
> @@ -1 +1,3 @@
>  include $(SRCDIR)/$(TEST_DIR)/Makefile.$(ARCH)
> +
> +COMMON_CFLAGS += -Wa,--divide

Some weeks ago, I also played with an elf cross compiler and came to the
same conclusion, that we need this option there. Unfortunately, it does
not work with clang:

 https://gitlab.com/huth/kvm-unit-tests/-/jobs/707986800#L1629

You could try to wrap it with "cc-option" instead ... or use a proper
check in the configure script to detect whether it's needed or not.

And can you please put it next to the other COMMON_CFLAGS in
x86/Makefile.common instead of x86/Makefile?

 Thanks,
  Thomas


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

* Re: [kvm-unit-tests PATCH 3/7] x86: Makefile: Fix linkage of realmode on x86_64-elf binutils
  2020-08-10 13:06 ` [kvm-unit-tests PATCH 3/7] x86: Makefile: Fix linkage of realmode on x86_64-elf binutils Roman Bolshakov
@ 2020-08-28  5:44   ` Thomas Huth
  0 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2020-08-28  5:44 UTC (permalink / raw)
  To: Roman Bolshakov, kvm; +Cc: Cameron Esfahani

On 10/08/2020 15.06, Roman Bolshakov wrote:
> 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>
> 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 2ea9c9f..8230ac0 100644
> --- a/x86/Makefile.common
> +++ b/x86/Makefile.common
> @@ -66,7 +66,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

Reviewed-by: Thomas Huth <thuth@redhat.com>


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

* Re: [kvm-unit-tests PATCH 5/7] lib: x86: Use portable format macros for uint32_t
  2020-08-10 13:06 ` [kvm-unit-tests PATCH 5/7] lib: x86: Use portable format macros for uint32_t Roman Bolshakov
@ 2020-08-28  5:49   ` Thomas Huth
  0 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2020-08-28  5:49 UTC (permalink / raw)
  To: Roman Bolshakov, kvm; +Cc: Alex Bennée, Andrew Jones, Cameron Esfahani

On 10/08/2020 15.06, 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>
> 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.  */
> 

Reviewed-by: Thomas Huth <thuth@redhat.com>


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

* Re: [kvm-unit-tests PATCH 6/7] configure: Add an option to specify getopt
  2020-08-10 13:06 ` [kvm-unit-tests PATCH 6/7] configure: Add an option to specify getopt Roman Bolshakov
@ 2020-08-28  5:55   ` Thomas Huth
  2020-08-28  7:12     ` Roman Bolshakov
  0 siblings, 1 reply; 24+ messages in thread
From: Thomas Huth @ 2020-08-28  5:55 UTC (permalink / raw)
  To: Roman Bolshakov, kvm; +Cc: Cameron Esfahani

On 10/08/2020 15.06, 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(-)

Is this still required with a newer version of bash? The one that ships
with macOS is just too old...

I assume that getopt is a builtin function in newer versions of the bash?

Last time we discussed, we agreed that Bash v4.2 would be a reasonable
minimum for the kvm-unit-tests:

 https://www.spinics.net/lists/kvm/msg222139.html

Thus if the user installed bash from homebrew on macos, we should be fine?

Could you maybe replace this patch with a check for a minimum version of
bash instead?

 Thomas


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

* Re: [kvm-unit-tests PATCH 1/7] x86: Makefile: Allow division on x86_64-elf binutils
  2020-08-28  5:00   ` Thomas Huth
@ 2020-08-28  6:54     ` Roman Bolshakov
  2020-08-28  7:24       ` Thomas Huth
  2020-08-31 17:30     ` Roman Bolshakov
  1 sibling, 1 reply; 24+ messages in thread
From: Roman Bolshakov @ 2020-08-28  6:54 UTC (permalink / raw)
  To: Thomas Huth; +Cc: kvm, Cameron Esfahani

On Fri, Aug 28, 2020 at 07:00:19AM +0200, Thomas Huth wrote:
> On 10/08/2020 15.06, Roman Bolshakov wrote:
> > 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.
> > 
> > The option is ignored on the Linux target of GNU binutils.
> > 
> > 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>
> > ---
> >  x86/Makefile | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/x86/Makefile b/x86/Makefile
> > index 8a007ab..22afbb9 100644
> > --- a/x86/Makefile
> > +++ b/x86/Makefile
> > @@ -1 +1,3 @@
> >  include $(SRCDIR)/$(TEST_DIR)/Makefile.$(ARCH)
> > +
> > +COMMON_CFLAGS += -Wa,--divide
> 
> Some weeks ago, I also played with an elf cross compiler and came to the
> same conclusion, that we need this option there. Unfortunately, it does
> not work with clang:
> 
>  https://gitlab.com/huth/kvm-unit-tests/-/jobs/707986800#L1629
> 
> You could try to wrap it with "cc-option" instead ... or use a proper
> check in the configure script to detect whether it's needed or not.
> 

Hi Thomas,

Thanks for reviewing the series. I'll look into both options and will
test with both gcc and clang afterwards. I can also update .travis.yml
in a new patch to test the build on macOS.

> And can you please put it next to the other COMMON_CFLAGS in
> x86/Makefile.common instead of x86/Makefile?
> 

Sure.

Regards,
Roman

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

* Re: [kvm-unit-tests PATCH 6/7] configure: Add an option to specify getopt
  2020-08-28  5:55   ` Thomas Huth
@ 2020-08-28  7:12     ` Roman Bolshakov
  2020-08-28  7:34       ` Thomas Huth
  0 siblings, 1 reply; 24+ messages in thread
From: Roman Bolshakov @ 2020-08-28  7:12 UTC (permalink / raw)
  To: Thomas Huth; +Cc: kvm, Cameron Esfahani

On Fri, Aug 28, 2020 at 07:55:53AM +0200, Thomas Huth wrote:
> On 10/08/2020 15.06, 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(-)
> 
> Is this still required with a newer version of bash? The one that ships
> with macOS is just too old...
> 
> I assume that getopt is a builtin function in newer versions of the bash?
> 

Except it has `s` at the end. There's a getopts built-in in bash.
I'll try to replace external getopt with getopts.

> Last time we discussed, we agreed that Bash v4.2 would be a reasonable
> minimum for the kvm-unit-tests:
> 
>  https://www.spinics.net/lists/kvm/msg222139.html
> 
> Thus if the user installed bash from homebrew on macos, we should be fine?
> 
> Could you maybe replace this patch with a check for a minimum version of
> bash instead?
> 

No problem, if getopts works.

Thanks,
Roman

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

* Re: [kvm-unit-tests PATCH 1/7] x86: Makefile: Allow division on x86_64-elf binutils
  2020-08-28  6:54     ` Roman Bolshakov
@ 2020-08-28  7:24       ` Thomas Huth
  2020-08-28  7:47         ` Roman Bolshakov
  0 siblings, 1 reply; 24+ messages in thread
From: Thomas Huth @ 2020-08-28  7:24 UTC (permalink / raw)
  To: Roman Bolshakov; +Cc: kvm, Cameron Esfahani

On 28/08/2020 08.54, Roman Bolshakov wrote:
> On Fri, Aug 28, 2020 at 07:00:19AM +0200, Thomas Huth wrote:
>> On 10/08/2020 15.06, Roman Bolshakov wrote:
>>> 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.
>>>
>>> The option is ignored on the Linux target of GNU binutils.
>>>
>>> 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>
>>> ---
>>>  x86/Makefile | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/x86/Makefile b/x86/Makefile
>>> index 8a007ab..22afbb9 100644
>>> --- a/x86/Makefile
>>> +++ b/x86/Makefile
>>> @@ -1 +1,3 @@
>>>  include $(SRCDIR)/$(TEST_DIR)/Makefile.$(ARCH)
>>> +
>>> +COMMON_CFLAGS += -Wa,--divide
>>
>> Some weeks ago, I also played with an elf cross compiler and came to the
>> same conclusion, that we need this option there. Unfortunately, it does
>> not work with clang:
>>
>>  https://gitlab.com/huth/kvm-unit-tests/-/jobs/707986800#L1629
>>
>> You could try to wrap it with "cc-option" instead ... or use a proper
>> check in the configure script to detect whether it's needed or not.
>>
> 
> Hi Thomas,
> 
> Thanks for reviewing the series. I'll look into both options and will
> test with both gcc and clang afterwards. I can also update .travis.yml
> in a new patch to test the build on macOS.

That would be great, thanks! Note that you need at least Clang v10 (the
one from Fedora 32 is fine) to compile the kvm-unit-tests.

And if it's of any help, this was the stuff that I used in .travis.yml
for my experiments (might still be incomplete, though):

    - os: osx
      osx_image: xcode12
      addons:
        homebrew:
          packages:
            - bash
            - coreutils
            - qemu
            - x86_64-elf-gcc
      env:
      - CONFIG="--cross-prefix=x86_64-elf-"
      - BUILD_DIR="build"
      - TESTS="umip"
      - ACCEL="tcg"

    - os: osx
      osx_image: xcode12
      addons:
        homebrew:
          packages:
            - bash
            - coreutils
            - qemu
            - i386-elf-gcc
      env:
      - CONFIG="--arch=i386 --cross-prefix=x86_64-elf-"
      - BUILD_DIR="build"
      - TESTS="umip"
      - ACCEL="tcg"

 Thomas


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

* Re: [kvm-unit-tests PATCH 6/7] configure: Add an option to specify getopt
  2020-08-28  7:12     ` Roman Bolshakov
@ 2020-08-28  7:34       ` Thomas Huth
  0 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2020-08-28  7:34 UTC (permalink / raw)
  To: Roman Bolshakov; +Cc: kvm, Cameron Esfahani, Stefan Raspl, Andrew Jones

On 28/08/2020 09.12, Roman Bolshakov wrote:
> On Fri, Aug 28, 2020 at 07:55:53AM +0200, Thomas Huth wrote:
>> On 10/08/2020 15.06, 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(-)
>>
>> Is this still required with a newer version of bash? The one that ships
>> with macOS is just too old...
>>
>> I assume that getopt is a builtin function in newer versions of the bash?
>>
> 
> Except it has `s` at the end. There's a getopts built-in in bash.
> I'll try to replace external getopt with getopts.

Ouch, ok, I wasn't aware of the difference between getopt and getopts. I
guess your patch here is ok in that case.

Or maybe we should simply revert d4d34e6484825ee5734b042c215c06
("run_tests: fix command line options handling") and state in the docs
that options have to be given before the tests on the command line...

 Thomas


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

* Re: [kvm-unit-tests PATCH 1/7] x86: Makefile: Allow division on x86_64-elf binutils
  2020-08-28  7:24       ` Thomas Huth
@ 2020-08-28  7:47         ` Roman Bolshakov
  2020-08-28  7:56           ` Thomas Huth
  0 siblings, 1 reply; 24+ messages in thread
From: Roman Bolshakov @ 2020-08-28  7:47 UTC (permalink / raw)
  To: Thomas Huth; +Cc: kvm, Cameron Esfahani

On Fri, Aug 28, 2020 at 09:24:10AM +0200, Thomas Huth wrote:
> On 28/08/2020 08.54, Roman Bolshakov wrote:
> > On Fri, Aug 28, 2020 at 07:00:19AM +0200, Thomas Huth wrote:
> >> On 10/08/2020 15.06, Roman Bolshakov wrote:
> >>> 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.
> >>>
> >>> The option is ignored on the Linux target of GNU binutils.
> >>>
> >>> 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>
> >>> ---
> >>>  x86/Makefile | 2 ++
> >>>  1 file changed, 2 insertions(+)
> >>>
> >>> diff --git a/x86/Makefile b/x86/Makefile
> >>> index 8a007ab..22afbb9 100644
> >>> --- a/x86/Makefile
> >>> +++ b/x86/Makefile
> >>> @@ -1 +1,3 @@
> >>>  include $(SRCDIR)/$(TEST_DIR)/Makefile.$(ARCH)
> >>> +
> >>> +COMMON_CFLAGS += -Wa,--divide
> >>
> >> Some weeks ago, I also played with an elf cross compiler and came to the
> >> same conclusion, that we need this option there. Unfortunately, it does
> >> not work with clang:
> >>
> >>  https://gitlab.com/huth/kvm-unit-tests/-/jobs/707986800#L1629
> >>
> >> You could try to wrap it with "cc-option" instead ... or use a proper
> >> check in the configure script to detect whether it's needed or not.
> >>
> > 
> > Hi Thomas,
> > 
> > Thanks for reviewing the series. I'll look into both options and will
> > test with both gcc and clang afterwards. I can also update .travis.yml
> > in a new patch to test the build on macOS.
> 
> That would be great, thanks! Note that you need at least Clang v10 (the
> one from Fedora 32 is fine) to compile the kvm-unit-tests.
> 
> And if it's of any help, this was the stuff that I used in .travis.yml
> for my experiments (might still be incomplete, though):
> 
>     - os: osx
>       osx_image: xcode12
>       addons:
>         homebrew:
>           packages:
>             - bash
>             - coreutils
>             - qemu
>             - x86_64-elf-gcc
>       env:
>       - CONFIG="--cross-prefix=x86_64-elf-"
>       - BUILD_DIR="build"
>       - TESTS="umip"
>       - ACCEL="tcg"
> 
>     - os: osx
>       osx_image: xcode12
>       addons:
>         homebrew:
>           packages:
>             - bash
>             - coreutils
>             - qemu
>             - i386-elf-gcc

It's going to be i686-elf-gcc.

>       env:
>       - CONFIG="--arch=i386 --cross-prefix=x86_64-elf-"

`--cross-prefix=i686-elf-`, respectively.

>       - BUILD_DIR="build"
>       - TESTS="umip"
>       - ACCEL="tcg"
> 

Thanks, I'll use it as the basis. Do I need to add your Signed-off-by: here?
or Suggested-by: is enough?

IIRC all tests pass on TCG/5.1.

Regards,
Roman

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

* Re: [kvm-unit-tests PATCH 1/7] x86: Makefile: Allow division on x86_64-elf binutils
  2020-08-28  7:47         ` Roman Bolshakov
@ 2020-08-28  7:56           ` Thomas Huth
  2020-08-28  8:37             ` Roman Bolshakov
  0 siblings, 1 reply; 24+ messages in thread
From: Thomas Huth @ 2020-08-28  7:56 UTC (permalink / raw)
  To: Roman Bolshakov; +Cc: kvm, Cameron Esfahani

On 28/08/2020 09.47, Roman Bolshakov wrote:
> On Fri, Aug 28, 2020 at 09:24:10AM +0200, Thomas Huth wrote:
>> On 28/08/2020 08.54, Roman Bolshakov wrote:
>>> On Fri, Aug 28, 2020 at 07:00:19AM +0200, Thomas Huth wrote:
>>>> On 10/08/2020 15.06, Roman Bolshakov wrote:
>>>>> 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.
>>>>>
>>>>> The option is ignored on the Linux target of GNU binutils.
>>>>>
>>>>> 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>
>>>>> ---
>>>>>  x86/Makefile | 2 ++
>>>>>  1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/x86/Makefile b/x86/Makefile
>>>>> index 8a007ab..22afbb9 100644
>>>>> --- a/x86/Makefile
>>>>> +++ b/x86/Makefile
>>>>> @@ -1 +1,3 @@
>>>>>  include $(SRCDIR)/$(TEST_DIR)/Makefile.$(ARCH)
>>>>> +
>>>>> +COMMON_CFLAGS += -Wa,--divide
>>>>
>>>> Some weeks ago, I also played with an elf cross compiler and came to the
>>>> same conclusion, that we need this option there. Unfortunately, it does
>>>> not work with clang:
>>>>
>>>>  https://gitlab.com/huth/kvm-unit-tests/-/jobs/707986800#L1629
>>>>
>>>> You could try to wrap it with "cc-option" instead ... or use a proper
>>>> check in the configure script to detect whether it's needed or not.
>>>>
>>>
>>> Hi Thomas,
>>>
>>> Thanks for reviewing the series. I'll look into both options and will
>>> test with both gcc and clang afterwards. I can also update .travis.yml
>>> in a new patch to test the build on macOS.
>>
>> That would be great, thanks! Note that you need at least Clang v10 (the
>> one from Fedora 32 is fine) to compile the kvm-unit-tests.
>>
>> And if it's of any help, this was the stuff that I used in .travis.yml
>> for my experiments (might still be incomplete, though):
>>
>>     - os: osx
>>       osx_image: xcode12
>>       addons:
>>         homebrew:
>>           packages:
>>             - bash
>>             - coreutils
>>             - qemu
>>             - x86_64-elf-gcc
>>       env:
>>       - CONFIG="--cross-prefix=x86_64-elf-"
>>       - BUILD_DIR="build"
>>       - TESTS="umip"
>>       - ACCEL="tcg"
>>
>>     - os: osx
>>       osx_image: xcode12
>>       addons:
>>         homebrew:
>>           packages:
>>             - bash
>>             - coreutils
>>             - qemu
>>             - i386-elf-gcc
> 
> It's going to be i686-elf-gcc.

Ah, there are two flavours? Ok, good to know.

>>       env:
>>       - CONFIG="--arch=i386 --cross-prefix=x86_64-elf-"
> 
> `--cross-prefix=i686-elf-`, respectively.
> 
>>       - BUILD_DIR="build"
>>       - TESTS="umip"
>>       - ACCEL="tcg"
>>
> 
> Thanks, I'll use it as the basis. Do I need to add your Signed-off-by: here?
> or Suggested-by: is enough?

Suggested-by is fine.

> IIRC all tests pass on TCG/5.1.

Yes, please tweak the TEST="..:" lines accordingly, I just added one
test there for my initial tries.

 Thomas


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

* Re: [kvm-unit-tests PATCH 1/7] x86: Makefile: Allow division on x86_64-elf binutils
  2020-08-28  7:56           ` Thomas Huth
@ 2020-08-28  8:37             ` Roman Bolshakov
  0 siblings, 0 replies; 24+ messages in thread
From: Roman Bolshakov @ 2020-08-28  8:37 UTC (permalink / raw)
  To: Thomas Huth; +Cc: kvm, Cameron Esfahani

On Fri, Aug 28, 2020 at 09:56:28AM +0200, Thomas Huth wrote:
> On 28/08/2020 09.47, Roman Bolshakov wrote:
> > On Fri, Aug 28, 2020 at 09:24:10AM +0200, Thomas Huth wrote:
> >> On 28/08/2020 08.54, Roman Bolshakov wrote:
> >>> On Fri, Aug 28, 2020 at 07:00:19AM +0200, Thomas Huth wrote:
> >>>> On 10/08/2020 15.06, Roman Bolshakov wrote:
> >>>>> 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.
> >>>>>
> >>>>> The option is ignored on the Linux target of GNU binutils.
> >>>>>
> >>>>> 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>
> >>>>> ---
> >>>>>  x86/Makefile | 2 ++
> >>>>>  1 file changed, 2 insertions(+)
> >>>>>
> >>>>> diff --git a/x86/Makefile b/x86/Makefile
> >>>>> index 8a007ab..22afbb9 100644
> >>>>> --- a/x86/Makefile
> >>>>> +++ b/x86/Makefile
> >>>>> @@ -1 +1,3 @@
> >>>>>  include $(SRCDIR)/$(TEST_DIR)/Makefile.$(ARCH)
> >>>>> +
> >>>>> +COMMON_CFLAGS += -Wa,--divide
> >>>>
> >>>> Some weeks ago, I also played with an elf cross compiler and came to the
> >>>> same conclusion, that we need this option there. Unfortunately, it does
> >>>> not work with clang:
> >>>>
> >>>>  https://gitlab.com/huth/kvm-unit-tests/-/jobs/707986800#L1629
> >>>>
> >>>> You could try to wrap it with "cc-option" instead ... or use a proper
> >>>> check in the configure script to detect whether it's needed or not.
> >>>>
> >>>
> >>> Hi Thomas,
> >>>
> >>> Thanks for reviewing the series. I'll look into both options and will
> >>> test with both gcc and clang afterwards. I can also update .travis.yml
> >>> in a new patch to test the build on macOS.
> >>
> >> That would be great, thanks! Note that you need at least Clang v10 (the
> >> one from Fedora 32 is fine) to compile the kvm-unit-tests.
> >>
> >> And if it's of any help, this was the stuff that I used in .travis.yml
> >> for my experiments (might still be incomplete, though):
> >>
> >>     - os: osx
> >>       osx_image: xcode12
> >>       addons:
> >>         homebrew:
> >>           packages:
> >>             - bash
> >>             - coreutils
> >>             - qemu
> >>             - x86_64-elf-gcc
> >>       env:
> >>       - CONFIG="--cross-prefix=x86_64-elf-"
> >>       - BUILD_DIR="build"
> >>       - TESTS="umip"
> >>       - ACCEL="tcg"
> >>
> >>     - os: osx
> >>       osx_image: xcode12
> >>       addons:
> >>         homebrew:
> >>           packages:
> >>             - bash
> >>             - coreutils
> >>             - qemu
> >>             - i386-elf-gcc
> > 
> > It's going to be i686-elf-gcc.
> 
> Ah, there are two flavours? Ok, good to know.
> 

i386-elf-gcc package was configured as x86_64-pc-elf target and then was
renamed to x86_64-elf-gcc to match GCC target name (last December or in
January).

x86_64_elf-gcc can't properly link 32-bit ELF binaries because of GCC
PR16470
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=16470), PR32044
(https://gcc.gnu.org/bugzilla/show_bug.cgi?id=32044). A cross-compiler
is needed for that, that's why i686-elf-gcc was added in June/May:

https://github.com/Homebrew/homebrew-core/pull/54946/commits/94ee559e6c6284539f74662a85ec20c174436677

But as far as I understand i686-pc-elf target is the same as i386-pc-elf
in GCC/binutils (according to code inspection and a following discussion
freenode's #gcc channel).

FWIW, i686-elf-gcc can be installed in parallel with x86_64-elf-gcc.

Thanks,
Roman

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

* Re: [kvm-unit-tests PATCH 1/7] x86: Makefile: Allow division on x86_64-elf binutils
  2020-08-28  5:00   ` Thomas Huth
  2020-08-28  6:54     ` Roman Bolshakov
@ 2020-08-31 17:30     ` Roman Bolshakov
  2020-08-31 20:33       ` Thomas Huth
  1 sibling, 1 reply; 24+ messages in thread
From: Roman Bolshakov @ 2020-08-31 17:30 UTC (permalink / raw)
  To: Thomas Huth; +Cc: kvm, Cameron Esfahani

On Fri, Aug 28, 2020 at 07:00:19AM +0200, Thomas Huth wrote:
> On 10/08/2020 15.06, Roman Bolshakov wrote:
> > +
> > +COMMON_CFLAGS += -Wa,--divide
> 
> Some weeks ago, I also played with an elf cross compiler and came to the
> same conclusion, that we need this option there. Unfortunately, it does
> not work with clang:
> 
>  https://gitlab.com/huth/kvm-unit-tests/-/jobs/707986800#L1629
> 
> You could try to wrap it with "cc-option" instead ... or use a proper
> check in the configure script to detect whether it's needed or not.
> 

Hi Thomas,

I've wrapped it but clang can't deal with another option:
-Woverride-init

Even if I wrap it with cc-option and add wrapped clang's
-Winitializer-overrides, the build fails with:

x86/vmexit.c:577:5: error: no previous prototype for function 'main'
      [-Werror,-Wmissing-prototypes]
int main(int ac, char **av)
    ^
1 error generated.
<builtin>: recipe for target 'x86/vmexit.o' failed

I'm puzzled with this one.

CI log (ubuntu focal + clang 10):
https://travis-ci.com/github/roolebo/kvm-unit-tests/jobs/379561410

Now I wonder if wrong clang is used... Perhaps I should try
--cc=clang-10 in .travis.yml instead of --cc=clang.

Thanks,
Roman

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

* Re: [kvm-unit-tests PATCH 1/7] x86: Makefile: Allow division on x86_64-elf binutils
  2020-08-31 17:30     ` Roman Bolshakov
@ 2020-08-31 20:33       ` Thomas Huth
  0 siblings, 0 replies; 24+ messages in thread
From: Thomas Huth @ 2020-08-31 20:33 UTC (permalink / raw)
  To: Roman Bolshakov; +Cc: kvm, Cameron Esfahani

On 31/08/2020 19.30, Roman Bolshakov wrote:
> On Fri, Aug 28, 2020 at 07:00:19AM +0200, Thomas Huth wrote:
>> On 10/08/2020 15.06, Roman Bolshakov wrote:
>>> +
>>> +COMMON_CFLAGS += -Wa,--divide
>>
>> Some weeks ago, I also played with an elf cross compiler and came to the
>> same conclusion, that we need this option there. Unfortunately, it does
>> not work with clang:
>>
>>  https://gitlab.com/huth/kvm-unit-tests/-/jobs/707986800#L1629
>>
>> You could try to wrap it with "cc-option" instead ... or use a proper
>> check in the configure script to detect whether it's needed or not.
>>
> 
> Hi Thomas,
> 
> I've wrapped it but clang can't deal with another option:
> -Woverride-init
> 
> Even if I wrap it with cc-option and add wrapped clang's
> -Winitializer-overrides, the build fails with:
> 
> x86/vmexit.c:577:5: error: no previous prototype for function 'main'
>       [-Werror,-Wmissing-prototypes]
> int main(int ac, char **av)
>     ^
> 1 error generated.
> <builtin>: recipe for target 'x86/vmexit.o' failed
> 
> I'm puzzled with this one.
> 
> CI log (ubuntu focal + clang 10):
> https://travis-ci.com/github/roolebo/kvm-unit-tests/jobs/379561410
> 
> Now I wonder if wrong clang is used... Perhaps I should try
> --cc=clang-10 in .travis.yml instead of --cc=clang.

 Hi Roman,

yes, if you see the "no previous prototype for function 'main'" warning,
your Clang is too old, you really need the latest and greatest version
for the kvm-unit-tests. IIRC version 10 should be fine.

 Thomas


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

end of thread, other threads:[~2020-08-31 20:33 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-10 13:06 [kvm-unit-tests PATCH 0/7] Add support for generic ELF cross-compiler Roman Bolshakov
2020-08-10 13:06 ` [kvm-unit-tests PATCH 1/7] x86: Makefile: Allow division on x86_64-elf binutils Roman Bolshakov
2020-08-28  5:00   ` Thomas Huth
2020-08-28  6:54     ` Roman Bolshakov
2020-08-28  7:24       ` Thomas Huth
2020-08-28  7:47         ` Roman Bolshakov
2020-08-28  7:56           ` Thomas Huth
2020-08-28  8:37             ` Roman Bolshakov
2020-08-31 17:30     ` Roman Bolshakov
2020-08-31 20:33       ` Thomas Huth
2020-08-10 13:06 ` [kvm-unit-tests PATCH 2/7] x86: Replace instruction prefixes with spaces Roman Bolshakov
2020-08-28  4:34   ` Thomas Huth
2020-08-10 13:06 ` [kvm-unit-tests PATCH 3/7] x86: Makefile: Fix linkage of realmode on x86_64-elf binutils Roman Bolshakov
2020-08-28  5:44   ` Thomas Huth
2020-08-10 13:06 ` [kvm-unit-tests PATCH 4/7] lib: Bundle debugreg.h from the kernel Roman Bolshakov
2020-08-28  4:56   ` Thomas Huth
2020-08-10 13:06 ` [kvm-unit-tests PATCH 5/7] lib: x86: Use portable format macros for uint32_t Roman Bolshakov
2020-08-28  5:49   ` Thomas Huth
2020-08-10 13:06 ` [kvm-unit-tests PATCH 6/7] configure: Add an option to specify getopt Roman Bolshakov
2020-08-28  5:55   ` Thomas Huth
2020-08-28  7:12     ` Roman Bolshakov
2020-08-28  7:34       ` Thomas Huth
2020-08-10 13:06 ` [kvm-unit-tests PATCH 7/7] README: Update build instructions for macOS Roman Bolshakov
2020-08-26 16:52 ` [kvm-unit-tests PATCH 0/7] Add support for generic ELF cross-compiler Roman Bolshakov

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