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

[Anthony Liguori]

Andreas Färber <address@hidden> writes:

> Am 02.08.2012 21:40, schrieb Anthony Liguori:
>> Reset propagates.  There is unanimous consensus that this is the Right
>> Way to model reset.  There is also wide consensus that reset typically
>> will propagate through the composition tree although in some cases,
>> reset actually propagates through the bus (this mostly affects devices
>> that are children of /peripheral paths though).
>> 
>> The "root" of the composition tree is the machine.  The machine in the
>> abstract sense, not the QEMUMachine sense.  QEMUMachine::init() should
>> eventually become trivial--just create a handful of devices that
>> represent the core components of the machine with everything else being
>> created through composition.
>> 
>> Open coded logic in QEMUMachine::init is always bad.  Handling reset for
>> a specific device in QEMUMachine::init is bad.  That goes against the
>> idea of making QEMUMachine::init trivial.
>
> We don't seem in disagreement so far. No one is questioning bus resets.
> The issue at hand is specifically CPU reset, for which there is no bus,
> no container and thus must happen somehow at machine level.
>
> 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.

This means that the reset logic will be spread out.  A single hook in
QEMUMachine is much nicer.

> An alternative would be to have a CPUState::reset callback (in addition
> to CPUClass::reset) that would by default be NULL but could be used by
> the odd machines to piggy-back reset code. I think this is the safest
> solution, assuring that on every cpu_reset() the custom reset code is
> executed immediately.

I think the right way to handle reset for CPU's is exactly what's done
today.  The CPUState registers a reset handler via
qemu_register_reset().

Eventually, we would model CPUSocket, CPUCore, CPUThread (which is
essentially what is CPUState today).

Reset of CPUState would propagate its CPUCores, which would then
propagate to CPUThread.

The machine/board device would have 1-N link<CPUSocket> properties and
the reset handler for the board would propagate reset to the CPUSocket links.

> The other issue wrt reset callback placement is CPU hotplug, where I
> believe we need a callback at machine level in lack of a bus CPUs are
> attached to. When the CPU is plugged we need to assure it later gets
> reset by someone and added as a QOM child in the proper place.

Reset does two things today: tear down current state and establish
initial state.

The fact that we rely on an external call to reset to establish initial
state after construction is not ideal.  Initial state should be
established during construction either by explicitly calling a function
(it could be reset) or by setting initial state.

> Currently
> we don't have that. If we iterate through CPUs as done here we would get
> that for free, otherwise we may need to register reset callbacks on
> hotplug and unregister on hot-unplug at QEMUMachine level.

If you hotplug a CPUSocket by setting a link<> on the machine device,
then this all Just Works without any special handling.

This is what makes propagation so attractive.  You don't have to
maintain a list of everything that needs to be reset along with data
descriptions of the order.  That gets figured out lazily during reset.

Regards,

Anthony Liguori

>
> I am all ears for practical solutions, but theoretical talk about
> containers and reset propagation doesn't seem to get us a solution.
> Please say what container you mean and how/where your solutions are
> supposed to work in code and how which of the proposals should be improved.
>
> Thanks,
> Andreas
>
> -- 
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg