[Qemu-devel] [PATCH v5 0/9] Clock framework API.

Damien Hedde posted 9 patches 5 years, 6 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20181002142443.30976-1-damien.hedde@greensocs.com
Test docker-clang@ubuntu failed
Test checkpatch passed
docs/devel/clock.txt           | 163 +++++++++
Makefile.objs                  |   1 +
include/hw/char/cadence_uart.h |   3 +
include/hw/clock-port.h        | 136 ++++++++
include/hw/qdev-clock.h        | 129 +++++++
include/hw/qdev-core.h         |  14 +
include/hw/qdev.h              |   1 +
hw/arm/xilinx_zynq.c           |  17 +-
hw/char/cadence_uart.c         |  92 ++++-
hw/core/clock-port.c           | 159 +++++++++
hw/core/qdev-clock.c           | 166 +++++++++
hw/core/qdev.c                 |  29 ++
hw/misc/zynq_slcr.c            | 607 ++++++++++++++++++++-------------
qdev-monitor.c                 |  12 +
hw/char/trace-events           |   3 +
hw/core/Makefile.objs          |   3 +-
hw/core/trace-events           |   7 +
17 files changed, 1289 insertions(+), 253 deletions(-)
create mode 100644 docs/devel/clock.txt
create mode 100644 include/hw/clock-port.h
create mode 100644 include/hw/qdev-clock.h
create mode 100644 hw/core/clock-port.c
create mode 100644 hw/core/qdev-clock.c
create mode 100644 hw/core/trace-events
[Qemu-devel] [PATCH v5 0/9] Clock framework API.
Posted by Damien Hedde 5 years, 6 months ago
This series aims to add a way to model clocks in qemu between devices.
This allows to model the clock tree of a platform allowing us to inspect clock
configuration and detect problems such as disabled clock or bad configured
pll.

This series is a reroll of the v4 sent recently without the reset feature as
requested by Peter. I also took into account the reviews about migration and
other suggestions.

This framework was originally discussed in 2017, here:
https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg07218.html

For the user, the framework is now very similar to the device's gpio API.
Clocks inputs and outputs can be added in devices during initialization phase.
Then an input can be connected to an output: it means every time the output
clock changes, a callback in the input is triggered allowing any action to be
taken. A difference with gpios is that several inputs can be connected to a
single output without doing any glue.

Behind the scene, there is 2 objects: a clock input which is a placeholder for
a callback, and a clock output which is a list of inputs. The value transferred
between an output and an input is an integer representing the clock frequency.
The input clock callback is called every time the clock frequency changes.
The input side holds a cached value of the frequency (the output does not need
one). This allows a device to fetch its input clock frequency at any time
without caching it itself.

This internal state is added to handle migration in order not to update and
propagate clocks during it (it adds cross-device and order-specific effects).
Each device handles its input clock migration by adding the clock frequency in
its own vmstate description.

Regarding the migration strategy, discussion started in the v4 patches.
The problem is that we add some kind of wire between different devices and 
we face propagation issue.
The chosen solution allows migration compatibility from a platform version
with no implemented clocks to a platform with clocks. A migrated device that
have a new input clock is responsible to initialize its frequency during
migration. Each input clock having its own state, such initialization will not
have any side-effect on other input clock connected to the same output.
Output clocks, having no state, are not set during migration: If a clock output
frequency changes due to migration (eg: clock computation bug-fix), the effects
will be delayed. Eventually the clock output will be updated after migration if
some (software) event trigger a clock update, but it can not be guaranteed.

There is also the problem of initialization which is very much like the
migration. Currently, in the zynq example, clocks outputs are initialized in
the clock controller device_reset. According to Peter, outputs should not be
set in device_reset, so we need a better way.

It is not simple, since we can have complicated cases with several propagation
step.
We can't initialize outputs (without propagating) during device init and uses
inputs value in device_reset to complete the initialization.
Consider the case where we have a device generating a frequency, another device
doing a clock division, then a device using this clock.
[DevClockGenerator] -> clk1 -> [DevClockDiv] -> clk2 -> [Dev]
I don't see how we can handle such initialization without doing some
propagation phase at some point.

Regarding clock gating. The 0 frequency value means the clock is gated.
If need be, a gate device can be built taking an input gpio and clock and
generating an output clock.

