Skip to contentSkip to conversation

S/390 host fixed

PATCH In review Ulrich Hecht 5 participants 11 messages Jul 30, 2007 – Sep 13, 2007 Kbuild

lore
Share & tools
lore

Ulrich Hecht

author
#0 lore raw

PATCH 1/1 S/390 host fixed

Hi!

S/390 host support has been broken for a long time (since 0.4.2 or something like that). I finally got around to fix it, adding disassembly support on the way.

CU
Uli

Signature · 1 line
SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg)
Makefile.target +3 -0
diff -ruN qemu/Makefile.target qemu-s390/Makefile.target
@@ -347,6 +347,9 @@
 ifeq ($(findstring sh4, $(TARGET_ARCH) $(ARCH)),sh4)
 LIBOBJS+=sh4-dis.o
 endif
+ifeq ($(findstring s390, $(TARGET_ARCH) $(ARCH)),s390)
+LIBOBJS+=s390-dis.o
+endif
 
 ifdef CONFIG_GDBSTUB
 OBJS+=gdbstub.o
configure +2 -2
diff -ruN qemu/configure qemu-s390/configure
@@ -53,7 +53,7 @@
   mips64)
     cpu="mips64"
   ;;
-  s390)
+  s390*)
     cpu="s390"
   ;;
   sparc|sun4[cdmuv])

@@ -300,7 +300,7 @@
 if [ "$bsd" = "yes" -o "$darwin" = "yes" -o "$mingw32" = "yes" ] ; then
     AIOLIBS=
 else
-    AIOLIBS="-lrt"
+    AIOLIBS="-lrt -lpthread"
 fi
 
 # default flags for all hosts
dis-asm.h +3 -0
diff -ruN qemu/dis-asm.h qemu-s390/dis-asm.h
@@ -202,6 +202,8 @@
   bfd_arch_mn10300,    /* Matsushita MN10300 */
   bfd_arch_last
   };
+#define bfd_mach_s390_31 31
+#define bfd_mach_s390_64 64
 
 typedef struct symbol_cache_entry
 {

@@ -379,6 +381,7 @@
 extern int print_insn_tic30		PARAMS ((bfd_vma, disassemble_info*));
 extern int print_insn_ppc		PARAMS ((bfd_vma, disassemble_info*));
 extern int print_insn_alpha             PARAMS ((bfd_vma, disassemble_info*));
+extern int print_insn_s390		PARAMS ((bfd_vma, disassemble_info*));
 
 #if 0
 /* Fetch the disassembler for a given BFD, if that support is available.  */
disas.c +2 -0
diff -ruN qemu/disas.c qemu-s390/disas.c
@@ -269,6 +269,8 @@
     print_insn = print_insn_little_mips;
 #elif defined(__m68k__)
     print_insn = print_insn_m68k;
+#elif defined(__s390__)
+    print_insn = print_insn_s390;
 #else
     fprintf(out, "0x%lx: Asm output not supported on this arch\n",
 	    (long) code);
dyngen-exec.h +1 -1
diff -ruN qemu/dyngen-exec.h qemu-s390/dyngen-exec.h
@@ -254,7 +254,7 @@
 #define GOTO_LABEL_PARAM(n) asm volatile ("b " ASM_NAME(__op_gen_label) #n)
 #elif defined(__s390__)
 #define EXIT_TB() asm volatile ("br %r14")
-#define GOTO_LABEL_PARAM(n) asm volatile ("b " ASM_NAME(__op_gen_label) #n)
+#define GOTO_LABEL_PARAM(n) asm volatile ("bras %r7,8; .long " ASM_NAME(__op_gen_label) #n "; l %r7, 0(%r7); br %r7")
 #elif defined(__alpha__)
 #define EXIT_TB() asm volatile ("ret")
 #elif defined(__ia64__)
exec-all.h +18 -0
diff -ruN qemu/exec-all.h qemu-s390/exec-all.h
@@ -340,6 +340,24 @@
 		  "1:\n");\
 } while (0)
 
