Hi Peter,
On 1/17/24 10:15, pet...@redhat.com wrote:
> From: Peter Xu <pet...@redhat.com>
>
> QEMU resets do not have a way to order reset hooks. Add one coarse grained
> reset stage so that some devices can be reset later than some others.
I would precise that the lowest stage has the highest priority and is
handled first.
>
> Signed-off-by: Peter Xu <pet...@redhat.com>
> ---
> include/sysemu/reset.h | 5 ++++
> hw/core/reset.c | 60 +++++++++++++++++++++++++++++++-----------
> 2 files changed, 49 insertions(+), 16 deletions(-)
>
> diff --git a/include/sysemu/reset.h b/include/sysemu/reset.h
> index 609e4d50c2..0de697ce9f 100644
> --- a/include/sysemu/reset.h
> +++ b/include/sysemu/reset.h
> @@ -5,9 +5,14 @@
>
> typedef void QEMUResetHandler(void *opaque);
>
> +#define QEMU_RESET_STAGES_N 2
> +
> void qemu_register_reset(QEMUResetHandler *func, void *opaque);
> +void qemu_register_reset_one(QEMUResetHandler *func, void *opaque,
> + bool skip_snap, int stage);
> void qemu_register_reset_nosnapshotload(QEMUResetHandler *func, void
> *opaque);
> void qemu_unregister_reset(QEMUResetHandler *func, void *opaque);
> +void qemu_unregister_reset_one(QEMUResetHandler *func, void *opaque, int
> stage);
> void qemu_devices_reset(ShutdownCause reason);
>
> #endif
> diff --git a/hw/core/reset.c b/hw/core/reset.c
> index 8cf60b2b09..a84c9bee84 100644
> --- a/hw/core/reset.c
> +++ b/hw/core/reset.c
> @@ -36,55 +36,83 @@ typedef struct QEMUResetEntry {
> bool skip_on_snapshot_load;
> } QEMUResetEntry;
>
> -static QTAILQ_HEAD(, QEMUResetEntry) reset_handlers =
> - QTAILQ_HEAD_INITIALIZER(reset_handlers);
> +typedef QTAILQ_HEAD(QEMUResetList, QEMUResetEntry) QEMUResetList;
> +static QEMUResetList reset_handlers[QEMU_RESET_STAGES_N];
>
> -static void qemu_register_reset_one(QEMUResetHandler *func, void *opaque,
> - bool skip_snap)
> +static void __attribute__((__constructor__)) qemu_reset_handlers_init(void)
> +{
> + QEMUResetList *head;
> + int i = 0;
nit: you may put the declarations within the block
> +
> + for (i = 0; i < QEMU_RESET_STAGES_N; i++) {
> + head = &reset_handlers[i];
> + QTAILQ_INIT(head);
> + }
> +}
> +
> +void qemu_register_reset_one(QEMUResetHandler *func, void *opaque,
> + bool skip_snap, int stage)
> {
> QEMUResetEntry *re = g_new0(QEMUResetEntry, 1);
> + QEMUResetList *head;
> +
> + assert(stage >= 0 && stage < QEMU_RESET_STAGES_N);
> + head = &reset_handlers[stage];
>
> re->func = func;
> re->opaque = opaque;
> re->skip_on_snapshot_load = skip_snap;
> - QTAILQ_INSERT_TAIL(&reset_handlers, re, entry);
> + QTAILQ_INSERT_TAIL(head, re, entry);
> }
>
> void qemu_register_reset(QEMUResetHandler *func, void *opaque)
> {
> - /* By default, do not skip during load of a snapshot */
Shouldn't the above comment stay since the statement is not affected by
this patch? Or remove it in previous patch?
> - qemu_register_reset_one(func, opaque, false);
> + qemu_register_reset_one(func, opaque, false, 0);
> }
>
> void qemu_register_reset_nosnapshotload(QEMUResetHandler *func, void *opaque)
> {
> - qemu_register_reset_one(func, opaque, true);
> + qemu_register_reset_one(func, opaque, true, 0);
> }
>
> -void qemu_unregister_reset(QEMUResetHandler *func, void *opaque)
> +void qemu_unregister_reset_one(QEMUResetHandler *func, void *opaque, int
> stage)
> {
> + QEMUResetList *head;
> QEMUResetEntry *re;
>
> - QTAILQ_FOREACH(re, &reset_handlers, entry) {
> + assert(stage >= 0 && stage < QEMU_RESET_STAGES_N);
> + head = &reset_handlers[stage];
> +
> + QTAILQ_FOREACH(re, head, entry) {
> if (re->func == func && re->opaque == opaque) {
> - QTAILQ_REMOVE(&reset_handlers, re, entry);
> + QTAILQ_REMOVE(head, re, entry);
> g_free(re);
> return;
> }
> }
> }
>
> +void qemu_unregister_reset(QEMUResetHandler *func, void *opaque)
> +{
> + qemu_unregister_reset_one(func, opaque, 0);
> +}
> +
> void qemu_devices_reset(ShutdownCause reason)
> {
> QEMUResetEntry *re, *nre;
> + QEMUResetList *head;
> + int stage;
>
> /* reset all devices */
> - QTAILQ_FOREACH_SAFE(re, &reset_handlers, entry, nre) {
> - if (reason == SHUTDOWN_CAUSE_SNAPSHOT_LOAD &&
> - re->skip_on_snapshot_load) {
> - continue;
> + for (stage = 0; stage < QEMU_RESET_STAGES_N; stage++) {
> + head = &reset_handlers[stage];
> + QTAILQ_FOREACH_SAFE(re, head, entry, nre) {
> + if (reason == SHUTDOWN_CAUSE_SNAPSHOT_LOAD &&
> + re->skip_on_snapshot_load) {
> + continue;
> + }
> + re->func(re->opaque);
> }
> - re->func(re->opaque);
> }
> }
>
Thanks
Eric
