All of lore.kernel.org
 help / color / mirror / Atom feed
From: Damien Hedde <damien.hedde@greensocs.com>
To: qemu-devel@nongnu.org
Cc: mark.burton@greensocs.com, edgari@xilinx.com,
	"Damien Hedde" <damien.hedde@greensocs.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	"Daniel P. Berrangé" <berrange@redhat.com>,
	"Eduardo Habkost" <eduardo@habkost.net>
Subject: [PATCH v5 5/6] RFC qapi/device_add: handle the rom_order_override when cold-plugging
Date: Thu, 19 May 2022 17:34:01 +0200	[thread overview]
Message-ID: <20220519153402.41540-6-damien.hedde@greensocs.com> (raw)
In-Reply-To: <20220519153402.41540-1-damien.hedde@greensocs.com>

rom_set_order_override() and rom_reset_order_override() were called
in qemu_create_cli_devices() to set the rom_order_override value
once and for all when creating the devices added on CLI.

Unfortunately this won't work with qapi commands.

Move the calls inside device_add so that it will be done in every
case:
+ CLI option: -device
+ QAPI command: device_add

rom_[set|reset]_order_override() are implemented in hw/core/loader.c
They either do nothing or call fw_cfg_[set|reset]_order_override().
The later functions are implemented in hw/nvram/fw_cfg.c and only
change an integer value of a "global" variable.
In consequence, there are no complex side effects involved and we can
safely move them from outside the -device option loop to the inner
function.

Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---

I see 2 other ways to handle this:

1. Adding a new option to device_add.

We could add a new boolean (_rom_order_override_ for example) option
tot he qapi command. This flag would then be forced to "true" when
handling "-device" parameter from CLI.
The flag default could be:
- always false
- false on hot-plug, true on cold-plug.

2. Adding a new qapi command
We could add one or two commands to do the
rom_[set|reset]_order_override() operation.
---
 softmmu/qdev-monitor.c | 11 +++++++++++
 softmmu/vl.c           |  2 --
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
index d68ef883b5..7cbee2b0d8 100644
--- a/softmmu/qdev-monitor.c
+++ b/softmmu/qdev-monitor.c
@@ -43,6 +43,7 @@
 #include "hw/qdev-properties.h"
 #include "hw/clock.h"
 #include "hw/boards.h"
+#include "hw/loader.h"
 
 /*
  * Aliases were a bad idea from the start.  Let's keep them
@@ -673,6 +674,10 @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts,
         return NULL;
     }
 
+    if (!is_hotplug) {
+        rom_set_order_override(FW_CFG_ORDER_OVERRIDE_DEVICE);
+    }
+
     /* create device */
     dev = qdev_new(driver);
 
@@ -714,6 +719,9 @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts,
     if (!qdev_realize(DEVICE(dev), bus, errp)) {
         goto err_del_dev;
     }
+    if (!is_hotplug) {
+        rom_reset_order_override();
+    }
     return dev;
 
 err_del_dev:
@@ -721,6 +729,9 @@ err_del_dev:
         object_unparent(OBJECT(dev));
         object_unref(OBJECT(dev));
     }
+    if (!is_hotplug) {
+        rom_reset_order_override();
+    }
     return NULL;
 }
 
diff --git a/softmmu/vl.c b/softmmu/vl.c
index ea15e37973..5a0d54b595 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2635,7 +2635,6 @@ static void qemu_create_cli_devices(void)
     }
 
     /* init generic devices */
-    rom_set_order_override(FW_CFG_ORDER_OVERRIDE_DEVICE);
     qemu_opts_foreach(qemu_find_opts("device"),
                       device_init_func, NULL, &error_fatal);
     QTAILQ_FOREACH(opt, &device_opts, next) {
@@ -2652,7 +2651,6 @@ static void qemu_create_cli_devices(void)
         object_unref(OBJECT(dev));
         loc_pop(&opt->loc);
     }
-    rom_reset_order_override();
 }
 
 static void qemu_machine_creation_done(void)
-- 
2.36.1



  parent reply	other threads:[~2022-05-19 15:49 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-19 15:33 [PATCH v5 0/6] QAPI support for device cold-plug Damien Hedde
2022-05-19 15:33 ` [PATCH v5 1/6] machine: add phase_get() and document phase_check()/advance() Damien Hedde
2022-05-24 19:56   ` Jim Shu
2022-05-19 15:33 ` [PATCH v5 2/6] machine&vl: introduce phase_until() to handle phase transitions Damien Hedde
2022-05-24 19:56   ` Jim Shu
2022-05-19 15:33 ` [PATCH v5 3/6] vl: support machine-initialized target in phase_until() Damien Hedde
2022-05-24 19:56   ` Jim Shu
2022-05-19 15:34 ` [PATCH v5 4/6] qapi/device_add: compute is_hotplug flag Damien Hedde
2022-05-24 19:57   ` Jim Shu
2022-05-19 15:34 ` Damien Hedde [this message]
2022-05-19 15:34 ` [PATCH v5 6/6] qapi/device_add: Allow execution in machine initialized phase Damien Hedde
2022-05-24 19:58   ` Jim Shu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220519153402.41540-6-damien.hedde@greensocs.com \
    --to=damien.hedde@greensocs.com \
    --cc=berrange@redhat.com \
    --cc=edgari@xilinx.com \
    --cc=eduardo@habkost.net \
    --cc=mark.burton@greensocs.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.