+#elif defined(__s390__)
+/* GCC spills R13, so we have to restore it before branching away */
+
+#define GOTO_TB(opname, tbparam, n)\
+do {\
+    static void __attribute__((used)) *dummy ## n = &&dummy_label ## n;\
+    static void __attribute__((used)) *__op_label ## n \
+        __asm__(ASM_OP_LABEL_NAME(n, opname)) = &&label ## n;\
+	__asm__ __volatile__ ( \
+		"l %%r13,52(%%r15)\n" \
+		"br %0\n" \
+	: : "r" (((TranslationBlock*)tbparam)->tb_next[n]));\
+	\
+	for(;*((int*)0);); /* just to keep GCC busy */ \
+label ## n: ;\
+dummy_label ## n: ;\
+} while(0)
+
 #else
 
 /* jump to next block operations (more portable code, does not need
s390-dis.c +1711 -0

1,711 lines not on this page · raw message

target-i386/helper.c +3 -0
diff -ruN qemu/target-i386/helper.c qemu-s390/target-i386/helper.c
@@ -3852,6 +3852,9 @@
     int ret;
     unsigned long pc;
     CPUX86State *saved_env;
+#ifdef __s390__
+    retaddr = (void*)((unsigned long)retaddr & 0x7fffffffUL);
+#endif
 
     /* XXX: hack to restore env in all cases, even if not called from
        generated code */
target-i386/translate.c +4 -0
diff -ruN qemu/target-i386/translate.c qemu-s390/target-i386/translate.c
@@ -1795,7 +1795,11 @@
         case CC_OP_SUBW:
         case CC_OP_SUBL:
         case CC_OP_SUBQ:
+#ifdef __s390__
+            func = NULL; /* does not work on S/390 for unknown reasons */
+#else
             func = gen_jcc_sub[s->cc_op - CC_OP_SUBB][jcc_op];
+#endif
             break;
             
             /* some jumps are easy to compute */
#1 lore raw
Quoted 8 lines · Ulrich Hecht

Hi!

S/390 host support has been broken for a long time (since 0.4.2 or something like that). I finally got around to fix it, adding disassembly support on the way.

CU
Uli

Do you or anyone else have any plans to add S/390 target support?

Or is there some other S/390 emulator on which it is possible to run S/390 Linux? Hercules is very slow and not really usable for compilation.

Sunil

author
#2 lore raw
In reply to Ulrich Hecht
Quoted 3 lines · Ulrich Hecht

S/390 host support has been broken for a long time (since 0.4.2 or something like that). I finally got around to fix it, adding disassembly support on the way.

And here's an even better patch that also fixes non-i386 targets. MIPS needs a little workaround to keep GCC from creating a jump table, something that dyngen cannot handle. Alpha does not build because of an ICE.

CU
Uli

Signature · 1 line
SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg)
Makefile.target +3 -0
diff -ruN qemu/Makefile.target qemu-s390/Makefile.target
@@ -347,6 +347,9 @@
 ifeq ($(findstring sh4, $(TARGET_ARCH) $(ARCH)),sh4)
 LIBOBJS+=sh4-dis.o
 endif
+ifeq ($(findstring s390, $(TARGET_ARCH) $(ARCH)),s390)
+LIBOBJS+=s390-dis.o
+endif
 
 ifdef CONFIG_GDBSTUB
 OBJS+=gdbstub.o
configure +2 -2
diff -ruN qemu/configure qemu-s390/configure
@@ -53,7 +53,7 @@
   mips64)
     cpu="mips64"
   ;;
-  s390)
+  s390*)
     cpu="s390"
   ;;
   sparc|sun4[cdmuv])

@@ -300,7 +300,7 @@
 if [ "$bsd" = "yes" -o "$darwin" = "yes" -o "$mingw32" = "yes" ] ; then
     AIOLIBS=
 else
-    AIOLIBS="-lrt"
+    AIOLIBS="-lrt -lpthread"
 fi
 
 # default flags for all hosts
dis-asm.h +3 -0
diff -ruN qemu/dis-asm.h qemu-s390/dis-asm.h
@@ -202,6 +202,8 @@
   bfd_arch_mn10300,    /* Matsushita MN10300 */
   bfd_arch_last
   };
