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

[David Gibson]

On Wed, Aug 08, 2012 at 12:32:39AM +0200, Andreas Färber wrote:
> Am 08.08.2012 00:02, schrieb Benjamin Herrenschmidt:
> > On Fri, 2012-08-03 at 17:01 +0200, Andreas Färber wrote:
> >>
> >> I have posted a suggestion where CPU reset is triggered by "the
> >> machine
> >> as an abstract concept" (needs a bit of tweaking still, but the
> >> general
> >> idea is there).
> >> Based on that, shouldn't it be rather easy to add a Notifier similar
> >> to
> >> "machine init done" that lets individual machines do post-reset setup?
> >> I.e. not have QEMUMachine trigger and control the reset.
> >>
> > 
> > Note that we really want pre and post reset vs the device reset.
> > 
> > That's why the machine should be the one in charge. The top level of the
> > reset sequencing is -not- the CPU, it's the machine. All machines (or
> > SoCs) have some kind of reset controller and provide facilities for
> > resetting individual devices, busses, processor cores.... the global
> > "system" reset (when it exists) itself might have interesting ordering
> > or sequencing requirements.
> > 
> > Now, to fix our immediate problem on ppc for 1.2 the hook proposed by
> > Anthony for which David sent a patch does the job just fine, it allows
> > us to clean out all our iommu tables before the device-reset, meaning
> > that in-flights DMA cannot overwrite the various "files" (SLOF image
> > etc.... that are auto-loaded via reset handlers implicitely created by
> > load_image_targphys), and we can then do some post-initializations as
> > well to get things ready for a restart (rebuild the device-tree, etc...)
> 
> That's all good, except for embedded machines without such implicit
> reset handling. It does contradict the "a machine is just a config file,
> setting up QOM objects" concept, but I was not the one to push that! :)
> 
> What I was thinking about however were those mentioned individual cores
> being reset using cpu_reset(). If we want to piggy-back some
> machine-specific register initialization for individual CPUStates then
> QEMUMachine::reset is not going to be enough because it only gets
> triggered for complete system reset. My suggestion was thus to just call
> cpu_reset() in your QEMUMachine::reset and have cpu_reset() take care of
> its initialization wherever called from. Any of these solutions are easy
> to implement for 1.2 if agreement is reached what people want.

So, I more or less reaslied that myself and my new version of the
reset patch (which I expect to send out later today) kind of does
that.  I no longer do the machine specific CPU state setup from the
QEMUMachine::reset, it's done from the per-cpu reset handler.  The
QEMUMachine::reset just does the special setup that's only for the
CPU0 entry conditions, which *is* specific to a full system reset (not
that I think we can get an individual CPU reset on pseries, anyway).

> What I am missing from Anthony's side is some communication to machine
> maintainers on the course to adopt before applying random patches. Right
> now x86 and ppc are moving into opposite directions and arm, mips, etc.
> maintainers may not even be aware of ongoing changes, and there's a
> pending uc32 machine that should be reviewed in this light.

So.. having the CPU reset at the top of the tree definitely makes no
sense - if nothing else, *which* cpu when there's more than one.

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