qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v7 03/11] hw/core: create Resettable QOM interface


From: Philippe Mathieu-Daudé
Subject: Re: [PATCH v7 03/11] hw/core: create Resettable QOM interface
Date: Mon, 20 Jan 2020 10:18:07 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.2

On 1/20/20 10:08 AM, Damien Hedde wrote:
On 1/18/20 7:42 AM, Philippe Mathieu-Daudé wrote:
On 1/15/20 1:36 PM, Damien Hedde wrote:
This commit defines an interface allowing multi-phase reset. This aims
to solve a problem of the actual single-phase reset (built in
DeviceClass and BusClass): reset behavior is dependent on the order
in which reset handlers are called. In particular doing external
side-effect (like setting an qemu_irq) is problematic because receiving
object may not be reset yet.

The Resettable interface divides the reset in 3 well defined phases.
To reset an object tree, all 1st phases are executed then all 2nd then
all 3rd. See the comments in include/hw/resettable.h for a more complete
description. The interface defines 3 phases to let the future
possibility of holding an object into reset for some time.

The qdev/qbus reset in DeviceClass and BusClass will be modified in
following commits to use this interface. A mechanism is provided
to allow executing a transitional reset handler in place of the 2nd
phase which is executed in children-then-parent order inside a tree.
This will allow to transition devices and buses smoothly while
keeping the exact current qdev/qbus reset behavior for now.

Documentation will be added in a following commit.

Signed-off-by: Damien Hedde <address@hidden>
Reviewed-by: Richard Henderson <address@hidden>
---

v7 update: un-nest struct ResettablePhases
---
   Makefile.objs           |   1 +
   include/hw/resettable.h | 211 +++++++++++++++++++++++++++++++++++
   hw/core/resettable.c    | 238 ++++++++++++++++++++++++++++++++++++++++
   hw/core/Makefile.objs   |   1 +
   hw/core/trace-events    |  17 +++
   5 files changed, 468 insertions(+)
   create mode 100644 include/hw/resettable.h
   create mode 100644 hw/core/resettable.c

diff --git a/Makefile.objs b/Makefile.objs
index 7c1e50f9d6..9752d549b4 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -191,6 +191,7 @@ trace-events-subdirs += migration
   trace-events-subdirs += net
   trace-events-subdirs += ui
   endif
+trace-events-subdirs += hw/core
   trace-events-subdirs += hw/display
   trace-events-subdirs += qapi
   trace-events-subdirs += qom