+#define bfd_mach_s390_31 31
+#define bfd_mach_s390_64 64
 
 typedef struct symbol_cache_entry
 {

@@ -379,6 +381,7 @@
 extern int print_insn_tic30		PARAMS ((bfd_vma, disassemble_info*));
 extern int print_insn_ppc		PARAMS ((bfd_vma, disassemble_info*));
 extern int print_insn_alpha             PARAMS ((bfd_vma, disassemble_info*));
+extern int print_insn_s390		PARAMS ((bfd_vma, disassemble_info*));
 
 #if 0
 /* Fetch the disassembler for a given BFD, if that support is available.  */
disas.c +2 -0
diff -ruN qemu/disas.c qemu-s390/disas.c
@@ -269,6 +269,8 @@
     print_insn = print_insn_little_mips;
 #elif defined(__m68k__)
     print_insn = print_insn_m68k;
+#elif defined(__s390__)
+    print_insn = print_insn_s390;
 #else
     fprintf(out, "0x%lx: Asm output not supported on this arch\n",
 	    (long) code);
dyngen-exec.h +1 -1
diff -ruN qemu/dyngen-exec.h qemu-s390/dyngen-exec.h
@@ -254,7 +254,7 @@
 #define GOTO_LABEL_PARAM(n) asm volatile ("b " ASM_NAME(__op_gen_label) #n)
 #elif defined(__s390__)
 #define EXIT_TB() asm volatile ("br %r14")
-#define GOTO_LABEL_PARAM(n) asm volatile ("b " ASM_NAME(__op_gen_label) #n)
+#define GOTO_LABEL_PARAM(n) asm volatile ("bras %r7,8; .long " ASM_NAME(__op_gen_label) #n "; l %r7, 0(%r7); br %r7")
 #elif defined(__alpha__)
 #define EXIT_TB() asm volatile ("ret")
 #elif defined(__ia64__)
exec-all.h +18 -0
diff -ruN qemu/exec-all.h qemu-s390/exec-all.h
@@ -340,6 +340,24 @@
 		  "1:\n");\
 } while (0)
 
+#elif defined(__s390__)
+/* GCC spills R13, so we have to restore it before branching away */
+
+#define GOTO_TB(opname, tbparam, n)\
+do {\
+    static void __attribute__((used)) *dummy ## n = &&dummy_label ## n;\
+    static void __attribute__((used)) *__op_label ## n \
+        __asm__(ASM_OP_LABEL_NAME(n, opname)) = &&label ## n;\
+	__asm__ __volatile__ ( \
+		"l %%r13,52(%%r15)\n" \
+		"br %0\n" \
+	: : "r" (((TranslationBlock*)tbparam)->tb_next[n]));\
+	\
+	for(;*((int*)0);); /* just to keep GCC busy */ \
+label ## n: ;\
+dummy_label ## n: ;\
+} while(0)
+
 #else
 
 /* jump to next block operations (more portable code, does not need

Files qemu/hda and qemu-s390/hda differ

s390-dis.c +1711 -0

1,711 lines not on this page · raw message

target-alpha/op_helper.c +3 -0
diff -ruN qemu/target-alpha/op_helper.c qemu-s390/target-alpha/op_helper.c
@@ -1229,6 +1229,9 @@
     CPUState *saved_env;
     target_phys_addr_t pc;
     int ret;
+#ifdef __s390__
+    retaddr = (void*)((unsigned long)retaddr & 0x7fffffffUL);
+#endif
 
     /* XXX: hack to restore env in all cases, even if not called from
        generated code */
target-arm/op_helper.c +3 -0
diff -ruN qemu/target-arm/op_helper.c qemu-s390/target-arm/op_helper.c
@@ -202,6 +202,9 @@
     CPUState *saved_env;
     target_phys_addr_t pc;
     int ret;
+#ifdef __s390__
+    retaddr = (void*)((unsigned long)retaddr & 0x7fffffffUL);
+#endif
 
     /* XXX: hack to restore env in all cases, even if not called from
        generated code */
target-i386/helper.c +3 -0
diff -ruN qemu/target-i386/helper.c qemu-s390/target-i386/helper.c
@@ -3852,6 +3852,9 @@
     int ret;
     unsigned long pc;
     CPUX86State *saved_env;
+#ifdef __s390__
+    retaddr = (void*)((unsigned long)retaddr & 0x7fffffffUL);
+#endif
 
     /* XXX: hack to restore env in all cases, even if not called from
        generated code */
target-i386/translate.c +8 -0
diff -ruN qemu/target-i386/translate.c qemu-s390/target-i386/translate.c
@@ -1795,7 +1795,11 @@
         case CC_OP_SUBW:
         case CC_OP_SUBL:
         case CC_OP_SUBQ:
+#ifdef __s390__
+            func = NULL; /* does not work on S/390 for unknown reasons */
+#else
             func = gen_jcc_sub[s->cc_op - CC_OP_SUBB][jcc_op];
+#endif
             break;
             
             /* some jumps are easy to compute */

@@ -1843,7 +1847,11 @@
                 func = gen_jcc_sub[(s->cc_op - CC_OP_ADDB) % 4][jcc_op];
                 break;
             case JCC_S:
