All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Jan Beulich" <JBeulich@suse.com>
To: xen-devel <xen-devel@lists.xenproject.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: [PATCH 1/7] x86emul: don't special case fetching the immediate of PUSH
Date: Thu, 11 Aug 2016 06:03:36 -0600	[thread overview]
Message-ID: <57AC85B8020000780010509E@prv-mh.provo.novell.com> (raw)
In-Reply-To: <57AC83530200007800105078@prv-mh.provo.novell.com>

[-- Attachment #1: Type: text/plain, Size: 7862 bytes --]

These immediates follow the standard patterns in all modes, so they're
better fetched by the generic source operand handling code.

To facilitate testing, instead of adding yet another of these pretty
convoluted individual test cases, simply introduce another blowfish run
with -mno-accumulate-outgoing-args (the additional -Dstatic is to
keep the compiler from converting the calling convention to
"regparm(3)", which I did observe it does).

To make this introduction of a new blowfish pass (and potential further
ones later one) have less impact on the readability of the final code,
abstract all such "binary blob" executions via a table to iterate
through.

The resulting native code execution adjustment also uncovered a lack of
clobbers on the asm() in the 64-bit case, which is being fixed at once.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/tools/tests/x86_emulator/Makefile
+++ b/tools/tests/x86_emulator/Makefile
@@ -11,21 +11,21 @@ all: $(TARGET)
 run: $(TARGET)
 	./$(TARGET)
 
-.PHONY: blowfish.h
-blowfish.h:
-	rm -f blowfish.bin
-	XEN_TARGET_ARCH=x86_32 make -f blowfish.mk all
-	(echo "static unsigned int blowfish32_code[] = {"; \
-	od -v -t x blowfish.bin | sed 's/^[0-9]* /0x/' | sed 's/ /, 0x/g' | sed 's/$$/,/';\
-	echo "};") >$@
-	rm -f blowfish.bin
-ifeq ($(XEN_COMPILE_ARCH),x86_64)
-	XEN_TARGET_ARCH=x86_64 make -f blowfish.mk all
-	(echo "static unsigned int blowfish64_code[] = {"; \
-	od -v -t x blowfish.bin | sed 's/^[0-9]* /0x/' | sed 's/ /, 0x/g' | sed 's/$$/,/';\
-	echo "};") >>$@
-	rm -f blowfish.bin
-endif
+cflags-x86_32 := "-mno-accumulate-outgoing-args -Dstatic="
+
+blowfish.h: blowfish.c blowfish.mk Makefile
+	rm -f $@.new blowfish.bin
+	$(foreach arch,$(filter-out $(XEN_COMPILE_ARCH),x86_32) $(XEN_COMPILE_ARCH), \
+	    for cflags in "" $(cflags-$(arch)); do \
+		$(MAKE) -f blowfish.mk XEN_TARGET_ARCH=$(arch) BLOWFISH_CFLAGS="$$cflags" all; \
+		flavor=$$(echo $${cflags} | sed -e 's, .*,,' -e 'y,-=,__,') ; \
+		(echo "static unsigned int blowfish_$(arch)$${flavor}[] = {"; \
+		 od -v -t x blowfish.bin | sed -e 's/^[0-9]* /0x/' -e 's/ /, 0x/g' -e 's/$$/,/'; \
+		 echo "};") >>$@.new; \
+		rm -f blowfish.bin; \
+	    done; \
+	)
+	mv $@.new $@
 
 $(TARGET): x86_emulate.o test_x86_emulator.o
 	$(HOSTCC) -o $@ $^
--- a/tools/tests/x86_emulator/blowfish.mk
+++ b/tools/tests/x86_emulator/blowfish.mk
@@ -5,7 +5,7 @@ include $(XEN_ROOT)/tools/Rules.mk
 
 $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
 
-CFLAGS += -fno-builtin -msoft-float
+CFLAGS += -fno-builtin -msoft-float $(BLOWFISH_CFLAGS)
 
 .PHONY: all
 all: blowfish.bin
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -1,4 +1,5 @@
 #include <errno.h>
+#include <limits.h>
 #include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -12,6 +13,21 @@
 #include "x86_emulate/x86_emulate.h"
 #include "blowfish.h"
 
+static const struct {
+    const void *code;
+    size_t size;
+    unsigned int bitness;
+    const char*name;
+} blobs[] = {
+    { blowfish_x86_32, sizeof(blowfish_x86_32), 32, "blowfish" },
+    { blowfish_x86_32_mno_accumulate_outgoing_args,
+      sizeof(blowfish_x86_32_mno_accumulate_outgoing_args),
+      32, "blowfish (push)" },
+#ifdef __x86_64__
+    { blowfish_x86_64, sizeof(blowfish_x86_64), 64, "blowfish" },
+#endif
+};
+
 #define MMAP_SZ 16384
 
 /* EFLAGS bit definitions. */
@@ -943,25 +959,19 @@ int main(int argc, char **argv)
     else
         printf("skipped\n");
 
-    for ( j = 1; j <= 2; j++ )
+    for ( j = 0; j < sizeof(blobs) / sizeof(*blobs); j++ )
     {
-#if defined(__i386__)
-        if ( j == 2 ) break;
-        memcpy(res, blowfish32_code, sizeof(blowfish32_code));
-#else
-        ctxt.addr_size = 16 << j;
-        ctxt.sp_size   = 16 << j;
-        memcpy(res, (j == 1) ? blowfish32_code : blowfish64_code,
-               (j == 1) ? sizeof(blowfish32_code) : sizeof(blowfish64_code));
-#endif
-        printf("Testing blowfish %u-bit code sequence", j*32);
+        memcpy(res, blobs[j].code, blobs[j].size);
+        ctxt.addr_size = ctxt.sp_size = blobs[j].bitness;
+
+        printf("Testing %s %u-bit code sequence",
+               blobs[j].name, ctxt.addr_size);
         regs.eax = 2;
         regs.edx = 1;
         regs.eip = (unsigned long)res;
         regs.esp = (unsigned long)res + MMAP_SZ - 4;
-        if ( j == 2 )
+        if ( ctxt.addr_size == 64 )
         {
-            ctxt.addr_size = ctxt.sp_size = 64;
             *(uint32_t *)(unsigned long)regs.esp = 0;
             regs.esp -= 4;
         }
@@ -983,20 +993,27 @@ int main(int argc, char **argv)
              (regs.eax != 2) || (regs.edx != 1) )
             goto fail;
         printf("okay\n");
-    }
 
