Re: [Qemu-devel] [PATCH] S/390 host fixed

2007-09-12 Thread Thiemo Seufer
Ulrich Hecht wrote:
[snip]
> > > +#ifdef __s390__
> > > +retaddr = (void*)((unsigned long)retaddr & 0x7fffUL);
> > > +#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.

> > > +#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




Re: [Qemu-devel] [PATCH] S/390 host fixed

2007-08-01 Thread Dan Shearer
On Wed, Aug 01, 2007 at 02:02:13AM +0100, Paul Brook wrote:

> 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. 

-- 
Dan Shearer
[EMAIL PROTECTED]




Re: [Qemu-devel] [PATCH] S/390 host fixed

2007-08-01 Thread Ulrich Hecht
On Wednesday 01 August 2007 01:59, Thiemo Seufer wrote:
> > -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.

> > +#ifdef __s390__
> > +retaddr = (void*)((unsigned long)retaddr & 0x7fffUL);
> > +#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.

> > +#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. :)

> > +#ifdef __s390__
> > +if(!T1)
> > +T0 = (int32_t)env->fcr0;
> > +else if(T1 == 25)
[...]
>
> 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

-- 
SUSE LINUX Products GmbH, GF: Markus Rex, HRB 16746 (AG Nürnberg)




Re: [Qemu-devel] [PATCH] S/390 host fixed

2007-07-31 Thread Paul Brook
> >  void op_cfc1 (void)
> >  {
> > +#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




Re: [Qemu-devel] [PATCH] S/390 host fixed

2007-07-31 Thread Thiemo Seufer
Ulrich Hecht wrote:
> On Monday 30 July 2007 13:49, Ulrich Hecht wrote:
> > 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]
> @@ -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]
> diff -ruN qemu/target-alpha/op_helper.c qemu-s390/target-alpha/op_helper.c
> --- qemu/target-alpha/op_helper.c 2007-04-05 06:58:33.0 +
> +++ qemu-s390/target-alpha/op_helper.c2007-07-30 12:16:31.0 
> +
> @@ -1229,6 +1229,9 @@
>  CPUState *saved_env;
>  target_phys_addr_t pc;
>  int ret;
> +#ifdef __s390__
> +retaddr = (void*)((unsigned long)retaddr & 0x7fffUL);
> +#endif

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

[snip]
> diff -ruN qemu/target-i386/translate.c qemu-s390/target-i386/translate.c
> --- qemu/target-i386/translate.c  2007-06-26 08:35:18.0 +
> +++ qemu-s390/target-i386/translate.c 2007-07-30 13:57:39.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]
> diff -ruN qemu/target-mips/op.c qemu-s390/target-mips/op.c
> --- qemu/target-mips/op.c 2007-06-25 17:34:33.0 +
> +++ qemu-s390/target-mips/op.c2007-07-30 13:34:08.0 +
> @@ -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 & 0x0f83) | ((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




Re: [Qemu-devel] [PATCH] S/390 host fixed

2007-07-30 Thread Sunil Amitkumar Janki

Ulrich Hecht wrote:

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