I've tested this patchset running Xilinx's Linux on the xilinx-zynq-a9 machine.
Clocks are correctly updated and we ends up with a configured baudrate of 115601
on the console uart (for a theoretical 115200) which is nice. "cadence_uart*" and
"clock*" traces can be enabled to see what's going on in this platform.

We are considering switching to a generic payload evolution of this API.
For example by specifying the qom carried type when adding an input/output to
a device. This would allow us, for example, to add a power input port to handle
power gating or others ports without modifying the device state structure.

Any comments and suggestion are welcomed.

The patches are organised as follows:
+ Patches 1 to 4 adds the clock support in qemu.
+ Patch 5 add some documentation in docs/devel
+ Patches 6 to 9 adds the uart's clocks to the xilinx_zynq platform as an
example for this framework. It updates the zynq's slcr clock controller, the 
cadence_uart device, and the zynq toplevel platform.

Compared to v4, the changes are:
  - no more reset flag, we are talking frequency only here
  - name changes to better match gpio api
  - migration strategy change
  - sysbus patch removed (was becoming complicated due too migration changes)

Compared to v3, the following notable changes happen:
  - Bindings are now fixed during machine realisation,
  - Input clock objects have been removed from the point of view of the
user, i.e. nothing need to be added in the device state. A device can
now declare a input clock by calling qdev_init_clock_in() (similarly of
what is done for GPIOs),
  - Callbacks called on clock change now return void. The input owner is
responsible of making necessary updates accordingly (e.g. update another
clock output, or modify its internal state) (again, mostly like GPIOs).

Thanks to the Xilinx QEMU team who sponsored this development.


Damien Hedde (9):
  hw/core/clock-port: introduce clock port objects
  qdev: add clock input&output support to devices.
  qdev-monitor: print the device's clock with info qtree
  qdev-clock: introduce an init array to ease the device construction
  docs/clocks: add device's clock documentation
  hw/misc/zynq_slcr: use standard register definition
  hw/misc/zynq_slcr: add clock generation for uarts
  hw/char/cadence_uart: add clock support
  hw/arm/xilinx_zynq: connect uart clocks to slcr

 docs/devel/clock.txt           | 163 +++++++++
 Makefile.objs                  |   1 +
 include/hw/char/cadence_uart.h |   3 +
 include/hw/clock-port.h        | 136 ++++++++
 include/hw/qdev-clock.h        | 129 +++++++
 include/hw/qdev-core.h         |  14 +
 include/hw/qdev.h              |   1 +
 hw/arm/xilinx_zynq.c           |  17 +-
 hw/char/cadence_uart.c         |  92 ++++-
 hw/core/clock-port.c           | 159 +++++++++
 hw/core/qdev-clock.c           | 166 +++++++++
 hw/core/qdev.c                 |  29 ++
 hw/misc/zynq_slcr.c            | 607 ++++++++++++++++++++-------------
 qdev-monitor.c                 |  12 +
 hw/char/trace-events           |   3 +
 hw/core/Makefile.objs          |   3 +-
 hw/core/trace-events           |   7 +
 17 files changed, 1289 insertions(+), 253 deletions(-)
 create mode 100644 docs/devel/clock.txt
 create mode 100644 include/hw/clock-port.h
 create mode 100644 include/hw/qdev-clock.h
 create mode 100644 hw/core/clock-port.c
 create mode 100644 hw/core/qdev-clock.c
 create mode 100644 hw/core/trace-events

-- 
2.19.0


Re: [Qemu-devel] [PATCH v5 0/9] Clock framework API.
Posted by Philippe Mathieu-Daudé 5 years, 6 months ago
Hi Damien,