+#ifdef __s390__
+		func = NULL;
+#else
                 func = gen_jcc_sub[(s->cc_op - CC_OP_ADDB) % 4][jcc_op];
+#endif
                 break;
             default:
                 func = NULL;
target-m68k/op_helper.c +3 -0
diff -ruN qemu/target-m68k/op_helper.c qemu-s390/target-m68k/op_helper.c
@@ -55,6 +55,9 @@
     CPUState *saved_env;
     target_phys_addr_t pc;
     int ret;
+#ifdef __s390__
+    retaddr = (void*)((unsigned long)retaddr & 0x7fffffffUL);
+#endif
 
     /* XXX: hack to restore env in all cases, even if not called from
        generated code */
target-mips/op.c +13 -0
diff -ruN qemu/target-mips/op.c qemu-s390/target-mips/op.c
@@ -1616,6 +1616,18 @@
 
 void op_cfc1 (void)
 {
+#ifdef __s390__
+    if(!T1)
+        T0 = (int32_t)env->fcr0;
+    else if(T1 == 25)
+        T0 = ((env->fcr31 >> 24) & 0xfe) | ((env->fcr31 >> 23) & 0x1);
+    else if(T1 == 26)
+        T0 = env->fcr31 & 0x0003f07c;
+    else if(T1 == 28)
+        T0 = (env->fcr31 & 0x00000f83) | ((env->fcr31 >> 22) & 0x4);
+    else
+        T0 = (int32_t)env->fcr31;
+#else    
     switch (T1) {
     case 0:
         T0 = (int32_t)env->fcr0;

@@ -1633,6 +1645,7 @@
         T0 = (int32_t)env->fcr31;
         break;
     }
+#endif
     DEBUG_FPU_STATE();
     RETURN();
 }
target-mips/op_helper.c +3 -0
diff -ruN qemu/target-mips/op_helper.c qemu-s390/target-mips/op_helper.c
@@ -571,6 +571,9 @@
     CPUState *saved_env;
     unsigned long pc;
     int ret;
+#ifdef __s390__
+    retaddr = (void*)((unsigned long)retaddr & 0x7fffffffUL);
+#endif
 
     /* XXX: hack to restore env in all cases, even if not called from
        generated code */
target-ppc/op_helper.c +3 -0
diff -ruN qemu/target-ppc/op_helper.c qemu-s390/target-ppc/op_helper.c
@@ -2228,6 +2228,9 @@
     CPUState *saved_env;
     target_phys_addr_t pc;
     int ret;
+#ifdef __s390__
+    retaddr = (void*)((unsigned long)retaddr & 0x7fffffffUL);
+#endif
 
     /* XXX: hack to restore env in all cases, even if not called from
        generated code */
target-sh4/op_helper.c +3 -0
diff -ruN qemu/target-sh4/op_helper.c qemu-s390/target-sh4/op_helper.c
@@ -48,6 +48,9 @@
     CPUState *saved_env;
     unsigned long pc;
     int ret;
+#ifdef __s390__
+    retaddr = (void*)((unsigned long)retaddr & 0x7fffffffUL);
+#endif
 
     /* XXX: hack to restore env in all cases, even if not called from
        generated code */
target-sparc/op_helper.c +3 -0
diff -ruN qemu/target-sparc/op_helper.c qemu-s390/target-sparc/op_helper.c
@@ -1070,6 +1070,9 @@
     int ret;
     unsigned long pc;
     CPUState *saved_env;
+#ifdef __s390__
+    retaddr = (void*)((unsigned long)retaddr & 0x7fffffffUL);
+#endif
 
     /* XXX: hack to restore env in all cases, even if not called from
        generated code */
#3 lore raw
Quoted 9 lines · Ulrich Hecht
Quoted 3 lines · Ulrich Hecht

S/390 host support has been broken for a long time (since 0.4.2 or something like that). I finally got around to fix it, adding disassembly support on the way.

And here's an even better patch that also fixes non-i386 targets. MIPS needs a little workaround to keep GCC from creating a jump table, something that dyngen cannot handle. Alpha does not build because of an ICE.

I committed the bits I were comfortable with, which left those:

[snip]

configure line 303

Quoted 6 lines
configure +1 -1
@@ -300,7 +300,7 @@
 if [ "$bsd" = "yes" -o "$darwin" = "yes" -o "$mingw32" = "yes" ] ; then
     AIOLIBS=
 else
-    AIOLIBS="-lrt"
+    AIOLIBS="-lrt -lpthread"

Why is this needed? Linux toolchains should add -lpthread implicitly.

[snip]

