From 8915c118599d47c8d73f0b5982d28289c3ad797f Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Sun, 18 Feb 2024 15:57:11 +0900 Subject: hw/qdev: Pass bus argument to qdev_hotplug_allowed() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In preparation of checking the parent bus is hot(un)pluggable in a few commits, pass a 'bus' argument to qdev_hotplug_allowed(). Signed-off-by: Akihiko Odaki [PMD: Split from bigger patch, part 1/6] Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Igor Mammedov Message-Id: <20250110091908.64454-2-philmd@linaro.org> --- hw/core/qdev-hotplug.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'hw/core/qdev-hotplug.c') diff --git a/hw/core/qdev-hotplug.c b/hw/core/qdev-hotplug.c index d495d0e9c7..19fbb11a31 100644 --- a/hw/core/qdev-hotplug.c +++ b/hw/core/qdev-hotplug.c @@ -30,7 +30,7 @@ HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev) return NULL; } -bool qdev_hotplug_allowed(DeviceState *dev, Error **errp) +bool qdev_hotplug_allowed(DeviceState *dev, BusState *bus, Error **errp) { MachineState *machine; MachineClass *mc; -- cgit 1.4.1 From 206d602e9b73d9079449b44899d572624d57390a Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Sun, 18 Feb 2024 15:57:11 +0900 Subject: hw/qdev: Factor qdev_hotunplug_allowed() out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Factor qdev_hotunplug_allowed() out of qdev_unplug(). Start checking the device is not blocked. Signed-off-by: Akihiko Odaki [PMD: Split from bigger patch, part 2/6] Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Igor Mammedov Message-Id: <20250110091908.64454-3-philmd@linaro.org> --- hw/core/qdev-hotplug.c | 5 +++++ include/hw/qdev-core.h | 1 + system/qdev-monitor.c | 2 +- 3 files changed, 7 insertions(+), 1 deletion(-) (limited to 'hw/core/qdev-hotplug.c') diff --git a/hw/core/qdev-hotplug.c b/hw/core/qdev-hotplug.c index 19fbb11a31..dc35110e73 100644 --- a/hw/core/qdev-hotplug.c +++ b/hw/core/qdev-hotplug.c @@ -47,6 +47,11 @@ bool qdev_hotplug_allowed(DeviceState *dev, BusState *bus, Error **errp) return true; } +bool qdev_hotunplug_allowed(DeviceState *dev, Error **errp) +{ + return !qdev_unplug_blocked(dev, errp); +} + HotplugHandler *qdev_get_bus_hotplug_handler(DeviceState *dev) { if (dev->parent_bus) { diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h index 930b00fb09..530f3da702 100644 --- a/include/hw/qdev-core.h +++ b/include/hw/qdev-core.h @@ -541,6 +541,7 @@ void qdev_set_legacy_instance_id(DeviceState *dev, int alias_id, HotplugHandler *qdev_get_bus_hotplug_handler(DeviceState *dev); HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev); bool qdev_hotplug_allowed(DeviceState *dev, BusState *bus, Error **errp); +bool qdev_hotunplug_allowed(DeviceState *dev, Error **errp); /** * qdev_get_hotplug_handler() - Get handler responsible for device wiring diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c index 83388dc0c4..511d1aa83c 100644 --- a/system/qdev-monitor.c +++ b/system/qdev-monitor.c @@ -909,7 +909,7 @@ void qdev_unplug(DeviceState *dev, Error **errp) HotplugHandlerClass *hdc; Error *local_err = NULL; - if (qdev_unplug_blocked(dev, errp)) { + if (!qdev_hotunplug_allowed(dev, errp)) { return; } -- cgit 1.4.1 From f2694f1b1a1a38c586ee6f00e88b729828012d3a Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Sun, 18 Feb 2024 15:57:11 +0900 Subject: hw/qdev: Introduce qdev_hotplug_unplug_allowed_common() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce qdev_hotplug_unplug_allowed_common() to hold common code between checking hot-plug/unplug is allowed. Signed-off-by: Akihiko Odaki [PMD: Split from bigger patch, part 3/6] Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Igor Mammedov Message-Id: <20250110091908.64454-4-philmd@linaro.org> --- hw/core/qdev-hotplug.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) (limited to 'hw/core/qdev-hotplug.c') diff --git a/hw/core/qdev-hotplug.c b/hw/core/qdev-hotplug.c index dc35110e73..168d796474 100644 --- a/hw/core/qdev-hotplug.c +++ b/hw/core/qdev-hotplug.c @@ -30,12 +30,22 @@ HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev) return NULL; } +static bool qdev_hotplug_unplug_allowed_common(DeviceState *dev, BusState *bus, + Error **errp) +{ + return true; +} + bool qdev_hotplug_allowed(DeviceState *dev, BusState *bus, Error **errp) { MachineState *machine; MachineClass *mc; Object *m_obj = qdev_get_machine(); + if (!qdev_hotplug_unplug_allowed_common(dev, bus, errp)) { + return false; + } + if (object_dynamic_cast(m_obj, TYPE_MACHINE)) { machine = MACHINE(m_obj); mc = MACHINE_GET_CLASS(machine); @@ -49,7 +59,8 @@ bool qdev_hotplug_allowed(DeviceState *dev, BusState *bus, Error **errp) bool qdev_hotunplug_allowed(DeviceState *dev, Error **errp) { - return !qdev_unplug_blocked(dev, errp); + return !qdev_unplug_blocked(dev, errp) && + qdev_hotplug_unplug_allowed_common(dev, dev->parent_bus, errp); } HotplugHandler *qdev_get_bus_hotplug_handler(DeviceState *dev) -- cgit 1.4.1 From 1bff035be76cc30ff0fc4e8f0b0fbda84db7d1dc Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Sun, 18 Feb 2024 15:57:11 +0900 Subject: hw/qdev: Check DevClass::hotpluggable in hotplug_unplug_allowed_common MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Check the same code once in the common helper. Signed-off-by: Akihiko Odaki [PMD: Split from bigger patch, part 4/6] Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Igor Mammedov Message-Id: <20250110091908.64454-5-philmd@linaro.org> --- hw/core/qdev-hotplug.c | 9 +++++++++ system/qdev-monitor.c | 10 +--------- 2 files changed, 10 insertions(+), 9 deletions(-) (limited to 'hw/core/qdev-hotplug.c') diff --git a/hw/core/qdev-hotplug.c b/hw/core/qdev-hotplug.c index 168d796474..1d77fffb5e 100644 --- a/hw/core/qdev-hotplug.c +++ b/hw/core/qdev-hotplug.c @@ -12,6 +12,7 @@ #include "qemu/osdep.h" #include "hw/qdev-core.h" #include "hw/boards.h" +#include "qapi/error.h" HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev) { @@ -33,6 +34,14 @@ HotplugHandler *qdev_get_machine_hotplug_handler(DeviceState *dev) static bool qdev_hotplug_unplug_allowed_common(DeviceState *dev, BusState *bus, Error **errp) { + DeviceClass *dc = DEVICE_GET_CLASS(dev); + + if (!dc->hotpluggable) { + error_setg(errp, "Device '%s' does not support hotplugging", + object_get_typename(OBJECT(dev))); + return false; + } + return true; } diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c index 511d1aa83c..81f747b38f 100644 --- a/system/qdev-monitor.c +++ b/system/qdev-monitor.c @@ -263,8 +263,7 @@ static DeviceClass *qdev_get_device_class(const char **driver, Error **errp) } dc = DEVICE_CLASS(oc); - if (!dc->user_creatable || - (phase_check(PHASE_MACHINE_READY) && !dc->hotpluggable)) { + if (!dc->user_creatable) { error_setg(errp, QERR_INVALID_PARAMETER_VALUE, "driver", "a pluggable device type"); return NULL; @@ -904,7 +903,6 @@ static DeviceState *find_device_state(const char *id, bool use_generic_error, void qdev_unplug(DeviceState *dev, Error **errp) { - DeviceClass *dc = DEVICE_GET_CLASS(dev); HotplugHandler *hotplug_ctrl; HotplugHandlerClass *hdc; Error *local_err = NULL; @@ -919,12 +917,6 @@ void qdev_unplug(DeviceState *dev, Error **errp) return; } - if (!dc->hotpluggable) { - error_setg(errp, "Device '%s' does not support hotplugging", - object_get_typename(OBJECT(dev))); - return; - } - if (migration_is_running() && !dev->allow_unplug_during_migration) { error_setg(errp, "device_del not allowed while migrating"); return; -- cgit 1.4.1 From ccaca8929d53b23e2f7acc45cc88d05fc0479a1b Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Sun, 18 Feb 2024 15:57:11 +0900 Subject: hw/qdev: Check qbus_is_hotpluggable in hotplug_unplug_allowed_common MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Check the same code once in the common helper. Signed-off-by: Akihiko Odaki [PMD: Split from bigger patch, part 5/6] Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Igor Mammedov Message-Id: <20250110091908.64454-6-philmd@linaro.org> --- hw/core/qdev-hotplug.c | 8 ++++++++ system/qdev-monitor.c | 11 ----------- 2 files changed, 8 insertions(+), 11 deletions(-) (limited to 'hw/core/qdev-hotplug.c') diff --git a/hw/core/qdev-hotplug.c b/hw/core/qdev-hotplug.c index 1d77fffb5e..f6422cd0e4 100644 --- a/hw/core/qdev-hotplug.c +++ b/hw/core/qdev-hotplug.c @@ -42,6 +42,14 @@ static bool qdev_hotplug_unplug_allowed_common(DeviceState *dev, BusState *bus, return false; } + if (bus) { + if (!qbus_is_hotpluggable(bus)) { + error_setg(errp, "Bus '%s' does not support hotplugging", + bus->name); + return false; + } + } + return true; } diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c index 81f747b38f..e27d25c585 100644 --- a/system/qdev-monitor.c +++ b/system/qdev-monitor.c @@ -675,11 +675,6 @@ DeviceState *qdev_device_add_from_qdict(const QDict *opts, return NULL; } - if (phase_check(PHASE_MACHINE_READY) && bus && !qbus_is_hotpluggable(bus)) { - error_setg(errp, "Bus '%s' does not support hotplugging", bus->name); - return NULL; - } - if (migration_is_running()) { error_setg(errp, "device_add not allowed while migrating"); return NULL; @@ -911,12 +906,6 @@ void qdev_unplug(DeviceState *dev, Error **errp) return; } - if (dev->parent_bus && !qbus_is_hotpluggable(dev->parent_bus)) { - error_setg(errp, "Bus '%s' does not support hotplugging", - dev->parent_bus->name); - return; - } - if (migration_is_running() && !dev->allow_unplug_during_migration) { error_setg(errp, "device_del not allowed while migrating"); return; -- cgit 1.4.1 From 937874a83d149a1b871a569d6abded3fdd302267 Mon Sep 17 00:00:00 2001 From: Akihiko Odaki Date: Sun, 18 Feb 2024 15:57:11 +0900 Subject: hw/qdev: Check machine_hotplug_handler in hotplug_unplug_allowed_common MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit 03fcbd9dc508 ("qdev: Check for the availability of a hotplug controller before adding a device") says: > The qdev_unplug() function contains a g_assert(hotplug_ctrl) > statement, so QEMU crashes when the user tries to device_add + > device_del a device that does not have a corresponding hotplug > controller. > The code in qdev_device_add() already checks whether the bus has a > proper hotplug controller, but for devices that do not have a > corresponding bus, here is no appropriate check available yet. In that > case we should check whether the machine itself provides a suitable > hotplug controller and refuse to plug the device if none is available. However, it forgot to add the corresponding check to qdev_unplug(). Check the machine hotplug handler once in the common qdev_hotplug_unplug_allowed_common() helper so both hotplug and hot-unplug path are covered. Fixes: 7716b8ca74 ("qdev: HotplugHandler: Add support for unplugging BUS-less devices") Signed-off-by: Akihiko Odaki [PMD: Split from bigger patch, part 6/6] Signed-off-by: Philippe Mathieu-Daudé Reviewed-by: Igor Mammedov Message-Id: <20250110091908.64454-7-philmd@linaro.org> --- hw/core/qdev-hotplug.c | 10 ++++++++++ system/qdev-monitor.c | 14 +++----------- 2 files changed, 13 insertions(+), 11 deletions(-) (limited to 'hw/core/qdev-hotplug.c') diff --git a/hw/core/qdev-hotplug.c b/hw/core/qdev-hotplug.c index f6422cd0e4..ff176dc1bb 100644 --- a/hw/core/qdev-hotplug.c +++ b/hw/core/qdev-hotplug.c @@ -48,6 +48,16 @@ static bool qdev_hotplug_unplug_allowed_common(DeviceState *dev, BusState *bus, bus->name); return false; } + } else { + if (!qdev_get_machine_hotplug_handler(dev)) { + /* + * No bus, no machine hotplug handler --> device is not hotpluggable + */ + error_setg(errp, + "Device '%s' can not be hotplugged on this machine", + object_get_typename(OBJECT(dev))); + return false; + } } return true; diff --git a/system/qdev-monitor.c b/system/qdev-monitor.c index e27d25c585..861c25c855 100644 --- a/system/qdev-monitor.c +++ b/system/qdev-monitor.c @@ -684,17 +684,9 @@ 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 (!qdev_hotplug_allowed(dev, bus, errp)) { - goto err_del_dev; - } - - if (!bus && !qdev_get_machine_hotplug_handler(dev)) { - /* No bus, no machine hotplug handler --> device is not hotpluggable */ - error_setg(errp, "Device '%s' can not be hotplugged on this machine", - driver); - goto err_del_dev; - } + if (phase_check(PHASE_MACHINE_READY) && + !qdev_hotplug_allowed(dev, bus, errp)) { + goto err_del_dev; } /* -- cgit 1.4.1