On 02/10/2018 16:24, Damien Hedde wrote:
> This series aims to add a way to model clocks in qemu between devices.
> This allows to model the clock tree of a platform allowing us to inspect clock
> configuration and detect problems such as disabled clock or bad configured
> pll.
> 
> This series is a reroll of the v4 sent recently without the reset feature as
> requested by Peter. I also took into account the reviews about migration and
> other suggestions.
> 
> This framework was originally discussed in 2017, here:
> https://lists.gnu.org/archive/html/qemu-devel/2017-02/msg07218.html
> 
> For the user, the framework is now very similar to the device's gpio API.
> Clocks inputs and outputs can be added in devices during initialization phase.
> Then an input can be connected to an output: it means every time the output
> clock changes, a callback in the input is triggered allowing any action to be
> taken. A difference with gpios is that several inputs can be connected to a
> single output without doing any glue.
> 
> Behind the scene, there is 2 objects: a clock input which is a placeholder for
> a callback, and a clock output which is a list of inputs. The value transferred
> between an output and an input is an integer representing the clock frequency.
> The input clock callback is called every time the clock frequency changes.
> The input side holds a cached value of the frequency (the output does not need
> one). This allows a device to fetch its input clock frequency at any time
> without caching it itself.
> 
> This internal state is added to handle migration in order not to update and
> propagate clocks during it (it adds cross-device and order-specific effects).
> Each device handles its input clock migration by adding the clock frequency in
> its own vmstate description.
> 
> Regarding the migration strategy, discussion started in the v4 patches.
> The problem is that we add some kind of wire between different devices and 
> we face propagation issue.
> The chosen solution allows migration compatibility from a platform version
> with no implemented clocks to a platform with clocks. A migrated device that
> have a new input clock is responsible to initialize its frequency during
> migration. Each input clock having its own state, such initialization will not
> have any side-effect on other input clock connected to the same output.
> Output clocks, having no state, are not set during migration: If a clock output
> frequency changes due to migration (eg: clock computation bug-fix), the effects
> will be delayed. Eventually the clock output will be updated after migration if
> some (software) event trigger a clock update, but it can not be guaranteed.
> 
> There is also the problem of initialization which is very much like the
> migration. Currently, in the zynq example, clocks outputs are initialized in
> the clock controller device_reset. According to Peter, outputs should not be
> set in device_reset, so we need a better way.
> 
> It is not simple, since we can have complicated cases with several propagation
> step.
> We can't initialize outputs (without propagating) during device init and uses
> inputs value in device_reset to complete the initialization.
> Consider the case where we have a device generating a frequency, another device
> doing a clock division, then a device using this clock.
> [DevClockGenerator] -> clk1 -> [DevClockDiv] -> clk2 -> [Dev]
> I don't see how we can handle such initialization without doing some
> propagation phase at some point.
> 
> Regarding clock gating. The 0 frequency value means the clock is gated.
> If need be, a gate device can be built taking an input gpio and clock and
> generating an output clock.
> 
> I've tested this patchset running Xilinx's Linux on the xilinx-zynq-a9 machine.
> Clocks are correctly updated and we ends up with a configured baudrate of 115601
> on the console uart (for a theoretical 115200) which is nice. "cadence_uart*" and
> "clock*" traces can be enabled to see what's going on in this platform.
> 
> We are considering switching to a generic payload evolution of this API.
> For example by specifying the qom carried type when adding an input/output to
> a device. This would allow us, for example, to add a power input port to handle
> power gating or others ports without modifying the device state structure.
> 
> Any comments and suggestion are welcomed.

How would you instanciate devices and connect their clocks from the
command line (with the -device option)?

Should clocked devices have DeviceClass::user_creatable = false by default?

Thanks,

Phil.

Re: [Qemu-devel] [PATCH v5 0/9] Clock framework API.
Posted by Damien Hedde 5 years, 6 months ago
Hi Philippe,

On 10/4/18 6:13 PM, Philippe Mathieu-Daudé wrote:
> Hi Damien,
> 
> On 02/10/2018 16:24, Damien Hedde wrote:
>> This series aims to add a way to model clocks in qemu between devices.
>> This allows to model the clock tree of a platform allowing us to inspect clock
>> configuration and detect problems such as disabled clock or bad configured
>> pll.
>>
>> [...]
>>
>> Any comments and suggestion are welcomed.
> 
> How would you instanciate devices and connect their clocks from the
> command line (with the -device option)?
I didn't not thought about that. I'm not sure to understand how this is
done for a gpio for example. Is this done by setting the link property
manually ?

If that's the case, I suppose we can somehow add a link property on the
input side to the output side to achieve that. The other way seem not
possible due to the fact the output side has a dynamically allocated
list of inputs.
> 
> Should clocked devices have DeviceClass::user_creatable = false by default?
It would be a solution, but I don't think we want this ?
> 
> Thanks,
> 
> Phil.
> 