-    printf("%-40s", "Testing blowfish native execution...");    
-    asm volatile (
+        if ( ctxt.addr_size != sizeof(void *) * CHAR_BIT )
+            continue;
+
+        i = printf("Testing %s native execution...", blobs[j].name);
+        asm volatile (
 #if defined(__i386__)
-        "movl $0x100000,%%ecx; call *%%ecx"
+            "movl $0x100000,%%ecx; call *%%ecx"
 #else
-        "movl $0x100000,%%ecx; call *%%rcx"
+            "movl $0x100000,%%ecx; call *%%rcx"
 #endif
-        : "=a" (regs.eax), "=d" (regs.edx)
-        : "0" (2), "1" (1) : "ecx" );
-    if ( (regs.eax != 2) || (regs.edx != 1) )
-        goto fail;
-    printf("okay\n");
+            : "=a" (regs.eax), "=d" (regs.edx)
+            : "0" (2), "1" (1) : "ecx"
+#ifdef __x86_64__
+              , "rsi", "rdi", "r8", "r9", "r10", "r11"
+#endif
+        );
+        if ( (regs.eax != 2) || (regs.edx != 1) )
+            goto fail;
+        printf("%*sokay\n", i < 40 ? 40 - i : 0, "");
+    }
 
     return 0;
 
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -94,8 +94,8 @@ static uint8_t opcode_table[256] = {
     ImplicitOps, ImplicitOps, DstReg|SrcMem|ModRM, DstReg|SrcNone|ModRM|Mov,
     0, 0, 0, 0,
     /* 0x68 - 0x6F */
-    ImplicitOps|Mov, DstReg|SrcImm|ModRM|Mov,
-    ImplicitOps|Mov, DstReg|SrcImmByte|ModRM|Mov,
+    DstImplicit|SrcImm|Mov, DstReg|SrcImm|ModRM|Mov,
+    DstImplicit|SrcImmByte|Mov, DstReg|SrcImmByte|ModRM|Mov,
     ImplicitOps|Mov, ImplicitOps|Mov, ImplicitOps|Mov, ImplicitOps|Mov,
     /* 0x70 - 0x77 */
     ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
@@ -2335,10 +2335,15 @@ x86_emulate(
         break;
 
     case 0x68: /* push imm{16,32,64} */
-        src.val = ((op_bytes == 2)
-                   ? (int32_t)insn_fetch_type(int16_t)
-                   : insn_fetch_type(int32_t));
-        goto push;
+    case 0x6a: /* push imm8 */
+    push:
+        d |= Mov; /* force writeback */
+        dst.type  = OP_MEM;
+        dst.bytes = mode_64bit() && (op_bytes == 4) ? 8 : op_bytes;
+        dst.val = src.val;
+        dst.mem.seg = x86_seg_ss;
+        dst.mem.off = sp_pre_dec(dst.bytes);
+        break;
 
     case 0x69: /* imul imm16/32 */
     case 0x6b: /* imul imm8 */
@@ -2349,19 +2354,6 @@ x86_emulate(
             goto done;
         goto imul;
 
-    case 0x6a: /* push imm8 */
-        src.val = insn_fetch_type(int8_t);
-    push:
-        d |= Mov; /* force writeback */
-        dst.type  = OP_MEM;
-        dst.bytes = op_bytes;
-        if ( mode_64bit() && (dst.bytes == 4) )
-            dst.bytes = 8;
-        dst.val = src.val;
-        dst.mem.seg = x86_seg_ss;
-        dst.mem.off = sp_pre_dec(dst.bytes);
-        break;
-
     case 0x6c ... 0x6d: /* ins %dx,%es:%edi */ {
         unsigned long nr_reps = get_rep_prefix();
         unsigned int port = (uint16_t)_regs.edx;



[-- Attachment #2: x86emul-push-imm-generic.patch --]
[-- Type: text/plain, Size: 7920 bytes --]

x86emul: don't special case fetching the immediate of PUSH

These immediates follow the standard patterns in all modes, so they're
better fetched by the generic source operand handling code.

To facilitate testing, instead of adding yet another of these pretty
convoluted individual test cases, simply introduce another blowfish run
with -mno-accumulate-outgoing-args (the additional -Dstatic is to
keep the compiler from converting the calling convention to
"regparm(3)", which I did observe it does).

To make this introduction of a new blowfish pass (and potential further
ones later one) have less impact on the readability of the final code,
abstract all such "binary blob" executions via a table to iterate
through.

The resulting native code execution adjustment also uncovered a lack of
clobbers on the asm() in the 64-bit case, which is being fixed at once.

Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/tools/tests/x86_emulator/Makefile
+++ b/tools/tests/x86_emulator/Makefile
@@ -11,21 +11,21 @@ all: $(TARGET)
 run: $(TARGET)
 	./$(TARGET)
 
-.PHONY: blowfish.h
-blowfish.h:
-	rm -f blowfish.bin
-	XEN_TARGET_ARCH=x86_32 make -f blowfish.mk all
-	(echo "static unsigned int blowfish32_code[] = {"; \
-	od -v -t x blowfish.bin | sed 's/^[0-9]* /0x/' | sed 's/ /, 0x/g' | sed 's/$$/,/';\
-	echo "};") >$@
-	rm -f blowfish.bin
-ifeq ($(XEN_COMPILE_ARCH),x86_64)
-	XEN_TARGET_ARCH=x86_64 make -f blowfish.mk all
-	(echo "static unsigned int blowfish64_code[] = {"; \
-	od -v -t x blowfish.bin | sed 's/^[0-9]* /0x/' | sed 's/ /, 0x/g' | sed 's/$$/,/';\
-	echo "};") >>$@
-	rm -f blowfish.bin
-endif
+cflags-x86_32 := "-mno-accumulate-outgoing-args -Dstatic="
+
+blowfish.h: blowfish.c blowfish.mk Makefile
+	rm -f $@.new blowfish.bin
+	$(foreach arch,$(filter-out $(XEN_COMPILE_ARCH),x86_32) $(XEN_COMPILE_ARCH), \
+	    for cflags in "" $(cflags-$(arch)); do \
+		$(MAKE) -f blowfish.mk XEN_TARGET_ARCH=$(arch) BLOWFISH_CFLAGS="$$cflags" all; \
+		flavor=$$(echo $${cflags} | sed -e 's, .*,,' -e 'y,-=,__,') ; \
+		(echo "static unsigned int blowfish_$(arch)$${flavor}[] = {"; \
+		 od -v -t x blowfish.bin | sed -e 's/^[0-9]* /0x/' -e 's/ /, 0x/g' -e 's/$$/,/'; \
+		 echo "};") >>$@.new; \
+		rm -f blowfish.bin; \
+	    done; \
+	)
+	mv $@.new $@
 
 $(TARGET): x86_emulate.o test_x86_emulator.o
 	$(HOSTCC) -o $@ $^
--- a/tools/tests/x86_emulator/blowfish.mk
+++ b/tools/tests/x86_emulator/blowfish.mk
@@ -5,7 +5,7 @@ include $(XEN_ROOT)/tools/Rules.mk
 
 $(call cc-options-add,CFLAGS,CC,$(EMBEDDED_EXTRA_CFLAGS))
 
-CFLAGS += -fno-builtin -msoft-float
+CFLAGS += -fno-builtin -msoft-float $(BLOWFISH_CFLAGS)
 
 .PHONY: all
 all: blowfish.bin
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -1,4 +1,5 @@
 #include <errno.h>
+#include <limits.h>
 #include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
@@ -12,6 +13,21 @@
 #include "x86_emulate/x86_emulate.h"
 #include "blowfish.h"
 
+static const struct {
+    const void *code;
+    size_t size;
+    unsigned int bitness;
+    const char*name;
+} blobs[] = {
+    { blowfish_x86_32, sizeof(blowfish_x86_32), 32, "blowfish" },
+    { blowfish_x86_32_mno_accumulate_outgoing_args,
+      sizeof(blowfish_x86_32_mno_accumulate_outgoing_args),
+      32, "blowfish (push)" },
+#ifdef __x86_64__
+    { blowfish_x86_64, sizeof(blowfish_x86_64), 64, "blowfish" },
+#endif
+};
+
 #define MMAP_SZ 16384
 
 /* EFLAGS bit definitions. */
@@ -943,25 +959,19 @@ int main(int argc, char **argv)
     else
         printf("skipped\n");
 
-    for ( j = 1; j <= 2; j++ )
+    for ( j = 0; j < sizeof(blobs) / sizeof(*blobs); j++ )
     {
-#if defined(__i386__)
-        if ( j == 2 ) break;
-        memcpy(res, blowfish32_code, sizeof(blowfish32_code));
-#else
-        ctxt.addr_size = 16 << j;
-        ctxt.sp_size   = 16 << j;
-        memcpy(res, (j == 1) ? blowfish32_code : blowfish64_code,
-               (j == 1) ? sizeof(blowfish32_code) : sizeof(blowfish64_code));
-#endif
-        printf("Testing blowfish %u-bit code sequence", j*32);
+        memcpy(res, blobs[j].code, blobs[j].size);
+        ctxt.addr_size = ctxt.sp_size = blobs[j].bitness;
+
+        printf("Testing %s %u-bit code sequence",
+               blobs[j].name, ctxt.addr_size);
         regs.eax = 2;
         regs.edx = 1;
         regs.eip = (unsigned long)res;
         regs.esp = (unsigned long)res + MMAP_SZ - 4;
-        if ( j == 2 )
+        if ( ctxt.addr_size == 64 )
         {
-            ctxt.addr_size = ctxt.sp_size = 64;
             *(uint32_t *)(unsigned long)regs.esp = 0;
             regs.esp -= 4;
         }
@@ -983,20 +993,27 @@ int main(int argc, char **argv)
              (regs.eax != 2) || (regs.edx != 1) )
             goto fail;
         printf("okay\n");
-    }
 
-    printf("%-40s", "Testing blowfish native execution...");    
-    asm volatile (
+        if ( ctxt.addr_size != sizeof(void *) * CHAR_BIT )
+            continue;
+
+        i = printf("Testing %s native execution...", blobs[j].name);
+        asm volatile (
 #if defined(__i386__)
-        "movl $0x100000,%%ecx; call *%%ecx"
+            "movl $0x100000,%%ecx; call *%%ecx"
 #else
-        "movl $0x100000,%%ecx; call *%%rcx"
+            "movl $0x100000,%%ecx; call *%%rcx"
 #endif
-        : "=a" (regs.eax), "=d" (regs.edx)
-        : "0" (2), "1" (1) : "ecx" );
-    if ( (regs.eax != 2) || (regs.edx != 1) )
-        goto fail;
-    printf("okay\n");
+            : "=a" (regs.eax), "=d" (regs.edx)
+            : "0" (2), "1" (1) : "ecx"
+#ifdef __x86_64__
+              , "rsi", "rdi", "r8", "r9", "r10", "r11"
+#endif
+        );
+        if ( (regs.eax != 2) || (regs.edx != 1) )
+            goto fail;
+        printf("%*sokay\n", i < 40 ? 40 - i : 0, "");
+    }
 
     return 0;
 
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -94,8 +94,8 @@ static uint8_t opcode_table[256] = {
     ImplicitOps, ImplicitOps, DstReg|SrcMem|ModRM, DstReg|SrcNone|ModRM|Mov,
     0, 0, 0, 0,
     /* 0x68 - 0x6F */
-    ImplicitOps|Mov, DstReg|SrcImm|ModRM|Mov,
-    ImplicitOps|Mov, DstReg|SrcImmByte|ModRM|Mov,
+    DstImplicit|SrcImm|Mov, DstReg|SrcImm|ModRM|Mov,
+    DstImplicit|SrcImmByte|Mov, DstReg|SrcImmByte|ModRM|Mov,
     ImplicitOps|Mov, ImplicitOps|Mov, ImplicitOps|Mov, ImplicitOps|Mov,
     /* 0x70 - 0x77 */
     ImplicitOps, ImplicitOps, ImplicitOps, ImplicitOps,
@@ -2335,10 +2335,15 @@ x86_emulate(
         break;
 
     case 0x68: /* push imm{16,32,64} */
-        src.val = ((op_bytes == 2)
-                   ? (int32_t)insn_fetch_type(int16_t)
-                   : insn_fetch_type(int32_t));
-        goto push;
+    case 0x6a: /* push imm8 */
+    push:
+        d |= Mov; /* force writeback */
+        dst.type  = OP_MEM;
+        dst.bytes = mode_64bit() && (op_bytes == 4) ? 8 : op_bytes;
+        dst.val = src.val;
+        dst.mem.seg = x86_seg_ss;
+        dst.mem.off = sp_pre_dec(dst.bytes);
+        break;
 
     case 0x69: /* imul imm16/32 */
     case 0x6b: /* imul imm8 */
@@ -2349,19 +2354,6 @@ x86_emulate(
             goto done;
         goto imul;
 
-    case 0x6a: /* push imm8 */
-        src.val = insn_fetch_type(int8_t);
-    push:
-        d |= Mov; /* force writeback */
-        dst.type  = OP_MEM;
-        dst.bytes = op_bytes;
-        if ( mode_64bit() && (dst.bytes == 4) )
-            dst.bytes = 8;
-        dst.val = src.val;
-        dst.mem.seg = x86_seg_ss;
-        dst.mem.off = sp_pre_dec(dst.bytes);
-        break;
-
     case 0x6c ... 0x6d: /* ins %dx,%es:%edi */ {
         unsigned long nr_reps = get_rep_prefix();
         unsigned int port = (uint16_t)_regs.edx;

[-- Attachment #3: Type: text/plain, Size: 127 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

  reply	other threads:[~2016-08-11 12:03 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-11 11:53 [PATCH 0/7] x86emul: misc small adjustments Jan Beulich
2016-08-11 12:03 ` Jan Beulich [this message]
2016-08-11 12:58   ` [PATCH 1/7] x86emul: don't special case fetching the immediate of PUSH Andrew Cooper
2016-08-11 13:26     ` Jan Beulich
2016-08-11 17:33       ` Andrew Cooper
2016-08-11 12:04 ` [PATCH 2/7] x86emul: don't special case fetching immediates of near and short branches Jan Beulich
2016-08-11 13:19   ` Andrew Cooper
2016-08-11 13:27     ` Jan Beulich
2016-08-11 12:04 ` [PATCH 3/7] x86emul: all push flavors are data moves Jan Beulich
2016-08-11 13:40   ` Andrew Cooper
2016-08-11 12:05 ` [PATCH 4/7] x86emul: fold SrcImmByte fetching Jan Beulich
2016-08-11 13:41   ` Andrew Cooper
2016-08-11 14:09     ` Jan Beulich
2016-08-11 15:06       ` Andrew Cooper
2016-08-11 12:06 ` [PATCH 5/7] x86emul: don't special case fetching unsigned 8-bit immediates Jan Beulich
2016-08-11 16:32   ` Andrew Cooper
2016-08-11 16:44     ` Jan Beulich
2016-08-11 17:38       ` Andrew Cooper
2016-08-12 10:50         ` Jan Beulich
2016-08-12 11:47           ` Andrew Cooper
2016-08-11 12:06 ` [PATCH 6/7] x86emul: use DstEax where possible Jan Beulich
2016-08-11 16:41   ` Andrew Cooper
2016-08-11 12:07 ` [PATCH 7/7] x86emul: introduce SrcImm16 Jan Beulich
2016-08-11 17:23   ` Andrew Cooper

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=57AC85B8020000780010509E@prv-mh.provo.novell.com \
    --to=jbeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.