Re: [Qemu-devel] [PATCH 2/2] pseries: Use new hook to correct reset sequence

[David Gibson <david@gibson.dropbear.id.au>]

On Thu, Aug 02, 2012 at 05:44:49PM +0200, Andreas Färber wrote:
> Am 02.08.2012 04:10, schrieb David Gibson:
> > A number of things need to occur during reset of the PAPR paravirtualized
> > platform in a specific order.  For example, the hash table needs to be
> > cleared before the CPUs are reset, so that they initialize their register
> > state correctly, and the CPUs need to have their main reset called before
> > we set up the entry point state on the boot cpu.  We also need to have
> > the main qdev reset happen before the creation and installation of the
> > device tree for the new boot, because we need the state of the devices
> > settled to correctly construct the device tree.
> > 
> > Currently reset of pseries is broken in a number of ways, and in other
> > cases works largely by accident. This patch uses the new QEMUMachine reset
> > hook to correct these problems, by replacing the several existing spapr
> > reset hooks with one new machine hook which ensures that the various stages
> > happen in the correct order.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  hw/spapr.c |   66 +++++++++++++++++++++++++++++++++---------------------------
> >  1 file changed, 36 insertions(+), 30 deletions(-)
> > 
> > diff --git a/hw/spapr.c b/hw/spapr.c
> > index 2453bae..1e60ec1 100644
> > --- a/hw/spapr.c
> > +++ b/hw/spapr.c
> > @@ -582,29 +582,22 @@ static void spapr_reset_htab(sPAPREnvironment *spapr)
> >      }
> >  }
> >  
> > -static void spapr_reset(void *opaque)
> > +static void spapr_reset_cpu(CPUPPCState *env)
> >  {
> > -    sPAPREnvironment *spapr = (sPAPREnvironment *)opaque;
> > -
> > -    /* Reset the hash table & recalc the RMA */
> > -    spapr_reset_htab(spapr);
> > -
> > -    /* Load the fdt */
> > -    spapr_finalize_fdt(spapr, spapr->fdt_addr, spapr->rtas_addr,
> > -                       spapr->rtas_size);
> > -}
> > -
> > -static void spapr_cpu_reset(void *opaque)
> > -{
> > -    PowerPCCPU *cpu = opaque;
> > -    CPUPPCState *env = &cpu->env;
> > +    PowerPCCPU *cpu = container_of(env, PowerPCCPU, env);
> 
> NACK. Please don't undo the cleanups I have applied! Functions should
> take a QOM PowerPCCPU, not its internal CPUPPCState. Fields are
> gradually being moved from CPUxxxState into CPUState.

Um, ok.  So how do I iterate the PowerPCCPUs instead of the CPUPPCStates?

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