Re: [Qemu-devel] Significant performance regression in qemu-system-mips.

[Aurelien Jarno]

On Fri, Mar 26, 2010 at 04:47:51PM -0500, Rob Landley wrote:
> On Friday 26 March 2010 14:00:00 Aurelien Jarno wrote:
> > > I'm not asking anyone to care about me personally, I'm asking them to
> > > care about specific technical issues.  If those issues don't interest
> > > you, they don't interest you.
> > >
> > > Speaking of ppc, last month I sent this patch:
> > >
> > >   http://lists.gnu.org/archive/html/qemu-devel/2010-02/msg00917.html
> > >
> > > And I was under the impression people agreed with it:
> > >
> > >   http://lists.gnu.org/archive/html/qemu-devel/2010-02/msg01044.html
> > >   http://lists.gnu.org/archive/html/qemu-devel/2010-02/msg01714.html
> > >
> > > But today's -git is still having that same issue, and the same patch
> > > still applies cleanly and fixes it for me.
> >
> > Re-read the last link you quoted, and especially this part:
> > | The
> > | same way using CONFIG_BSD in linux-user/elfload.c doesn't make sense,
> > | as this code will never been compiled.
> 
> I didn't know the BSD comments werer addressed at me.  (I haven't got a BSD
> test system.)
> 
> > While your patch goes in the good direction, it doesn't mean it is
> > correct. Conditionally compiling code on CONFIG_BSD in a Linux specific
> > file doesn't make sense.
> 
> Ok.
> 
> > I am pretty fine applying a correct patch if you send a new one.
> 
> By which you mean rip out the whole #ifdef block?

Yes.

> Here you go:

This looks much better. Can you please resend it with the changes below
and a Signed-off-by: ?

> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index 682a813..44405dd 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -518,12 +518,11 @@ do {                                                    
>                 \
>  static inline void init_thread(struct target_pt_regs *_regs, struct 
> image_info *infop)
>  {
>      abi_ulong pos = infop->start_stack;

This line should probably be removed...

> -    abi_ulong tmp;
>  #if defined(TARGET_PPC64) && !defined(TARGET_ABI32)
>      abi_ulong entry, toc;
>  #endif
>  
> -    _regs->gpr[1] = infop->start_stack;
> +    _regs->gpr[1] = pos;

...instead of doing the change here.

>  #if defined(TARGET_PPC64) && !defined(TARGET_ABI32)
>      entry = ldq_raw(infop->entry) + infop->load_addr;
>      toc = ldq_raw(infop->entry + 8) + infop->load_addr;
> @@ -531,17 +530,6 @@ static inline void init_thread(struct target_pt_regs 
> *_regs, struct image_info *
>      infop->entry = entry;
>  #endif
>      _regs->nip = infop->entry;
> -    /* Note that isn't exactly what regular kernel does
> -     * but this is what the ABI wants and is needed to allow
> -     * execution of PPC BSD programs.
> -     */
> -    /* FIXME - what to for failure of get_user()? */
> -    get_user_ual(_regs->gpr[3], pos);
> -    pos += sizeof(abi_ulong);
> -    _regs->gpr[4] = pos;
> -    for (tmp = 1; tmp != 0; pos += sizeof(abi_ulong))
> -        tmp = ldl(pos);
> -    _regs->gpr[5] = pos;
>  }
>  
>  #define ELF_EXEC_PAGESIZE    4096
> 
> Re-tested and it works fine for me.
> 
> > I have no problem with you having no interest in sh4, a lot of people
> > are in you case. I don't think it gives you the right to describe the
> > sh4 kernel maintainer as "sh4 linux-kernel maintainer officially doesn't
> > care about anybody who isn't employed by his company", or later "It's
> > not real hardware, it's a one-company toy". This is not something
> > reflected in link you quoted. Paul Mundt has been nicely answering your
> > question on this thread.
> 
> Ok, I agree I was a bit harsh.  (He's the one who introduced his employer into
> the discussion, but I suspect I read more into that than he meant by it.)
> 

I think you misunderstood him. You were talking about Super-Hitachi
which is a train [1] from Hitachi (not his employer), while he was talking
about Super-H which is a CPU [2] from Renesas (his employer).

He has the right to not care about trains ;-)

[1] http://en.wikipedia.org/wiki/Hitachi_(Japanese_train)
[2] http://en.wikipedia.org/wiki/SuperH

-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
address@hidden                 http://www.aurel32.net