target-alpha/op_helper.c line 1234

Quoted 10 lines

diff -ruN qemu/target-alpha/op_helper.c qemu-s390/target-alpha/op_helper.c

target-alpha/op_helper.c +3 -0
@@ -1229,6 +1229,9 @@
     CPUState *saved_env;
     target_phys_addr_t pc;
     int ret;
+#ifdef __s390__
+    retaddr = (void*)((unsigned long)retaddr & 0x7fffffffUL);
+#endif

All of those look weird. Is this a null-extension vs. sign-extension issue?

[snip]

target-i386/translate.c line 1802

Quoted 12 lines

diff -ruN qemu/target-i386/translate.c qemu-s390/target-i386/translate.c

target-i386/translate.c +4 -0
@@ -1795,7 +1795,11 @@
         case CC_OP_SUBW:
         case CC_OP_SUBL:
         case CC_OP_SUBQ:
+#ifdef __s390__
+            func = NULL; /* does not work on S/390 for unknown reasons */
+#else
             func = gen_jcc_sub[s->cc_op - CC_OP_SUBB][jcc_op];
+#endif

Hum. It wold be good to know what happens here.

[snip]

Quoted 9 lines

diff -ruN qemu/target-mips/op.c qemu-s390/target-mips/op.c

target-mips/op.c +2 -0
@@ -1616,6 +1616,18 @@
 
 void op_cfc1 (void)
 {
+#ifdef __s390__
+    if(!T1)

target-mips/op.c line 1630

Quoted 10 lines
target-mips/op.c +10 -0
+        T0 = (int32_t)env->fcr0;
+    else if(T1 == 25)
+        T0 = ((env->fcr31 >> 24) & 0xfe) | ((env->fcr31 >> 23) & 0x1);
+    else if(T1 == 26)
+        T0 = env->fcr31 & 0x0003f07c;
+    else if(T1 == 28)
+        T0 = (env->fcr31 & 0x00000f83) | ((env->fcr31 >> 22) & 0x4);
+    else
+        T0 = (int32_t)env->fcr31;
+#else

I guess this breaks when you _breathe_ at the compiler. Inventing switch-table support in dyngen would be preferable (if possible...).

Thiemo

#4 lore raw
Quoted 8 lines

target-mips/op.c line 1621

Quoted 5 lines

void op_cfc1 (void)
{

target-mips/op.c +3 -0
+#ifdef __s390__
+    if(!T1)
+        T0 = (int32_t)env->fcr0;

I guess this breaks when you _breathe_ at the compiler. Inventing switch-table support in dyngen would be preferable (if possible...).

Actually, I'm surprised this doesn't break on other hosts. Jump tables are death on all targets, I guess s390 gcc happens to use different heuristics for expanding switch statements.

I suspect making dyngen handle jump tables is not going to happen.

Paul

#5 lore raw
Quoted 1 line · Paul Brook

I suspect making dyngen handle jump tables is not going to happen.

Which brings up the question of dyngen. Very clever and a very good way of bootstrapping QEMU in the early days, but too brittle going forwards now the rest of QEMU works so well.

You had an alternative approach to dyngen, pointed at here:
http://lists.gnu.org/archive/html/qemu-devel/2006-10/msg00227.html

Any thoughts about the future of hand-coded backends in QEMU? I wasn't convinced at first (and, given resources, it isn't nearly as good as a special-purpose language for describing CPUs at this level) but I now think your work is a very good solution for bootstrapping QEMU to the next level of users.

Signature · 2 lines
Dan Shearer
[email protected]
#6 lore raw

Re: [Qemu-devel] replacing dyngen (was: S/390 host fixed)

Quoted 7 lines · Dan Shearer
Quoted 1 line · Paul Brook

I suspect making dyngen handle jump tables is not going to happen.

Which brings up the question of dyngen. Very clever and a very good way of bootstrapping QEMU in the early days, but too brittle going forwards now the rest of QEMU works so well.

Quoted 8 lines

You had an alternative approach to dyngen, pointed at here:
http://lists.gnu.org/archive/html/qemu-devel/2006-10/msg00227.html

Any thoughts about the future of hand-coded backends in QEMU? I wasn't convinced at first (and, given resources, it isn't nearly as good as a special-purpose language for describing CPUs at this level) but I now think your work is a very good solution for bootstrapping QEMU to the next level of users.

There's also a goolge SoC project to use llvm for code generation.
http://code.google.com/p/llvm-qemu/
This works, however it's also significantly slower than current qemu (no more than half the speed). It may be possible to improve this, but llvm is a fairly heavyweight framework so I'm not convinced it is ever going to be competitive for generalexecution speed.