Thanks,

Damien

Re: [Qemu-devel] [PATCH v5 0/9] Clock framework API.
Posted by Peter Maydell 5 years, 6 months ago
On 11 October 2018 at 17:20, Damien Hedde <damien.hedde@greensocs.com> wrote:
>
> Hi Philippe,
>
> On 10/4/18 6:13 PM, Philippe Mathieu-Daudé wrote:
>> Hi Damien,
>>
>> On 02/10/2018 16:24, Damien Hedde wrote:
>>> This series aims to add a way to model clocks in qemu between devices.
>>> This allows to model the clock tree of a platform allowing us to inspect clock
>>> configuration and detect problems such as disabled clock or bad configured
>>> pll.
>>>
>>> [...]
>>>
>>> Any comments and suggestion are welcomed.
>>
>> How would you instanciate devices and connect their clocks from the
>> command line (with the -device option)?
> I didn't not thought about that. I'm not sure to understand how this is
> done for a gpio for example. Is this done by setting the link property
> manually ?

You can't wire up GPIOs on the command line. I don't think we
really need to be able to wire up clocks on the command line either,
do we?

> Should clocked devices have DeviceClass::user_creatable = false by default?

How many devices have a clock and nothing else that would cause
them to be non-user-creatable (ie no GPIOs, no IRQ lines, no
memory-mapped memory regions) ?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v5 0/9] Clock framework API.
Posted by Philippe Mathieu-Daudé 5 years, 6 months ago
On 11/10/2018 18:23, Peter Maydell wrote:
> On 11 October 2018 at 17:20, Damien Hedde <damien.hedde@greensocs.com> wrote:
>>
>> Hi Philippe,
>>
>> On 10/4/18 6:13 PM, Philippe Mathieu-Daudé wrote:
>>> Hi Damien,
>>>
>>> On 02/10/2018 16:24, Damien Hedde wrote:
>>>> This series aims to add a way to model clocks in qemu between devices.
>>>> This allows to model the clock tree of a platform allowing us to inspect clock
>>>> configuration and detect problems such as disabled clock or bad configured
>>>> pll.
>>>>
>>>> [...]
>>>>
>>>> Any comments and suggestion are welcomed.
>>>
>>> How would you instanciate devices and connect their clocks from the
>>> command line (with the -device option)?
>> I didn't not thought about that. I'm not sure to understand how this is
>> done for a gpio for example. Is this done by setting the link property
>> manually ?
> 
> You can't wire up GPIOs on the command line. I don't think we
> really need to be able to wire up clocks on the command line either,
> do we?

This would be a big mess.

> 
>> Should clocked devices have DeviceClass::user_creatable = false by default?
> 
> How many devices have a clock and nothing else that would cause
> them to be non-user-creatable (ie no GPIOs, no IRQ lines, no
> memory-mapped memory regions) ?

I'm not sure I understood your question.

But I understand than as devices consuming GPIO/IRQ, clocked device
can't be instantiate from command line, and I am happy with that.

Re: [Qemu-devel] [PATCH v5 0/9] Clock framework API.
Posted by Peter Maydell 5 years, 6 months ago
On 11 October 2018 at 18:00, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> On 11/10/2018 18:23, Peter Maydell wrote:
>> How many devices have a clock and nothing else that would cause
>> them to be non-user-creatable (ie no GPIOs, no IRQ lines, no
>> memory-mapped memory regions) ?
>
> I'm not sure I understood your question.

Sorry, let me try rephrasing.

We started with the question of whether devices with clocks should be
marked not user creatable. It's already the case that devices with any
of IRQs, GPIOs or memory-mapped registers can't be user created.
(In particular, any sysbus device is not user-creatable by default.)
So the question was intended to ask how often it matters whether
we can't wire up devices with clocks on the command line.
Are there any devices which would have clocks, but aren't *already*
ruled out of being user creatable for other reasons? If there
aren't any, it doesn't really matter much that we don't have
a mechanism for command line clock wiring.

So I think the answer is:
 * almost all users of this API framework are going to be
   sysbus devices; those are already not user-creatable
 * any device that uses this framework, and which is not
   a sysbus device (but instead a plain old qdev device)
   will need to mark itself as not-user-creatable by hand,
   the same as if it uses the qdev gpio APIs

