* [Qemu-devel] [RFC PATCH v2 00/12] Multi-phase reset
@ 2019-06-04 16:25 Damien Hedde
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 01/12] Create Resettable QOM interface Damien Hedde
` (12 more replies)
0 siblings, 13 replies; 15+ messages in thread
From: Damien Hedde @ 2019-06-04 16:25 UTC (permalink / raw)
To: qemu-devel
Cc: edgar.iglesias, peter.maydell, marc.burton, alistair, qemu-arm,
Damien Hedde, marcandre.lureau, pbonzini, philmd, luc.michel
Hi all,
Here's the second version of the multi-phase reset proposal patches.
# DESCRIPTION
Basically the reset procedure is split in 3 parts:
INIT PHASE: Reset the object internal state, put a resetting flag and do the
same for the reset subtree. No side effect on other devices to guarantee
that, in a reset domain, everything get initialized first. This corresponds
mostly to what is currently done in the device/bus reset method.
HOLD PHASE: This phase allows to control a reset with a IO. When a IO control
reset procedure based on the IO level (not edge), we may need to assert
the reset, wait some time, and finally de-assert the reset. The consequence
is that such a device can stay in a "resetting state" and may need to show
this state to other devices through its outputs. For example, a clock
controller will typically shutdown its clocks when it is in resetting state.
EXIT PHASE: This phase sets outputs to state after reset. For a clock controller
it starts the clocks. It also clears the "resetting" flag. A device should
not react to inputs until this flag has been cleared. During this phase,
outputs are propagated.
The Resettable interface is detailed in the added doc in patch 7.
# CHANGE SINCE V1
The series now focus only on the Resettable interface (no more ResetDomain).
Proposed changed have been applied:
- reset_count getter/modifier methods
- a foreach_child method
This last method allows some flexibility on what is the hierarchy of reset.
There is some discussion ongoing about this point. Althought the series does
not aim to modify the qdev/qbus-centric reset behavior, it is not fixed. An
object specialization can override it.
# RESET DEPRECATION
There is 3 changes in the current way of handling reset (for users or
developers of Devices)
1. qdev/qbus_reset_all
Theses functions are deprecated by this series and should be replaced by
direct call to device_reset or bus_reset. There is only a few existing calls
so I can probably replace them all and delete the original functions.
2. old's device_reset
There is a few call to this function, I renamed it *device_legacy_reset* to
handle the transition. This function allowed to reset only a given device
(and not its eventual qbus subtree). This behavior is not possible with
the Resettable interface. At first glance it seemed that it is used only on
device having no sub-buses so we could just use the new device_reset.
But I need to look at them more closely to be sure. If this behavior is really
needed (but why would we not reset children ?), it's possible to do a specific
function for Device to to 3-phases reset without the children.
3. DeviceClass's reset and BusClass's methods
This is the major change. The method is deprecated because reset methods are
now located in the interface class. In the series, I make the init phase
redirect to the original reset method of DeviceClass (or BusClass). There a
lot of use of the method and transitioning to 3-phases only reset will need
some work.
# MIGRATION
Bus reset state migration is right now not handled.
Regarding "migration-during-reset should Just Work, without necessarily
needing any specific changes for a device". The included patch define a vmsd
subsection which must to be added to every device main vmsd structure for
migration-during-reset of theses devices to work. I tried to find a way to
avoid that but really don't see how to achieve that.
So in the current state of this series, migration can only be supported for
leaf device (in term of qdev/qbus) where we add the subsection.
I'm not sure the migration is the problem here. I doubt a device will
support staying in reset state (which is a new feature) without manual
intervention. So migration of this unsupported state without any specific
change may not be a real asset.
The series is organised as follow:
- Patch 1 adds Resettable interface
- Patches 2 and 3 rename device_reset function by device_legacy_reset to prepare
the transition.
- Patches 4 to 6 make the changes in Device and Bus classes.
- Patches 7 add some doc
- Patches 8 to 12 modify the xilinx_zynq to add 3-phases reset support in the
uart and the slcr (the reset controller).
Thank you for your feedback,
Damien
Damien Hedde (12):
Create Resettable QOM interface
add device_legacy_reset function to do the transition with
device_reset
replace all occurences of device_reset by device_legacy_reset
make Device and Bus Resettable
Add function to control reset with gpio inputs
add vmstate description for device reset state
add doc about Resettable interface
hw/misc/zynq_slcr: use standard register definition
convert cadence_uart to 3-phases reset
Convert zynq's slcr to 3-phases reset
Add uart reset support in zynq_slcr
Connect the uart reset gpios in the zynq platform
docs/devel/reset.txt | 151 +++++++++
hw/arm/xilinx_zynq.c | 14 +-
hw/audio/intel-hda.c | 2 +-
hw/char/cadence_uart.c | 81 ++++-
hw/core/Makefile.objs | 2 +
hw/core/bus.c | 60 ++++
hw/core/qdev-vmstate.c | 34 +++
hw/core/qdev.c | 124 +++++++-
hw/core/resettable.c | 121 ++++++++
hw/hyperv/hyperv.c | 2 +-
hw/i386/pc.c | 2 +-
hw/ide/microdrive.c | 8 +-
hw/intc/spapr_xive.c | 2 +-
hw/misc/zynq_slcr.c | 543 ++++++++++++++++++---------------
hw/ppc/pnv_psi.c | 2 +-
hw/ppc/spapr_pci.c | 2 +-
hw/ppc/spapr_vio.c | 2 +-
hw/s390x/s390-pci-inst.c | 2 +-
hw/scsi/vmw_pvscsi.c | 2 +-
hw/sd/omap_mmc.c | 2 +-
hw/sd/pl181.c | 2 +-
include/hw/char/cadence_uart.h | 10 +-
include/hw/qdev-core.h | 112 ++++++-
include/hw/resettable.h | 104 +++++++
tests/Makefile.include | 1 +
25 files changed, 1105 insertions(+), 282 deletions(-)
create mode 100644 docs/devel/reset.txt
create mode 100644 hw/core/qdev-vmstate.c
create mode 100644 hw/core/resettable.c
create mode 100644 include/hw/resettable.h
--
2.21.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [RFC PATCH v2 01/12] Create Resettable QOM interface
2019-06-04 16:25 [Qemu-devel] [RFC PATCH v2 00/12] Multi-phase reset Damien Hedde
@ 2019-06-04 16:25 ` Damien Hedde
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 02/12] add device_legacy_reset function to do the transition with device_reset Damien Hedde
` (11 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Damien Hedde @ 2019-06-04 16:25 UTC (permalink / raw)
To: qemu-devel
Cc: edgar.iglesias, peter.maydell, marc.burton, alistair, qemu-arm,
Damien Hedde, marcandre.lureau, pbonzini, philmd, luc.michel
This commit defines an interface allowing multi-phase reset.
The phases are INIT, HOLD and EXIT. Each phase has an associated method
in the class.
The reset of a Resettable is controlled with 2 functions:
- resettable_assert_reset which starts the reset operation.
- resettable_deassert_reset which ends the reset operation.
There is also a `resettable_reset` helper function which does assert then
deassert allowing to do a proper reset in one call.
The Resettable interface is "reentrant", _assert_ can be called several
times and _deasert_ must be called the same number of times so that the
Resettable leave reset state. It is supported by keeping a counter of
the current number of _assert_ calls. The counter maintainance is done
though 3 methods get/increment/decrement_count.
Reset hierarchy is also supported. Each Resettable may have
sub-Resettable objects. When resetting a Resettable, it is propagated to
its children using the foreach_child method.
The reset is first propagated to the children before being applied to the
Resettable. This will allow to replace current qdev_reset mechanism by this
interface without side-effects on reset order.
Note: I used an uint32 for the count. This match the type already used
in the existing resetting counter in hw/scsi/vmw_pvscsi.c for the
PVSCSIState.
Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
hw/core/Makefile.objs | 1 +
hw/core/resettable.c | 121 ++++++++++++++++++++++++++++++++++++++++
include/hw/resettable.h | 104 ++++++++++++++++++++++++++++++++++
3 files changed, 226 insertions(+)
create mode 100644 hw/core/resettable.c
create mode 100644 include/hw/resettable.h
diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
index a799c83815..97007454a8 100644
--- a/hw/core/Makefile.objs
+++ b/hw/core/Makefile.objs
@@ -1,6 +1,7 @@
# core qdev-related obj files, also used by *-user:
common-obj-y += qdev.o qdev-properties.o
common-obj-y += bus.o reset.o
+common-obj-y += resettable.o
common-obj-$(CONFIG_SOFTMMU) += qdev-fw.o
common-obj-$(CONFIG_SOFTMMU) += fw-path-provider.o
# irq.o needed for qdev GPIO handling:
diff --git a/hw/core/resettable.c b/hw/core/resettable.c
new file mode 100644
index 0000000000..59954dac05
--- /dev/null
+++ b/hw/core/resettable.c
@@ -0,0 +1,121 @@
+/*
+ * Resettable interface.
+ *
+ * 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.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/module.h"
+#include "hw/resettable.h"
+
+#define RESETTABLE_MAX_COUNT 50
+
+#define RESETTABLE_GET_CLASS(obj) \
+ OBJECT_GET_CLASS(ResettableClass, (obj), TYPE_RESETTABLE)
+
+static void resettable_init_phase(Object *obj, bool cold);
+
+static void resettable_cold_init_phase(Object *obj)
+{
+ resettable_init_phase(obj, true);
+}
+
+static void resettable_warm_init_phase(Object *obj)
+{
+ resettable_init_phase(obj, false);
+}
+
+static void resettable_init_phase(Object *obj, bool cold)
+{
+ void (*func)(Object *);
+ ResettableClass *rc = RESETTABLE_GET_CLASS(obj);
+ uint32_t count;
+
+ count = rc->increment_count(obj);
+ /* this assert is triggered by an eventual reset loop */
+ assert(count <= RESETTABLE_MAX_COUNT);
+
+ func = cold ? resettable_cold_init_phase : resettable_warm_init_phase;
+ rc->foreach_child(obj, func);
+
+ if (rc->phases.init) {
+ rc->phases.init(obj, cold);
+ }
+}
+
+static void resettable_hold_phase(Object *obj)
+{
+ ResettableClass *rc = RESETTABLE_GET_CLASS(obj);
+
+ rc->foreach_child(obj, resettable_hold_phase);
+
+ if (rc->phases.hold) {
+ rc->phases.hold(obj);
+ }
+}
+
+static void resettable_exit_phase(Object *obj)
+{
+ ResettableClass *rc = RESETTABLE_GET_CLASS(obj);
+
+ rc->foreach_child(obj, resettable_exit_phase);
+
+ assert(rc->get_count(obj) > 0);
+ if (rc->decrement_count(obj) == 0 && rc->phases.exit) {
+ rc->phases.exit(obj);
+ }
+}
+
+void resettable_assert_reset(Object *obj, bool cold)
+{
+ resettable_init_phase(obj, cold);
+ resettable_hold_phase(obj);
+}
+
+void resettable_deassert_reset(Object *obj)
+{
+ resettable_exit_phase(obj);
+}
+
+void resettable_reset(Object *obj, bool cold)
+{
+ resettable_assert_reset(obj, cold);
+ resettable_deassert_reset(obj);
+}
+
+void resettable_class_set_parent_reset_phases(ResettableClass *rc,
+ ResettableInitPhase init,
+ ResettableHoldPhase hold,
+ ResettableExitPhase exit,
+ ResettablePhases *parent_phases)
+{
+ *parent_phases = rc->phases;
+ if (init) {
+ rc->phases.init = init;
+ }
+ if (hold) {
+ rc->phases.hold = hold;
+ }
+ if (exit) {
+ rc->phases.exit = exit;
+ }
+}
+
+static const TypeInfo resettable_interface_info = {
+ .name = TYPE_RESETTABLE,
+ .parent = TYPE_INTERFACE,
+ .class_size = sizeof(ResettableClass),
+};
+
+static void reset_register_types(void)
+{
+ type_register_static(&resettable_interface_info);
+}
+
+type_init(reset_register_types)
diff --git a/include/hw/resettable.h b/include/hw/resettable.h
new file mode 100644
index 0000000000..39522b9b51
--- /dev/null
+++ b/include/hw/resettable.h
@@ -0,0 +1,104 @@
+#ifndef HW_RESETTABLE_H
+#define HW_RESETTABLE_H
+
+#include "qom/object.h"
+
+#define TYPE_RESETTABLE "resettable"
+
+#define RESETTABLE_CLASS(class) \
+ OBJECT_CLASS_CHECK(ResettableClass, (class), TYPE_RESETTABLE)
+
+/*
+ * ResettableClass:
+ * Interface for resettable objects.
+ *
+ * The reset operation is divided in several phases each represented by a
+ * method.
+ *
+ * Each Ressetable must maintain a reset counter in its state, 3 methods allows
+ * to interact with it.
+ *
+ * @phases.init: should reset local state only. Takes a bool @cold argument
+ * specifying whether the reset is cold or warm. It must not do side-effect
+ * on others objects.
+ *
+ * @phases.hold: side-effects action on others objects due to staying in a
+ * resetting state.
+ *
+ * @phases.exit: leave the reset state, may do side-effects action on others
+ * objects.
+ *
+ * @get_count: Get the current reset count
+ * @increment_count: Increment the reset count, returns the new count
+ * @decrement_count: decrement the reset count, returns the new count
+ *
+ * @foreach_child: Executes a given function on every Resettable child.
+ * A child is not a QOM child, but a child a reset meaning.
+ */
+typedef void (*ResettableInitPhase)(Object *obj, bool cold);
+typedef void (*ResettableHoldPhase)(Object *obj);
+typedef void (*ResettableExitPhase)(Object *obj);
+typedef uint32_t (*ResettableGetCount)(Object *obj);
+typedef uint32_t (*ResettableIncrementCount)(Object *obj);
+typedef uint32_t (*ResettableDecrementCount)(Object *obj);
+typedef void (*ResettableForeachChild)(Object *obj, void (*visitor)(Object *));
+typedef struct ResettableClass {
+ InterfaceClass parent_class;
+
+ struct ResettablePhases {
+ ResettableInitPhase init;
+ ResettableHoldPhase hold;
+ ResettableExitPhase exit;
+ } phases;
+
+ ResettableGetCount get_count;
+ ResettableIncrementCount increment_count;
+ ResettableDecrementCount decrement_count;
+ ResettableForeachChild foreach_child;
+} ResettableClass;
+typedef struct ResettablePhases ResettablePhases;
+
+/**
+ * resettable_assert_reset:
+ * Increments the reset count and executes the init and hold phases.
+ * Each time resettable_assert_reset is called, resettable_deassert_reset
+ * must eventually be called once.
+ * It will also impact reset children.
+ *
+ * @obj object to reset, must implement Resettable interface.
+ * @cold boolean indicating the type of reset (cold or warm)
+ */
+void resettable_assert_reset(Object *obj, bool cold);
+
+/**
+ * resettable_deassert_reset:
+ * Decrements the reset count by one and executes the exit phase if it hits
+ * zero.
+ * It will also impact reset children.
+ *
+ * @obj object to reset, must implement Resettable interface.
+ */
+void resettable_deassert_reset(Object *obj);
+
+/**
+ * resettable_reset:
+ * Calling this function is equivalent to call @assert_reset then
+ * @deassert_reset.
+ */
+void resettable_reset(Object *obj, bool cold);
+
+/**
+ * resettable_class_set_parent_reset_phases:
+ *
+ * Save @rc current reset phases into @parent_phases and override @rc phases
+ * by the given new methods (@init, @hold and @exit).
+ * Each phase is overriden only if the new one is not NULL allowing to
+ * override a subset of phases.
+ */
+void resettable_class_set_parent_reset_phases(ResettableClass *rc,
+ ResettableInitPhase init,
+ ResettableHoldPhase hold,
+ ResettableExitPhase exit,
+ ResettablePhases *parent_phases);
+
+#endif
--
2.21.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [RFC PATCH v2 02/12] add device_legacy_reset function to do the transition with device_reset
2019-06-04 16:25 [Qemu-devel] [RFC PATCH v2 00/12] Multi-phase reset Damien Hedde
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 01/12] Create Resettable QOM interface Damien Hedde
@ 2019-06-04 16:25 ` Damien Hedde
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 03/12] replace all occurences of device_reset by device_legacy_reset Damien Hedde
` (10 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Damien Hedde @ 2019-06-04 16:25 UTC (permalink / raw)
To: qemu-devel
Cc: edgar.iglesias, peter.maydell, marc.burton, alistair, qemu-arm,
Damien Hedde, marcandre.lureau, pbonzini, philmd, luc.michel
This function has device_reset behavior and will allow to change
device_reset prototype while keeping the functionality.
Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
hw/core/qdev.c | 2 +-
include/hw/qdev-core.h | 9 +++++++--
2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index f9b6efe509..90037ba70c 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -1086,7 +1086,7 @@ void device_class_set_parent_unrealize(DeviceClass *dc,
dc->unrealize = dev_unrealize;
}
-void device_reset(DeviceState *dev)
+void device_legacy_reset(DeviceState *dev)
{
DeviceClass *klass = DEVICE_GET_CLASS(dev);
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index fa55dc10ae..537dd0054d 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -406,11 +406,16 @@ char *qdev_get_own_fw_dev_path_from_handler(BusState *bus, DeviceState *dev);
void qdev_machine_init(void);
/**
- * @device_reset
+ * device_legacy_reset:
*
* Reset a single device (by calling the reset method).
*/
-void device_reset(DeviceState *dev);
+void device_legacy_reset(DeviceState *dev);
+
+static inline void device_reset(DeviceState *dev)
+{
+ device_legacy_reset(dev);
+}
void device_class_set_parent_reset(DeviceClass *dc,
DeviceReset dev_reset,
--
2.21.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [RFC PATCH v2 03/12] replace all occurences of device_reset by device_legacy_reset
2019-06-04 16:25 [Qemu-devel] [RFC PATCH v2 00/12] Multi-phase reset Damien Hedde
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 01/12] Create Resettable QOM interface Damien Hedde
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 02/12] add device_legacy_reset function to do the transition with device_reset Damien Hedde
@ 2019-06-04 16:25 ` Damien Hedde
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 04/12] make Device and Bus Resettable Damien Hedde
` (9 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Damien Hedde @ 2019-06-04 16:25 UTC (permalink / raw)
To: qemu-devel
Cc: edgar.iglesias, peter.maydell, marc.burton, alistair, qemu-arm,
Damien Hedde, marcandre.lureau, pbonzini, philmd, luc.michel
Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
hw/audio/intel-hda.c | 2 +-
hw/hyperv/hyperv.c | 2 +-
hw/i386/pc.c | 2 +-
hw/ide/microdrive.c | 8 ++++----
hw/intc/spapr_xive.c | 2 +-
hw/ppc/pnv_psi.c | 2 +-
hw/ppc/spapr_pci.c | 2 +-
hw/ppc/spapr_vio.c | 2 +-
hw/s390x/s390-pci-inst.c | 2 +-
hw/scsi/vmw_pvscsi.c | 2 +-
hw/sd/omap_mmc.c | 2 +-
hw/sd/pl181.c | 2 +-
12 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/hw/audio/intel-hda.c b/hw/audio/intel-hda.c
index 33e333cc26..c674b9c0bb 100644
--- a/hw/audio/intel-hda.c
+++ b/hw/audio/intel-hda.c
@@ -1085,7 +1085,7 @@ static void intel_hda_reset(DeviceState *dev)
QTAILQ_FOREACH(kid, &d->codecs.qbus.children, sibling) {
DeviceState *qdev = kid->child;
cdev = HDA_CODEC_DEVICE(qdev);
- device_reset(DEVICE(cdev));
+ device_legacy_reset(DEVICE(cdev));
d->state_sts |= (1 << cdev->cad);
}
intel_hda_update_irq(d);
diff --git a/hw/hyperv/hyperv.c b/hw/hyperv/hyperv.c
index 8758635227..ec57417a3d 100644
--- a/hw/hyperv/hyperv.c
+++ b/hw/hyperv/hyperv.c
@@ -139,7 +139,7 @@ void hyperv_synic_reset(CPUState *cs)
SynICState *synic = get_synic(cs);
if (synic) {
- device_reset(DEVICE(synic));
+ device_legacy_reset(DEVICE(synic));
}
}
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index edc240bcbf..c6d72c9a5d 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -2634,7 +2634,7 @@ static void pc_machine_reset(void)
cpu = X86_CPU(cs);
if (cpu->apic_state) {
- device_reset(cpu->apic_state);
+ device_legacy_reset(cpu->apic_state);
}
}
}
diff --git a/hw/ide/microdrive.c b/hw/ide/microdrive.c
index 34bb98dce8..b1350eb54f 100644
--- a/hw/ide/microdrive.c
+++ b/hw/ide/microdrive.c
@@ -171,7 +171,7 @@ static void md_attr_write(PCMCIACardState *card, uint32_t at, uint8_t value)
case 0x00: /* Configuration Option Register */
s->opt = value & 0xcf;
if (value & OPT_SRESET) {
- device_reset(DEVICE(s));
+ device_legacy_reset(DEVICE(s));
}
md_interrupt_update(s);
break;
@@ -314,7 +314,7 @@ static void md_common_write(PCMCIACardState *card, uint32_t at, uint16_t value)
case 0xe: /* Device Control */
s->ctrl = value;
if (value & CTRL_SRST) {
- device_reset(DEVICE(s));
+ device_legacy_reset(DEVICE(s));
}
md_interrupt_update(s);
break;
@@ -539,7 +539,7 @@ static int dscm1xxxx_attach(PCMCIACardState *card)
md->attr_base = pcc->cis[0x74] | (pcc->cis[0x76] << 8);
md->io_base = 0x0;
- device_reset(DEVICE(md));
+ device_legacy_reset(DEVICE(md));
md_interrupt_update(md);
return 0;
@@ -549,7 +549,7 @@ static int dscm1xxxx_detach(PCMCIACardState *card)
{
MicroDriveState *md = MICRODRIVE(card);
- device_reset(DEVICE(md));
+ device_legacy_reset(DEVICE(md));
return 0;
}
diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
index 62e0ef8fa5..bea4582f33 100644
--- a/hw/intc/spapr_xive.c
+++ b/hw/intc/spapr_xive.c
@@ -1528,7 +1528,7 @@ static target_ulong h_int_reset(PowerPCCPU *cpu,
return H_PARAMETER;
}
- device_reset(DEVICE(xive));
+ device_legacy_reset(DEVICE(xive));
if (kvm_irqchip_in_kernel()) {
Error *local_err = NULL;
diff --git a/hw/ppc/pnv_psi.c b/hw/ppc/pnv_psi.c
index 5345c8389e..d79c8c62be 100644
--- a/hw/ppc/pnv_psi.c
+++ b/hw/ppc/pnv_psi.c
@@ -702,7 +702,7 @@ static void pnv_psi_p9_mmio_write(void *opaque, hwaddr addr,
break;
case PSIHB9_INTERRUPT_CONTROL:
if (val & PSIHB9_IRQ_RESET) {
- device_reset(DEVICE(&psi9->source));
+ device_legacy_reset(DEVICE(&psi9->source));
}
psi->regs[reg] = val;
break;
diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index 97961b0128..d89290ac28 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1858,7 +1858,7 @@ static int spapr_phb_children_reset(Object *child, void *opaque)
DeviceState *dev = (DeviceState *) object_dynamic_cast(child, TYPE_DEVICE);
if (dev) {
- device_reset(dev);
+ device_legacy_reset(dev);
}
return 0;
diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c
index 583c13deda..5a0b5cc35c 100644
--- a/hw/ppc/spapr_vio.c
+++ b/hw/ppc/spapr_vio.c
@@ -306,7 +306,7 @@ int spapr_vio_send_crq(SpaprVioDevice *dev, uint8_t *crq)
static void spapr_vio_quiesce_one(SpaprVioDevice *dev)
{
if (dev->tcet) {
- device_reset(DEVICE(dev->tcet));
+ device_legacy_reset(DEVICE(dev->tcet));
}
free_crq(dev);
}
diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index be2896232d..d532597566 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -243,7 +243,7 @@ int clp_service_call(S390CPU *cpu, uint8_t r2, uintptr_t ra)
stw_p(&ressetpci->hdr.rsp, CLP_RC_SETPCIFN_FHOP);
goto out;
}
- device_reset(DEVICE(pbdev));
+ device_legacy_reset(DEVICE(pbdev));
pbdev->fh &= ~FH_MASK_ENABLE;
pbdev->state = ZPCI_FS_DISABLED;
stl_p(&ressetpci->fh, pbdev->fh);
diff --git a/hw/scsi/vmw_pvscsi.c b/hw/scsi/vmw_pvscsi.c
index 584b4be07e..7ad3d05b9b 100644
--- a/hw/scsi/vmw_pvscsi.c
+++ b/hw/scsi/vmw_pvscsi.c
@@ -834,7 +834,7 @@ pvscsi_on_cmd_reset_device(PVSCSIState *s)
if (sdev != NULL) {
s->resetting++;
- device_reset(&sdev->qdev);
+ device_legacy_reset(&sdev->qdev);
s->resetting--;
return PVSCSI_COMMAND_PROCESSING_SUCCEEDED;
}
diff --git a/hw/sd/omap_mmc.c b/hw/sd/omap_mmc.c
index d0c98ca021..24a1edc149 100644
--- a/hw/sd/omap_mmc.c
+++ b/hw/sd/omap_mmc.c
@@ -317,7 +317,7 @@ void omap_mmc_reset(struct omap_mmc_s *host)
* into any bus, and we must reset it manually. When omap_mmc is
* QOMified this must move into the QOM reset function.
*/
- device_reset(DEVICE(host->card));
+ device_legacy_reset(DEVICE(host->card));
}
static uint64_t omap_mmc_read(void *opaque, hwaddr offset,
diff --git a/hw/sd/pl181.c b/hw/sd/pl181.c
index 3ad7e925c5..03f859ec33 100644
--- a/hw/sd/pl181.c
+++ b/hw/sd/pl181.c
@@ -479,7 +479,7 @@ static void pl181_reset(DeviceState *d)
/* Since we're still using the legacy SD API the card is not plugged
* into any bus, and we must reset it manually.
*/
- device_reset(DEVICE(s->card));
+ device_legacy_reset(DEVICE(s->card));
}
static void pl181_init(Object *obj)
--
2.21.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [RFC PATCH v2 04/12] make Device and Bus Resettable
2019-06-04 16:25 [Qemu-devel] [RFC PATCH v2 00/12] Multi-phase reset Damien Hedde
` (2 preceding siblings ...)
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 03/12] replace all occurences of device_reset by device_legacy_reset Damien Hedde
@ 2019-06-04 16:25 ` Damien Hedde
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 05/12] Add function to control reset with gpio inputs Damien Hedde
` (8 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Damien Hedde @ 2019-06-04 16:25 UTC (permalink / raw)
To: qemu-devel
Cc: edgar.iglesias, peter.maydell, marc.burton, alistair, qemu-arm,
Damien Hedde, marcandre.lureau, pbonzini, philmd, luc.michel
This add Resettable interface implementation for both Bus and Device.
The init phase default implementation is to call the legacy reset
handler.
qdev/bus_reset_all implementations are modified to use the new
device_reset / bus_reset function.
Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
hw/core/bus.c | 60 ++++++++++++++++++++++++++++++++++++++
hw/core/qdev.c | 65 +++++++++++++++++++++++++++++++++++-------
include/hw/qdev-core.h | 63 ++++++++++++++++++++++++++++++++++------
tests/Makefile.include | 1 +
4 files changed, 170 insertions(+), 19 deletions(-)
diff --git a/hw/core/bus.c b/hw/core/bus.c
index e09843f6ab..c731e5cac7 100644
--- a/hw/core/bus.c
+++ b/hw/core/bus.c
@@ -21,6 +21,7 @@
#include "qemu-common.h"
#include "hw/qdev.h"
#include "qapi/error.h"
+#include "hw/resettable.h"
void qbus_set_hotplug_handler(BusState *bus, Object *handler, Error **errp)
{
@@ -67,6 +68,54 @@ int qbus_walk_children(BusState *bus,
return 0;
}
+void bus_reset(BusState *bus, bool cold)
+{
+ resettable_reset(OBJECT(bus), cold);
+}
+
+bool bus_is_resetting(BusState *bus)
+{
+ return (bus->resetting != 0);
+}
+
+static uint32_t bus_get_reset_count(Object *obj)
+{
+ BusState *bus = BUS(obj);
+ return bus->resetting;
+}
+
+static uint32_t bus_increment_reset_count(Object *obj)
+{
+ BusState *bus = BUS(obj);
+ return ++bus->resetting;
+}
+
+static uint32_t bus_decrement_reset_count(Object *obj)
+{
+ BusState *bus = BUS(obj);
+ return --bus->resetting;
+}
+
+static void bus_foreach_reset_child(Object *obj, void (*func)(Object *))
+{
+ BusState *bus = BUS(obj);
+ BusChild *kid;
+
+ QTAILQ_FOREACH(kid, &bus->children, sibling) {
+ func(OBJECT(kid->child));
+ }
+}
+
+static void bus_reset_init_phase(Object *obj, bool cold)
+{
+ BusState *bus = BUS(obj);
+ BusClass *bc = BUS_GET_CLASS(obj);
+
+ if (bc->reset) {
+ bc->reset(bus);
+ }
+}
+
static void qbus_realize(BusState *bus, DeviceState *parent, const char *name)
{
const char *typename = object_get_typename(OBJECT(bus));
@@ -204,9 +253,16 @@ static char *default_bus_get_fw_dev_path(DeviceState *dev)
static void bus_class_init(ObjectClass *class, void *data)
{
BusClass *bc = BUS_CLASS(class);
+ ResettableClass *rc = RESETTABLE_CLASS(class);
class->unparent = bus_unparent;
bc->get_fw_dev_path = default_bus_get_fw_dev_path;
+
+ rc->phases.init = bus_reset_init_phase;
+ rc->get_count = bus_get_reset_count;
+ rc->increment_count = bus_increment_reset_count;
+ rc->decrement_count = bus_decrement_reset_count;
+ rc->foreach_child = bus_foreach_reset_child;
}
static void qbus_finalize(Object *obj)
@@ -225,6 +281,10 @@ static const TypeInfo bus_info = {
.instance_init = qbus_initfn,
.instance_finalize = qbus_finalize,
.class_init = bus_class_init,
+ .interfaces = (InterfaceInfo[]) {
+ { TYPE_RESETTABLE },
+ { }
+ },
};
static void bus_register_types(void)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 90037ba70c..8c3911c2bd 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -254,25 +254,56 @@ HotplugHandler *qdev_get_hotplug_handler(DeviceState *dev)
return hotplug_ctrl;
}
-static int qdev_reset_one(DeviceState *dev, void *opaque)
+void device_reset(DeviceState *dev, bool cold)
{
- device_reset(dev);
+ resettable_reset(OBJECT(dev), cold);
+}
- return 0;
+bool device_is_resetting(DeviceState *dev)
+{
+ return (dev->resetting != 0);
+}
+
+static uint32_t device_get_reset_count(Object *obj)
+{
+ DeviceState *dev = DEVICE(obj);
+ return dev->resetting;
}
-static int qbus_reset_one(BusState *bus, void *opaque)
+static uint32_t device_increment_reset_count(Object *obj)
{
- BusClass *bc = BUS_GET_CLASS(bus);
- if (bc->reset) {
- bc->reset(bus);
+ DeviceState *dev = DEVICE(obj);
+ return ++dev->resetting;
+}
+
+static uint32_t device_decrement_reset_count(Object *obj)
+{
+ DeviceState *dev = DEVICE(obj);
+ return --dev->resetting;
+}
+
+static void device_foreach_reset_child(Object *obj, void (*func)(Object *))
+{
+ DeviceState *dev = DEVICE(obj);
+ BusState *bus;
+ QLIST_FOREACH(bus, &dev->child_bus, sibling) {
+ func(OBJECT(bus));
+ }
+}
+
+static void device_reset_init_phase(Object *obj, bool cold)
+{
+ DeviceState *dev = DEVICE(obj);
+ DeviceClass *dc = DEVICE_GET_CLASS(dev);
+
+ if (dc->reset) {
+ dc->reset(dev);
}
- return 0;
}
void qdev_reset_all(DeviceState *dev)
{
- qdev_walk_children(dev, NULL, NULL, qdev_reset_one, qbus_reset_one, NULL);
+ device_reset(dev, false);
}
void qdev_reset_all_fn(void *opaque)
@@ -282,7 +313,7 @@ void qdev_reset_all_fn(void *opaque)
void qbus_reset_all(BusState *bus)
{
- qbus_walk_children(bus, NULL, NULL, qdev_reset_one, qbus_reset_one, NULL);
+ bus_reset(bus, false);
}
void qbus_reset_all_fn(void *opaque)
@@ -864,7 +895,7 @@ static void device_set_realized(Object *obj, bool value, Error **errp)
}
}
if (dev->hotplugged) {
- device_reset(dev);
+ device_reset(dev, true);
}
dev->pending_deleted_event = false;
@@ -954,6 +985,7 @@ static void device_initfn(Object *obj)
dev->instance_id_alias = -1;
dev->realized = false;
+ dev->resetting = 0;
object_property_add_bool(obj, "realized",
device_get_realized, device_set_realized, NULL);
@@ -1049,6 +1081,7 @@ static void device_unparent(Object *obj)
static void device_class_init(ObjectClass *class, void *data)
{
DeviceClass *dc = DEVICE_CLASS(class);
+ ResettableClass *rc = RESETTABLE_CLASS(class);
class->unparent = device_unparent;
@@ -1060,6 +1093,12 @@ static void device_class_init(ObjectClass *class, void *data)
*/
dc->hotpluggable = true;
dc->user_creatable = true;
+
+ rc->phases.init = device_reset_init_phase;
+ rc->get_count = device_get_reset_count;
+ rc->increment_count = device_increment_reset_count;
+ rc->decrement_count = device_decrement_reset_count;
+ rc->foreach_child = device_foreach_reset_child;
}
void device_class_set_parent_reset(DeviceClass *dc,
@@ -1117,6 +1156,10 @@ static const TypeInfo device_type_info = {
.class_init = device_class_init,
.abstract = true,
.class_size = sizeof(DeviceClass),
+ .interfaces = (InterfaceInfo[]) {
+ { TYPE_RESETTABLE },
+ { }
+ },
};
static void qdev_register_types(void)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 537dd0054d..658a419350 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -6,6 +6,7 @@
#include "qom/object.h"
#include "hw/irq.h"
#include "hw/hotplug.h"
+#include "hw/resettable.h"
enum {
DEV_NVECTORS_UNSPECIFIED = -1,
@@ -107,6 +108,11 @@ typedef struct DeviceClass {
bool hotpluggable;
/* callbacks */
+ /*
+ * Reset method here is deprecated and replaced by methods in the
+ * resettable class interface to implement a multi-phase reset.
+ * TODO: remove once every reset callback is unused
+ */
DeviceReset reset;
DeviceRealize realize;
DeviceUnrealize unrealize;
@@ -131,6 +137,8 @@ struct NamedGPIOList {
/**
* DeviceState:
* @realized: Indicates whether the device has been fully constructed.
+ * @resetting: Indicates whether the device is under reset. Also
+ * used to count how many times reset has been initiated on the device.
*
* This structure should not be accessed directly. We declare it here
* so that it can be embedded in individual device state structures.
@@ -152,6 +160,7 @@ struct DeviceState {
int num_child_bus;
int instance_id_alias;
int alias_required_for_version;
+ uint32_t resetting;
};
struct DeviceListener {
@@ -198,6 +207,8 @@ typedef struct BusChild {
/**
* BusState:
* @hotplug_handler: link to a hotplug handler associated with bus.
+ * @resetting: Indicates whether the device is under reset. Also
+ * used to count how many times reset has been initiated on the device.
*/
struct BusState {
Object obj;
@@ -209,6 +220,7 @@ struct BusState {
int num_children;
QTAILQ_HEAD(, BusChild) children;
QLIST_ENTRY(BusState) sibling;
+ uint32_t resetting;
};
/**
@@ -375,11 +387,36 @@ int qdev_walk_children(DeviceState *dev,
qdev_walkerfn *post_devfn, qbus_walkerfn *post_busfn,
void *opaque);
-void qdev_reset_all(DeviceState *dev);
-void qdev_reset_all_fn(void *opaque);
+/**
+ * device_reset:
+ * Resets the device @dev, @cold tell whether to do a cold or warm reset.
+ * Uses the ressetable interface.
+ * Base behavior is to reset the device and its qdev/qbus subtree.
+ */
+void device_reset(DeviceState *dev, bool cold);
/**
- * @qbus_reset_all:
+ * bus_reset:
+ * Resets the bus @bus, @cold tell whether to do a cold or warm reset.
+ * Uses the ressetable interface.
+ * Base behavior is to reset the bus and its qdev/qbus subtree.
+ */
+void bus_reset(BusState *bus, bool cold);
+
+/**
+ * device_is_resetting:
+ * Tell whether the device @dev is currently under reset.
+ */
+bool device_is_resetting(DeviceState *dev);
+
+/**
+ * bus_is_resetting:
+ * Tell whether the bus @bus is currently under reset.
+ */
+bool bus_is_resetting(BusState *bus);
+
+/**
+ * qbus/qdev_reset_all:
* @bus: Bus to be reset.
*
* Reset @bus and perform a bus-level ("hard") reset of all devices connected
@@ -387,7 +424,13 @@ void qdev_reset_all_fn(void *opaque);
* hard reset means that qbus_reset_all will reset all state of the device.
* For PCI devices, for example, this will include the base address registers
* or configuration space.
+ *
+ * Theses functions are deprecated, please use device/bus_reset or
+ * resettable_reset_* instead
+ * TODO: remove them when all occurence are removed
*/
+void qdev_reset_all(DeviceState *dev);
+void qdev_reset_all_fn(void *opaque);
void qbus_reset_all(BusState *bus);
void qbus_reset_all_fn(void *opaque);
@@ -409,17 +452,21 @@ void qdev_machine_init(void);
* device_legacy_reset:
*
* Reset a single device (by calling the reset method).
+ *
+ * This function is deprecated, please use device_reset() instead.
+ * TODO: remove the function when all occurences are removed.
*/
void device_legacy_reset(DeviceState *dev);
-static inline void device_reset(DeviceState *dev)
-{
- device_legacy_reset(dev);
-}
-
+/**
+ * device_class_set_parent_reset:
+ * TODO: remove the function when DeviceClass's reset method
+ * is not used anymore.
+ */
void device_class_set_parent_reset(DeviceClass *dc,
DeviceReset dev_reset,
DeviceReset *parent_reset);
+
void device_class_set_parent_realize(DeviceClass *dc,
DeviceRealize dev_realize,
DeviceRealize *parent_realize);
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 46a36c2c95..5b25905907 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -547,6 +547,7 @@ tests/fp/%:
tests/test-qdev-global-props$(EXESUF): tests/test-qdev-global-props.o \
hw/core/qdev.o hw/core/qdev-properties.o hw/core/hotplug.o\
+ hw/core/resettable.o \
hw/core/bus.o \
hw/core/irq.o \
hw/core/fw-path-provider.o \
--
2.21.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [RFC PATCH v2 05/12] Add function to control reset with gpio inputs
2019-06-04 16:25 [Qemu-devel] [RFC PATCH v2 00/12] Multi-phase reset Damien Hedde
` (3 preceding siblings ...)
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 04/12] make Device and Bus Resettable Damien Hedde
@ 2019-06-04 16:25 ` Damien Hedde
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 06/12] add vmstate description for device reset state Damien Hedde
` (7 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Damien Hedde @ 2019-06-04 16:25 UTC (permalink / raw)
To: qemu-devel
Cc: edgar.iglesias, peter.maydell, marc.burton, alistair, qemu-arm,
Damien Hedde, marcandre.lureau, pbonzini, philmd, luc.michel
It adds the possibility to add 2 gpios to control the warm and cold reset.
With theses ios, the reset can be maintained during some time.
Each io is associated with a state to detect level changes.
The cold reset io function is named power_gate as it is really the
meaning of this io.
Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
hw/core/qdev.c | 57 ++++++++++++++++++++++++++++++++++++++++++
include/hw/qdev-core.h | 44 ++++++++++++++++++++++++++++++++
2 files changed, 101 insertions(+)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 8c3911c2bd..89405398ae 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -429,6 +429,61 @@ void qdev_init_gpio_in(DeviceState *dev, qemu_irq_handler handler, int n)
qdev_init_gpio_in_named(dev, handler, NULL, n);
}
+static void device_reset_handler(DeviceState *dev, bool cold, bool level)
+{
+ DeviceResetInputState *dris;
+
+ dris = cold ? &dev->cold_reset_input : &dev->warm_reset_input;
+
+ if (level == dris->state) {
+ /* io state has not changed */
+ return;
+ }
+
+ dris->state = level;
+ switch (dris->type) {
+ case DEVICE_ACTIVE_LOW:
+ level = !level;
+ case DEVICE_ACTIVE_HIGH:
+ if (level) {
+ resettable_assert_reset(OBJECT(dev), cold);
+ } else {
+ resettable_deassert_reset(OBJECT(dev));
+ }
+ break;
+ }
+}
+
+static void device_cold_reset_handler(void *opaque, int n, int level)
+{
+ device_reset_handler((DeviceState *) opaque, true, level);
+}
+
+static void device_warm_reset_handler(void *opaque, int n, int level)
+{
+ device_reset_handler((DeviceState *) opaque, false, level);
+}
+
+void qdev_init_reset_gpio_in_named(DeviceState *dev, const char *name,
+ bool cold, DeviceActiveType type)
+{
+ qemu_irq_handler handler;
+
+ if (cold) {
+ assert(!dev->cold_reset_input.exists);
+ dev->cold_reset_input.exists = true;
+ dev->cold_reset_input.type = type;
+ handler = device_cold_reset_handler;
+ } else {
+ assert(!dev->warm_reset_input.exists);
+ dev->warm_reset_input.exists = true;
+ dev->warm_reset_input.type = type;
+ handler = device_warm_reset_handler;
+ }
+
+ qdev_init_gpio_in_named(dev, handler, name, 1);
+}
+
void qdev_init_gpio_out_named(DeviceState *dev, qemu_irq *pins,
const char *name, int n)
{
@@ -986,6 +1041,8 @@ static void device_initfn(Object *obj)
dev->instance_id_alias = -1;
dev->realized = false;
dev->resetting = 0;
+ dev->cold_reset_input.exists = false;
+ dev->warm_reset_input.exists = false;
object_property_add_bool(obj, "realized",
device_get_realized, device_set_realized, NULL);
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 658a419350..693f79b385 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -134,6 +134,17 @@ struct NamedGPIOList {
QLIST_ENTRY(NamedGPIOList) node;
};
+typedef enum DeviceActiveType {
+ DEVICE_ACTIVE_LOW,
+ DEVICE_ACTIVE_HIGH,
+} DeviceActiveType;
+
+typedef struct DeviceResetInputState {
+ bool exists;
+ DeviceActiveType type;
+ bool state;
+} DeviceResetInputState;
+
/**
* DeviceState:
* @realized: Indicates whether the device has been fully constructed.
@@ -161,6 +172,8 @@ struct DeviceState {
int instance_id_alias;
int alias_required_for_version;
uint32_t resetting;
+ DeviceResetInputState cold_reset_input;
+ DeviceResetInputState warm_reset_input;
};
struct DeviceListener {
@@ -362,6 +375,37 @@ static inline void qdev_init_gpio_in_named(DeviceState *dev,
void qdev_pass_gpios(DeviceState *dev, DeviceState *container,
const char *name);
+/**
+ * qdev_init_reset_gpio_in_named:
+ * Create a gpio controlling the warm or cold reset of the device.
+ * @cold specify whether it triggers cold or warm reset
+ * @type what kind of reset io it is
+ */
+void qdev_init_reset_gpio_in_named(DeviceState *dev, const char *name,
+ bool cold, DeviceActiveType type);
+
+/**
+ * qdev_init_warm_reset_gpio:
+ * Create a reset input to control the device warm reset.
+ */
+static inline void qdev_init_warm_reset_gpio(DeviceState *dev,
+ const char *name,
+ DeviceActiveType type)
+{
+ qdev_init_reset_gpio_in_named(dev, name, false, type);
+}
+
+/**
+ * qdev_init_power_gate_gpio:
+ * Create a power gate input to control the device cold reset.
+ */
+static inline void qdev_init_power_gate_gpio(DeviceState *dev,
+ const char *name,
+ DeviceActiveType type)
+{
+ qdev_init_reset_gpio_in_named(dev, name, true, type);
+}
+
BusState *qdev_get_parent_bus(DeviceState *dev);
/*** BUS API. ***/
--
2.21.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [RFC PATCH v2 06/12] add vmstate description for device reset state
2019-06-04 16:25 [Qemu-devel] [RFC PATCH v2 00/12] Multi-phase reset Damien Hedde
` (4 preceding siblings ...)
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 05/12] Add function to control reset with gpio inputs Damien Hedde
@ 2019-06-04 16:25 ` Damien Hedde
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 07/12] add doc about Resettable interface Damien Hedde
` (6 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Damien Hedde @ 2019-06-04 16:25 UTC (permalink / raw)
To: qemu-devel
Cc: edgar.iglesias, peter.maydell, marc.burton, alistair, qemu-arm,
Damien Hedde, marcandre.lureau, pbonzini, philmd, luc.michel
The `device_vmstate_reset` can be added by device specialization, as
vmsd subsection, to migrate the reset related device state part.
It contains the resetting counter and the reset inputs current status.
Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
hw/core/Makefile.objs | 1 +
hw/core/qdev-vmstate.c | 34 ++++++++++++++++++++++++++++++++++
include/hw/qdev-core.h | 6 ++++++
3 files changed, 41 insertions(+)
create mode 100644 hw/core/qdev-vmstate.c
diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
index 97007454a8..b3b4990005 100644
--- a/hw/core/Makefile.objs
+++ b/hw/core/Makefile.objs
@@ -4,6 +4,7 @@ common-obj-y += bus.o reset.o
common-obj-y += resettable.o
common-obj-$(CONFIG_SOFTMMU) += qdev-fw.o
common-obj-$(CONFIG_SOFTMMU) += fw-path-provider.o
+common-obj-$(CONFIG_SOFTMMU) += qdev-vmstate.o
# irq.o needed for qdev GPIO handling:
common-obj-y += irq.o
common-obj-y += hotplug.o
diff --git a/hw/core/qdev-vmstate.c b/hw/core/qdev-vmstate.c
new file mode 100644
index 0000000000..5322b70b0e
--- /dev/null
+++ b/hw/core/qdev-vmstate.c
@@ -0,0 +1,34 @@
+/*
+ * Device vmstate
+ *
+ * Copyright (c) 2019 GreenSocs
+ *
+ * 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.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/qdev.h"
+#include "migration/vmstate.h"
+
+static bool device_vmstate_reset_needed(void *opaque)
+{
+ DeviceState *dev = (DeviceState *) opaque;
+ return dev->resetting;
+}
+
+const struct VMStateDescription device_vmstate_reset = {
+ .name = "device_reset",
+ .version_id = 0,
+ .minimum_version_id = 0,
+ .needed = device_vmstate_reset_needed,
+ .fields = (VMStateField[]) {
+ VMSTATE_UINT32(resetting, DeviceState),
+ VMSTATE_BOOL(cold_reset_input.state, DeviceState),
+ VMSTATE_BOOL(warm_reset_input.state, DeviceState),
+ VMSTATE_END_OF_LIST()
+ }
+};
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 693f79b385..596a5bbead 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -546,4 +546,10 @@ static inline bool qbus_is_hotpluggable(BusState *bus)
void device_listener_register(DeviceListener *listener);
void device_listener_unregister(DeviceListener *listener);
+/**
+ * device_vmstate_reset:
+ * vmstate entry for reset related state
+ */
+extern const struct VMStateDescription device_vmstate_reset;
+
#endif
--
2.21.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [RFC PATCH v2 07/12] add doc about Resettable interface
2019-06-04 16:25 [Qemu-devel] [RFC PATCH v2 00/12] Multi-phase reset Damien Hedde
` (5 preceding siblings ...)
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 06/12] add vmstate description for device reset state Damien Hedde
@ 2019-06-04 16:25 ` Damien Hedde
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 08/12] hw/misc/zynq_slcr: use standard register definition Damien Hedde
` (5 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Damien Hedde @ 2019-06-04 16:25 UTC (permalink / raw)
To: qemu-devel
Cc: edgar.iglesias, peter.maydell, marc.burton, alistair, qemu-arm,
Damien Hedde, marcandre.lureau, pbonzini, philmd, luc.michel
Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
docs/devel/reset.txt | 151 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 151 insertions(+)
create mode 100644 docs/devel/reset.txt
diff --git a/docs/devel/reset.txt b/docs/devel/reset.txt
new file mode 100644
index 0000000000..32c211d8ca
--- /dev/null
+++ b/docs/devel/reset.txt
@@ -0,0 +1,151 @@
+
+=====
+Reset
+=====
+
+The reset of qemu objects is handled using the Resettable interface declared
+in *include/hw/resettable.h*.
+As of now DeviceClass and BusClass implement this interface.
+
+
+Triggering reset
+----------------
+
+The function *resettable_reset* must be used to trigger a reset on a given
+object.
+void resettable_reset(Object *obj, bool cold)
+
+The parameter *obj* must implement the Resettable interface.
+The parameter *cold* is a boolean specifying whether to do a cold or warm
+reset
+
+For Devices and Buses there is also the corresponding helpers:
+void device_reset(Device *dev, bool cold)
+void bus_reset(Device *dev, bool cold)
+
+If one wants to put an object into a reset state. There is the
+*resettable_assert_reset* function.
+void resettable_assert_reset(Object *obj, bool cold)
+
+One must eventually call the function *resettable_deassert_reset* to end the
+reset state:
+void resettable_deassert_reset(Object *obj, bool cold)
+
+Calling *resettable_assert_reset* then *resettable_deassert_reset* is the
+same as calling *resettable_reset*.
+
+It is possible to interleave multiple calls to
+ - resettable_reset,
+ - resettable_assert_reset, and
+ - resettable_deassert_reset.
+The only constraint is that *resettable_deassert_reset* must be called once
+per *resettable_assert_reset* call so that the object leaves the reset state.
+
+Therefore there may be several reset sources/controllers of a given object.
+The interface handle everything and the controllers do not need to know
+anything about each others. The object will leave reset state only when all
+controllers released their reset.
+
+All theses functions must called while holding the iothread lock.
+
+
+Implementing reset for a Resettable object : Multi-phase reset
+--------------------------------------------------------------
+
+The Resettable uses a multi-phase mechanism to handle some ordering constraints
+when resetting multiple object at the same time. For a given object the reset
+procedure is split into three different phases executed in order:
+ 1 INIT: This phase should set/reset the state of the Resettable it has when is
+ in reset state. Side-effects to others object is forbidden (such as
+ setting IO level).
+ 2 HOLD: This phase corresponds to the external side-effects due to staying into
+ the reset state.
+ 3 EXIT: This phase corresponds to leaving the reset state. It have both
+ local and external effects.
+
+*resettable_assert_reset* does the INIT and HOLD phases. While
+*resettable_deassert_reset* does the EXIT phase.
+
+When resetting multiple object at the same time. The interface executes the
+given phase of the objects before going to the next phase. This guarantee that
+all INIT phases are done before any HOLD phase and so on.
+
+There is three methods in the interface so must be implemented in an object.
+The methods corresponds to the three phases:
+```
+typedef void (*ResettableInitPhase)(Object *obj, bool cold);
+typedef void (*ResettableHoldPhase)(Object *obj);
+typedef void (*ResettableExitPhase)(Object *obj);
+typedef struct ResettableClass {
+ InterfaceClass parent_class;
+
+ struct ResettablePhases {
+ ResettableInitPhase init;
+ ResettableHoldPhase hold;
+ ResettableExitPhase exit;
+ } phases;
+ [...]
+} ResettableClass;
+```
+
+Note that only the init method takes the bool parameter, if the warm/cold
+information is needed by other methods it should be stored somewhere in the
+object state.
+
+Theses methods should be updated when specializing an object. For this the
+helper function *resettable_class_set_parent_reset_phases* can be used to
+backup parent methods while changing the specialized ones.
+void resettable_class_set_parent_reset_phases(ResettableClass *rc,
+ ResettableInitPhase init,
+ ResettableHoldPhase hold,
+ ResettableExitPhase exit,
+ ResettablePhases *parent_phases);
+
+
+Implementing the base Resettable behavior : Re-entrance and Hierarchy
+---------------------------------------------------------------------
+
+There is four others methods in the interface to handle the base mechanics
+of the Resettable interface. The methods should be implemented in object
+base class. DeviceClass and BusClass implement them.
+
+```
+typedef uint32_t (*ResettableGetCount)(Object *obj);
+typedef uint32_t (*ResettableIncrementCount)(Object *obj);
+typedef uint32_t (*ResettableDecrementCount)(Object *obj);
+typedef void (*ResettableForeachChild)(Object *obj, void (*visitor)(Object *));
+typedef struct ResettableClass {
+ InterfaceClass parent_class;
+
+ [...]
+
+ ResettableGetCount get_count;
+ ResettableIncrementCount increment_count;
+ ResettableDecrementCount decrement_count;
+ ResettableForeachChild foreach_child;
+} ResettableClass;
+```
+
+As stated above, several reset procedures can be concurrent on an object.
+This is handled with the three methods *get_count*, *increment_count* and
+*decrement_count*. An object is in reset state if the count is non-zero.
+
+The reset hierarchy is handled using the *foreach_child* method. This method
+executes a given function on every reset "child".
+
+In DeviceClass and BusClass the base behavior is to mimic the legacy qdev
+reset. Reset hierarchy follows the qdev/qbus tree.
+
+Reset control through GPIO
+--------------------------
+
+For devices, two reset inputs can be added: one for the cold, one the warm
+reset. This is done using the following function.
+```
+typedef enum DeviceActiveType {
+ DEVICE_ACTIVE_LOW,
+ DEVICE_ACTIVE_HIGH,
+} DeviceActiveType;
+void qdev_init_reset_gpio_in_named(DeviceState *dev, const char *name,
+ bool cold, DeviceActiveType type);
+```
--
2.21.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [RFC PATCH v2 08/12] hw/misc/zynq_slcr: use standard register definition
2019-06-04 16:25 [Qemu-devel] [RFC PATCH v2 00/12] Multi-phase reset Damien Hedde
` (6 preceding siblings ...)
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 07/12] add doc about Resettable interface Damien Hedde
@ 2019-06-04 16:25 ` Damien Hedde
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 09/12] convert cadence_uart to 3-phases reset Damien Hedde
` (4 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Damien Hedde @ 2019-06-04 16:25 UTC (permalink / raw)
To: qemu-devel
Cc: edgar.iglesias, peter.maydell, Alistair Francis, marc.burton,
alistair, qemu-arm, Damien Hedde, marcandre.lureau, pbonzini,
philmd, luc.michel
Replace the zynq_slcr registers enum and macros using the
hw/registerfields.h macros.
Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
---
hw/misc/zynq_slcr.c | 472 ++++++++++++++++++++++----------------------
1 file changed, 236 insertions(+), 236 deletions(-)
diff --git a/hw/misc/zynq_slcr.c b/hw/misc/zynq_slcr.c
index d6bdd027ef..baa13d1316 100644
--- a/hw/misc/zynq_slcr.c
+++ b/hw/misc/zynq_slcr.c
@@ -20,6 +20,7 @@
#include "hw/sysbus.h"
#include "sysemu/sysemu.h"
#include "qemu/log.h"
+#include "hw/registerfields.h"
#ifndef ZYNQ_SLCR_ERR_DEBUG
#define ZYNQ_SLCR_ERR_DEBUG 0
@@ -35,138 +36,135 @@
#define XILINX_LOCK_KEY 0x767b
#define XILINX_UNLOCK_KEY 0xdf0d
-#define R_PSS_RST_CTRL_SOFT_RST 0x1
-
-enum {
- SCL = 0x000 / 4,
- LOCK,
- UNLOCK,
- LOCKSTA,
-
- ARM_PLL_CTRL = 0x100 / 4,
- DDR_PLL_CTRL,
- IO_PLL_CTRL,
- PLL_STATUS,
- ARM_PLL_CFG,
- DDR_PLL_CFG,
- IO_PLL_CFG,
-
- ARM_CLK_CTRL = 0x120 / 4,
- DDR_CLK_CTRL,
- DCI_CLK_CTRL,
- APER_CLK_CTRL,
- USB0_CLK_CTRL,
- USB1_CLK_CTRL,
- GEM0_RCLK_CTRL,
- GEM1_RCLK_CTRL,
- GEM0_CLK_CTRL,
- GEM1_CLK_CTRL,
- SMC_CLK_CTRL,
- LQSPI_CLK_CTRL,
- SDIO_CLK_CTRL,
- UART_CLK_CTRL,
- SPI_CLK_CTRL,
- CAN_CLK_CTRL,
- CAN_MIOCLK_CTRL,
- DBG_CLK_CTRL,
- PCAP_CLK_CTRL,
- TOPSW_CLK_CTRL,
+REG32(SCL, 0x000)
+REG32(LOCK, 0x004)
+REG32(UNLOCK, 0x008)
+REG32(LOCKSTA, 0x00c)
+
+REG32(ARM_PLL_CTRL, 0x100)
+REG32(DDR_PLL_CTRL, 0x104)
+REG32(IO_PLL_CTRL, 0x108)
+REG32(PLL_STATUS, 0x10c)
+REG32(ARM_PLL_CFG, 0x110)
+REG32(DDR_PLL_CFG, 0x114)
+REG32(IO_PLL_CFG, 0x118)
+
+REG32(ARM_CLK_CTRL, 0x120)
+REG32(DDR_CLK_CTRL, 0x124)
+REG32(DCI_CLK_CTRL, 0x128)
+REG32(APER_CLK_CTRL, 0x12c)
+REG32(USB0_CLK_CTRL, 0x130)
+REG32(USB1_CLK_CTRL, 0x134)
+REG32(GEM0_RCLK_CTRL, 0x138)
+REG32(GEM1_RCLK_CTRL, 0x13c)
+REG32(GEM0_CLK_CTRL, 0x140)
+REG32(GEM1_CLK_CTRL, 0x144)
+REG32(SMC_CLK_CTRL, 0x148)
+REG32(LQSPI_CLK_CTRL, 0x14c)
+REG32(SDIO_CLK_CTRL, 0x150)
+REG32(UART_CLK_CTRL, 0x154)
+REG32(SPI_CLK_CTRL, 0x158)
+REG32(CAN_CLK_CTRL, 0x15c)
+REG32(CAN_MIOCLK_CTRL, 0x160)
+REG32(DBG_CLK_CTRL, 0x164)
+REG32(PCAP_CLK_CTRL, 0x168)
+REG32(TOPSW_CLK_CTRL, 0x16c)
#define FPGA_CTRL_REGS(n, start) \
- FPGA ## n ## _CLK_CTRL = (start) / 4, \
- FPGA ## n ## _THR_CTRL, \
- FPGA ## n ## _THR_CNT, \
- FPGA ## n ## _THR_STA,
- FPGA_CTRL_REGS(0, 0x170)
- FPGA_CTRL_REGS(1, 0x180)
- FPGA_CTRL_REGS(2, 0x190)
- FPGA_CTRL_REGS(3, 0x1a0)
-
- BANDGAP_TRIP = 0x1b8 / 4,
- PLL_PREDIVISOR = 0x1c0 / 4,
- CLK_621_TRUE,
-
- PSS_RST_CTRL = 0x200 / 4,
- DDR_RST_CTRL,
- TOPSW_RESET_CTRL,
- DMAC_RST_CTRL,
- USB_RST_CTRL,
- GEM_RST_CTRL,
- SDIO_RST_CTRL,
- SPI_RST_CTRL,
- CAN_RST_CTRL,
- I2C_RST_CTRL,
- UART_RST_CTRL,
- GPIO_RST_CTRL,
- LQSPI_RST_CTRL,
- SMC_RST_CTRL,
- OCM_RST_CTRL,
- FPGA_RST_CTRL = 0x240 / 4,
- A9_CPU_RST_CTRL,
-
- RS_AWDT_CTRL = 0x24c / 4,
- RST_REASON,
-
- REBOOT_STATUS = 0x258 / 4,
- BOOT_MODE,
-
- APU_CTRL = 0x300 / 4,
- WDT_CLK_SEL,
-
- TZ_DMA_NS = 0x440 / 4,
- TZ_DMA_IRQ_NS,
- TZ_DMA_PERIPH_NS,
-
- PSS_IDCODE = 0x530 / 4,
-
- DDR_URGENT = 0x600 / 4,
- DDR_CAL_START = 0x60c / 4,
- DDR_REF_START = 0x614 / 4,
- DDR_CMD_STA,
- DDR_URGENT_SEL,
- DDR_DFI_STATUS,
-
- MIO = 0x700 / 4,
+ REG32(FPGA ## n ## _CLK_CTRL, (start)) \
+ REG32(FPGA ## n ## _THR_CTRL, (start) + 0x4)\
+ REG32(FPGA ## n ## _THR_CNT, (start) + 0x8)\
+ REG32(FPGA ## n ## _THR_STA, (start) + 0xc)
+FPGA_CTRL_REGS(0, 0x170)
+FPGA_CTRL_REGS(1, 0x180)
+FPGA_CTRL_REGS(2, 0x190)
+FPGA_CTRL_REGS(3, 0x1a0)
+
+REG32(BANDGAP_TRIP, 0x1b8)
+REG32(PLL_PREDIVISOR, 0x1c0)
+REG32(CLK_621_TRUE, 0x1c4)
+
+REG32(PSS_RST_CTRL, 0x200)
+ FIELD(PSS_RST_CTRL, SOFT_RST, 0, 1)
+REG32(DDR_RST_CTRL, 0x204)
+REG32(TOPSW_RESET_CTRL, 0x208)
+REG32(DMAC_RST_CTRL, 0x20c)
+REG32(USB_RST_CTRL, 0x210)
+REG32(GEM_RST_CTRL, 0x214)
+REG32(SDIO_RST_CTRL, 0x218)
+REG32(SPI_RST_CTRL, 0x21c)
+REG32(CAN_RST_CTRL, 0x220)
+REG32(I2C_RST_CTRL, 0x224)
+REG32(UART_RST_CTRL, 0x228)
+REG32(GPIO_RST_CTRL, 0x22c)
+REG32(LQSPI_RST_CTRL, 0x230)
+REG32(SMC_RST_CTRL, 0x234)
+REG32(OCM_RST_CTRL, 0x238)
+REG32(FPGA_RST_CTRL, 0x240)
+REG32(A9_CPU_RST_CTRL, 0x244)
+
+REG32(RS_AWDT_CTRL, 0x24c)
+REG32(RST_REASON, 0x250)
+
+REG32(REBOOT_STATUS, 0x258)
+REG32(BOOT_MODE, 0x25c)
+
+REG32(APU_CTRL, 0x300)
+REG32(WDT_CLK_SEL, 0x304)
+
+REG32(TZ_DMA_NS, 0x440)
+REG32(TZ_DMA_IRQ_NS, 0x444)
+REG32(TZ_DMA_PERIPH_NS, 0x448)
+
+REG32(PSS_IDCODE, 0x530)
+
+REG32(DDR_URGENT, 0x600)
+REG32(DDR_CAL_START, 0x60c)
+REG32(DDR_REF_START, 0x614)
+REG32(DDR_CMD_STA, 0x618)
+REG32(DDR_URGENT_SEL, 0x61c)
+REG32(DDR_DFI_STATUS, 0x620)
+
+REG32(MIO, 0x700)
#define MIO_LENGTH 54
- MIO_LOOPBACK = 0x804 / 4,
- MIO_MST_TRI0,
- MIO_MST_TRI1,
+REG32(MIO_LOOPBACK, 0x804)
+REG32(MIO_MST_TRI0, 0x808)
+REG32(MIO_MST_TRI1, 0x80c)
- SD0_WP_CD_SEL = 0x830 / 4,
- SD1_WP_CD_SEL,
+REG32(SD0_WP_CD_SEL, 0x830)
+REG32(SD1_WP_CD_SEL, 0x834)
- LVL_SHFTR_EN = 0x900 / 4,
- OCM_CFG = 0x910 / 4,
+REG32(LVL_SHFTR_EN, 0x900)
+REG32(OCM_CFG, 0x910)
- CPU_RAM = 0xa00 / 4,
+REG32(CPU_RAM, 0xa00)
- IOU = 0xa30 / 4,
+REG32(IOU, 0xa30)
- DMAC_RAM = 0xa50 / 4,
+REG32(DMAC_RAM, 0xa50)
- AFI0 = 0xa60 / 4,
- AFI1 = AFI0 + 3,
- AFI2 = AFI1 + 3,
- AFI3 = AFI2 + 3,
+REG32(AFI0, 0xa60)
+REG32(AFI1, 0xa6c)
+REG32(AFI2, 0xa78)
+REG32(AFI3, 0xa84)
#define AFI_LENGTH 3
- OCM = 0xa90 / 4,
+REG32(OCM, 0xa90)
- DEVCI_RAM = 0xaa0 / 4,
+REG32(DEVCI_RAM, 0xaa0)
- CSG_RAM = 0xab0 / 4,
+REG32(CSG_RAM, 0xab0)
- GPIOB_CTRL = 0xb00 / 4,
- GPIOB_CFG_CMOS18,
- GPIOB_CFG_CMOS25,
- GPIOB_CFG_CMOS33,
- GPIOB_CFG_HSTL = 0xb14 / 4,
- GPIOB_DRVR_BIAS_CTRL,
+REG32(GPIOB_CTRL, 0xb00)
+REG32(GPIOB_CFG_CMOS18, 0xb04)
+REG32(GPIOB_CFG_CMOS25, 0xb08)
+REG32(GPIOB_CFG_CMOS33, 0xb0c)
+REG32(GPIOB_CFG_HSTL, 0xb14)
+REG32(GPIOB_DRVR_BIAS_CTRL, 0xb18)
- DDRIOB = 0xb40 / 4,
+REG32(DDRIOB, 0xb40)
#define DDRIOB_LENGTH 14
-};
#define ZYNQ_SLCR_MMIO_SIZE 0x1000
#define ZYNQ_SLCR_NUM_REGS (ZYNQ_SLCR_MMIO_SIZE / 4)
@@ -189,150 +187,152 @@ static void zynq_slcr_reset(DeviceState *d)
DB_PRINT("RESET\n");
- s->regs[LOCKSTA] = 1;
+ s->regs[R_LOCKSTA] = 1;
/* 0x100 - 0x11C */
- s->regs[ARM_PLL_CTRL] = 0x0001A008;
- s->regs[DDR_PLL_CTRL] = 0x0001A008;
- s->regs[IO_PLL_CTRL] = 0x0001A008;
- s->regs[PLL_STATUS] = 0x0000003F;
- s->regs[ARM_PLL_CFG] = 0x00014000;
- s->regs[DDR_PLL_CFG] = 0x00014000;
- s->regs[IO_PLL_CFG] = 0x00014000;
+ s->regs[R_ARM_PLL_CTRL] = 0x0001A008;
+ s->regs[R_DDR_PLL_CTRL] = 0x0001A008;
+ s->regs[R_IO_PLL_CTRL] = 0x0001A008;
+ s->regs[R_PLL_STATUS] = 0x0000003F;
+ s->regs[R_ARM_PLL_CFG] = 0x00014000;
+ s->regs[R_DDR_PLL_CFG] = 0x00014000;
+ s->regs[R_IO_PLL_CFG] = 0x00014000;
/* 0x120 - 0x16C */
- s->regs[ARM_CLK_CTRL] = 0x1F000400;
- s->regs[DDR_CLK_CTRL] = 0x18400003;
- s->regs[DCI_CLK_CTRL] = 0x01E03201;
- s->regs[APER_CLK_CTRL] = 0x01FFCCCD;
- s->regs[USB0_CLK_CTRL] = s->regs[USB1_CLK_CTRL] = 0x00101941;
- s->regs[GEM0_RCLK_CTRL] = s->regs[GEM1_RCLK_CTRL] = 0x00000001;
- s->regs[GEM0_CLK_CTRL] = s->regs[GEM1_CLK_CTRL] = 0x00003C01;
- s->regs[SMC_CLK_CTRL] = 0x00003C01;
- s->regs[LQSPI_CLK_CTRL] = 0x00002821;
- s->regs[SDIO_CLK_CTRL] = 0x00001E03;
- s->regs[UART_CLK_CTRL] = 0x00003F03;
- s->regs[SPI_CLK_CTRL] = 0x00003F03;
- s->regs[CAN_CLK_CTRL] = 0x00501903;
- s->regs[DBG_CLK_CTRL] = 0x00000F03;
- s->regs[PCAP_CLK_CTRL] = 0x00000F01;
+ s->regs[R_ARM_CLK_CTRL] = 0x1F000400;
+ s->regs[R_DDR_CLK_CTRL] = 0x18400003;
+ s->regs[R_DCI_CLK_CTRL] = 0x01E03201;
+ s->regs[R_APER_CLK_CTRL] = 0x01FFCCCD;
+ s->regs[R_USB0_CLK_CTRL] = s->regs[R_USB1_CLK_CTRL] = 0x00101941;
+ s->regs[R_GEM0_RCLK_CTRL] = s->regs[R_GEM1_RCLK_CTRL] = 0x00000001;
+ s->regs[R_GEM0_CLK_CTRL] = s->regs[R_GEM1_CLK_CTRL] = 0x00003C01;
+ s->regs[R_SMC_CLK_CTRL] = 0x00003C01;
+ s->regs[R_LQSPI_CLK_CTRL] = 0x00002821;
+ s->regs[R_SDIO_CLK_CTRL] = 0x00001E03;
+ s->regs[R_UART_CLK_CTRL] = 0x00003F03;
+ s->regs[R_SPI_CLK_CTRL] = 0x00003F03;
+ s->regs[R_CAN_CLK_CTRL] = 0x00501903;
+ s->regs[R_DBG_CLK_CTRL] = 0x00000F03;
+ s->regs[R_PCAP_CLK_CTRL] = 0x00000F01;
/* 0x170 - 0x1AC */
- s->regs[FPGA0_CLK_CTRL] = s->regs[FPGA1_CLK_CTRL] = s->regs[FPGA2_CLK_CTRL]
- = s->regs[FPGA3_CLK_CTRL] = 0x00101800;
- s->regs[FPGA0_THR_STA] = s->regs[FPGA1_THR_STA] = s->regs[FPGA2_THR_STA]
- = s->regs[FPGA3_THR_STA] = 0x00010000;
+ s->regs[R_FPGA0_CLK_CTRL] = s->regs[R_FPGA1_CLK_CTRL]
+ = s->regs[R_FPGA2_CLK_CTRL]
+ = s->regs[R_FPGA3_CLK_CTRL] = 0x00101800;
+ s->regs[R_FPGA0_THR_STA] = s->regs[R_FPGA1_THR_STA]
+ = s->regs[R_FPGA2_THR_STA]
+ = s->regs[R_FPGA3_THR_STA] = 0x00010000;
/* 0x1B0 - 0x1D8 */
- s->regs[BANDGAP_TRIP] = 0x0000001F;
- s->regs[PLL_PREDIVISOR] = 0x00000001;
- s->regs[CLK_621_TRUE] = 0x00000001;
+ s->regs[R_BANDGAP_TRIP] = 0x0000001F;
+ s->regs[R_PLL_PREDIVISOR] = 0x00000001;
+ s->regs[R_CLK_621_TRUE] = 0x00000001;
/* 0x200 - 0x25C */
- s->regs[FPGA_RST_CTRL] = 0x01F33F0F;
- s->regs[RST_REASON] = 0x00000040;
+ s->regs[R_FPGA_RST_CTRL] = 0x01F33F0F;
+ s->regs[R_RST_REASON] = 0x00000040;
- s->regs[BOOT_MODE] = 0x00000001;
+ s->regs[R_BOOT_MODE] = 0x00000001;
/* 0x700 - 0x7D4 */
for (i = 0; i < 54; i++) {
- s->regs[MIO + i] = 0x00001601;
+ s->regs[R_MIO + i] = 0x00001601;
}
for (i = 2; i <= 8; i++) {
- s->regs[MIO + i] = 0x00000601;
+ s->regs[R_MIO + i] = 0x00000601;
}
- s->regs[MIO_MST_TRI0] = s->regs[MIO_MST_TRI1] = 0xFFFFFFFF;
+ s->regs[R_MIO_MST_TRI0] = s->regs[R_MIO_MST_TRI1] = 0xFFFFFFFF;
- s->regs[CPU_RAM + 0] = s->regs[CPU_RAM + 1] = s->regs[CPU_RAM + 3]
- = s->regs[CPU_RAM + 4] = s->regs[CPU_RAM + 7]
- = 0x00010101;
- s->regs[CPU_RAM + 2] = s->regs[CPU_RAM + 5] = 0x01010101;
- s->regs[CPU_RAM + 6] = 0x00000001;
+ s->regs[R_CPU_RAM + 0] = s->regs[R_CPU_RAM + 1] = s->regs[R_CPU_RAM + 3]
+ = s->regs[R_CPU_RAM + 4] = s->regs[R_CPU_RAM + 7]
+ = 0x00010101;
+ s->regs[R_CPU_RAM + 2] = s->regs[R_CPU_RAM + 5] = 0x01010101;
+ s->regs[R_CPU_RAM + 6] = 0x00000001;
- s->regs[IOU + 0] = s->regs[IOU + 1] = s->regs[IOU + 2] = s->regs[IOU + 3]
- = 0x09090909;
- s->regs[IOU + 4] = s->regs[IOU + 5] = 0x00090909;
- s->regs[IOU + 6] = 0x00000909;
+ s->regs[R_IOU + 0] = s->regs[R_IOU + 1] = s->regs[R_IOU + 2]
+ = s->regs[R_IOU + 3] = 0x09090909;
+ s->regs[R_IOU + 4] = s->regs[R_IOU + 5] = 0x00090909;
+ s->regs[R_IOU + 6] = 0x00000909;
- s->regs[DMAC_RAM] = 0x00000009;
+ s->regs[R_DMAC_RAM] = 0x00000009;
- s->regs[AFI0 + 0] = s->regs[AFI0 + 1] = 0x09090909;
- s->regs[AFI1 + 0] = s->regs[AFI1 + 1] = 0x09090909;
- s->regs[AFI2 + 0] = s->regs[AFI2 + 1] = 0x09090909;
- s->regs[AFI3 + 0] = s->regs[AFI3 + 1] = 0x09090909;
- s->regs[AFI0 + 2] = s->regs[AFI1 + 2] = s->regs[AFI2 + 2]
- = s->regs[AFI3 + 2] = 0x00000909;
+ s->regs[R_AFI0 + 0] = s->regs[R_AFI0 + 1] = 0x09090909;
+ s->regs[R_AFI1 + 0] = s->regs[R_AFI1 + 1] = 0x09090909;
+ s->regs[R_AFI2 + 0] = s->regs[R_AFI2 + 1] = 0x09090909;
+ s->regs[R_AFI3 + 0] = s->regs[R_AFI3 + 1] = 0x09090909;
+ s->regs[R_AFI0 + 2] = s->regs[R_AFI1 + 2] = s->regs[R_AFI2 + 2]
+ = s->regs[R_AFI3 + 2] = 0x00000909;
- s->regs[OCM + 0] = 0x01010101;
- s->regs[OCM + 1] = s->regs[OCM + 2] = 0x09090909;
+ s->regs[R_OCM + 0] = 0x01010101;
+ s->regs[R_OCM + 1] = s->regs[R_OCM + 2] = 0x09090909;
- s->regs[DEVCI_RAM] = 0x00000909;
- s->regs[CSG_RAM] = 0x00000001;
+ s->regs[R_DEVCI_RAM] = 0x00000909;
+ s->regs[R_CSG_RAM] = 0x00000001;
- s->regs[DDRIOB + 0] = s->regs[DDRIOB + 1] = s->regs[DDRIOB + 2]
- = s->regs[DDRIOB + 3] = 0x00000e00;
- s->regs[DDRIOB + 4] = s->regs[DDRIOB + 5] = s->regs[DDRIOB + 6]
- = 0x00000e00;
- s->regs[DDRIOB + 12] = 0x00000021;
+ s->regs[R_DDRIOB + 0] = s->regs[R_DDRIOB + 1] = s->regs[R_DDRIOB + 2]
+ = s->regs[R_DDRIOB + 3] = 0x00000e00;
+ s->regs[R_DDRIOB + 4] = s->regs[R_DDRIOB + 5] = s->regs[R_DDRIOB + 6]
+ = 0x00000e00;
+ s->regs[R_DDRIOB + 12] = 0x00000021;
}
static bool zynq_slcr_check_offset(hwaddr offset, bool rnw)
{
switch (offset) {
- case LOCK:
- case UNLOCK:
- case DDR_CAL_START:
- case DDR_REF_START:
+ case R_LOCK:
+ case R_UNLOCK:
+ case R_DDR_CAL_START:
+ case R_DDR_REF_START:
return !rnw; /* Write only */
- case LOCKSTA:
- case FPGA0_THR_STA:
- case FPGA1_THR_STA:
- case FPGA2_THR_STA:
- case FPGA3_THR_STA:
- case BOOT_MODE:
- case PSS_IDCODE:
- case DDR_CMD_STA:
- case DDR_DFI_STATUS:
- case PLL_STATUS:
+ case R_LOCKSTA:
+ case R_FPGA0_THR_STA:
+ case R_FPGA1_THR_STA:
+ case R_FPGA2_THR_STA:
+ case R_FPGA3_THR_STA:
+ case R_BOOT_MODE:
+ case R_PSS_IDCODE:
+ case R_DDR_CMD_STA:
+ case R_DDR_DFI_STATUS:
+ case R_PLL_STATUS:
return rnw;/* read only */
- case SCL:
- case ARM_PLL_CTRL ... IO_PLL_CTRL:
- case ARM_PLL_CFG ... IO_PLL_CFG:
- case ARM_CLK_CTRL ... TOPSW_CLK_CTRL:
- case FPGA0_CLK_CTRL ... FPGA0_THR_CNT:
- case FPGA1_CLK_CTRL ... FPGA1_THR_CNT:
- case FPGA2_CLK_CTRL ... FPGA2_THR_CNT:
- case FPGA3_CLK_CTRL ... FPGA3_THR_CNT:
- case BANDGAP_TRIP:
- case PLL_PREDIVISOR:
- case CLK_621_TRUE:
- case PSS_RST_CTRL ... A9_CPU_RST_CTRL:
- case RS_AWDT_CTRL:
- case RST_REASON:
- case REBOOT_STATUS:
- case APU_CTRL:
- case WDT_CLK_SEL:
- case TZ_DMA_NS ... TZ_DMA_PERIPH_NS:
- case DDR_URGENT:
- case DDR_URGENT_SEL:
- case MIO ... MIO + MIO_LENGTH - 1:
- case MIO_LOOPBACK ... MIO_MST_TRI1:
- case SD0_WP_CD_SEL:
- case SD1_WP_CD_SEL:
- case LVL_SHFTR_EN:
- case OCM_CFG:
- case CPU_RAM:
- case IOU:
- case DMAC_RAM:
- case AFI0 ... AFI3 + AFI_LENGTH - 1:
- case OCM:
- case DEVCI_RAM:
- case CSG_RAM:
- case GPIOB_CTRL ... GPIOB_CFG_CMOS33:
- case GPIOB_CFG_HSTL:
- case GPIOB_DRVR_BIAS_CTRL:
- case DDRIOB ... DDRIOB + DDRIOB_LENGTH - 1:
+ case R_SCL:
+ case R_ARM_PLL_CTRL ... R_IO_PLL_CTRL:
+ case R_ARM_PLL_CFG ... R_IO_PLL_CFG:
+ case R_ARM_CLK_CTRL ... R_TOPSW_CLK_CTRL:
+ case R_FPGA0_CLK_CTRL ... R_FPGA0_THR_CNT:
+ case R_FPGA1_CLK_CTRL ... R_FPGA1_THR_CNT:
+ case R_FPGA2_CLK_CTRL ... R_FPGA2_THR_CNT:
+ case R_FPGA3_CLK_CTRL ... R_FPGA3_THR_CNT:
+ case R_BANDGAP_TRIP:
+ case R_PLL_PREDIVISOR:
+ case R_CLK_621_TRUE:
+ case R_PSS_RST_CTRL ... R_A9_CPU_RST_CTRL:
+ case R_RS_AWDT_CTRL:
+ case R_RST_REASON:
+ case R_REBOOT_STATUS:
+ case R_APU_CTRL:
+ case R_WDT_CLK_SEL:
+ case R_TZ_DMA_NS ... R_TZ_DMA_PERIPH_NS:
+ case R_DDR_URGENT:
+ case R_DDR_URGENT_SEL:
+ case R_MIO ... R_MIO + MIO_LENGTH - 1:
+ case R_MIO_LOOPBACK ... R_MIO_MST_TRI1:
+ case R_SD0_WP_CD_SEL:
+ case R_SD1_WP_CD_SEL:
+ case R_LVL_SHFTR_EN:
+ case R_OCM_CFG:
+ case R_CPU_RAM:
+ case R_IOU:
+ case R_DMAC_RAM:
+ case R_AFI0 ... R_AFI3 + AFI_LENGTH - 1:
+ case R_OCM:
+ case R_DEVCI_RAM:
+ case R_CSG_RAM:
+ case R_GPIOB_CTRL ... R_GPIOB_CFG_CMOS33:
+ case R_GPIOB_CFG_HSTL:
+ case R_GPIOB_DRVR_BIAS_CTRL:
+ case R_DDRIOB ... R_DDRIOB + DDRIOB_LENGTH - 1:
return true;
default:
return false;
@@ -370,24 +370,24 @@ static void zynq_slcr_write(void *opaque, hwaddr offset,
}
switch (offset) {
- case SCL:
- s->regs[SCL] = val & 0x1;
+ case R_SCL:
+ s->regs[R_SCL] = val & 0x1;
return;
- case LOCK:
+ case R_LOCK:
if ((val & 0xFFFF) == XILINX_LOCK_KEY) {
DB_PRINT("XILINX LOCK 0xF8000000 + 0x%x <= 0x%x\n", (int)offset,
(unsigned)val & 0xFFFF);
- s->regs[LOCKSTA] = 1;
+ s->regs[R_LOCKSTA] = 1;
} else {
DB_PRINT("WRONG XILINX LOCK KEY 0xF8000000 + 0x%x <= 0x%x\n",
(int)offset, (unsigned)val & 0xFFFF);
}
return;
- case UNLOCK:
+ case R_UNLOCK:
if ((val & 0xFFFF) == XILINX_UNLOCK_KEY) {
DB_PRINT("XILINX UNLOCK 0xF8000000 + 0x%x <= 0x%x\n", (int)offset,
(unsigned)val & 0xFFFF);
- s->regs[LOCKSTA] = 0;
+ s->regs[R_LOCKSTA] = 0;
} else {
DB_PRINT("WRONG XILINX UNLOCK KEY 0xF8000000 + 0x%x <= 0x%x\n",
(int)offset, (unsigned)val & 0xFFFF);
@@ -395,7 +395,7 @@ static void zynq_slcr_write(void *opaque, hwaddr offset,
return;
}
- if (s->regs[LOCKSTA]) {
+ if (s->regs[R_LOCKSTA]) {
qemu_log_mask(LOG_GUEST_ERROR,
"SCLR registers are locked. Unlock them first\n");
return;
@@ -403,8 +403,8 @@ static void zynq_slcr_write(void *opaque, hwaddr offset,
s->regs[offset] = val;
switch (offset) {
- case PSS_RST_CTRL:
- if (val & R_PSS_RST_CTRL_SOFT_RST) {
+ case R_PSS_RST_CTRL:
+ if (FIELD_EX32(val, PSS_RST_CTRL, SOFT_RST)) {
qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
}
break;
--
2.21.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [RFC PATCH v2 09/12] convert cadence_uart to 3-phases reset
2019-06-04 16:25 [Qemu-devel] [RFC PATCH v2 00/12] Multi-phase reset Damien Hedde
` (7 preceding siblings ...)
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 08/12] hw/misc/zynq_slcr: use standard register definition Damien Hedde
@ 2019-06-04 16:25 ` Damien Hedde
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 10/12] Convert zynq's slcr " Damien Hedde
` (3 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Damien Hedde @ 2019-06-04 16:25 UTC (permalink / raw)
To: qemu-devel
Cc: edgar.iglesias, peter.maydell, marc.burton, alistair, qemu-arm,
Damien Hedde, marcandre.lureau, pbonzini, philmd, luc.michel
Split the existing reset procedure into 3 phases.
Test the resetting flag to discard register accesses
and character reception.
Also adds a active high reset io.
Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
hw/char/cadence_uart.c | 81 +++++++++++++++++++++++++++++++++++++++---
1 file changed, 77 insertions(+), 4 deletions(-)
diff --git a/hw/char/cadence_uart.c b/hw/char/cadence_uart.c
index fbdbd463bb..27e1c70678 100644
--- a/hw/char/cadence_uart.c
+++ b/hw/char/cadence_uart.c
@@ -38,6 +38,18 @@
#define DB_PRINT(...)
#endif
+#define CADENCE_UART_CLASS(class) \
+ OBJECT_CLASS_CHECK(CadenceUartClass, (class), TYPE_CADENCE_UART)
+#define CADENCE_UART_GET_CLASS(obj) \
+ OBJECT_GET_CLASS(CadenceUartClass, (obj), TYPE_CADENCE_UART)
+
+typedef struct CadenceUartClass {
+ /*< private >*/
+ SysBusDeviceClass parent_class;
+
+ struct ResettablePhases parent_reset_phases;
+} CadenceUartClass;
+
#define UART_SR_INTR_RTRIG 0x00000001
#define UART_SR_INTR_REMPTY 0x00000002
#define UART_SR_INTR_RFUL 0x00000004
@@ -222,6 +234,10 @@ static int uart_can_receive(void *opaque)
int ret = MAX(CADENCE_UART_RX_FIFO_SIZE, CADENCE_UART_TX_FIFO_SIZE);
uint32_t ch_mode = s->r[R_MR] & UART_MR_CHMODE;
+ if (device_is_resetting((DeviceState *) opaque)) {
+ return 0;
+ }
+
if (ch_mode == NORMAL_MODE || ch_mode == ECHO_MODE) {
ret = MIN(ret, CADENCE_UART_RX_FIFO_SIZE - s->rx_count);
}
@@ -337,6 +353,10 @@ static void uart_receive(void *opaque, const uint8_t *buf, int size)
CadenceUARTState *s = opaque;
uint32_t ch_mode = s->r[R_MR] & UART_MR_CHMODE;
+ if (device_is_resetting((DeviceState *) opaque)) {
+ return;
+ }
+
if (ch_mode == NORMAL_MODE || ch_mode == ECHO_MODE) {
uart_write_rx_fifo(opaque, buf, size);
}
@@ -350,6 +370,10 @@ static void uart_event(void *opaque, int event)
CadenceUARTState *s = opaque;
uint8_t buf = '\0';
+ if (device_is_resetting((DeviceState *) opaque)) {
+ return;
+ }
+
if (event == CHR_EVENT_BREAK) {
uart_write_rx_fifo(opaque, &buf, 1);
}
@@ -382,6 +406,10 @@ static void uart_write(void *opaque, hwaddr offset,
{
CadenceUARTState *s = opaque;
+ if (device_is_resetting((DeviceState *)opaque)) {
+ return;
+ }
+
DB_PRINT(" offset:%x data:%08x\n", (unsigned)offset, (unsigned)value);
offset >>= 2;
if (offset >= CADENCE_UART_R_MAX) {
@@ -440,6 +468,10 @@ static uint64_t uart_read(void *opaque, hwaddr offset,
CadenceUARTState *s = opaque;
uint32_t c = 0;
+ if (device_is_resetting((DeviceState *)opaque)) {
+ return 0;
+ }
+
offset >>= 2;
if (offset >= CADENCE_UART_R_MAX) {
c = 0;
@@ -459,9 +491,14 @@ static const MemoryRegionOps uart_ops = {
.endianness = DEVICE_NATIVE_ENDIAN,
};
-static void cadence_uart_reset(DeviceState *dev)
+static void cadence_uart_reset_init(Object *obj, bool cold)
{
- CadenceUARTState *s = CADENCE_UART(dev);
+ CadenceUARTState *s = CADENCE_UART(obj);
+ CadenceUartClass *cc = CADENCE_UART_GET_CLASS(obj);
+
+ if (cc->parent_reset_phases.init) {
+ cc->parent_reset_phases.init(obj, cold);
+ }
s->r[R_CR] = 0x00000128;
s->r[R_IMR] = 0;
@@ -470,6 +507,28 @@ static void cadence_uart_reset(DeviceState *dev)
s->r[R_BRGR] = 0x0000028B;
s->r[R_BDIV] = 0x0000000F;
s->r[R_TTRIG] = 0x00000020;
+}
+
+static void cadence_uart_reset_hold(Object *obj)
+{
+ CadenceUARTState *s = CADENCE_UART(obj);
+ CadenceUartClass *cc = CADENCE_UART_GET_CLASS(obj);
+
+ if (cc->parent_reset_phases.hold) {
+ cc->parent_reset_phases.hold(obj);
+ }
+
+ qemu_set_irq(s->irq, 0);
+}
+
+static void cadence_uart_reset_exit(Object *obj)
+{
+ CadenceUARTState *s = CADENCE_UART(obj);
+ CadenceUartClass *cc = CADENCE_UART_GET_CLASS(obj);
+
+ if (cc->parent_reset_phases.exit) {
+ cc->parent_reset_phases.exit(obj);
+ }
uart_rx_reset(s);
uart_tx_reset(s);
@@ -498,6 +557,8 @@ static void cadence_uart_init(Object *obj)
sysbus_init_irq(sbd, &s->irq);
s->char_tx_time = (NANOSECONDS_PER_SECOND / 9600) * 10;
+
+ qdev_init_warm_reset_gpio(DEVICE(obj), "rst", DEVICE_ACTIVE_HIGH);
}
static int cadence_uart_post_load(void *opaque, int version_id)
@@ -532,6 +593,10 @@ static const VMStateDescription vmstate_cadence_uart = {
VMSTATE_UINT32(rx_wpos, CadenceUARTState),
VMSTATE_TIMER_PTR(fifo_trigger_handle, CadenceUARTState),
VMSTATE_END_OF_LIST()
+ },
+ .subsections = (const VMStateDescription * []) {
+ &device_vmstate_reset,
+ NULL
}
};
@@ -543,12 +608,19 @@ static Property cadence_uart_properties[] = {
static void cadence_uart_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
+ ResettableClass *rc = RESETTABLE_CLASS(klass);
+ CadenceUartClass *cc = CADENCE_UART_CLASS(klass);
dc->realize = cadence_uart_realize;
dc->vmsd = &vmstate_cadence_uart;
- dc->reset = cadence_uart_reset;
dc->props = cadence_uart_properties;
- }
+
+ resettable_class_set_parent_reset_phases(rc,
+ cadence_uart_reset_init,
+ cadence_uart_reset_hold,
+ cadence_uart_reset_exit,
+ &cc->parent_reset_phases);
+}
static const TypeInfo cadence_uart_info = {
.name = TYPE_CADENCE_UART,
@@ -556,6 +628,7 @@ static const TypeInfo cadence_uart_info = {
.instance_size = sizeof(CadenceUARTState),
.instance_init = cadence_uart_init,
.class_init = cadence_uart_class_init,
+ .class_size = sizeof(CadenceUartClass),
};
static void cadence_uart_register_types(void)
--
2.21.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [RFC PATCH v2 10/12] Convert zynq's slcr to 3-phases reset
2019-06-04 16:25 [Qemu-devel] [RFC PATCH v2 00/12] Multi-phase reset Damien Hedde
` (8 preceding siblings ...)
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 09/12] convert cadence_uart to 3-phases reset Damien Hedde
@ 2019-06-04 16:25 ` Damien Hedde
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 11/12] Add uart reset support in zynq_slcr Damien Hedde
` (2 subsequent siblings)
12 siblings, 0 replies; 15+ messages in thread
From: Damien Hedde @ 2019-06-04 16:25 UTC (permalink / raw)
To: qemu-devel
Cc: edgar.iglesias, peter.maydell, marc.burton, alistair, qemu-arm,
Damien Hedde, marcandre.lureau, pbonzini, philmd, luc.michel
Change the legacy reset function into the init phase and test the
resetting flag in register accesses.
Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
hw/misc/zynq_slcr.c | 39 +++++++++++++++++++++++++++++++++++----
1 file changed, 35 insertions(+), 4 deletions(-)
diff --git a/hw/misc/zynq_slcr.c b/hw/misc/zynq_slcr.c
index baa13d1316..c6d2bba966 100644
--- a/hw/misc/zynq_slcr.c
+++ b/hw/misc/zynq_slcr.c
@@ -171,6 +171,17 @@ REG32(DDRIOB, 0xb40)
#define TYPE_ZYNQ_SLCR "xilinx,zynq_slcr"
#define ZYNQ_SLCR(obj) OBJECT_CHECK(ZynqSLCRState, (obj), TYPE_ZYNQ_SLCR)
+#define ZYNQ_SLCR_CLASS(class) \
+ OBJECT_CLASS_CHECK(ZynqSLCRClass, (class), TYPE_ZYNQ_SLCR)
+#define ZYNQ_SLCR_GET_CLASS(obj) \
+ OBJECT_GET_CLASS(ZynqSLCRClass, (obj), TYPE_ZYNQ_SLCR)
+
+typedef struct ZynqSLCRClass {
+ /*< private >*/
+ SysBusDeviceClass parent_class;
+
+ struct ResettablePhases parent_reset_phases;
+} ZynqSLCRClass;
typedef struct ZynqSLCRState {
SysBusDevice parent_obj;
@@ -180,13 +191,18 @@ typedef struct ZynqSLCRState {
uint32_t regs[ZYNQ_SLCR_NUM_REGS];
} ZynqSLCRState;
-static void zynq_slcr_reset(DeviceState *d)
+static void zynq_slcr_reset_init(Object *obj, bool cold)
{
- ZynqSLCRState *s = ZYNQ_SLCR(d);
+ ZynqSLCRState *s = ZYNQ_SLCR(obj);
+ ZynqSLCRClass *zc = ZYNQ_SLCR_GET_CLASS(obj);
int i;
DB_PRINT("RESET\n");
+ if (zc->parent_reset_phases.init) {
+ zc->parent_reset_phases.init(obj, cold);
+ }
+
s->regs[R_LOCKSTA] = 1;
/* 0x100 - 0x11C */
s->regs[R_ARM_PLL_CTRL] = 0x0001A008;
@@ -276,7 +292,6 @@ static void zynq_slcr_reset(DeviceState *d)
s->regs[R_DDRIOB + 12] = 0x00000021;
}
-
static bool zynq_slcr_check_offset(hwaddr offset, bool rnw)
{
switch (offset) {
@@ -346,6 +361,10 @@ static uint64_t zynq_slcr_read(void *opaque, hwaddr offset,
offset /= 4;
uint32_t ret = s->regs[offset];
+ if (device_is_resetting((DeviceState *) opaque)) {
+ return 0;
+ }
+
if (!zynq_slcr_check_offset(offset, true)) {
qemu_log_mask(LOG_GUEST_ERROR, "zynq_slcr: Invalid read access to "
" addr %" HWADDR_PRIx "\n", offset * 4);
@@ -361,6 +380,10 @@ static void zynq_slcr_write(void *opaque, hwaddr offset,
ZynqSLCRState *s = (ZynqSLCRState *)opaque;
offset /= 4;
+ if (device_is_resetting((DeviceState *) opaque)) {
+ return;
+ }
+
DB_PRINT("addr: %08" HWADDR_PRIx " data: %08" PRIx64 "\n", offset * 4, val);
if (!zynq_slcr_check_offset(offset, false)) {
@@ -439,9 +462,16 @@ static const VMStateDescription vmstate_zynq_slcr = {
static void zynq_slcr_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
+ ResettableClass *rc = RESETTABLE_CLASS(klass);
+ ZynqSLCRClass *zc = ZYNQ_SLCR_CLASS(klass);
dc->vmsd = &vmstate_zynq_slcr;
- dc->reset = zynq_slcr_reset;
+
+ resettable_class_set_parent_reset_phases(rc,
+ zynq_slcr_reset_init,
+ NULL,
+ NULL,
+ &zc->parent_reset_phases);
}
static const TypeInfo zynq_slcr_info = {
@@ -450,6 +480,7 @@ static const TypeInfo zynq_slcr_info = {
.parent = TYPE_SYS_BUS_DEVICE,
.instance_size = sizeof(ZynqSLCRState),
.instance_init = zynq_slcr_init,
+ .class_size = sizeof(ZynqSLCRClass),
};
static void zynq_slcr_register_types(void)
--
2.21.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [RFC PATCH v2 11/12] Add uart reset support in zynq_slcr
2019-06-04 16:25 [Qemu-devel] [RFC PATCH v2 00/12] Multi-phase reset Damien Hedde
` (9 preceding siblings ...)
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 10/12] Convert zynq's slcr " Damien Hedde
@ 2019-06-04 16:25 ` Damien Hedde
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 12/12] Connect the uart reset gpios in the zynq platform Damien Hedde
2019-06-18 16:13 ` [Qemu-devel] [RFC PATCH v2 00/12] Multi-phase reset Peter Maydell
12 siblings, 0 replies; 15+ messages in thread
From: Damien Hedde @ 2019-06-04 16:25 UTC (permalink / raw)
To: qemu-devel
Cc: edgar.iglesias, peter.maydell, marc.burton, alistair, qemu-arm,
Damien Hedde, marcandre.lureau, pbonzini, philmd, luc.michel
Add two gpio outputs to control the uart resets.
Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
hw/misc/zynq_slcr.c | 36 +++++++++++++++++++++++++++++++++++-
1 file changed, 35 insertions(+), 1 deletion(-)
diff --git a/hw/misc/zynq_slcr.c b/hw/misc/zynq_slcr.c
index c6d2bba966..6649c93a90 100644
--- a/hw/misc/zynq_slcr.c
+++ b/hw/misc/zynq_slcr.c
@@ -96,6 +96,10 @@ REG32(SPI_RST_CTRL, 0x21c)
REG32(CAN_RST_CTRL, 0x220)
REG32(I2C_RST_CTRL, 0x224)
REG32(UART_RST_CTRL, 0x228)
+ FIELD(UART_RST_CTRL, UART0_CPU1X_RST, 0, 1)
+ FIELD(UART_RST_CTRL, UART1_CPU1X_RST, 1, 1)
+ FIELD(UART_RST_CTRL, UART0_REF_RST, 2, 1)
+ FIELD(UART_RST_CTRL, UART1_REF_RST, 3, 1)
REG32(GPIO_RST_CTRL, 0x22c)
REG32(LQSPI_RST_CTRL, 0x230)
REG32(SMC_RST_CTRL, 0x234)
@@ -189,8 +193,14 @@ typedef struct ZynqSLCRState {
MemoryRegion iomem;
uint32_t regs[ZYNQ_SLCR_NUM_REGS];
+
+ qemu_irq uart0_rst;
+ qemu_irq uart1_rst;
} ZynqSLCRState;
+#define ZYNQ_SLCR_REGFIELD_TO_OUT(state, irq, reg, field) \
+ qemu_set_irq((state)->irq, ARRAY_FIELD_EX32((state)->regs, reg, field) != 0)
+
static void zynq_slcr_reset_init(Object *obj, bool cold)
{
ZynqSLCRState *s = ZYNQ_SLCR(obj);
@@ -292,6 +302,24 @@ static void zynq_slcr_reset_init(Object *obj, bool cold)
s->regs[R_DDRIOB + 12] = 0x00000021;
}
+static void zynq_slcr_compute_uart_reset(ZynqSLCRState *s)
+{
+ ZYNQ_SLCR_REGFIELD_TO_OUT(s, uart0_rst, UART_RST_CTRL, UART0_REF_RST);
+ ZYNQ_SLCR_REGFIELD_TO_OUT(s, uart1_rst, UART_RST_CTRL, UART1_REF_RST);
+}
+
+static void zynq_slcr_reset_hold(Object *obj)
+{
+ ZynqSLCRState *s = ZYNQ_SLCR(obj);
+ ZynqSLCRClass *zc = ZYNQ_SLCR_GET_CLASS(obj);
+
+ if (zc->parent_reset_phases.hold) {
+ zc->parent_reset_phases.hold(obj);
+ }
+
+ zynq_slcr_compute_uart_reset(s);
+}
+
static bool zynq_slcr_check_offset(hwaddr offset, bool rnw)
{
switch (offset) {
@@ -431,6 +459,9 @@ static void zynq_slcr_write(void *opaque, hwaddr offset,
qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
}
break;
+ case R_UART_RST_CTRL:
+ zynq_slcr_compute_uart_reset(s);
+ break;
}
}
@@ -447,6 +478,9 @@ static void zynq_slcr_init(Object *obj)
memory_region_init_io(&s->iomem, obj, &slcr_ops, s, "slcr",
ZYNQ_SLCR_MMIO_SIZE);
sysbus_init_mmio(SYS_BUS_DEVICE(obj), &s->iomem);
+
+ qdev_init_gpio_out_named(DEVICE(obj), &s->uart0_rst, "uart0_rst", 1);
+ qdev_init_gpio_out_named(DEVICE(obj), &s->uart1_rst, "uart1_rst", 1);
}
static const VMStateDescription vmstate_zynq_slcr = {
@@ -469,7 +503,7 @@ static void zynq_slcr_class_init(ObjectClass *klass, void *data)
resettable_class_set_parent_reset_phases(rc,
zynq_slcr_reset_init,
- NULL,
+ zynq_slcr_reset_hold,
NULL,
&zc->parent_reset_phases);
}
--
2.21.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [RFC PATCH v2 12/12] Connect the uart reset gpios in the zynq platform
2019-06-04 16:25 [Qemu-devel] [RFC PATCH v2 00/12] Multi-phase reset Damien Hedde
` (10 preceding siblings ...)
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 11/12] Add uart reset support in zynq_slcr Damien Hedde
@ 2019-06-04 16:25 ` Damien Hedde
2019-06-18 16:13 ` [Qemu-devel] [RFC PATCH v2 00/12] Multi-phase reset Peter Maydell
12 siblings, 0 replies; 15+ messages in thread
From: Damien Hedde @ 2019-06-04 16:25 UTC (permalink / raw)
To: qemu-devel
Cc: edgar.iglesias, peter.maydell, marc.burton, alistair, qemu-arm,
Damien Hedde, marcandre.lureau, pbonzini, philmd, luc.michel
Connect the two uart reset inputs to the slcr corresponding outputs.
Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
hw/arm/xilinx_zynq.c | 14 ++++++++------
include/hw/char/cadence_uart.h | 10 +++++++++-
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/hw/arm/xilinx_zynq.c b/hw/arm/xilinx_zynq.c
index 198e3f9763..ed7549a6a2 100644
--- a/hw/arm/xilinx_zynq.c
+++ b/hw/arm/xilinx_zynq.c
@@ -166,7 +166,7 @@ static void zynq_init(MachineState *machine)
MemoryRegion *address_space_mem = get_system_memory();
MemoryRegion *ext_ram = g_new(MemoryRegion, 1);
MemoryRegion *ocm_ram = g_new(MemoryRegion, 1);
- DeviceState *dev;
+ DeviceState *dev, *slcr;
SysBusDevice *busdev;
qemu_irq pic[64];
int n;
@@ -211,9 +211,9 @@ static void zynq_init(MachineState *machine)
1, 0x0066, 0x0022, 0x0000, 0x0000, 0x0555, 0x2aa,
0);
- dev = qdev_create(NULL, "xilinx,zynq_slcr");
- qdev_init_nofail(dev);
- sysbus_mmio_map(SYS_BUS_DEVICE(dev), 0, 0xF8000000);
+ slcr = qdev_create(NULL, "xilinx,zynq_slcr");
+ qdev_init_nofail(slcr);
+ sysbus_mmio_map(SYS_BUS_DEVICE(slcr), 0, 0xF8000000);
dev = qdev_create(NULL, TYPE_A9MPCORE_PRIV);
qdev_prop_set_uint32(dev, "num-cpu", 1);
@@ -234,8 +234,10 @@ static void zynq_init(MachineState *machine)
sysbus_create_simple("xlnx,ps7-usb", 0xE0002000, pic[53-IRQ_OFFSET]);
sysbus_create_simple("xlnx,ps7-usb", 0xE0003000, pic[76-IRQ_OFFSET]);
- cadence_uart_create(0xE0000000, pic[59 - IRQ_OFFSET], serial_hd(0));
- cadence_uart_create(0xE0001000, pic[82 - IRQ_OFFSET], serial_hd(1));
+ cadence_uart_create(0xE0000000, pic[59 - IRQ_OFFSET], serial_hd(0),
+ slcr, "uart0_rst", 0);
+ cadence_uart_create(0xE0001000, pic[82 - IRQ_OFFSET], serial_hd(1),
+ slcr, "uart1_rst", 0);
sysbus_create_varargs("cadence_ttc", 0xF8001000,
pic[42-IRQ_OFFSET], pic[43-IRQ_OFFSET], pic[44-IRQ_OFFSET], NULL);
diff --git a/include/hw/char/cadence_uart.h b/include/hw/char/cadence_uart.h
index 118e3f10de..b7489a711f 100644
--- a/include/hw/char/cadence_uart.h
+++ b/include/hw/char/cadence_uart.h
@@ -51,7 +51,10 @@ typedef struct {
static inline DeviceState *cadence_uart_create(hwaddr addr,
qemu_irq irq,
- Chardev *chr)
+ Chardev *chr,
+ DeviceState *rst_dev,
+ const char *rst_name,
+ int rst_n)
{
DeviceState *dev;
SysBusDevice *s;
@@ -63,6 +66,11 @@ static inline DeviceState *cadence_uart_create(hwaddr addr,
sysbus_mmio_map(s, 0, addr);
sysbus_connect_irq(s, 0, irq);
+ if (rst_dev) {
+ qdev_connect_gpio_out_named(rst_dev, rst_name, rst_n,
+ qdev_get_gpio_in_named(dev, "rst", 0));
+ }
+
return dev;
}
--
2.21.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [RFC PATCH v2 00/12] Multi-phase reset
2019-06-04 16:25 [Qemu-devel] [RFC PATCH v2 00/12] Multi-phase reset Damien Hedde
` (11 preceding siblings ...)
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 12/12] Connect the uart reset gpios in the zynq platform Damien Hedde
@ 2019-06-18 16:13 ` Peter Maydell
2019-06-27 9:13 ` Damien Hedde
12 siblings, 1 reply; 15+ messages in thread
From: Peter Maydell @ 2019-06-18 16:13 UTC (permalink / raw)
To: Damien Hedde
Cc: Edgar Iglesias, marc.burton, Alistair Francis, QEMU Developers,
qemu-arm, Paolo Bonzini, Marc-André Lureau,
Philippe Mathieu-Daudé,
Luc Michel
On Tue, 4 Jun 2019 at 17:25, Damien Hedde <damien.hedde@greensocs.com> wrote:
>
> Hi all,
>
> Here's the second version of the multi-phase reset proposal patches.
Sorry for leaving this one hanging again. Some initial general
comments...
> # DESCRIPTION
>
> Basically the reset procedure is split in 3 parts:
> INIT PHASE: Reset the object internal state, put a resetting flag and do the
> same for the reset subtree. No side effect on other devices to guarantee
> that, in a reset domain, everything get initialized first. This corresponds
> mostly to what is currently done in the device/bus reset method.
>
> HOLD PHASE: This phase allows to control a reset with a IO. When a IO control
> reset procedure based on the IO level (not edge), we may need to assert
> the reset, wait some time, and finally de-assert the reset. The consequence
> is that such a device can stay in a "resetting state" and may need to show
> this state to other devices through its outputs. For example, a clock
> controller will typically shutdown its clocks when it is in resetting state.
>
> EXIT PHASE: This phase sets outputs to state after reset. For a clock controller
> it starts the clocks. It also clears the "resetting" flag. A device should
> not react to inputs until this flag has been cleared. During this phase,
> outputs are propagated.
>
> The Resettable interface is detailed in the added doc in patch 7.
>
> # CHANGE SINCE V1
>
> The series now focus only on the Resettable interface (no more ResetDomain).
> Proposed changed have been applied:
> - reset_count getter/modifier methods
> - a foreach_child method
>
> This last method allows some flexibility on what is the hierarchy of reset.
> There is some discussion ongoing about this point. Althought the series does
> not aim to modify the qdev/qbus-centric reset behavior, it is not fixed. An
> object specialization can override it.
I've looked through the patches, and I think my current concerns
(stated briefly) are:
* I don't think we have the "don't call device implementations of
reset phase functions multiple times" semantics that I wanted;
OTOH I think the logic I suggested for those in comments on the
v1 of this series wouldn't work either.
* handling of "call the parent class's reset" is not very clean
(but this is a general QOM design issue)
* migration (see below)
> # RESET DEPRECATION
>
> There is 3 changes in the current way of handling reset (for users or
> developers of Devices)
>
> 1. qdev/qbus_reset_all
>
> Theses functions are deprecated by this series and should be replaced by
> direct call to device_reset or bus_reset. There is only a few existing calls
> so I can probably replace them all and delete the original functions.
Sounds good.
> 2. old's device_reset
>
> There is a few call to this function, I renamed it *device_legacy_reset* to
> handle the transition. This function allowed to reset only a given device
> (and not its eventual qbus subtree). This behavior is not possible with
> the Resettable interface. At first glance it seemed that it is used only on
> device having no sub-buses so we could just use the new device_reset.
> But I need to look at them more closely to be sure. If this behavior is really
> needed (but why would we not reset children ?), it's possible to do a specific
> function for Device to to 3-phases reset without the children.
Users of this function:
hw/audio/intel-hda.c
-- used by the HDA device to reset the HDACodecDevice, which has
no child buses
hw/hyperv/hyperv.c
-- resets the SynICState, which I think has no child buses
hw/i386/pc.c
-- resets the APIC, which has no child buses. (This reset is only
done as a workaround for lack of reset phases: the whole machine
is reset and then the APIC is re-reset last to undo any changes
that other devices might have made to it. Probably making the APIC
support phased reset would allow us to drop this hack.)
hw/ide/microdrive.c
-- called here to reset the MicroDriveState object. This does have
a child bus, but md_reset() explicitly calls ide_bus_reset(),
so it should be possible to clean this up.
hw/intc/spapr_xive.c
-- resets the SpaprXive device, which I think has no child buses
hw/ppc/pnv_psi.c
-- resets a XiveSource, which I think has no child buses
hw/ppc/spapr_pci.c
-- resets every QOM child of the SpaprPhbState. This one will
require closer checking, but my guess is that if there's a child
with a child bus then failure to reset the bus would be a bug.
hw/ppc/spapr_vio.c
-- resets an SpaprTceTable. This needs attention from an Spapr
expert but is probably ok.
hw/s390x/s390-pci-inst.c
-- resets the S390PCIBusDevice. Needs S390 expertise, but probably
either no child buses or failure to reset them is a bug.
hw/scsi/vmw_pvscsi.c
-- resets an individual SCSIDevice. I don't think those have child buses.
hw/sd/omap_mmc.c
-- resetting an SDState, which has no child bus
hw/sd/pl181.c
-- ditto.
So there are one or two not-totally-trivial cases but we should
be able to deal with these.
> 3. DeviceClass's reset and BusClass's methods
>
> This is the major change. The method is deprecated because reset methods are
> now located in the interface class. In the series, I make the init phase
> redirect to the original reset method of DeviceClass (or BusClass). There a
> lot of use of the method and transitioning to 3-phases only reset will need
> some work.
I think it should be possible to do the conversion mechanically
(eg with coccinelle). We can look at doing that later.
> # MIGRATION
>
> Bus reset state migration is right now not handled.
>
> Regarding "migration-during-reset should Just Work, without necessarily
> needing any specific changes for a device". The included patch define a vmsd
> subsection which must to be added to every device main vmsd structure for
> migration-during-reset of theses devices to work. I tried to find a way to
> avoid that but really don't see how to achieve that.
>
> So in the current state of this series, migration can only be supported for
> leaf device (in term of qdev/qbus) where we add the subsection.
>
> I'm not sure the migration is the problem here. I doubt a device will
> support staying in reset state (which is a new feature) without manual
> intervention. So migration of this unsupported state without any specific
> change may not be a real asset.
The approach I thought would be good turns out not to actually work,
so I need to think a bit more about this.
I think what I would like to achieve is "default to doing the right
thing" -- ideally devices that add support for being held in reset
should not need to manually do something to make the bus/device reset
state be migrated properly. Otherwise we have an easy mistake to
make (forgetting to do the necessary handling of migration) when
adding a new device or making a device support being held in reset.
Regarding "devices not supporting staying in reset state without
manual intervention" do you mean that they might not correctly
deal with incoming signals or guest register write attempts
in the held-in-reset state? That's true, but I don't think it's
too terrible. (In particular it's a bug that can be fixed without
breaking migration compatibility; and it seems unlikely that guest
software would be trying to read or write a device that it's put
into reset.)
thanks
-- PMM
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [RFC PATCH v2 00/12] Multi-phase reset
2019-06-18 16:13 ` [Qemu-devel] [RFC PATCH v2 00/12] Multi-phase reset Peter Maydell
@ 2019-06-27 9:13 ` Damien Hedde
0 siblings, 0 replies; 15+ messages in thread
From: Damien Hedde @ 2019-06-27 9:13 UTC (permalink / raw)
To: Peter Maydell
Cc: Edgar Iglesias, Alistair Francis, mark.burton, QEMU Developers,
qemu-arm, Paolo Bonzini, Marc-André Lureau,
Philippe Mathieu-Daudé,
Luc Michel
Hi
On 6/18/19 6:13 PM, Peter Maydell wrote:
> On Tue, 4 Jun 2019 at 17:25, Damien Hedde <damien.hedde@greensocs.com> wrote:
>>
>> Hi all,
>>
>> Here's the second version of the multi-phase reset proposal patches.
>
> I've looked through the patches, and I think my current concerns
> (stated briefly) are:
> * I don't think we have the "don't call device implementations of
> reset phase functions multiple times" semantics that I wanted;
> OTOH I think the logic I suggested for those in comments on the
> v1 of this series wouldn't work either.
I see now what you meant. I think it is possible but the cold/warm make
it more complicated. It will need a "get_reset_kind" method which
returns whether it's cold or warm.
> * handling of "call the parent class's reset" is not very clean
> (but this is a general QOM design issue)
Is it the current good way of doing this ? (some kind of
override_this_parent_method function)
> * migration (see below)
>
>> # RESET DEPRECATION
>>
>> There is 3 changes in the current way of handling reset (for users or
>> developers of Devices)
>>
>> 1. qdev/qbus_reset_all
>>
>> Theses functions are deprecated by this series and should be replaced by
>> direct call to device_reset or bus_reset. There is only a few existing calls
>> so I can probably replace them all and delete the original functions.
>
> Sounds good.
>
>> 2. old's device_reset
>>
>> There is a few call to this function, I renamed it *device_legacy_reset* to
>> handle the transition. This function allowed to reset only a given device
>> (and not its eventual qbus subtree). This behavior is not possible with
>> the Resettable interface. At first glance it seemed that it is used only on
>> device having no sub-buses so we could just use the new device_reset.
>> But I need to look at them more closely to be sure. If this behavior is really
>> needed (but why would we not reset children ?), it's possible to do a specific
>> function for Device to to 3-phases reset without the children.
>
> Users of this function:
> hw/audio/intel-hda.c
> -- used by the HDA device to reset the HDACodecDevice, which has
> no child buses
> hw/hyperv/hyperv.c
> -- resets the SynICState, which I think has no child buses
> hw/i386/pc.c
> -- resets the APIC, which has no child buses. (This reset is only
> done as a workaround for lack of reset phases: the whole machine
> is reset and then the APIC is re-reset last to undo any changes
> that other devices might have made to it. Probably making the APIC
> support phased reset would allow us to drop this hack.)
> hw/ide/microdrive.c
> -- called here to reset the MicroDriveState object. This does have
> a child bus, but md_reset() explicitly calls ide_bus_reset(),
> so it should be possible to clean this up.
> hw/intc/spapr_xive.c
> -- resets the SpaprXive device, which I think has no child buses
> hw/ppc/pnv_psi.c
> -- resets a XiveSource, which I think has no child buses
> hw/ppc/spapr_pci.c
> -- resets every QOM child of the SpaprPhbState. This one will
> require closer checking, but my guess is that if there's a child
> with a child bus then failure to reset the bus would be a bug.
> hw/ppc/spapr_vio.c
> -- resets an SpaprTceTable. This needs attention from an Spapr
> expert but is probably ok.
> hw/s390x/s390-pci-inst.c
> -- resets the S390PCIBusDevice. Needs S390 expertise, but probably
> either no child buses or failure to reset them is a bug.
> hw/scsi/vmw_pvscsi.c
> -- resets an individual SCSIDevice. I don't think those have child buses.
> hw/sd/omap_mmc.c
> -- resetting an SDState, which has no child bus
> hw/sd/pl181.c
> -- ditto.
>
> So there are one or two not-totally-trivial cases but we should
> be able to deal with these.
Thanks for listing theses. Do you think I should includes all the
switches in the series or just the trivial ones ?
>
>> 3. DeviceClass's reset and BusClass's methods
>>
>> This is the major change. The method is deprecated because reset methods are
>> now located in the interface class. In the series, I make the init phase
>> redirect to the original reset method of DeviceClass (or BusClass). There a
>> lot of use of the method and transitioning to 3-phases only reset will need
>> some work.
>
> I think it should be possible to do the conversion mechanically
> (eg with coccinelle). We can look at doing that later.
>
>> # MIGRATION
>>
>> Bus reset state migration is right now not handled.
>>
>> Regarding "migration-during-reset should Just Work, without necessarily
>> needing any specific changes for a device". The included patch define a vmsd
>> subsection which must to be added to every device main vmsd structure for
>> migration-during-reset of theses devices to work. I tried to find a way to
>> avoid that but really don't see how to achieve that.
>>
>> So in the current state of this series, migration can only be supported for
>> leaf device (in term of qdev/qbus) where we add the subsection.
>>
>> I'm not sure the migration is the problem here. I doubt a device will
>> support staying in reset state (which is a new feature) without manual
>> intervention. So migration of this unsupported state without any specific
>> change may not be a real asset.
>
> The approach I thought would be good turns out not to actually work,
> so I need to think a bit more about this.
>
> I think what I would like to achieve is "default to doing the right
> thing" -- ideally devices that add support for being held in reset
> should not need to manually do something to make the bus/device reset
> state be migrated properly. Otherwise we have an easy mistake to
> make (forgetting to do the necessary handling of migration) when
> adding a new device or making a device support being held in reset.
To have a default behavior, we need to somehow handle that in the
DeviceClass.
Ideally speaking, this subsection could be handled during the device
realization, when the vmsd is registered. But since we cannot
dynamically add a new subsection to the device vmsd structure, we have
to forge an extended new one (the same with additional subsection). It
seems technically possible but this feels like a bad hacky idea.
>
> Regarding "devices not supporting staying in reset state without
> manual intervention" do you mean that they might not correctly
> deal with incoming signals or guest register write attempts
> in the held-in-reset state? That's true, but I don't think it's
> too terrible. (In particular it's a bug that can be fixed without
> breaking migration compatibility; and it seems unlikely that guest
> software would be trying to read or write a device that it's put
> into reset.)
Yes it is what I meant.
>
> thanks
> -- PMM
>
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2019-06-27 9:14 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-04 16:25 [Qemu-devel] [RFC PATCH v2 00/12] Multi-phase reset Damien Hedde
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 01/12] Create Resettable QOM interface Damien Hedde
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 02/12] add device_legacy_reset function to do the transition with device_reset Damien Hedde
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 03/12] replace all occurences of device_reset by device_legacy_reset Damien Hedde
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 04/12] make Device and Bus Resettable Damien Hedde
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 05/12] Add function to control reset with gpio inputs Damien Hedde
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 06/12] add vmstate description for device reset state Damien Hedde
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 07/12] add doc about Resettable interface Damien Hedde
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 08/12] hw/misc/zynq_slcr: use standard register definition Damien Hedde
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 09/12] convert cadence_uart to 3-phases reset Damien Hedde
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 10/12] Convert zynq's slcr " Damien Hedde
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 11/12] Add uart reset support in zynq_slcr Damien Hedde
2019-06-04 16:25 ` [Qemu-devel] [RFC PATCH v2 12/12] Connect the uart reset gpios in the zynq platform Damien Hedde
2019-06-18 16:13 ` [Qemu-devel] [RFC PATCH v2 00/12] Multi-phase reset Peter Maydell
2019-06-27 9:13 ` Damien Hedde
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.