One interesting thing that did come out of that project is that it's fairly trivial (a few hours work) to turn the existing code into a simple interpreter. Obviously this incurs a large performance hit, but also makes qemu trivially portable to other hosts.

My hand coded generator is currently ~20% slower that dyngen.
I still think this (or something similar) is the way forward, however I haven't made time to do any work on it recently. Some parts of this code effectively are a "special language" for describing a CPU, that happen to be implemented using the C preprocessor :-)

Paul

#7 lore raw

Re: [Qemu-devel] replacing dyngen (was: S/390 host fixed)

Quoted 3 lines · Paul Brook

Some parts of this code effectively are a "special language" for describing a CPU, that happen to be implemented using the C preprocessor :-)

Right, in the same way that QEMU devices written in C are in a device language? :-)

Signature · 2 lines
Dan Shearer
[email protected]
#8 lore raw

Re: [Qemu-devel] replacing dyngen (was: S/390 host fixed)

Quoted 7 lines · Dan Shearer
Quoted 3 lines · Paul Brook

Some parts of this code effectively are a "special language" for describing a CPU, that happen to be implemented using the C preprocessor :-)

Right, in the same way that QEMU devices written in C are in a device language? :-)

Not really. The cpu backends are mainly defining parameters for the generic code. Most of it doesn't look much like C. The implementation of the code generator itself is in C, but that's shared between all targets.

Paul

author
#9 lore raw
In reply to Thiemo Seufer
Quoted 4 lines · Thiemo Seufer

configure line 303

Quoted 2 lines
configure +1 -1
-    AIOLIBS="-lrt"
+    AIOLIBS="-lrt -lpthread"

Why is this needed? Linux toolchains should add -lpthread implicitly.

Our SLES9 toolchain seems not to. It's a near-cosmetic change.

Quoted 6 lines

target-alpha/op_helper.c line 1234

Quoted 3 lines
target-alpha/op_helper.c +3 -0
+#ifdef __s390__
+    retaddr = (void*)((unsigned long)retaddr & 0x7fffffffUL);
+#endif

All of those look weird. Is this a null-extension vs. sign-extension issue?

S/390 has a 31 (thirty-one) bit address space; the MSB of the PSW is not part of the address and must be masked out. This is simply part of the architecture, there's no way around it.

Quoted 7 lines
Quoted 5 lines
(quoted diff) +2 -0
+#ifdef __s390__
+            func = NULL; /* does not work on S/390 for unknown

reasons */ +#else

             func = gen_jcc_sub[s->cc_op - CC_OP_SUBB][jcc_op];

+#endif

Hum. It wold be good to know what happens here.

Indeed. :)

Quoted 4 lines
Quoted 4 lines
target-mips/op.c +4 -0
+#ifdef __s390__
+    if(!T1)
+        T0 = (int32_t)env->fcr0;
+    else if(T1 == 25)

[...]

Quoted 2 lines

I guess this breaks when you _breathe_ at the compiler.

Probably, but that is true for a significant part of the QEMU codebase...

CU
Uli

Signature · 1 line
SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg)
#10 lore raw

Ulrich Hecht wrote:
[snip]

Quoted 10 lines
Quoted 6 lines

target-alpha/op_helper.c line 1234

Quoted 3 lines
target-alpha/op_helper.c +3 -0
+#ifdef __s390__
+    retaddr = (void*)((unsigned long)retaddr & 0x7fffffffUL);
+#endif

All of those look weird. Is this a null-extension vs. sign-extension issue?

S/390 has a 31 (thirty-one) bit address space; the MSB of the PSW is not part of the address and must be masked out. This is simply part of the architecture, there's no way around it.

It looks like the retaddr value is always acquired via the GETPC macro, which does a __builtin_return_address(0). Could you check that
- Gcc's __builtin_return_address isn't just broken
- If the compiler is ok, test if twiddling the bit in the GETPC macro works for s390. I suspect this catches more instances of the problem and might even explain the odd breakage you saw.

Quoted 9 lines
Quoted 7 lines
Quoted 5 lines
(quoted diff) +2 -0
+#ifdef __s390__
+            func = NULL; /* does not work on S/390 for unknown

reasons */ +#else

             func = gen_jcc_sub[s->cc_op - CC_OP_SUBB][jcc_op];

+#endif

Hum. It wold be good to know what happens here.

Indeed. :)

Thiemo