* [PATCH v5 1/6] machine: add phase_get() and document phase_check()/advance()
2022-05-19 15:33 [PATCH v5 0/6] QAPI support for device cold-plug Damien Hedde
@ 2022-05-19 15:33 ` 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
` (4 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Damien Hedde @ 2022-05-19 15:33 UTC (permalink / raw)
To: qemu-devel
Cc: mark.burton, edgari, Damien Hedde, Philippe Mathieu-Daudé,
Paolo Bonzini, Daniel P. Berrangé,
Eduardo Habkost
phase_get() returns the current phase, we'll use it in next
commit.
Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
include/hw/qdev-core.h | 19 +++++++++++++++++++
hw/core/qdev.c | 5 +++++
2 files changed, 24 insertions(+)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 92c3d65208..e29c705b74 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -887,7 +887,26 @@ typedef enum MachineInitPhase {
PHASE_MACHINE_READY,
} MachineInitPhase;
+/*
+ * phase_get:
+ * Returns the current phase
+ */
+MachineInitPhase phase_get(void);
+
+/**
+ * phase_check:
+ * Test if current phase is at least @phase.
+ *
+ * Returns true if this is the case.
+ */
extern bool phase_check(MachineInitPhase phase);
+
+/**
+ * @phase_advance:
+ * Update the current phase to @phase.
+ *
+ * Must only be used to make a single phase step.
+ */
extern void phase_advance(MachineInitPhase phase);
#endif
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 84f3019440..632dc0a4be 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -910,6 +910,11 @@ Object *qdev_get_machine(void)
static MachineInitPhase machine_phase;
+MachineInitPhase phase_get(void)
+{
+ return machine_phase;
+}
+
bool phase_check(MachineInitPhase phase)
{
return machine_phase >= phase;
--
2.36.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v5 1/6] machine: add phase_get() and document phase_check()/advance()
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
0 siblings, 0 replies; 12+ messages in thread
From: Jim Shu @ 2022-05-24 19:56 UTC (permalink / raw)
To: Damien Hedde
Cc: qemu-devel@nongnu.org Developers, mark.burton, edgari,
Philippe Mathieu-Daudé,
Paolo Bonzini, Daniel P. Berrangé,
Eduardo Habkost
[-- Attachment #1: Type: text/plain, Size: 1691 bytes --]
Tested-by: Jim Shu <jim.shu@sifive.com>
On Thu, May 19, 2022 at 11:41 PM Damien Hedde <damien.hedde@greensocs.com>
wrote:
> phase_get() returns the current phase, we'll use it in next
> commit.
>
> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> include/hw/qdev-core.h | 19 +++++++++++++++++++
> hw/core/qdev.c | 5 +++++
> 2 files changed, 24 insertions(+)
>
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index 92c3d65208..e29c705b74 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -887,7 +887,26 @@ typedef enum MachineInitPhase {
> PHASE_MACHINE_READY,
> } MachineInitPhase;
>
> +/*
> + * phase_get:
> + * Returns the current phase
> + */
> +MachineInitPhase phase_get(void);
> +
> +/**
> + * phase_check:
> + * Test if current phase is at least @phase.
> + *
> + * Returns true if this is the case.
> + */
> extern bool phase_check(MachineInitPhase phase);
> +
> +/**
> + * @phase_advance:
> + * Update the current phase to @phase.
> + *
> + * Must only be used to make a single phase step.
> + */
> extern void phase_advance(MachineInitPhase phase);
>
> #endif
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index 84f3019440..632dc0a4be 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -910,6 +910,11 @@ Object *qdev_get_machine(void)
>
> static MachineInitPhase machine_phase;
>
> +MachineInitPhase phase_get(void)
> +{
> + return machine_phase;
> +}
> +
> bool phase_check(MachineInitPhase phase)
> {
> return machine_phase >= phase;
> --
> 2.36.1
>
>
>
[-- Attachment #2: Type: text/html, Size: 2342 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v5 2/6] machine&vl: introduce phase_until() to handle phase transitions
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-19 15:33 ` 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
` (3 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Damien Hedde @ 2022-05-19 15:33 UTC (permalink / raw)
To: qemu-devel
Cc: mark.burton, edgari, Damien Hedde, Paolo Bonzini,
Daniel P. Berrangé,
Eduardo Habkost
phase_until() is implemented in vl.c and is meant to be used
to make startup progress up to a specified phase being reached().
At this point, no behavior change is introduced: phase_until()
only supports a single double transition corresponding
to the functionality of qmp_exit_preconfig():
+ accel-created -> machine-initialized -> machine-ready
As a result qmp_exit_preconfig() now uses phase_until().
This commit is a preparation to support cold plugging a device
using qapi command (which will be introduced in a following commit).
For this we need fine grain control of the phase.
Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
v5:
+ refactor to avoid indentation change
---
include/hw/qdev-core.h | 14 +++++++++++++
softmmu/vl.c | 46 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 60 insertions(+)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index e29c705b74..5f73d06408 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -909,4 +909,18 @@ extern bool phase_check(MachineInitPhase phase);
*/
extern void phase_advance(MachineInitPhase phase);
+/**
+ * @phase_until:
+ * @phase: the target phase
+ * @errp: error report
+ *
+ * Make the machine init progress until the target phase is reached.
+ *
+ * Its is a no-op is the target phase is the current or an earlier
+ * phase.
+ *
+ * Returns true in case of success.
+ */
+extern bool phase_until(MachineInitPhase phase, Error **errp);
+
#endif
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 84a31eba76..7f8d15b5b8 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2702,11 +2702,17 @@ void qmp_x_exit_preconfig(Error **errp)
error_setg(errp, "The command is permitted only before machine initialization");
return;
}
+ phase_until(PHASE_MACHINE_READY, errp);
+}
+static void qemu_phase_ready(Error **errp)
+{
qemu_init_board();
+ /* phase is now PHASE_MACHINE_INITIALIZED. */
qemu_create_cli_devices();
cxl_fixed_memory_window_link_targets(errp);
qemu_machine_creation_done();
+ /* Phase is now PHASE_MACHINE_READY. */
if (loadvm) {
load_snapshot(loadvm, NULL, false, NULL, &error_fatal);
@@ -2729,6 +2735,46 @@ void qmp_x_exit_preconfig(Error **errp)
}
}
+bool phase_until(MachineInitPhase phase, Error **errp)
+{
+ ERRP_GUARD();
+ if (!phase_check(PHASE_ACCEL_CREATED)) {
+ error_setg(errp, "Phase transition is not supported until accelerator"
+ " is created");
+ return false;
+ }
+
+ while (!phase_check(phase)) {
+ MachineInitPhase cur_phase = phase_get();
+
+ switch (cur_phase) {
+ case PHASE_ACCEL_CREATED:
+ qemu_phase_ready(errp);
+ break;
+
+ default:
+ /*
+ * If we end up here, it is because we miss a case above.
+ */
+ error_setg(&error_abort, "Requested phase transition is not"
+ " implemented");
+ return false;
+ }
+
+ if (*errp) {
+ return false;
+ }
+
+ /*
+ * Ensure we made some progress.
+ * With the default case above, it should be enough to prevent
+ * any infinite loop.
+ */
+ assert(cur_phase < phase_get());
+ }
+ return true;
+}
+
void qemu_init(int argc, char **argv, char **envp)
{
QemuOpts *opts;
--
2.36.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v5 2/6] machine&vl: introduce phase_until() to handle phase transitions
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
0 siblings, 0 replies; 12+ messages in thread
From: Jim Shu @ 2022-05-24 19:56 UTC (permalink / raw)
To: Damien Hedde
Cc: qemu-devel@nongnu.org Developers, mark.burton, edgari,
Paolo Bonzini, Daniel P. Berrangé,
Eduardo Habkost
[-- Attachment #1: Type: text/plain, Size: 3781 bytes --]
Tested-by: Jim Shu <jim.shu@sifive.com>
On Thu, May 19, 2022 at 11:41 PM Damien Hedde <damien.hedde@greensocs.com>
wrote:
> phase_until() is implemented in vl.c and is meant to be used
> to make startup progress up to a specified phase being reached().
> At this point, no behavior change is introduced: phase_until()
> only supports a single double transition corresponding
> to the functionality of qmp_exit_preconfig():
> + accel-created -> machine-initialized -> machine-ready
>
> As a result qmp_exit_preconfig() now uses phase_until().
>
> This commit is a preparation to support cold plugging a device
> using qapi command (which will be introduced in a following commit).
> For this we need fine grain control of the phase.
>
> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
> ---
>
> v5:
> + refactor to avoid indentation change
> ---
> include/hw/qdev-core.h | 14 +++++++++++++
> softmmu/vl.c | 46 ++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 60 insertions(+)
>
> diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
> index e29c705b74..5f73d06408 100644
> --- a/include/hw/qdev-core.h
> +++ b/include/hw/qdev-core.h
> @@ -909,4 +909,18 @@ extern bool phase_check(MachineInitPhase phase);
> */
> extern void phase_advance(MachineInitPhase phase);
>
> +/**
> + * @phase_until:
> + * @phase: the target phase
> + * @errp: error report
> + *
> + * Make the machine init progress until the target phase is reached.
> + *
> + * Its is a no-op is the target phase is the current or an earlier
> + * phase.
> + *
> + * Returns true in case of success.
> + */
> +extern bool phase_until(MachineInitPhase phase, Error **errp);
> +
> #endif
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 84a31eba76..7f8d15b5b8 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -2702,11 +2702,17 @@ void qmp_x_exit_preconfig(Error **errp)
> error_setg(errp, "The command is permitted only before machine
> initialization");
> return;
> }
> + phase_until(PHASE_MACHINE_READY, errp);
> +}
>
> +static void qemu_phase_ready(Error **errp)
> +{
> qemu_init_board();
> + /* phase is now PHASE_MACHINE_INITIALIZED. */
> qemu_create_cli_devices();
> cxl_fixed_memory_window_link_targets(errp);
> qemu_machine_creation_done();
> + /* Phase is now PHASE_MACHINE_READY. */
>
> if (loadvm) {
> load_snapshot(loadvm, NULL, false, NULL, &error_fatal);
> @@ -2729,6 +2735,46 @@ void qmp_x_exit_preconfig(Error **errp)
> }
> }
>
> +bool phase_until(MachineInitPhase phase, Error **errp)
> +{
> + ERRP_GUARD();
> + if (!phase_check(PHASE_ACCEL_CREATED)) {
> + error_setg(errp, "Phase transition is not supported until
> accelerator"
> + " is created");
> + return false;
> + }
> +
> + while (!phase_check(phase)) {
> + MachineInitPhase cur_phase = phase_get();
> +
> + switch (cur_phase) {
> + case PHASE_ACCEL_CREATED:
> + qemu_phase_ready(errp);
> + break;
> +
> + default:
> + /*
> + * If we end up here, it is because we miss a case above.
> + */
> + error_setg(&error_abort, "Requested phase transition is not"
> + " implemented");
> + return false;
> + }
> +
> + if (*errp) {
> + return false;
> + }
> +
> + /*
> + * Ensure we made some progress.
> + * With the default case above, it should be enough to prevent
> + * any infinite loop.
> + */
> + assert(cur_phase < phase_get());
> + }
> + return true;
> +}
> +
> void qemu_init(int argc, char **argv, char **envp)
> {
> QemuOpts *opts;
> --
> 2.36.1
>
>
>
[-- Attachment #2: Type: text/html, Size: 4842 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v5 3/6] vl: support machine-initialized target in phase_until()
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-19 15:33 ` [PATCH v5 2/6] machine&vl: introduce phase_until() to handle phase transitions Damien Hedde
@ 2022-05-19 15:33 ` 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
` (2 subsequent siblings)
5 siblings, 1 reply; 12+ messages in thread
From: Damien Hedde @ 2022-05-19 15:33 UTC (permalink / raw)
To: qemu-devel
Cc: mark.burton, edgari, Damien Hedde, Philippe Mathieu-Daudé,
Paolo Bonzini
phase_until() now supports the following transitions:
+ accel-created -> machine-initialized
+ machine-initialized -> machine-ready
As a consequence we can now support the use of qmp_exit_preconfig()
from phases _accel-created_ and _machine-initialized_.
This commit is a preparation to support cold plugging a device
using qapi (which will be introduced in a following commit). For this
we need fine grain control of the phase.
Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
v5: update due to refactor of previous commit
---
softmmu/vl.c | 26 +++++++++++++++++++++-----
1 file changed, 21 insertions(+), 5 deletions(-)
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 7f8d15b5b8..ea15e37973 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2698,8 +2698,9 @@ static void qemu_machine_creation_done(void)
void qmp_x_exit_preconfig(Error **errp)
{
- if (phase_check(PHASE_MACHINE_INITIALIZED)) {
- error_setg(errp, "The command is permitted only before machine initialization");
+ if (phase_check(PHASE_MACHINE_READY)) {
+ error_setg(errp, "The command is permitted only before"
+ " machine is ready");
return;
}
phase_until(PHASE_MACHINE_READY, errp);
@@ -2707,9 +2708,6 @@ void qmp_x_exit_preconfig(Error **errp)
static void qemu_phase_ready(Error **errp)
{
- qemu_init_board();
- /* phase is now PHASE_MACHINE_INITIALIZED. */
- qemu_create_cli_devices();
cxl_fixed_memory_window_link_targets(errp);
qemu_machine_creation_done();
/* Phase is now PHASE_MACHINE_READY. */
@@ -2749,6 +2747,24 @@ bool phase_until(MachineInitPhase phase, Error **errp)
switch (cur_phase) {
case PHASE_ACCEL_CREATED:
+ qemu_init_board();
+ /* Phase is now PHASE_MACHINE_INITIALIZED. */
+ /*
+ * Handle CLI devices now in order leave this case in a state
+ * where we can cold plug devices with QMP. The following call
+ * handles the CLI options:
+ * + -fw_cfg (has side effects on device cold plug)
+ * + -device
+ */
+ qemu_create_cli_devices();
+ /*
+ * At this point all CLI options are handled apart:
+ * + -S (autostart)
+ * + -incoming
+ */
+ break;
+
+ case PHASE_MACHINE_INITIALIZED:
qemu_phase_ready(errp);
break;
--
2.36.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v5 3/6] vl: support machine-initialized target in phase_until()
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
0 siblings, 0 replies; 12+ messages in thread
From: Jim Shu @ 2022-05-24 19:56 UTC (permalink / raw)
To: Damien Hedde
Cc: qemu-devel@nongnu.org Developers, mark.burton, edgari,
Philippe Mathieu-Daudé,
Paolo Bonzini
[-- Attachment #1: Type: text/plain, Size: 2872 bytes --]
Tested-by: Jim Shu <jim.shu@sifive.com>
On Thu, May 19, 2022 at 11:36 PM Damien Hedde <damien.hedde@greensocs.com>
wrote:
> phase_until() now supports the following transitions:
> + accel-created -> machine-initialized
> + machine-initialized -> machine-ready
>
> As a consequence we can now support the use of qmp_exit_preconfig()
> from phases _accel-created_ and _machine-initialized_.
>
> This commit is a preparation to support cold plugging a device
> using qapi (which will be introduced in a following commit). For this
> we need fine grain control of the phase.
>
> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>
> v5: update due to refactor of previous commit
> ---
> softmmu/vl.c | 26 +++++++++++++++++++++-----
> 1 file changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 7f8d15b5b8..ea15e37973 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -2698,8 +2698,9 @@ static void qemu_machine_creation_done(void)
>
> void qmp_x_exit_preconfig(Error **errp)
> {
> - if (phase_check(PHASE_MACHINE_INITIALIZED)) {
> - error_setg(errp, "The command is permitted only before machine
> initialization");
> + if (phase_check(PHASE_MACHINE_READY)) {
> + error_setg(errp, "The command is permitted only before"
> + " machine is ready");
> return;
> }
> phase_until(PHASE_MACHINE_READY, errp);
> @@ -2707,9 +2708,6 @@ void qmp_x_exit_preconfig(Error **errp)
>
> static void qemu_phase_ready(Error **errp)
> {
> - qemu_init_board();
> - /* phase is now PHASE_MACHINE_INITIALIZED. */
> - qemu_create_cli_devices();
> cxl_fixed_memory_window_link_targets(errp);
> qemu_machine_creation_done();
> /* Phase is now PHASE_MACHINE_READY. */
> @@ -2749,6 +2747,24 @@ bool phase_until(MachineInitPhase phase, Error
> **errp)
>
> switch (cur_phase) {
> case PHASE_ACCEL_CREATED:
> + qemu_init_board();
> + /* Phase is now PHASE_MACHINE_INITIALIZED. */
> + /*
> + * Handle CLI devices now in order leave this case in a state
> + * where we can cold plug devices with QMP. The following call
> + * handles the CLI options:
> + * + -fw_cfg (has side effects on device cold plug)
> + * + -device
> + */
> + qemu_create_cli_devices();
> + /*
> + * At this point all CLI options are handled apart:
> + * + -S (autostart)
> + * + -incoming
> + */
> + break;
> +
> + case PHASE_MACHINE_INITIALIZED:
> qemu_phase_ready(errp);
> break;
>
> --
> 2.36.1
>
>
>
[-- Attachment #2: Type: text/html, Size: 3728 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v5 4/6] qapi/device_add: compute is_hotplug flag
2022-05-19 15:33 [PATCH v5 0/6] QAPI support for device cold-plug Damien Hedde
` (2 preceding siblings ...)
2022-05-19 15:33 ` [PATCH v5 3/6] vl: support machine-initialized target in phase_until() Damien Hedde
@ 2022-05-19 15:34 ` Damien Hedde
2022-05-24 19:57 ` Jim Shu
2022-05-19 15:34 ` [PATCH v5 5/6] RFC qapi/device_add: handle the rom_order_override when cold-plugging Damien Hedde
2022-05-19 15:34 ` [PATCH v5 6/6] qapi/device_add: Allow execution in machine initialized phase Damien Hedde
5 siblings, 1 reply; 12+ messages in thread
From: Damien Hedde @ 2022-05-19 15:34 UTC (permalink / raw)
To: qemu-devel
Cc: mark.burton, edgari, Damien Hedde, Philippe Mathieu-Daudé,
Paolo Bonzini, Daniel P. Berrangé,
Eduardo Habkost
Instead of checking the phase everytime, just store the result
in a flag. We will use more of it in the following commit.
Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
softmmu/qdev-monitor.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
index 12fe60c467..d68ef883b5 100644
--- a/softmmu/qdev-monitor.c
+++ b/softmmu/qdev-monitor.c
@@ -619,6 +619,7 @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts,
char *id;
DeviceState *dev = NULL;
BusState *bus = NULL;
+ bool is_hotplug = phase_check(PHASE_MACHINE_READY);
driver = qdict_get_try_str(opts, "driver");
if (!driver) {
@@ -662,7 +663,7 @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts,
return NULL;
}
- if (phase_check(PHASE_MACHINE_READY) && bus && !qbus_is_hotpluggable(bus)) {
+ if (is_hotplug && bus && !qbus_is_hotpluggable(bus)) {
error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name);
return NULL;
}
@@ -676,7 +677,7 @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts,
dev = qdev_new(driver);
/* Check whether the hotplug is allowed by the machine */
- if (phase_check(PHASE_MACHINE_READY)) {
+ if (is_hotplug) {
if (!qdev_hotplug_allowed(dev, errp)) {
goto err_del_dev;
}
--
2.36.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v5 4/6] qapi/device_add: compute is_hotplug flag
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
0 siblings, 0 replies; 12+ messages in thread
From: Jim Shu @ 2022-05-24 19:57 UTC (permalink / raw)
To: Damien Hedde
Cc: qemu-devel@nongnu.org Developers, mark.burton, edgari,
Philippe Mathieu-Daudé,
Paolo Bonzini, Daniel P. Berrangé,
Eduardo Habkost
[-- Attachment #1: Type: text/plain, Size: 1704 bytes --]
Tested-by: Jim Shu <jim.shu@sifive.com>
On Thu, May 19, 2022 at 11:37 PM Damien Hedde <damien.hedde@greensocs.com>
wrote:
> Instead of checking the phase everytime, just store the result
> in a flag. We will use more of it in the following commit.
>
> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> softmmu/qdev-monitor.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
> index 12fe60c467..d68ef883b5 100644
> --- a/softmmu/qdev-monitor.c
> +++ b/softmmu/qdev-monitor.c
> @@ -619,6 +619,7 @@ DeviceState *qdev_device_add_from_qdict(const QDict
> *opts,
> char *id;
> DeviceState *dev = NULL;
> BusState *bus = NULL;
> + bool is_hotplug = phase_check(PHASE_MACHINE_READY);
>
> driver = qdict_get_try_str(opts, "driver");
> if (!driver) {
> @@ -662,7 +663,7 @@ DeviceState *qdev_device_add_from_qdict(const QDict
> *opts,
> return NULL;
> }
>
> - if (phase_check(PHASE_MACHINE_READY) && bus &&
> !qbus_is_hotpluggable(bus)) {
> + if (is_hotplug && bus && !qbus_is_hotpluggable(bus)) {
> error_setg(errp, QERR_BUS_NO_HOTPLUG, bus->name);
> return NULL;
> }
> @@ -676,7 +677,7 @@ DeviceState *qdev_device_add_from_qdict(const QDict
> *opts,
> dev = qdev_new(driver);
>
> /* Check whether the hotplug is allowed by the machine */
> - if (phase_check(PHASE_MACHINE_READY)) {
> + if (is_hotplug) {
> if (!qdev_hotplug_allowed(dev, errp)) {
> goto err_del_dev;
> }
> --
> 2.36.1
>
>
>
[-- Attachment #2: Type: text/html, Size: 2391 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v5 5/6] RFC qapi/device_add: handle the rom_order_override when cold-plugging
2022-05-19 15:33 [PATCH v5 0/6] QAPI support for device cold-plug Damien Hedde
` (3 preceding siblings ...)
2022-05-19 15:34 ` [PATCH v5 4/6] qapi/device_add: compute is_hotplug flag Damien Hedde
@ 2022-05-19 15:34 ` Damien Hedde
2022-05-19 15:34 ` [PATCH v5 6/6] qapi/device_add: Allow execution in machine initialized phase Damien Hedde
5 siblings, 0 replies; 12+ messages in thread
From: Damien Hedde @ 2022-05-19 15:34 UTC (permalink / raw)
To: qemu-devel
Cc: mark.burton, edgari, Damien Hedde, Paolo Bonzini,
Daniel P. Berrangé,
Eduardo Habkost
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH v5 6/6] qapi/device_add: Allow execution in machine initialized phase
2022-05-19 15:33 [PATCH v5 0/6] QAPI support for device cold-plug Damien Hedde
` (4 preceding siblings ...)
2022-05-19 15:34 ` [PATCH v5 5/6] RFC qapi/device_add: handle the rom_order_override when cold-plugging Damien Hedde
@ 2022-05-19 15:34 ` Damien Hedde
2022-05-24 19:58 ` Jim Shu
5 siblings, 1 reply; 12+ messages in thread
From: Damien Hedde @ 2022-05-19 15:34 UTC (permalink / raw)
To: qemu-devel
Cc: mark.burton, edgari, Mirela Grujic, Damien Hedde,
Dr. David Alan Gilbert, Markus Armbruster, Eric Blake,
Paolo Bonzini, Daniel P. Berrangé,
Eduardo Habkost
From: Mirela Grujic <mirela.grujic@greensocs.com>
This commit allows to use the QMP command to add a cold-plugged
device like we can do with the CLI option -device.
Note: for device_add command in qdev.json adding the 'allow-preconfig'
option has no effect because the command appears to bypass QAPI (see
TODO at qapi/qdev.json:61). The option is added there solely to
document the intent.
For the same reason, the flags have to be explicitly set in
monitor_init_qmp_commands() when the device_add command is registered.
Signed-off-by: Mirela Grujic <mirela.grujic@greensocs.com>
Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
---
v4:
+ use phase_until()
+ add missing flag in hmp-commands.hx
---
qapi/qdev.json | 3 ++-
monitor/misc.c | 2 +-
softmmu/qdev-monitor.c | 4 ++++
hmp-commands.hx | 1 +
4 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/qapi/qdev.json b/qapi/qdev.json
index 26cd10106b..2e2de41499 100644
--- a/qapi/qdev.json
+++ b/qapi/qdev.json
@@ -77,7 +77,8 @@
{ 'command': 'device_add',
'data': {'driver': 'str', '*bus': 'str', '*id': 'str'},
'gen': false, # so we can get the additional arguments
- 'features': ['json-cli', 'json-cli-hotplug'] }
+ 'features': ['json-cli', 'json-cli-hotplug'],
+ 'allow-preconfig': true }
##
# @device_del:
diff --git a/monitor/misc.c b/monitor/misc.c
index 6c5bb82d3b..d3d413d70c 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -233,7 +233,7 @@ static void monitor_init_qmp_commands(void)
qmp_init_marshal(&qmp_commands);
qmp_register_command(&qmp_commands, "device_add",
- qmp_device_add, 0, 0);
+ qmp_device_add, QCO_ALLOW_PRECONFIG, 0);
QTAILQ_INIT(&qmp_cap_negotiation_commands);
qmp_register_command(&qmp_cap_negotiation_commands, "qmp_capabilities",
diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
index 7cbee2b0d8..c53f62be51 100644
--- a/softmmu/qdev-monitor.c
+++ b/softmmu/qdev-monitor.c
@@ -855,6 +855,10 @@ void qmp_device_add(QDict *qdict, QObject **ret_data, Error **errp)
QemuOpts *opts;
DeviceState *dev;
+ if (!phase_until(PHASE_MACHINE_INITIALIZED, errp)) {
+ return;
+ }
+
opts = qemu_opts_from_qdict(qemu_find_opts("device"), qdict, errp);
if (!opts) {
return;
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 03e6a73d1f..0091b8e2dd 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -672,6 +672,7 @@ ERST
.help = "add device, like -device on the command line",
.cmd = hmp_device_add,
.command_completion = device_add_completion,
+ .flags = "p",
},
SRST
--
2.36.1
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH v5 6/6] qapi/device_add: Allow execution in machine initialized phase
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
0 siblings, 0 replies; 12+ messages in thread
From: Jim Shu @ 2022-05-24 19:58 UTC (permalink / raw)
To: Damien Hedde
Cc: qemu-devel@nongnu.org Developers, mark.burton, edgari,
Mirela Grujic, Dr. David Alan Gilbert, Markus Armbruster,
Eric Blake, Paolo Bonzini, Daniel P. Berrangé,
Eduardo Habkost
[-- Attachment #1: Type: text/plain, Size: 2987 bytes --]
Tested-by: Jim Shu <jim.shu@sifive.com>
On Thu, May 19, 2022 at 11:37 PM Damien Hedde <damien.hedde@greensocs.com>
wrote:
> From: Mirela Grujic <mirela.grujic@greensocs.com>
>
> This commit allows to use the QMP command to add a cold-plugged
> device like we can do with the CLI option -device.
>
> Note: for device_add command in qdev.json adding the 'allow-preconfig'
> option has no effect because the command appears to bypass QAPI (see
> TODO at qapi/qdev.json:61). The option is added there solely to
> document the intent.
> For the same reason, the flags have to be explicitly set in
> monitor_init_qmp_commands() when the device_add command is registered.
>
> Signed-off-by: Mirela Grujic <mirela.grujic@greensocs.com>
> Signed-off-by: Damien Hedde <damien.hedde@greensocs.com>
> ---
>
> v4:
> + use phase_until()
> + add missing flag in hmp-commands.hx
> ---
> qapi/qdev.json | 3 ++-
> monitor/misc.c | 2 +-
> softmmu/qdev-monitor.c | 4 ++++
> hmp-commands.hx | 1 +
> 4 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/qapi/qdev.json b/qapi/qdev.json
> index 26cd10106b..2e2de41499 100644
> --- a/qapi/qdev.json
> +++ b/qapi/qdev.json
> @@ -77,7 +77,8 @@
> { 'command': 'device_add',
> 'data': {'driver': 'str', '*bus': 'str', '*id': 'str'},
> 'gen': false, # so we can get the additional arguments
> - 'features': ['json-cli', 'json-cli-hotplug'] }
> + 'features': ['json-cli', 'json-cli-hotplug'],
> + 'allow-preconfig': true }
>
> ##
> # @device_del:
> diff --git a/monitor/misc.c b/monitor/misc.c
> index 6c5bb82d3b..d3d413d70c 100644
> --- a/monitor/misc.c
> +++ b/monitor/misc.c
> @@ -233,7 +233,7 @@ static void monitor_init_qmp_commands(void)
> qmp_init_marshal(&qmp_commands);
>
> qmp_register_command(&qmp_commands, "device_add",
> - qmp_device_add, 0, 0);
> + qmp_device_add, QCO_ALLOW_PRECONFIG, 0);
>
> QTAILQ_INIT(&qmp_cap_negotiation_commands);
> qmp_register_command(&qmp_cap_negotiation_commands,
> "qmp_capabilities",
> diff --git a/softmmu/qdev-monitor.c b/softmmu/qdev-monitor.c
> index 7cbee2b0d8..c53f62be51 100644
> --- a/softmmu/qdev-monitor.c
> +++ b/softmmu/qdev-monitor.c
> @@ -855,6 +855,10 @@ void qmp_device_add(QDict *qdict, QObject **ret_data,
> Error **errp)
> QemuOpts *opts;
> DeviceState *dev;
>
> + if (!phase_until(PHASE_MACHINE_INITIALIZED, errp)) {
> + return;
> + }
> +
> opts = qemu_opts_from_qdict(qemu_find_opts("device"), qdict, errp);
> if (!opts) {
> return;
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 03e6a73d1f..0091b8e2dd 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -672,6 +672,7 @@ ERST
> .help = "add device, like -device on the command line",
> .cmd = hmp_device_add,
> .command_completion = device_add_completion,
> + .flags = "p",
> },
>
> SRST
> --
> 2.36.1
>
>
>
[-- Attachment #2: Type: text/html, Size: 4148 bytes --]
^ permalink raw reply [flat|nested] 12+ messages in thread