diff --git a/include/hw/resettable.h b/include/hw/resettable.h
new file mode 100644
index 0000000000..58b3df4c22
--- /dev/null
+++ b/include/hw/resettable.h
@@ -0,0 +1,211 @@
+/*
+ * Resettable interface header.
+ *
+ * Copyright (c) 2019 GreenSocs SAS
+ *
+ * Authors:
+ *   Damien Hedde
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef HW_RESETTABLE_H
+#define HW_RESETTABLE_H
+
+#include "qom/object.h"
+
+#define TYPE_RESETTABLE_INTERFACE "resettable"
+
+#define RESETTABLE_CLASS(class) \
+    OBJECT_CLASS_CHECK(ResettableClass, (class),
TYPE_RESETTABLE_INTERFACE)
+
+#define RESETTABLE_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(ResettableClass, (obj), TYPE_RESETTABLE_INTERFACE)
+
+typedef struct ResettableState ResettableState;
+
+/**
+ * ResetType:
+ * Types of reset.
+ *
+ * + Cold: reset resulting from a power cycle of the object.
+ *
+ * TODO: Support has to be added to handle more types. In particular,
+ * ResettableState structure needs to be expanded.
+ */
+typedef enum ResetType {
+    RESET_TYPE_COLD,
+} ResetType;
+
+/*
+ * ResettableClass:
+ * Interface for resettable objects.
+ *
+ * See docs/devel/reset.rst for more detailed information about how
QEMU models
+ * reset. This whole API must only be used when holding the iothread
mutex.
+ *
+ * All objects which can be reset must implement this interface;
+ * it is usually provided by a base class such as DeviceClass or
BusClass.
+ * Every Resettable object must maintain some state tracking the
+ * progress of a reset operation by providing a ResettableState
structure.
+ * The functions defined in this module take care of updating the
+ * state of the reset.
+ * The base class implementation of the interface provides this
+ * state and implements the associated method: get_state.
+ *
+ * Concrete object implementations (typically specific devices
+ * such as a UART model) should provide the functions
+ * for the phases.enter, phases.hold and phases.exit methods, which
+ * they can set in their class init function, either directly or
+ * by calling resettable_class_set_parent_phases().
+ * The phase methods are guaranteed to only only ever be called once
+ * for any reset event, in the order 'enter', 'hold', 'exit'.
+ * An object will always move quickly from 'enter' to 'hold'
+ * but might remain in 'hold' for an arbitrary period of time
+ * before eventually reset is deasserted and the 'exit' phase is called.
+ * Object implementations should be prepared for functions handling
+ * inbound connections from other devices (such as qemu_irq handler
+ * functions) to be called at any point during reset after their
+ * 'enter' method has been called.
+ *
+ * Users of a resettable object should not call these methods
+ * directly, but instead use the function resettable_reset().
+ *
+ * @phases.enter: This phase is called when the object enters reset. It
+ * should reset local state of the object, but it must not do
anything that
+ * has a side-effect on other objects, such as raising or lowering a
qemu_irq
+ * line or reading or writing guest memory. It takes the reset's type as
+ * argument.
+ *
+ * @phases.hold: This phase is called for entry into reset, once
every object
+ * in the system which is being reset has had its @phases.enter
method called.
+ * At this point devices can do actions that affect other objects.
+ *
+ * @phases.exit: This phase is called when the object leaves the
reset state.
+ * Actions affecting other objects are permitted.
+ *
+ * @get_state: Mandatory method which must return a pointer to a
+ * ResettableState.
+ *
+ * @get_transitional_function: transitional method to handle
Resettable objects
+ * not yet fully moved to this interface. It will be removed as soon
as it is
+ * not needed anymore. This method is optional and may return a
pointer to a
+ * function to be used instead of the phases. If the method exists
and returns
+ * a non-NULL function pointer then that function is executed as a
replacement
+ * of the 'hold' phase method taking the object as argument. The two
other phase
+ * methods are not executed.
+ *
+ * @child_foreach: Executes a given callback on every Resettable
child. Child
+ * in this context means a child in the qbus tree, so the children of
a qbus
+ * are the devices on it, and the children of a device are all the
buses it
+ * owns. This is not the same as the QOM object hierarchy. The
function takes
+ * additional opaque and ResetType arguments which must be passed
unmodified to
+ * the callback.
+ */
+typedef void (*ResettableEnterPhase)(Object *obj, ResetType type);
+typedef void (*ResettableHoldPhase)(Object *obj);
+typedef void (*ResettableExitPhase)(Object *obj);
+typedef ResettableState * (*ResettableGetState)(Object *obj);
+typedef void (*ResettableTrFunction)(Object *obj);
+typedef ResettableTrFunction (*ResettableGetTrFunction)(Object *obj);
+typedef void (*ResettableChildCallback)(Object *, void *opaque,
+                                        ResetType type);
+typedef void (*ResettableChildForeach)(Object *obj,
+                                       ResettableChildCallback cb,
+                                       void *opaque, ResetType type);
+typedef struct ResettablePhases {
+    ResettableEnterPhase enter;
+    ResettableHoldPhase hold;
+    ResettableExitPhase exit;
+} ResettablePhases;
+typedef struct ResettableClass {
+    InterfaceClass parent_class;
+
+    /* Phase methods */
+    ResettablePhases phases;
+
+    /* State access method */
+    ResettableGetState get_state;
+
+    /* Transitional method for legacy reset compatibility */
+    ResettableGetTrFunction get_transitional_function;
+
+    /* Hierarchy handling method */
+    ResettableChildForeach child_foreach;
+} ResettableClass;
+
+/**
+ * ResettableState:
+ * Structure holding reset related state. The fields should not be
accessed
+ * directly; the definition is here to allow further inclusion into
other
+ * objects.
+ *
+ * @count: Number of reset level the object is into. It is
incremented when
+ * the reset operation starts and decremented when it finishes.

Maybe you can add this comment and the variable in patch 5/11, that
would make patch 5 easier to review.

The variable is used in this patch so I cannot do that.

Oops you are right o_O ...


+ * @hold_phase_pending: flag which indicates that we need to invoke
the 'hold'
+ * phase handler for this object.
+ * @exit_phase_in_progress: true if we are currently in the exit phase
+ */
+struct ResettableState {
+    uint32_t count;

Maybe simply 'unsigned'?


Ok.

--
Damien




reply via email to

[Prev in Thread] Current Thread [Next in Thread]