Re: [Qemu-ppc] [PATCH] spapr: support CPU hotplug

[David Gibson]

On Mon, Aug 26, 2013 at 02:24:49PM +0200, Alexander Graf wrote:
> 
> On 23.08.2013, at 13:30, Alexey Kardashevskiy wrote:
> 
> > PAPR+ requires two RTAS calls to be supported by the hypervisor in
> > order to allow hotplugging VCPUs from the guest. The "start-cpu" RTAS
> > call was already there but "stop-self" was not.
> > 
> > This adds the "stop-self" RTAS call.
> > 
> > Signed-off-by: Alexey Kardashevskiy <address@hidden>
> > ---
> > hw/ppc/spapr_rtas.c | 14 ++++++++++++++
> > 1 file changed, 14 insertions(+)
> > 
> > diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
> > index 394ce05..8a4cfa0 100644
> > --- a/hw/ppc/spapr_rtas.c
> > +++ b/hw/ppc/spapr_rtas.c
> > @@ -202,6 +202,19 @@ static void rtas_start_cpu(PowerPCCPU *cpu_, 
> > sPAPREnvironment *spapr,
> >     rtas_st(rets, 0, -3);
> > }
> > 
> > +static void rtas_stop_self(PowerPCCPU *cpu, sPAPREnvironment *spapr,
> > +                           uint32_t token, uint32_t nargs,
> > +                           target_ulong args,
> > +                           uint32_t nret, target_ulong rets)
> > +{
> > +    CPUState *cs = CPU(cpu);
> > +    CPUPPCState *env = &cpu->env;
> > +
> > +    cs->halted = 1;
> > +    env->msr = 0;
> 
> So this is here to make sure we don't accidentally get out of halted state by 
> an interrupt on that vcpu. Could you please somehow make that part obvious? 
> Either by adding a comment or by only explicitly masking DEC and EE and a 
> comment :).
> 
> > +    cs->exit_request = 1;
> 
> This should probably be qemu_cpu_kick_self().

Uh, no, I don't think so.  This is there purely to make sure we exit
the inner loop, and actually test cpu_can_run() which will test
halted.  AFAICT qemu_cpu_kick_self() won't do anything similar.

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

pgp_bqYtmAjzb.pgp
Description: PGP signature