thanks
-- PMM

Re: [Qemu-devel] [PATCH v5 0/9] Clock framework API.
Posted by Philippe Mathieu-Daudé 5 years, 6 months ago
On 11/10/2018 19:12, Peter Maydell wrote:
> On 11 October 2018 at 18:00, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>> On 11/10/2018 18:23, Peter Maydell wrote:
>>> How many devices have a clock and nothing else that would cause
>>> them to be non-user-creatable (ie no GPIOs, no IRQ lines, no
>>> memory-mapped memory regions) ?
>>
>> I'm not sure I understood your question.
> 
> Sorry, let me try rephrasing.
> 
> We started with the question of whether devices with clocks should be
> marked not user creatable. It's already the case that devices with any
> of IRQs, GPIOs or memory-mapped registers can't be user created.
> (In particular, any sysbus device is not user-creatable by default.)
> So the question was intended to ask how often it matters whether
> we can't wire up devices with clocks on the command line.
> Are there any devices which would have clocks, but aren't *already*
> ruled out of being user creatable for other reasons? If there
> aren't any, it doesn't really matter much that we don't have
> a mechanism for command line clock wiring.

Thank you for clearing this out.

> 
> So I think the answer is:
>  * almost all users of this API framework are going to be
>    sysbus devices; those are already not user-creatable

Yes.

>  * any device that uses this framework, and which is not
>    a sysbus device (but instead a plain old qdev device)
>    will need to mark itself as not-user-creatable by hand,
>    the same as if it uses the qdev gpio APIs

OK, you answered my first question, it is now clearer to me :)

Thank you!

Phil.

Re: [Qemu-devel] [PATCH v5 0/9] Clock framework API.
Posted by Damien Hedde 5 years, 6 months ago
Hi Peter,

Sorry to bother you with this, but you said some time ago you would
write something about reset.

On 10/2/18 4:24 PM, Damien Hedde wrote:
> There is also the problem of initialization which is very much like the
> migration. Currently, in the zynq example, clocks outputs are initialized in
> the clock controller device_reset. According to Peter, outputs should not be
> set in device_reset, so we need a better way.

To illustrate the problem, lets take an example with the zynq case in
the patchset. We have a 2-stage clock tree:

+--------+            +----------------+              +------+
| Machine|>>base_clk>>|Clock controller|>>uart_clock>>| UART |
+--------+            +----------------+              +------+

At the end of the tree, the uart uses the clock to setup the backend
baudrate. The problem is that we need the whole clock path initialized
before having the right final frequency.

I've had some thought about it and I see several solutions

1. Set clock outputs in device_reset. But It can trigger side effects in
yet-non-reseted devices.

2. Have some kind of global 2nd stage reset to do all the output
propagation.

3. Try to propagate init values at platform building when doing the
clock connection.

4. (1) and (2) mixed. Have a per device 2nd stage reset "clock_reset"
(or any better name) method called when device_reset has been called and
all clock inputs have been initialized (by other device "clock_reset").
At the end of reset phase everything should have been propagated.

(1) being not-an-option and also (2) I think.

(3) seems complicated at best due to the unknown clock connection order.
And I'm not sure clock connection is the right moment to do this. In the
general case, a clock init value can depend on on some user-set/config
properties which will be applied later on I suppose (But I don't have a
clue at which point this operation -- the "realize" thing -- cab be done
in the platform startup sequence)

Do you think (4) is sensible ? It means any device requiring clock input
value will need to implement this new method. Basically this
"clock_reset" is just a part of the actual device_reset being delayed.
Obviously if there is a clock loop, the method will never be called.

--
Damien



Re: [Qemu-devel] [PATCH v5 0/9] Clock framework API.
Posted by Peter Maydell 5 years, 6 months ago
On 12 October 2018 at 16:26, Damien Hedde <damien.hedde@greensocs.com> wrote:
> Hi Peter,
>
> Sorry to bother you with this, but you said some time ago you would
> write something about reset.

Yeah, sorry about that. I haven't found (made) the time to think
the issues through yet. Let me just dump to email the notes I had.
This is a very "raw" set of notes, so it's not very organised and
has a lot of 'todo' bits in it...


Overall question -- does anybody know of other device modelling
systems that do a good job of modelling reset, and how they do it?
If we can borrow a proven design that might be better tha
thrashing one out ourseles...


My idea was roughly that we need a multi-phase reset, something like:

 PHASE 1:
   Devices reset internal state. No calls out to other devices
   (ie no setting IRQ line state, etc)
   Some common bit of code (dev base class?) marks the device
   reset as being in-progress by setting dev->resetting = true.

 PHASE 2:
   Devices update their output lines (calls to irq line setting
   functions, clock setting, etc)
   In this phase devices may be the recipients of callbacks for
   IRQ line level changes, etc, and must cope with this. A
   callback can tell it's being called for a level change during
   reset because dev->resetting will be true.
   (They can update their own outputs in response to input changes.)

 PHASE 3:
   Common code (dev base class) sets dev->resetting = false.
   (Individual devices don't get to do anything here, unless
   there's some painful case that requires it.)

The theory is that by decoupling reset into multiple phases
we can avoid most of the reset order issues, by having the
two ends do things in the right phase and/or by having
the destination end be able to identify when a change is
happening in the middle of reset.

Compatibility with existing device reset methods:
essentially the current rules are the same as the "phase 1"
rules, so we can just say that the current reset code in
all devices is phase 1 reset. Then we can adjust anything
that needs a phase 2 (which is probably not all that many
devices).

Places where reset order currently matters:
(this is just a list of things we should make sure our design
copes with)
 * wired-high IRQ lines, and device outputs that are asserted
   as the device comes out of reset
 * setting PC in the generic-loader device vs CPU reset
 * image load (rom image reset) vs Arm M-profile reset wanting
   to read the reset PC/SP from memory
 * Arm M-profile CPUs should have INITVTOR as a config signal input
 * x86 CPU vs APIC reset order ??
 * anything else?

We should have some sort of "reset domain" object which can
contain devices and also other reset domains. A reset domain's
reset is simple: its phase 1 reset is "for each child, do
phase 1 reset for that child", similarly for phases 2 and 3.
A reset domain has an overall "reset" method that says
"do phase 1 for all children, then phase 2, then phase 3".
This allows things like "this SoC has a reset line which
resets all its internal devices". (Do we want to give
reset domains input gpio lines to trigger reset, or is
being able to call a method on it sufficient?)

At the moment we have reset methods on devices:
 * each bus handles reset for the things on the bus
 * the (one and only) sysbus resets all the sysbus devices
 * devices on no bus don't get reset at all unless something
   takes special measures! (notably the CPUs are reset in
   a very ad-hoc way at the moment)
I guess this should be cleaned up by making each bus be
a 'reset domain', and having a "system" reset domain which gets
anything which doesn't have a parent reset domain (and which
we then use for whole-board power-on reset). Calling the
system reset domain's overall reset method will then do
the 3-phase reset for everything.

How would direct qemu_register_reset() calls fit into this
scheme? (Some of them are devices that could get QOM reset
methods instead. Perhaps we could get away with dumping the
rest into the 'master reset' domain, or leaving them as-is?
Some are for CPU reset which we definitely should clean up.)

We need to handle different types of reset somehow, in
particular "cold/power on reset" versus "warm reset".
Can we do this just with reset domain objects, or do we
need/want to pass each device's reset method a "type" argument?

thanks
-- PMM

Re: [Qemu-devel] [PATCH v5 0/9] Clock framework API.
Posted by Damien Hedde 5 years, 4 months ago

On 10/16/18 5:48 PM, Peter Maydell wrote:
> On 12 October 2018 at 16:26, Damien Hedde <damien.hedde@greensocs.com> wrote:
>> Hi Peter,
>>
>> Sorry to bother you with this, but you said some time ago you would
>> write something about reset.
> 
> Yeah, sorry about that. I haven't found (made) the time to think
> the issues through yet. Let me just dump to email the notes I had.
> This is a very "raw" set of notes, so it's not very organised and
> has a lot of 'todo' bits in it...
> 
> 
> Overall question -- does anybody know of other device modelling
> systems that do a good job of modelling reset, and how they do it?
> If we can borrow a proven design that might be better tha
> thrashing one out ourseles...
> 
> 
> My idea was roughly that we need a multi-phase reset, something like:
> 
>  PHASE 1:
>    Devices reset internal state. No calls out to other devices
>    (ie no setting IRQ line state, etc)
>    Some common bit of code (dev base class?) marks the device
>    reset as being in-progress by setting dev->resetting = true.
> 
>  PHASE 2:
>    Devices update their output lines (calls to irq line setting
>    functions, clock setting, etc)
>    In this phase devices may be the recipients of callbacks for
>    IRQ line level changes, etc, and must cope with this. A
>    callback can tell it's being called for a level change during
>    reset because dev->resetting will be true.
>    (They can update their own outputs in response to input changes.)
> 
>  PHASE 3:
>    Common code (dev base class) sets dev->resetting = false.
>    (Individual devices don't get to do anything here, unless
>    there's some painful case that requires it.)
> 
> The theory is that by decoupling reset into multiple phases
> we can avoid most of the reset order issues, by having the
> two ends do things in the right phase and/or by having
> the destination end be able to identify when a change is
> happening in the middle of reset.

This strategy seems fine to me.

> 
> Compatibility with existing device reset methods:
> essentially the current rules are the same as the "phase 1"
> rules, so we can just say that the current reset code in
> all devices is phase 1 reset. Then we can adjust anything
> that needs a phase 2 (which is probably not all that many
> devices).
> 
> Places where reset order currently matters:
> (this is just a list of things we should make sure our design
> copes with)
>  * wired-high IRQ lines, and device outputs that are asserted
>    as the device comes out of reset
>  * setting PC in the generic-loader device vs CPU reset
>  * image load (rom image reset) vs Arm M-profile reset wanting
>    to read the reset PC/SP from memory
>  * Arm M-profile CPUs should have INITVTOR as a config signal input
>  * x86 CPU vs APIC reset order ??
>  * anything else?
> 
> We should have some sort of "reset domain" object which can
> contain devices and also other reset domains. A reset domain's
> reset is simple: its phase 1 reset is "for each child, do
> phase 1 reset for that child", similarly for phases 2 and 3.
> A reset domain has an overall "reset" method that says
> "do phase 1 for all children, then phase 2, then phase 3".
> This allows things like "this SoC has a reset line which
> resets all its internal devices". (Do we want to give
> reset domains input gpio lines to trigger reset, or is
> being able to call a method on it sufficient?)

Consider the following use case: a platform with a controller that holds
a reset line on another device (starting with reset held).
At the beginning, the main (cold) reset will be triggered to start the
platform. During phase 2, the controller will set the reset line of the
other device (a gpio). Then we have phase 3. How do we handle this ?
Do we need to "delay" the phase 3 of the device until the controller
releases its reset line or do we consider the internal gpio reset line
to be outside of this reset scope ?

If we do that, we have 2 reset "sources": the main reset and the
internal gpio and there is some kind of OR operation to do between these
2 sources.

> 
> At the moment we have reset methods on devices:
>  * each bus handles reset for the things on the bus
>  * the (one and only) sysbus resets all the sysbus devices
>  * devices on no bus don't get reset at all unless something
>    takes special measures! (notably the CPUs are reset in
>    a very ad-hoc way at the moment)
> I guess this should be cleaned up by making each bus be
> a 'reset domain', and having a "system" reset domain which gets
> anything which doesn't have a parent reset domain (and which
> we then use for whole-board power-on reset). Calling the
> system reset domain's overall reset method will then do
> the 3-phase reset for everything>
> How would direct qemu_register_reset() calls fit into this
> scheme? (Some of them are devices that could get QOM reset
> methods instead. Perhaps we could get away with dumping the
> rest into the 'master reset' domain, or leaving them as-is?
> Some are for CPU reset which we definitely should clean up.)
> 
> We need to handle different types of reset somehow, in
> particular "cold/power on reset" versus "warm reset".
> Can we do this just with reset domain objects, or do we
> need/want to pass each device's reset method a "type" argument?

Maybe phase 1 only needs the type, phase 2 and 3 can only depends on the
device state.

Anyway, this reset problem seems outside of the scope of this patch set.
I think I'll do a reroll with the remark I've got so far, but it will
still miss the reset part to be well integrated. I can also propose
something in another rfc about the 3-phase reset to advance on that part.

--
Damien