Re: [Qemu-ppc] Implement instructions: yield, mdoio, mdoom

[David Gibson]

On Fri, Jun 24, 2016 at 12:15:58AM -0500, address@hidden wrote:
> David Gibson <address@hidden> wrote on 06/23/2016 09:45:48 
> PM:
> On Thu, Jun 23, 2016 at 08:10:08PM -0500, address@hidden wrote:
> 
> AL> In target-ppc/translate.c, the gen_or() function now has the ability
> AL> to pause the CPU via gen_pause().  However gen_or() does not implement
> AL> the PPC instructions yield (rs,ra,rb=27), mdoio (29), or mdoom (30),
> AL> which could be handled by simply calling gen_pause() for them.
> 
> DG> Uh.. having a glance at the description of these in the arch, I
> DG> don't think treating them as pause is correct.  I don't see
> DG> anything we can do with these in TCG that's better than the no-op
> DG> we currently treat them as.
> 
> Perhaps I don't understand what gen_pause() does, but it appears to
> only let other cores run, which I assume is what we'd want for
> 'yield'.  Otherwise what is the difference between gen_pause() and
> gen_wait()?

Ah.. hmm, yes you might be right.  I think I was mixing up what
gen_pause() did with gen_wait().  I'm having a bit of trouble working
out what an EXCP_HLT will actually do.  A bunch of places set it, but
hardly anywhere actually checks for it.

> I concur that the case for the mdoio or mdoom is less clear,
> but if a guest core is indicating a desire to let something else
> happen, then it seems that switching guest cores has the highest
> likelihood of conserving host execution time.

Well.. implementing mdoio and mdoom the same as yield should be
correct if non-optimal.  Generally the yield will be shorter, since
you're not actually evaluating the wakeup condition.

> The patch I was originally planning to submit for "yield" was the
> following:
> 
> in gen_or()
>         case 30:
>             /* Let a different processor run */
>             gen_update_nip(ctx, ctx->nip);
>             gen_helper_yield(cpu_env);
>             ctx->exception = POWERPC_EXCP_BRANCH;
>             break;
> 
> void helper_yield(CPUPPCState *env)
> {
>     PowerPCCPU *cpu = ppc_env_get_cpu(env);
>     CPUState *cs = CPU(cpu);
> 
>     /* Just let another CPU run.  */
>     cs->exception_index = EXCP_INTERRUPT;
>     cpu_loop_exit(cs);
> }
> 
> But the solution for gen_pause() seemed cleaner to me.
> 
> In my environment pause() and the above yield() behave similarly.
> However I still have an issue where I can't get gen_wait() to let the
> host sleep, so perhaps that is clouding my results.
> 

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature