diff mbox series

[kvm-unit-tests] x86: realmode: Workaround clang issues

Message ID 20200924120516.77299-1-r.bolshakov@yadro.com (mailing list archive)
State New, archived
Headers show
Series [kvm-unit-tests] x86: realmode: Workaround clang issues | expand

Commit Message

Roman Bolshakov Sept. 24, 2020, 12:05 p.m. UTC
clang doesn't properly support .code16gcc and generates wrong machine
code [1][2][3][4]. But the test works if object file is compiled with -m16 and
explicit suffixes are added for instructions.

1. https://lore.kernel.org/kvm/4d20fbce-d247-abf4-3ceb-da2c0d48fc50@redhat.com/
2. https://lore.kernel.org/kvm/20200915155959.GF52559@SPB-NB-133.local/
3. https://lore.kernel.org/kvm/788b7191-6987-9399-f352-2e661255157e@redhat.com/
4. https://lore.kernel.org/kvm/20200922212507.GA11460@SPB-NB-133.local/

Suggested-by: Thomas Huth <thuth@redhat.com>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
---
 .travis.yml         |  2 +-
 x86/Makefile.common |  2 +-
 x86/realmode.c      | 44 ++++++++++++++++++++++----------------------
 3 files changed, 24 insertions(+), 24 deletions(-)

Comments

Roman Bolshakov Sept. 24, 2020, 5:12 p.m. UTC | #1
On Thu, Sep 24, 2020 at 03:05:17PM +0300, Roman Bolshakov wrote:
> clang doesn't properly support .code16gcc and generates wrong machine
> code [1][2][3][4]. But the test works if object file is compiled with -m16 and
> explicit suffixes are added for instructions.
> 
> 1. https://lore.kernel.org/kvm/4d20fbce-d247-abf4-3ceb-da2c0d48fc50@redhat.com/
> 2. https://lore.kernel.org/kvm/20200915155959.GF52559@SPB-NB-133.local/
> 3. https://lore.kernel.org/kvm/788b7191-6987-9399-f352-2e661255157e@redhat.com/
> 4. https://lore.kernel.org/kvm/20200922212507.GA11460@SPB-NB-133.local/
> 
> Suggested-by: Thomas Huth <thuth@redhat.com>
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> ---
>  .travis.yml         |  2 +-
>  x86/Makefile.common |  2 +-
>  x86/realmode.c      | 44 ++++++++++++++++++++++----------------------
>  3 files changed, 24 insertions(+), 24 deletions(-)
> 
> diff --git a/.travis.yml b/.travis.yml
> index 2e5ae41..bd62190 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -24,7 +24,7 @@ jobs:
>        - 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"
> +               kvmclock_test msr pcid rdpru realmode rmap_chain s3 setjmp umip"
>        - ACCEL="kvm"
>  
>      - addons:
> diff --git a/x86/Makefile.common b/x86/Makefile.common
> index 090ce22..5567d66 100644
> --- a/x86/Makefile.common
> +++ b/x86/Makefile.common
> @@ -72,7 +72,7 @@ $(TEST_DIR)/realmode.elf: $(TEST_DIR)/realmode.o
>  	$(CC) -m32 -nostdlib -o $@ -Wl,-m,elf_i386 \
>  	      -Wl,-T,$(SRCDIR)/$(TEST_DIR)/realmode.lds $^
>  
> -$(TEST_DIR)/realmode.o: bits = 32
> +$(TEST_DIR)/realmode.o: bits = 16
>  
>  $(TEST_DIR)/kvmclock_test.elf: $(TEST_DIR)/kvmclock.o
>  
> diff --git a/x86/realmode.c b/x86/realmode.c
> index 7c2d776..c8a6ae0 100644
> --- a/x86/realmode.c
> +++ b/x86/realmode.c
> @@ -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 });
> @@ -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");
> -- 
> 2.28.0
> 

Hi,

I've noticed that the patch has been applied (thanks for that!) but a
test fails for centos-7. It has gcc that doesn't support "-m16":
https://gitlab.com/kvm-unit-tests/kvm-unit-tests/-/jobs/755387059

I'm going to come up with a patch that adds a test for the option and
fixes the issue for older gcc, then bits = 16 would be used for modern
compilers only.

Regards,
Roman
Paolo Bonzini Sept. 24, 2020, 5:21 p.m. UTC | #2
On 24/09/20 19:12, Roman Bolshakov wrote:
> I've noticed that the patch has been applied (thanks for that!) but a
> test fails for centos-7. It has gcc that doesn't support "-m16":
> https://gitlab.com/kvm-unit-tests/kvm-unit-tests/-/jobs/755387059
> 
> I'm going to come up with a patch that adds a test for the option and
> fixes the issue for older gcc, then bits = 16 would be used for modern
> compilers only.

Ok, cool!

Paolo
diff mbox series

Patch

diff --git a/.travis.yml b/.travis.yml
index 2e5ae41..bd62190 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -24,7 +24,7 @@  jobs:
       - 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"
+               kvmclock_test msr pcid rdpru realmode rmap_chain s3 setjmp umip"
       - ACCEL="kvm"
 
     - addons:
diff --git a/x86/Makefile.common b/x86/Makefile.common
index 090ce22..5567d66 100644
--- a/x86/Makefile.common
+++ b/x86/Makefile.common
@@ -72,7 +72,7 @@  $(TEST_DIR)/realmode.elf: $(TEST_DIR)/realmode.o
 	$(CC) -m32 -nostdlib -o $@ -Wl,-m,elf_i386 \
 	      -Wl,-T,$(SRCDIR)/$(TEST_DIR)/realmode.lds $^
 
-$(TEST_DIR)/realmode.o: bits = 32
+$(TEST_DIR)/realmode.o: bits = 16
 
 $(TEST_DIR)/kvmclock_test.elf: $(TEST_DIR)/kvmclock.o
 
diff --git a/x86/realmode.c b/x86/realmode.c
index 7c2d776..c8a6ae0 100644
--- a/x86/realmode.c
+++ b/x86/realmode.c
@@ -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 });
@@ -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");