| Message ID | 20200901085056.33391-11-r.bolshakov@yadro.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add support for generic ELF cross-compiler | expand |
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...
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
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
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
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
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
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
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:
.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(+)