* [Qemu-devel] [PATCH 1/9] define hotplug interface
2014-01-14 16:55 [Qemu-devel] [PATCH 0/9 v4] Refactor PCI/SHPC/PCIE hotplug to use a more generic hotplug API Igor Mammedov
@ 2014-01-14 16:55 ` Igor Mammedov
2014-01-14 16:55 ` [Qemu-devel] [PATCH 2/9] qdev: add to BusState "hotplug-handler" link Igor Mammedov
` (8 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Igor Mammedov @ 2014-01-14 16:55 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.crosthwaite, marcel.a, mst, aliguori, pbonzini, afaerber
Provide a generic hotplug interface for hotplug handlers.
Intended for replacing hotplug mechanism used by
PCI/PCIE/SHPC code and will be used for memory hotplug.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v3:
* amend commit description as requested by Peter Crosthwaite
* add <private|public> doc comments to type definitions
v2:
* s/device/handler/
* add hotplug_handler_plug/hotplug_handler_unplug API
v1:
it's scsi-bus like interface, but abstracted from bus altogether
since all current users care about in hotplug handlers, it's
hotplug device and hotplugged device and bus only serves
as a means to get access to hotplug device and it's callbacks.
---
hw/core/Makefile.objs | 1 +
hw/core/hotplug.c | 48 ++++++++++++++++++++++++++++++
include/hw/hotplug.h | 78 +++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 127 insertions(+), 0 deletions(-)
create mode 100644 hw/core/hotplug.c
create mode 100644 include/hw/hotplug.h
diff --git a/hw/core/Makefile.objs b/hw/core/Makefile.objs
index 950146c..9e324be 100644
--- a/hw/core/Makefile.objs
+++ b/hw/core/Makefile.objs
@@ -2,6 +2,7 @@
common-obj-y += qdev.o qdev-properties.o
# irq.o needed for qdev GPIO handling:
common-obj-y += irq.o
+common-obj-y += hotplug.o
common-obj-$(CONFIG_EMPTY_SLOT) += empty_slot.o
common-obj-$(CONFIG_XILINX_AXI) += stream.o
diff --git a/hw/core/hotplug.c b/hw/core/hotplug.c
new file mode 100644
index 0000000..5c3b5c9
--- /dev/null
+++ b/hw/core/hotplug.c
@@ -0,0 +1,48 @@
+/*
+ * Hotplug handler interface.
+ *
+ * Copyright (c) 2013 Red Hat Inc.
+ *
+ * Authors:
+ * Igor Mammedov <imammedo@redhat.com>,
+ *
+ * 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 "hw/hotplug.h"
+#include "qemu/module.h"
+
+void hotplug_handler_plug(HotplugHandler *plug_handler,
+ DeviceState *plugged_dev,
+ Error **errp)
+{
+ HotplugHandlerClass *hdc = HOTPLUG_HANDLER_GET_CLASS(plug_handler);
+
+ if (hdc->plug) {
+ hdc->plug(plug_handler, plugged_dev, errp);
+ }
+}
+
+void hotplug_handler_unplug(HotplugHandler *plug_handler,
+ DeviceState *plugged_dev,
+ Error **errp)
+{
+ HotplugHandlerClass *hdc = HOTPLUG_HANDLER_GET_CLASS(plug_handler);
+
+ if (hdc->unplug) {
+ hdc->unplug(plug_handler, plugged_dev, errp);
+ }
+}
+
+static const TypeInfo hotplug_handler_info = {
+ .name = TYPE_HOTPLUG_HANDLER,
+ .parent = TYPE_INTERFACE,
+ .class_size = sizeof(HotplugHandlerClass),
+};
+
+static void hotplug_handler_register_types(void)
+{
+ type_register_static(&hotplug_handler_info);
+}
+
+type_init(hotplug_handler_register_types)
diff --git a/include/hw/hotplug.h b/include/hw/hotplug.h
new file mode 100644
index 0000000..22ce92c
--- /dev/null
+++ b/include/hw/hotplug.h
@@ -0,0 +1,78 @@
+/*
+ * Hotplug handler interface.
+ *
+ * Copyright (c) 2013 Red Hat Inc.
+ *
+ * Authors:
+ * Igor Mammedov <imammedo@redhat.com>,
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+#ifndef HOTPLUG_H
+#define HOTPLUG_H
+
+#include "qom/object.h"
+#include "qemu/typedefs.h"
+
+#define TYPE_HOTPLUG_HANDLER "hotplug-handler"
+
+#define HOTPLUG_HANDLER_CLASS(klass) \
+ OBJECT_CLASS_CHECK(HotplugHandlerClass, (klass), TYPE_HOTPLUG_HANDLER)
+#define HOTPLUG_HANDLER_GET_CLASS(obj) \
+ OBJECT_GET_CLASS(HotplugHandlerClass, (obj), TYPE_HOTPLUG_HANDLER)
+#define HOTPLUG_HANDLER(obj) \
+ INTERFACE_CHECK(HotplugHandler, (obj), TYPE_HOTPLUG_HANDLER)
+
+
+typedef struct HotplugHandler {
+ /* <private> */
+ Object Parent;
+} HotplugHandler;
+
+/**
+ * hotplug_fn:
+ * @plug_handler: a device performing plug/uplug action
+ * @plugged_dev: a device that has been (un)plugged
+ * @errp: returns an error if this function fails
+ */
+typedef void (*hotplug_fn)(HotplugHandler *plug_handler,
+ DeviceState *plugged_dev, Error **errp);
+
+/**
+ * HotplugDeviceClass:
+ *
+ * Interface to be implemented by a device performing
+ * hardware (un)plug functions.
+ *
+ * @parent: Opaque parent interface.
+ * @plug: plug callback.
+ * @unplug: unplug callback.
+ */
+typedef struct HotplugHandlerClass {
+ /* <private> */
+ InterfaceClass parent;
+
+ /* <public> */
+ hotplug_fn plug;
+ hotplug_fn unplug;
+} HotplugHandlerClass;
+
+/**
+ * hotplug_handler_plug:
+ *
+ * Call #HotplugHandlerClass.plug callback of @plug_handler.
+ */
+void hotplug_handler_plug(HotplugHandler *plug_handler,
+ DeviceState *plugged_dev,
+ Error **errp);
+
+/**
+ * hotplug_handler_unplug:
+ *
+ * Call #HotplugHandlerClass.unplug callback of @plug_handler.
+ */
+void hotplug_handler_unplug(HotplugHandler *plug_handler,
+ DeviceState *plugged_dev,
+ Error **errp);
+#endif
--
1.7.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH 2/9] qdev: add to BusState "hotplug-handler" link
2014-01-14 16:55 [Qemu-devel] [PATCH 0/9 v4] Refactor PCI/SHPC/PCIE hotplug to use a more generic hotplug API Igor Mammedov
2014-01-14 16:55 ` [Qemu-devel] [PATCH 1/9] define hotplug interface Igor Mammedov
@ 2014-01-14 16:55 ` Igor Mammedov
2014-01-14 16:55 ` [Qemu-devel] [PATCH 3/9] qdev: add "hotpluggable" property to Device Igor Mammedov
` (7 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Igor Mammedov @ 2014-01-14 16:55 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.crosthwaite, marcel.a, mst, aliguori, pbonzini, afaerber
It will allow to reuse field with different BUSes, reducing code duplication.
Field is intended fot replacing 'hotplug_qdev' field in PCIBus and also
will allow to avoid adding equivalent field to DimmBus with possiblitity
to refactor other BUSes to use it instead of custom field.
In addition once all users of allow_hotplug field are converted to new
API, link could replace allow_hotplug in qdev hotplug code.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
hw/core/qdev.c | 4 ++++
include/hw/qdev-core.h | 5 +++++
2 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index 82a9123..c9f0c33 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -32,6 +32,7 @@
#include "qapi/visitor.h"
#include "qapi/qmp/qjson.h"
#include "monitor/monitor.h"
+#include "hw/hotplug.h"
int qdev_hotplug = 0;
static bool qdev_hot_added = false;
@@ -870,6 +871,9 @@ static void qbus_initfn(Object *obj)
BusState *bus = BUS(obj);
QTAILQ_INIT(&bus->children);
+ object_property_add_link(obj, QDEV_HOTPLUG_HANDLER_PROPERTY,
+ TYPE_HOTPLUG_HANDLER,
+ (Object **)&bus->hotplug_handler, NULL);
}
static char *default_bus_get_fw_dev_path(DeviceState *dev)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 2c4f140..58a5c69 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -8,6 +8,7 @@
#include "qom/object.h"
#include "hw/irq.h"
#include "qapi/error.h"
+#include "hw/hotplug.h"
enum {
DEV_NVECTORS_UNSPECIFIED = -1,
@@ -180,14 +181,18 @@ typedef struct BusChild {
QTAILQ_ENTRY(BusChild) sibling;
} BusChild;
+#define QDEV_HOTPLUG_HANDLER_PROPERTY "hotplug-handler"
+
/**
* BusState:
+ * @hotplug_device: link to a hotplug device associated with bus.
*/
struct BusState {
Object obj;
DeviceState *parent;
const char *name;
int allow_hotplug;
+ HotplugHandler *hotplug_handler;
int max_index;
QTAILQ_HEAD(ChildrenHead, BusChild) children;
QLIST_ENTRY(BusState) sibling;
--
1.7.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH 3/9] qdev: add "hotpluggable" property to Device
2014-01-14 16:55 [Qemu-devel] [PATCH 0/9 v4] Refactor PCI/SHPC/PCIE hotplug to use a more generic hotplug API Igor Mammedov
2014-01-14 16:55 ` [Qemu-devel] [PATCH 1/9] define hotplug interface Igor Mammedov
2014-01-14 16:55 ` [Qemu-devel] [PATCH 2/9] qdev: add to BusState "hotplug-handler" link Igor Mammedov
@ 2014-01-14 16:55 ` Igor Mammedov
2014-01-14 16:55 ` [Qemu-devel] [PATCH 4/9] hw/acpi: move typeinfo to the file end Igor Mammedov
` (6 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Igor Mammedov @ 2014-01-14 16:55 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.crosthwaite, marcel.a, mst, aliguori, pbonzini, afaerber
Currently it's possible to make PCIDevice not hotpluggable by using
no_hotplug field of PCIDeviceClass. However it limits this
only to PCI devices and prevents from generalizing hotplug code.
So add similar field to DeviceClass so it could be reused with other
Devices and would allow to replace PCI specific hotplug callbacks
with generic implementation.
In addition expose field as "hotpluggable" readonly property, to make
it possible to get it via QOM interface.
Make DeviceClass hotpluggable by default as it was assumed before.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v5:
* drop 'boolean' from doc comment of hotpluggable field
v4:
* s/hotplugable/hotpluggable/
v3:
* make DeviceClass hotpluggable by default
Since PCIDevice still uses internal no_hotlpug checks it shouldn't
reggress. And follow up patch that converts PCIDevices to use
"hotpluggable" property will take care about not hotpluggable PCI
devices explicitly setting "hotpluggable" to false in their class_init().
* move generic hotplug checks from
"7/11 qdev:pci: refactor PCIDevice to use generic "hotplugable" property"
to this patch
---
hw/core/qdev.c | 29 +++++++++++++++++++++++++++++
include/hw/qdev-core.h | 3 +++
2 files changed, 32 insertions(+), 0 deletions(-)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index c9f0c33..d8b83f1 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -215,6 +215,12 @@ void qdev_unplug(DeviceState *dev, Error **errp)
}
assert(dc->unplug != NULL);
+ if (!dc->hotpluggable) {
+ error_set(errp, QERR_DEVICE_NO_HOTPLUG,
+ object_get_typename(OBJECT(dev)));
+ return;
+ }
+
qdev_hot_removed = true;
if (dc->unplug(dev) < 0) {
@@ -694,6 +700,11 @@ static void device_set_realized(Object *obj, bool value, Error **err)
DeviceClass *dc = DEVICE_GET_CLASS(dev);
Error *local_err = NULL;
+ if (dev->hotplugged && !dc->hotpluggable) {
+ error_set(err, QERR_DEVICE_NO_HOTPLUG, object_get_typename(obj));
+ return;
+ }
+
if (value && !dev->realized) {
if (!obj->parent && local_err == NULL) {
static int unattached_count;
@@ -734,6 +745,14 @@ static void device_set_realized(Object *obj, bool value, Error **err)
dev->realized = value;
}
+static bool device_get_hotpluggable(Object *obj, Error **err)
+{
+ DeviceClass *dc = DEVICE_GET_CLASS(obj);
+ DeviceState *dev = DEVICE(obj);
+
+ return dc->hotpluggable && dev->parent_bus->allow_hotplug;
+}
+
static void device_initfn(Object *obj)
{
DeviceState *dev = DEVICE(obj);
@@ -750,6 +769,8 @@ static void device_initfn(Object *obj)
object_property_add_bool(obj, "realized",
device_get_realized, device_set_realized, NULL);
+ object_property_add_bool(obj, "hotpluggable",
+ device_get_hotpluggable, NULL, NULL);
class = object_get_class(OBJECT(dev));
do {
@@ -786,6 +807,14 @@ static void device_class_base_init(ObjectClass *class, void *data)
* so do not propagate them to the subclasses.
*/
klass->props = NULL;
+
+ /* by default all devices were considered as hotpluggable,
+ * so with intent to check it in generic qdev_unplug() /
+ * device_set_realized() functions make every device
+ * hotpluggable. Devices that shouldn't be hoplugable,
+ * should override it in their class_init()
+ */
+ klass->hotpluggable = true;
}
static void device_unparent(Object *obj)
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 58a5c69..9d58a9d 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -50,6 +50,8 @@ struct VMStateDescription;
* is changed to %true. Deprecated, new types inheriting directly from
* TYPE_DEVICE should use @realize instead, new leaf types should consult
* their respective parent type.
+ * @hotpluggable: indicates if #DeviceClass is hotpluggable, available
+ * as readonly "hotpluggable" property of #DeviceState instance
*
* # Realization #
* Devices are constructed in two stages,
@@ -110,6 +112,7 @@ typedef struct DeviceClass {
* TODO remove once we're there
*/
bool cannot_instantiate_with_device_add_yet;
+ bool hotpluggable;
/* callbacks */
void (*reset)(DeviceState *dev);
--
1.7.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH 4/9] hw/acpi: move typeinfo to the file end
2014-01-14 16:55 [Qemu-devel] [PATCH 0/9 v4] Refactor PCI/SHPC/PCIE hotplug to use a more generic hotplug API Igor Mammedov
` (2 preceding siblings ...)
2014-01-14 16:55 ` [Qemu-devel] [PATCH 3/9] qdev: add "hotpluggable" property to Device Igor Mammedov
@ 2014-01-14 16:55 ` Igor Mammedov
2014-01-14 16:55 ` [Qemu-devel] [PATCH 5/9] qdev:pci: refactor PCIDevice to use generic "hotpluggable" property Igor Mammedov
` (5 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Igor Mammedov @ 2014-01-14 16:55 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.crosthwaite, marcel.a, mst, aliguori, pbonzini, afaerber
do so to avoid not necessary forward declarations and
place typeinfo registration at the file end where it's
usualy expected.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
hw/acpi/piix4.c | 88 +++++++++++++++++++++++++++---------------------------
1 files changed, 44 insertions(+), 44 deletions(-)
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 20353b9..3dcaaca 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -504,50 +504,6 @@ i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
return s->smb.smbus;
}
-static Property piix4_pm_properties[] = {
- DEFINE_PROP_UINT32("smb_io_base", PIIX4PMState, smb_io_base, 0),
- DEFINE_PROP_UINT8(ACPI_PM_PROP_S3_DISABLED, PIIX4PMState, disable_s3, 0),
- DEFINE_PROP_UINT8(ACPI_PM_PROP_S4_DISABLED, PIIX4PMState, disable_s4, 0),
- DEFINE_PROP_UINT8(ACPI_PM_PROP_S4_VAL, PIIX4PMState, s4_val, 2),
- DEFINE_PROP_END_OF_LIST(),
-};
-
-static void piix4_pm_class_init(ObjectClass *klass, void *data)
-{
- DeviceClass *dc = DEVICE_CLASS(klass);
- PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
-
- k->no_hotplug = 1;
- k->init = piix4_pm_initfn;
- k->config_write = pm_write_config;
- k->vendor_id = PCI_VENDOR_ID_INTEL;
- k->device_id = PCI_DEVICE_ID_INTEL_82371AB_3;
- k->revision = 0x03;
- k->class_id = PCI_CLASS_BRIDGE_OTHER;
- dc->desc = "PM";
- dc->vmsd = &vmstate_acpi;
- dc->props = piix4_pm_properties;
- /*
- * Reason: part of PIIX4 southbridge, needs to be wired up,
- * e.g. by mips_malta_init()
- */
- dc->cannot_instantiate_with_device_add_yet = true;
-}
-
-static const TypeInfo piix4_pm_info = {
- .name = TYPE_PIIX4_PM,
- .parent = TYPE_PCI_DEVICE,
- .instance_size = sizeof(PIIX4PMState),
- .class_init = piix4_pm_class_init,
-};
-
-static void piix4_pm_register_types(void)
-{
- type_register_static(&piix4_pm_info);
-}
-
-type_init(piix4_pm_register_types)
-
static uint64_t gpe_readb(void *opaque, hwaddr addr, unsigned width)
{
PIIX4PMState *s = opaque;
@@ -757,3 +713,47 @@ static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev,
return 0;
}
+
+static Property piix4_pm_properties[] = {
+ DEFINE_PROP_UINT32("smb_io_base", PIIX4PMState, smb_io_base, 0),
+ DEFINE_PROP_UINT8(ACPI_PM_PROP_S3_DISABLED, PIIX4PMState, disable_s3, 0),
+ DEFINE_PROP_UINT8(ACPI_PM_PROP_S4_DISABLED, PIIX4PMState, disable_s4, 0),
+ DEFINE_PROP_UINT8(ACPI_PM_PROP_S4_VAL, PIIX4PMState, s4_val, 2),
+ DEFINE_PROP_END_OF_LIST(),
+};
+
+static void piix4_pm_class_init(ObjectClass *klass, void *data)
+{
+ DeviceClass *dc = DEVICE_CLASS(klass);
+ PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+
+ k->no_hotplug = 1;
+ k->init = piix4_pm_initfn;
+ k->config_write = pm_write_config;
+ k->vendor_id = PCI_VENDOR_ID_INTEL;
+ k->device_id = PCI_DEVICE_ID_INTEL_82371AB_3;
+ k->revision = 0x03;
+ k->class_id = PCI_CLASS_BRIDGE_OTHER;
+ dc->desc = "PM";
+ dc->vmsd = &vmstate_acpi;
+ dc->props = piix4_pm_properties;
+ /*
+ * Reason: part of PIIX4 southbridge, needs to be wired up,
+ * e.g. by mips_malta_init()
+ */
+ dc->cannot_instantiate_with_device_add_yet = true;
+}
+
+static const TypeInfo piix4_pm_info = {
+ .name = TYPE_PIIX4_PM,
+ .parent = TYPE_PCI_DEVICE,
+ .instance_size = sizeof(PIIX4PMState),
+ .class_init = piix4_pm_class_init,
+};
+
+static void piix4_pm_register_types(void)
+{
+ type_register_static(&piix4_pm_info);
+}
+
+type_init(piix4_pm_register_types)
--
1.7.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH 5/9] qdev:pci: refactor PCIDevice to use generic "hotpluggable" property
2014-01-14 16:55 [Qemu-devel] [PATCH 0/9 v4] Refactor PCI/SHPC/PCIE hotplug to use a more generic hotplug API Igor Mammedov
` (3 preceding siblings ...)
2014-01-14 16:55 ` [Qemu-devel] [PATCH 4/9] hw/acpi: move typeinfo to the file end Igor Mammedov
@ 2014-01-14 16:55 ` Igor Mammedov
2014-01-14 16:55 ` [Qemu-devel] [PATCH 6/9] acpi/piix4pm: convert ACPI PCI hotplug to use hotplug-handler API Igor Mammedov
` (4 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Igor Mammedov @ 2014-01-14 16:55 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.crosthwaite, marcel.a, mst, aliguori, pbonzini, afaerber
Get rid of PCIDevice specific PCIDeviceClass.no_hotplug and use
generic DeviceClass.hotpluggable field instead.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v2:
* move generic hotplug checks to
"qdev: add "hotpluggable" property to Device" patch
* s/hotplugable/hotpluggable/
---
hw/acpi/piix4.c | 10 +++++-----
hw/display/cirrus_vga.c | 2 +-
hw/display/qxl.c | 2 +-
hw/display/vga-pci.c | 2 +-
hw/display/vmware_vga.c | 2 +-
hw/i386/acpi-build.c | 6 +++---
hw/ide/piix.c | 4 ++--
hw/isa/piix4.c | 2 +-
hw/pci-host/piix.c | 6 +++---
hw/pci/pci.c | 11 +----------
hw/usb/hcd-ehci-pci.c | 2 +-
hw/usb/hcd-ohci.c | 2 +-
hw/usb/hcd-uhci.c | 2 +-
hw/usb/hcd-xhci.c | 2 +-
include/hw/pci/pci.h | 3 ---
15 files changed, 23 insertions(+), 35 deletions(-)
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 3dcaaca..c292753 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -304,9 +304,9 @@ static void acpi_piix_eject_slot(PIIX4PMState *s, unsigned slots)
QTAILQ_FOREACH_SAFE(kid, &bus->children, sibling, next) {
DeviceState *qdev = kid->child;
PCIDevice *dev = PCI_DEVICE(qdev);
- PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
+ DeviceClass *dc = DEVICE_GET_CLASS(dev);
if (PCI_SLOT(dev->devfn) == slot) {
- if (pc->no_hotplug) {
+ if (!dc->hotpluggable) {
slot_free = false;
} else {
object_unparent(OBJECT(qdev));
@@ -334,10 +334,10 @@ static void piix4_update_hotplug(PIIX4PMState *s)
QTAILQ_FOREACH_SAFE(kid, &bus->children, sibling, next) {
DeviceState *qdev = kid->child;
PCIDevice *pdev = PCI_DEVICE(qdev);
- PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pdev);
+ DeviceClass *dc = DEVICE_GET_CLASS(qdev);
int slot = PCI_SLOT(pdev->devfn);
- if (pc->no_hotplug) {
+ if (!dc->hotpluggable) {
s->pci0_hotplug_enable &= ~(1U << slot);
}
@@ -727,7 +727,6 @@ static void piix4_pm_class_init(ObjectClass *klass, void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
- k->no_hotplug = 1;
k->init = piix4_pm_initfn;
k->config_write = pm_write_config;
k->vendor_id = PCI_VENDOR_ID_INTEL;
@@ -742,6 +741,7 @@ static void piix4_pm_class_init(ObjectClass *klass, void *data)
* e.g. by mips_malta_init()
*/
dc->cannot_instantiate_with_device_add_yet = true;
+ dc->hotpluggable = false;
}
static const TypeInfo piix4_pm_info = {
diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index e4c345f..3a8fc0b 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -2996,7 +2996,6 @@ static void cirrus_vga_class_init(ObjectClass *klass, void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
- k->no_hotplug = 1;
k->init = pci_cirrus_vga_initfn;
k->romfile = VGABIOS_CIRRUS_FILENAME;
k->vendor_id = PCI_VENDOR_ID_CIRRUS;
@@ -3006,6 +3005,7 @@ static void cirrus_vga_class_init(ObjectClass *klass, void *data)
dc->desc = "Cirrus CLGD 54xx VGA";
dc->vmsd = &vmstate_pci_cirrus_vga;
dc->props = pci_vga_cirrus_properties;
+ dc->hotpluggable = false;
}
static const TypeInfo cirrus_vga_info = {
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index e4f172e..ec82e00 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -2299,7 +2299,6 @@ static void qxl_primary_class_init(ObjectClass *klass, void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
- k->no_hotplug = 1;
k->init = qxl_init_primary;
k->romfile = "vgabios-qxl.bin";
k->vendor_id = REDHAT_PCI_VENDOR_ID;
@@ -2310,6 +2309,7 @@ static void qxl_primary_class_init(ObjectClass *klass, void *data)
dc->reset = qxl_reset_handler;
dc->vmsd = &qxl_vmstate;
dc->props = qxl_properties;
+ dc->hotpluggable = false;
}
static const TypeInfo qxl_primary_info = {
diff --git a/hw/display/vga-pci.c b/hw/display/vga-pci.c
index b3a45c8..f74fc43 100644
--- a/hw/display/vga-pci.c
+++ b/hw/display/vga-pci.c
@@ -190,7 +190,6 @@ static void vga_class_init(ObjectClass *klass, void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
- k->no_hotplug = 1;
k->init = pci_std_vga_initfn;
k->romfile = "vgabios-stdvga.bin";
k->vendor_id = PCI_VENDOR_ID_QEMU;
@@ -198,6 +197,7 @@ static void vga_class_init(ObjectClass *klass, void *data)
k->class_id = PCI_CLASS_DISPLAY_VGA;
dc->vmsd = &vmstate_vga_pci;
dc->props = vga_pci_properties;
+ dc->hotpluggable = false;
set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories);
}
diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index aba292c..334e718 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -1296,7 +1296,6 @@ static void vmsvga_class_init(ObjectClass *klass, void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
- k->no_hotplug = 1;
k->init = pci_vmsvga_initfn;
k->romfile = "vgabios-vmware.bin";
k->vendor_id = PCI_VENDOR_ID_VMWARE;
@@ -1307,6 +1306,7 @@ static void vmsvga_class_init(ObjectClass *klass, void *data)
dc->reset = vmsvga_reset;
dc->vmsd = &vmstate_vmware_vga;
dc->props = vga_vmware_properties;
+ dc->hotpluggable = false;
set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories);
}
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 48312f5..4db799a 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -186,16 +186,16 @@ static void acpi_get_hotplug_info(AcpiMiscInfo *misc)
DIV_ROUND_UP(PCI_SLOT_MAX, BITS_PER_BYTE));
for (i = 0; i < ARRAY_SIZE(bus->devices); ++i) {
- PCIDeviceClass *pc;
+ DeviceClass *dc;
PCIDevice *pdev = bus->devices[i];
if (!pdev) {
continue;
}
- pc = PCI_DEVICE_GET_CLASS(pdev);
+ dc = DEVICE_GET_CLASS(pdev);
- if (pc->no_hotplug) {
+ if (!dc->hotpluggable) {
int slot = PCI_SLOT(i);
clear_bit(slot, misc->slot_hotplug_enable);
diff --git a/hw/ide/piix.c b/hw/ide/piix.c
index 9b5960b..0eda301 100644
--- a/hw/ide/piix.c
+++ b/hw/ide/piix.c
@@ -241,13 +241,13 @@ static void piix3_ide_class_init(ObjectClass *klass, void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
- k->no_hotplug = 1;
k->init = pci_piix_ide_initfn;
k->exit = pci_piix_ide_exitfn;
k->vendor_id = PCI_VENDOR_ID_INTEL;
k->device_id = PCI_DEVICE_ID_INTEL_82371SB_1;
k->class_id = PCI_CLASS_STORAGE_IDE;
set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
+ dc->hotpluggable = false;
}
static const TypeInfo piix3_ide_info = {
@@ -280,13 +280,13 @@ static void piix4_ide_class_init(ObjectClass *klass, void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
- k->no_hotplug = 1;
k->init = pci_piix_ide_initfn;
k->exit = pci_piix_ide_exitfn;
k->vendor_id = PCI_VENDOR_ID_INTEL;
k->device_id = PCI_DEVICE_ID_INTEL_82371AB;
k->class_id = PCI_CLASS_STORAGE_IDE;
set_bit(DEVICE_CATEGORY_STORAGE, dc->categories);
+ dc->hotpluggable = false;
}
static const TypeInfo piix4_ide_info = {
diff --git a/hw/isa/piix4.c b/hw/isa/piix4.c
index def6fe3..492cd22 100644
--- a/hw/isa/piix4.c
+++ b/hw/isa/piix4.c
@@ -107,7 +107,6 @@ static void piix4_class_init(ObjectClass *klass, void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
- k->no_hotplug = 1;
k->init = piix4_initfn;
k->vendor_id = PCI_VENDOR_ID_INTEL;
k->device_id = PCI_DEVICE_ID_INTEL_82371AB_0;
@@ -119,6 +118,7 @@ static void piix4_class_init(ObjectClass *klass, void *data)
* e.g. by mips_malta_init()
*/
dc->cannot_instantiate_with_device_add_yet = true;
+ dc->hotpluggable = false;
}
static const TypeInfo piix4_info = {
diff --git a/hw/pci-host/piix.c b/hw/pci-host/piix.c
index e89d5c1..ffdc853 100644
--- a/hw/pci-host/piix.c
+++ b/hw/pci-host/piix.c
@@ -628,7 +628,7 @@ static void piix3_class_init(ObjectClass *klass, void *data)
dc->desc = "ISA bridge";
dc->vmsd = &vmstate_piix3;
- k->no_hotplug = 1;
+ dc->hotpluggable = false;
k->init = piix3_initfn;
k->config_write = piix3_write_config;
k->vendor_id = PCI_VENDOR_ID_INTEL;
@@ -656,7 +656,7 @@ static void piix3_xen_class_init(ObjectClass *klass, void *data)
dc->desc = "ISA bridge";
dc->vmsd = &vmstate_piix3;
- k->no_hotplug = 1;
+ dc->hotpluggable = false;
k->init = piix3_initfn;
k->config_write = piix3_write_config_xen;
k->vendor_id = PCI_VENDOR_ID_INTEL;
@@ -682,7 +682,6 @@ static void i440fx_class_init(ObjectClass *klass, void *data)
DeviceClass *dc = DEVICE_CLASS(klass);
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
- k->no_hotplug = 1;
k->init = i440fx_initfn;
k->config_write = i440fx_write_config;
k->vendor_id = PCI_VENDOR_ID_INTEL;
@@ -696,6 +695,7 @@ static void i440fx_class_init(ObjectClass *klass, void *data)
* host-facing part, which can't be device_add'ed, yet.
*/
dc->cannot_instantiate_with_device_add_yet = true;
+ dc->hotpluggable = false;
}
static const TypeInfo i440fx_info = {
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index aa2a395..b8770ef 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -1733,11 +1733,7 @@ static int pci_qdev_init(DeviceState *qdev)
pci_dev->devfn);
if (pci_dev == NULL)
return -1;
- if (qdev->hotplugged && pc->no_hotplug) {
- qerror_report(QERR_DEVICE_NO_HOTPLUG, object_get_typename(OBJECT(pci_dev)));
- do_pci_unregister_device(pci_dev);
- return -1;
- }
+
if (pc->init) {
rc = pc->init(pci_dev);
if (rc != 0) {
@@ -1772,12 +1768,7 @@ static int pci_qdev_init(DeviceState *qdev)
static int pci_unplug_device(DeviceState *qdev)
{
PCIDevice *dev = PCI_DEVICE(qdev);
- PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(dev);
- if (pc->no_hotplug) {
- qerror_report(QERR_DEVICE_NO_HOTPLUG, object_get_typename(OBJECT(dev)));
- return -1;
- }
return dev->bus->hotplug(dev->bus->hotplug_qdev, dev,
PCI_HOTPLUG_DISABLED);
}
diff --git a/hw/usb/hcd-ehci-pci.c b/hw/usb/hcd-ehci-pci.c
index 0c98594..484a9bd 100644
--- a/hw/usb/hcd-ehci-pci.c
+++ b/hw/usb/hcd-ehci-pci.c
@@ -123,7 +123,7 @@ static void ehci_class_init(ObjectClass *klass, void *data)
k->init = usb_ehci_pci_initfn;
k->class_id = PCI_CLASS_SERIAL_USB;
k->config_write = usb_ehci_pci_write_config;
- k->no_hotplug = 1;
+ dc->hotpluggable = false;
dc->vmsd = &vmstate_ehci_pci;
dc->props = ehci_pci_properties;
}
diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c
index e38cdeb..3d35058 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -1993,10 +1993,10 @@ static void ohci_pci_class_init(ObjectClass *klass, void *data)
k->vendor_id = PCI_VENDOR_ID_APPLE;
k->device_id = PCI_DEVICE_ID_APPLE_IPID_USB;
k->class_id = PCI_CLASS_SERIAL_USB;
- k->no_hotplug = 1;
set_bit(DEVICE_CATEGORY_USB, dc->categories);
dc->desc = "Apple USB Controller";
dc->props = ohci_pci_properties;
+ dc->hotpluggable = false;
}
static const TypeInfo ohci_pci_info = {
diff --git a/hw/usb/hcd-uhci.c b/hw/usb/hcd-uhci.c
index 238d1d2..ad814b5 100644
--- a/hw/usb/hcd-uhci.c
+++ b/hw/usb/hcd-uhci.c
@@ -1318,7 +1318,7 @@ static void uhci_class_init(ObjectClass *klass, void *data)
k->device_id = info->device_id;
k->revision = info->revision;
k->class_id = PCI_CLASS_SERIAL_USB;
- k->no_hotplug = 1;
+ dc->hotpluggable = false;
dc->vmsd = &vmstate_uhci;
dc->props = uhci_properties;
set_bit(DEVICE_CATEGORY_USB, dc->categories);
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index bafe085..0fa814e 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -3798,6 +3798,7 @@ static void xhci_class_init(ObjectClass *klass, void *data)
dc->vmsd = &vmstate_xhci;
dc->props = xhci_properties;
dc->reset = xhci_reset;
+ dc->hotpluggable = false;
set_bit(DEVICE_CATEGORY_USB, dc->categories);
k->init = usb_xhci_initfn;
k->vendor_id = PCI_VENDOR_ID_NEC;
@@ -3805,7 +3806,6 @@ static void xhci_class_init(ObjectClass *klass, void *data)
k->class_id = PCI_CLASS_SERIAL_USB;
k->revision = 0x03;
k->is_express = 1;
- k->no_hotplug = 1;
}
static const TypeInfo xhci_info = {
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 754b82d..03d4bee 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -201,9 +201,6 @@ typedef struct PCIDeviceClass {
/* pcie stuff */
int is_express; /* is this device pci express? */
- /* device isn't hot-pluggable */
- int no_hotplug;
-
/* rom bar */
const char *romfile;
} PCIDeviceClass;
--
1.7.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH 6/9] acpi/piix4pm: convert ACPI PCI hotplug to use hotplug-handler API
2014-01-14 16:55 [Qemu-devel] [PATCH 0/9 v4] Refactor PCI/SHPC/PCIE hotplug to use a more generic hotplug API Igor Mammedov
` (4 preceding siblings ...)
2014-01-14 16:55 ` [Qemu-devel] [PATCH 5/9] qdev:pci: refactor PCIDevice to use generic "hotpluggable" property Igor Mammedov
@ 2014-01-14 16:55 ` Igor Mammedov
2014-01-14 16:55 ` [Qemu-devel] [PATCH 7/9] pci/shpc: convert SHPC " Igor Mammedov
` (3 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Igor Mammedov @ 2014-01-14 16:55 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.crosthwaite, marcel.a, mst, aliguori, pbonzini, afaerber
Split piix4_device_hotplug() into hotplug/unplug callbacks
and register them as "hotplug-handler" interface implementation of
PIIX4_PM device.
Replace pci_bus_hotplug() wiring with setting link on
PCI BUS "hotplug-handler" property to PIIX4_PM device.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v2:
* use error_abort to make error handling less verbose
---
hw/acpi/piix4.c | 68 ++++++++++++++++++++++++++++--------------------------
1 files changed, 35 insertions(+), 33 deletions(-)
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index c292753..20b1ea3 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -30,6 +30,8 @@
#include "hw/nvram/fw_cfg.h"
#include "exec/address-spaces.h"
#include "hw/acpi/piix4.h"
+#include "qapi/qmp/qerror.h"
+#include "hw/hotplug.h"
//#define DEBUG
@@ -107,7 +109,7 @@ typedef struct PIIX4PMState {
OBJECT_CHECK(PIIX4PMState, (obj), TYPE_PIIX4_PM)
static void piix4_acpi_system_hot_add_init(MemoryRegion *parent,
- PCIBus *bus, PIIX4PMState *s);
+ BusState *bus, PIIX4PMState *s);
#define ACPI_ENABLE 0xf1
#define ACPI_DISABLE 0xf0
@@ -459,7 +461,7 @@ static int piix4_pm_initfn(PCIDevice *dev)
qemu_add_machine_init_done_notifier(&s->machine_ready);
qemu_register_reset(piix4_reset, s);
- piix4_acpi_system_hot_add_init(pci_address_space_io(dev), dev->bus, s);
+ piix4_acpi_system_hot_add_init(pci_address_space_io(dev), BUS(dev->bus), s);
piix4_pm_add_propeties(s);
return 0;
@@ -645,11 +647,8 @@ static void piix4_cpu_added_req(Notifier *n, void *opaque)
piix4_cpu_hotplug_req(s, CPU(opaque), PLUG);
}
-static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev,
- PCIHotplugState state);
-
static void piix4_acpi_system_hot_add_init(MemoryRegion *parent,
- PCIBus *bus, PIIX4PMState *s)
+ BusState *bus, PIIX4PMState *s)
{
CPUState *cpu;
@@ -661,7 +660,9 @@ static void piix4_acpi_system_hot_add_init(MemoryRegion *parent,
"acpi-pci-hotplug", PCI_HOTPLUG_SIZE);
memory_region_add_subregion(parent, PCI_HOTPLUG_ADDR,
&s->io_pci);
- pci_bus_hotplug(bus, piix4_device_hotplug, DEVICE(s));
+ object_property_set_link(OBJECT(bus), OBJECT(s),
+ QDEV_HOTPLUG_HANDLER_PROPERTY, &error_abort);
+ bus->allow_hotplug = 1;
CPU_FOREACH(cpu) {
CPUClass *cc = CPU_GET_CLASS(cpu);
@@ -677,41 +678,35 @@ static void piix4_acpi_system_hot_add_init(MemoryRegion *parent,
qemu_register_cpu_added_notifier(&s->cpu_added_notifier);
}
-static void enable_device(PIIX4PMState *s, int slot)
+static void piix4_pci_device_hotplug_cb(HotplugHandler *hotplug_dev,
+ DeviceState *dev, Error **errp)
{
- s->ar.gpe.sts[0] |= PIIX4_PCI_HOTPLUG_STATUS;
- s->pci0_slot_device_present |= (1U << slot);
-}
-
-static void disable_device(PIIX4PMState *s, int slot)
-{
- s->ar.gpe.sts[0] |= PIIX4_PCI_HOTPLUG_STATUS;
- s->pci0_status.down |= (1U << slot);
-}
-
-static int piix4_device_hotplug(DeviceState *qdev, PCIDevice *dev,
- PCIHotplugState state)
-{
- int slot = PCI_SLOT(dev->devfn);
- PIIX4PMState *s = PIIX4_PM(qdev);
+ PCIDevice *pci_dev = PCI_DEVICE(dev);
+ int slot = PCI_SLOT(pci_dev->devfn);
+ PIIX4PMState *s = PIIX4_PM(hotplug_dev);
+ s->pci0_slot_device_present |= (1U << slot);
/* Don't send event when device is enabled during qemu machine creation:
* it is present on boot, no hotplug event is necessary. We do send an
* event when the device is disabled later. */
- if (state == PCI_COLDPLUG_ENABLED) {
- s->pci0_slot_device_present |= (1U << slot);
- return 0;
- }
-
- if (state == PCI_HOTPLUG_ENABLED) {
- enable_device(s, slot);
- } else {
- disable_device(s, slot);
+ if (!dev->hotplugged) {
+ return;
}
+ s->ar.gpe.sts[0] |= PIIX4_PCI_HOTPLUG_STATUS;
acpi_update_sci(&s->ar, s->irq);
+}
- return 0;
+static void piix4_pci_device_hot_unplug_cb(HotplugHandler *hotplug_dev,
+ DeviceState *dev, Error **errp)
+{
+ PCIDevice *pci_dev = PCI_DEVICE(dev);
+ int slot = PCI_SLOT(pci_dev->devfn);
+ PIIX4PMState *s = PIIX4_PM(hotplug_dev);
+
+ s->pci0_status.down |= (1U << slot);
+ s->ar.gpe.sts[0] |= PIIX4_PCI_HOTPLUG_STATUS;
+ acpi_update_sci(&s->ar, s->irq);
}
static Property piix4_pm_properties[] = {
@@ -726,6 +721,7 @@ static void piix4_pm_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+ HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
k->init = piix4_pm_initfn;
k->config_write = pm_write_config;
@@ -742,6 +738,8 @@ static void piix4_pm_class_init(ObjectClass *klass, void *data)
*/
dc->cannot_instantiate_with_device_add_yet = true;
dc->hotpluggable = false;
+ hc->plug = piix4_pci_device_hotplug_cb;
+ hc->unplug = piix4_pci_device_hot_unplug_cb;
}
static const TypeInfo piix4_pm_info = {
@@ -749,6 +747,10 @@ static const TypeInfo piix4_pm_info = {
.parent = TYPE_PCI_DEVICE,
.instance_size = sizeof(PIIX4PMState),
.class_init = piix4_pm_class_init,
+ .interfaces = (InterfaceInfo[]) {
+ { TYPE_HOTPLUG_HANDLER },
+ { }
+ }
};
static void piix4_pm_register_types(void)
--
1.7.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH 7/9] pci/shpc: convert SHPC hotplug to use hotplug-handler API
2014-01-14 16:55 [Qemu-devel] [PATCH 0/9 v4] Refactor PCI/SHPC/PCIE hotplug to use a more generic hotplug API Igor Mammedov
` (5 preceding siblings ...)
2014-01-14 16:55 ` [Qemu-devel] [PATCH 6/9] acpi/piix4pm: convert ACPI PCI hotplug to use hotplug-handler API Igor Mammedov
@ 2014-01-14 16:55 ` Igor Mammedov
2014-01-14 16:55 ` [Qemu-devel] [PATCH 8/9] pci/pcie: convert PCIE " Igor Mammedov
` (2 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Igor Mammedov @ 2014-01-14 16:55 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.crosthwaite, marcel.a, mst, aliguori, pbonzini, afaerber
Split shpc_device_hotplug() into hotplug/unplug callbacks
and register them as "hotplug-handler" interface implementation of
PCI_BRIDGE_DEV device.
Replace pci_bus_hotplug() wiring with setting link on PCI BUS
"hotplug-handler" property to PCI_BRIDGE_DEV device.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v2:
* drop error_is_set() and check local_error pointer directly
* keep original logic, i.e. do not abort in bridge init.
---
hw/pci-bridge/pci_bridge_dev.c | 9 +++
hw/pci/shpc.c | 127 +++++++++++++++++++++++++---------------
include/hw/pci/shpc.h | 8 +++
3 files changed, 97 insertions(+), 47 deletions(-)
diff --git a/hw/pci-bridge/pci_bridge_dev.c b/hw/pci-bridge/pci_bridge_dev.c
index 440e187..e68145c 100644
--- a/hw/pci-bridge/pci_bridge_dev.c
+++ b/hw/pci-bridge/pci_bridge_dev.c
@@ -26,6 +26,7 @@
#include "hw/pci/slotid_cap.h"
#include "exec/memory.h"
#include "hw/pci/pci_bus.h"
+#include "hw/hotplug.h"
#define TYPE_PCI_BRIDGE_DEV "pci-bridge"
#define PCI_BRIDGE_DEV(obj) \
@@ -136,6 +137,8 @@ static void pci_bridge_dev_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
PCIDeviceClass *k = PCI_DEVICE_CLASS(klass);
+ HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(klass);
+
k->init = pci_bridge_dev_initfn;
k->exit = pci_bridge_dev_exitfn;
k->config_write = pci_bridge_dev_write_config;
@@ -148,6 +151,8 @@ static void pci_bridge_dev_class_init(ObjectClass *klass, void *data)
dc->props = pci_bridge_dev_properties;
dc->vmsd = &pci_bridge_dev_vmstate;
set_bit(DEVICE_CATEGORY_BRIDGE, dc->categories);
+ hc->plug = shpc_device_hotplug_cb;
+ hc->unplug = shpc_device_hot_unplug_cb;
}
static const TypeInfo pci_bridge_dev_info = {
@@ -155,6 +160,10 @@ static const TypeInfo pci_bridge_dev_info = {
.parent = TYPE_PCI_BRIDGE,
.instance_size = sizeof(PCIBridgeDev),
.class_init = pci_bridge_dev_class_init,
+ .interfaces = (InterfaceInfo[]) {
+ { TYPE_HOTPLUG_HANDLER },
+ { }
+ }
};
static void pci_bridge_dev_register(void)
diff --git a/hw/pci/shpc.c b/hw/pci/shpc.c
index 576244b..df46280 100644
--- a/hw/pci/shpc.c
+++ b/hw/pci/shpc.c
@@ -7,6 +7,7 @@
#include "hw/pci/pci.h"
#include "hw/pci/pci_bus.h"
#include "hw/pci/msi.h"
+#include "qapi/qmp/qerror.h"
/* TODO: model power only and disabled slot states. */
/* TODO: handle SERR and wakeups */
@@ -490,65 +491,93 @@ static const MemoryRegionOps shpc_mmio_ops = {
.max_access_size = 4,
},
};
-
-static int shpc_device_hotplug(DeviceState *qdev, PCIDevice *affected_dev,
- PCIHotplugState hotplug_state)
+static void shpc_device_hotplug_common(PCIDevice *affected_dev, int *slot,
+ SHPCDevice *shpc, Error **errp)
{
int pci_slot = PCI_SLOT(affected_dev->devfn);
- uint8_t state;
- uint8_t led;
- PCIDevice *d = DO_UPCAST(PCIDevice, qdev, qdev);
- SHPCDevice *shpc = d->shpc;
- int slot = SHPC_PCI_TO_IDX(pci_slot);
- if (pci_slot < SHPC_IDX_TO_PCI(0) || slot >= shpc->nslots) {
- error_report("Unsupported PCI slot %d for standard hotplug "
- "controller. Valid slots are between %d and %d.",
- pci_slot, SHPC_IDX_TO_PCI(0),
- SHPC_IDX_TO_PCI(shpc->nslots) - 1);
- return -1;
+ *slot = SHPC_PCI_TO_IDX(pci_slot);
+
+ if (pci_slot < SHPC_IDX_TO_PCI(0) || *slot >= shpc->nslots) {
+ error_setg(errp, "Unsupported PCI slot %d for standard hotplug "
+ "controller. Valid slots are between %d and %d.",
+ pci_slot, SHPC_IDX_TO_PCI(0),
+ SHPC_IDX_TO_PCI(shpc->nslots) - 1);
+ return;
+ }
+}
+
+void shpc_device_hotplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
+ Error **errp)
+{
+ Error *local_err = NULL;
+ PCIDevice *pci_hotplug_dev = PCI_DEVICE(hotplug_dev);
+ SHPCDevice *shpc = pci_hotplug_dev->shpc;
+ int slot;
+
+ shpc_device_hotplug_common(PCI_DEVICE(dev), &slot, shpc, &local_err);
+ if (local_err) {
+ error_propagate(errp, local_err);
+ return;
}
+
/* Don't send event when device is enabled during qemu machine creation:
* it is present on boot, no hotplug event is necessary. We do send an
* event when the device is disabled later. */
- if (hotplug_state == PCI_COLDPLUG_ENABLED) {
+ if (!dev->hotplugged) {
shpc_set_status(shpc, slot, 0, SHPC_SLOT_STATUS_MRL_OPEN);
shpc_set_status(shpc, slot, SHPC_SLOT_STATUS_PRSNT_7_5W,
SHPC_SLOT_STATUS_PRSNT_MASK);
- return 0;
+ return;
}
- if (hotplug_state == PCI_HOTPLUG_DISABLED) {
- shpc->config[SHPC_SLOT_EVENT_LATCH(slot)] |= SHPC_SLOT_EVENT_BUTTON;
- state = shpc_get_status(shpc, slot, SHPC_SLOT_STATE_MASK);
- led = shpc_get_status(shpc, slot, SHPC_SLOT_PWR_LED_MASK);
- if (state == SHPC_STATE_DISABLED && led == SHPC_LED_OFF) {
- shpc_free_devices_in_slot(shpc, slot);
- shpc_set_status(shpc, slot, 1, SHPC_SLOT_STATUS_MRL_OPEN);
- shpc_set_status(shpc, slot, SHPC_SLOT_STATUS_PRSNT_EMPTY,
- SHPC_SLOT_STATUS_PRSNT_MASK);
- shpc->config[SHPC_SLOT_EVENT_LATCH(slot)] |=
- SHPC_SLOT_EVENT_MRL |
- SHPC_SLOT_EVENT_PRESENCE;
- }
+
+ /* This could be a cancellation of the previous removal.
+ * We check MRL state to figure out. */
+ if (shpc_get_status(shpc, slot, SHPC_SLOT_STATUS_MRL_OPEN)) {
+ shpc_set_status(shpc, slot, 0, SHPC_SLOT_STATUS_MRL_OPEN);
+ shpc_set_status(shpc, slot, SHPC_SLOT_STATUS_PRSNT_7_5W,
+ SHPC_SLOT_STATUS_PRSNT_MASK);
+ shpc->config[SHPC_SLOT_EVENT_LATCH(slot)] |=
+ SHPC_SLOT_EVENT_BUTTON |
+ SHPC_SLOT_EVENT_MRL |
+ SHPC_SLOT_EVENT_PRESENCE;
} else {
- /* This could be a cancellation of the previous removal.
- * We check MRL state to figure out. */
- if (shpc_get_status(shpc, slot, SHPC_SLOT_STATUS_MRL_OPEN)) {
- shpc_set_status(shpc, slot, 0, SHPC_SLOT_STATUS_MRL_OPEN);
- shpc_set_status(shpc, slot, SHPC_SLOT_STATUS_PRSNT_7_5W,
- SHPC_SLOT_STATUS_PRSNT_MASK);
- shpc->config[SHPC_SLOT_EVENT_LATCH(slot)] |=
- SHPC_SLOT_EVENT_BUTTON |
- SHPC_SLOT_EVENT_MRL |
- SHPC_SLOT_EVENT_PRESENCE;
- } else {
- /* Press attention button to cancel removal */
- shpc->config[SHPC_SLOT_EVENT_LATCH(slot)] |=
- SHPC_SLOT_EVENT_BUTTON;
- }
+ /* Press attention button to cancel removal */
+ shpc->config[SHPC_SLOT_EVENT_LATCH(slot)] |=
+ SHPC_SLOT_EVENT_BUTTON;
}
shpc_set_status(shpc, slot, 0, SHPC_SLOT_STATUS_66);
- shpc_interrupt_update(d);
- return 0;
+ shpc_interrupt_update(pci_hotplug_dev);
+}
+
+void shpc_device_hot_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
+ Error **errp)
+{
+ Error *local_err = NULL;
+ PCIDevice *pci_hotplug_dev = PCI_DEVICE(hotplug_dev);
+ SHPCDevice *shpc = pci_hotplug_dev->shpc;
+ uint8_t state;
+ uint8_t led;
+ int slot;
+
+ shpc_device_hotplug_common(PCI_DEVICE(dev), &slot, shpc, errp);
+ if (local_err) {
+ return;
+ }
+
+ shpc->config[SHPC_SLOT_EVENT_LATCH(slot)] |= SHPC_SLOT_EVENT_BUTTON;
+ state = shpc_get_status(shpc, slot, SHPC_SLOT_STATE_MASK);
+ led = shpc_get_status(shpc, slot, SHPC_SLOT_PWR_LED_MASK);
+ if (state == SHPC_STATE_DISABLED && led == SHPC_LED_OFF) {
+ shpc_free_devices_in_slot(shpc, slot);
+ shpc_set_status(shpc, slot, 1, SHPC_SLOT_STATUS_MRL_OPEN);
+ shpc_set_status(shpc, slot, SHPC_SLOT_STATUS_PRSNT_EMPTY,
+ SHPC_SLOT_STATUS_PRSNT_MASK);
+ shpc->config[SHPC_SLOT_EVENT_LATCH(slot)] |=
+ SHPC_SLOT_EVENT_MRL |
+ SHPC_SLOT_EVENT_PRESENCE;
+ }
+ shpc_set_status(shpc, slot, 0, SHPC_SLOT_STATUS_66);
+ shpc_interrupt_update(pci_hotplug_dev);
}
/* Initialize the SHPC structure in bridge's BAR. */
@@ -557,6 +586,7 @@ int shpc_init(PCIDevice *d, PCIBus *sec_bus, MemoryRegion *bar, unsigned offset)
int i, ret;
int nslots = SHPC_MAX_SLOTS; /* TODO: qdev property? */
SHPCDevice *shpc = d->shpc = g_malloc0(sizeof(*d->shpc));
+ BusState *bus = BUS(sec_bus);
shpc->sec_bus = sec_bus;
ret = shpc_cap_add_config(d);
if (ret) {
@@ -616,7 +646,10 @@ int shpc_init(PCIDevice *d, PCIBus *sec_bus, MemoryRegion *bar, unsigned offset)
d, "shpc-mmio", SHPC_SIZEOF(d));
shpc_cap_update_dword(d);
memory_region_add_subregion(bar, offset, &shpc->mmio);
- pci_bus_hotplug(sec_bus, shpc_device_hotplug, &d->qdev);
+
+ bus->allow_hotplug = 1;
+ object_property_set_link(OBJECT(sec_bus), OBJECT(d),
+ QDEV_HOTPLUG_HANDLER_PROPERTY, NULL);
d->cap_present |= QEMU_PCI_CAP_SHPC;
return 0;
diff --git a/include/hw/pci/shpc.h b/include/hw/pci/shpc.h
index 467911a..eef1a1a 100644
--- a/include/hw/pci/shpc.h
+++ b/include/hw/pci/shpc.h
@@ -4,6 +4,8 @@
#include "qemu-common.h"
#include "exec/memory.h"
#include "migration/vmstate.h"
+#include "qapi/error.h"
+#include "hw/hotplug.h"
struct SHPCDevice {
/* Capability offset in device's config space */
@@ -41,6 +43,12 @@ int shpc_init(PCIDevice *dev, PCIBus *sec_bus, MemoryRegion *bar, unsigned off);
void shpc_cleanup(PCIDevice *dev, MemoryRegion *bar);
void shpc_cap_write_config(PCIDevice *d, uint32_t addr, uint32_t val, int len);
+
+void shpc_device_hotplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
+ Error **errp);
+void shpc_device_hot_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
+ Error **errp);
+
extern VMStateInfo shpc_vmstate_info;
#define SHPC_VMSTATE(_field, _type) \
VMSTATE_BUFFER_UNSAFE_INFO(_field, _type, 0, shpc_vmstate_info, 0)
--
1.7.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH 8/9] pci/pcie: convert PCIE hotplug to use hotplug-handler API
2014-01-14 16:55 [Qemu-devel] [PATCH 0/9 v4] Refactor PCI/SHPC/PCIE hotplug to use a more generic hotplug API Igor Mammedov
` (6 preceding siblings ...)
2014-01-14 16:55 ` [Qemu-devel] [PATCH 7/9] pci/shpc: convert SHPC " Igor Mammedov
@ 2014-01-14 16:55 ` Igor Mammedov
2014-01-14 16:55 ` [Qemu-devel] [PATCH 9/9] hw/pci: switch to a generic hotplug handling for PCIDevice Igor Mammedov
2014-01-16 9:33 ` [Qemu-devel] [PATCH 0/9 v4] Refactor PCI/SHPC/PCIE hotplug to use a more generic hotplug API Michael S. Tsirkin
9 siblings, 0 replies; 15+ messages in thread
From: Igor Mammedov @ 2014-01-14 16:55 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.crosthwaite, marcel.a, mst, aliguori, pbonzini, afaerber
Split pcie_cap_slot_hotplug() into hotplug/unplug callbacks
and register them as "hotplug-handler" interface implementation of
PCIE_SLOT device.
Replace pci_bus_hotplug() wiring with setting link on PCI BUS
"hotplug-handler" property to PCI_BRIDGE_DEV device.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v2:
* keep original non abort behavior of pcie_cap_slot_init()
---
hw/pci/pcie.c | 67 ++++++++++++++++++++++++++++++------------------
hw/pci/pcie_port.c | 8 ++++++
include/hw/pci/pcie.h | 5 +++
3 files changed, 55 insertions(+), 25 deletions(-)
diff --git a/hw/pci/pcie.c b/hw/pci/pcie.c
index ca60cf2..66d234a 100644
--- a/hw/pci/pcie.c
+++ b/hw/pci/pcie.c
@@ -26,6 +26,7 @@
#include "hw/pci/pci_bus.h"
#include "hw/pci/pcie_regs.h"
#include "qemu/range.h"
+#include "qapi/qmp/qerror.h"
//#define DEBUG_PCIE
#ifdef DEBUG_PCIE
@@ -216,28 +217,20 @@ static void pcie_cap_slot_event(PCIDevice *dev, PCIExpressHotPlugEvent event)
hotplug_event_notify(dev);
}
-static int pcie_cap_slot_hotplug(DeviceState *qdev,
- PCIDevice *pci_dev, PCIHotplugState state)
+static void pcie_cap_slot_hotplug_common(PCIDevice *hotplug_dev,
+ DeviceState *dev,
+ uint8_t **exp_cap, Error **errp)
{
- PCIDevice *d = PCI_DEVICE(qdev);
- uint8_t *exp_cap = d->config + d->exp.exp_cap;
- uint16_t sltsta = pci_get_word(exp_cap + PCI_EXP_SLTSTA);
-
- /* Don't send event when device is enabled during qemu machine creation:
- * it is present on boot, no hotplug event is necessary. We do send an
- * event when the device is disabled later. */
- if (state == PCI_COLDPLUG_ENABLED) {
- pci_word_test_and_set_mask(exp_cap + PCI_EXP_SLTSTA,
- PCI_EXP_SLTSTA_PDS);
- return 0;
- }
+ PCIDevice *pci_dev = PCI_DEVICE(dev);
+ *exp_cap = hotplug_dev->config + hotplug_dev->exp.exp_cap;
+ uint16_t sltsta = pci_get_word(*exp_cap + PCI_EXP_SLTSTA);
PCIE_DEV_PRINTF(pci_dev, "hotplug state: %d\n", state);
if (sltsta & PCI_EXP_SLTSTA_EIS) {
/* the slot is electromechanically locked.
* This error is propagated up to qdev and then to HMP/QMP.
*/
- return -EBUSY;
+ error_setg_errno(errp, -EBUSY, "slot is electromechanically locked");
}
/* TODO: multifunction hot-plug.
@@ -245,18 +238,40 @@ static int pcie_cap_slot_hotplug(DeviceState *qdev,
* hot plugged/unplugged.
*/
assert(PCI_FUNC(pci_dev->devfn) == 0);
+}
- if (state == PCI_HOTPLUG_ENABLED) {
+void pcie_cap_slot_hotplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
+ Error **errp)
+{
+ uint8_t *exp_cap;
+
+ pcie_cap_slot_hotplug_common(PCI_DEVICE(hotplug_dev), dev, &exp_cap, errp);
+
+ /* Don't send event when device is enabled during qemu machine creation:
+ * it is present on boot, no hotplug event is necessary. We do send an
+ * event when the device is disabled later. */
+ if (!dev->hotplugged) {
pci_word_test_and_set_mask(exp_cap + PCI_EXP_SLTSTA,
PCI_EXP_SLTSTA_PDS);
- pcie_cap_slot_event(d, PCI_EXP_HP_EV_PDC);
- } else {
- object_unparent(OBJECT(pci_dev));
- pci_word_test_and_clear_mask(exp_cap + PCI_EXP_SLTSTA,
- PCI_EXP_SLTSTA_PDS);
- pcie_cap_slot_event(d, PCI_EXP_HP_EV_PDC);
+ return;
}
- return 0;
+
+ pci_word_test_and_set_mask(exp_cap + PCI_EXP_SLTSTA,
+ PCI_EXP_SLTSTA_PDS);
+ pcie_cap_slot_event(PCI_DEVICE(hotplug_dev), PCI_EXP_HP_EV_PDC);
+}
+
+void pcie_cap_slot_hot_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
+ Error **errp)
+{
+ uint8_t *exp_cap;
+
+ pcie_cap_slot_hotplug_common(PCI_DEVICE(hotplug_dev), dev, &exp_cap, errp);
+
+ object_unparent(OBJECT(dev));
+ pci_word_test_and_clear_mask(exp_cap + PCI_EXP_SLTSTA,
+ PCI_EXP_SLTSTA_PDS);
+ pcie_cap_slot_event(PCI_DEVICE(hotplug_dev), PCI_EXP_HP_EV_PDC);
}
/* pci express slot for pci express root/downstream port
@@ -264,6 +279,7 @@ static int pcie_cap_slot_hotplug(DeviceState *qdev,
void pcie_cap_slot_init(PCIDevice *dev, uint16_t slot)
{
uint32_t pos = dev->exp.exp_cap;
+ BusState *bus = BUS(pci_bridge_get_sec_bus(PCI_BRIDGE(dev)));
pci_word_test_and_set_mask(dev->config + pos + PCI_EXP_FLAGS,
PCI_EXP_FLAGS_SLOT);
@@ -305,8 +321,9 @@ void pcie_cap_slot_init(PCIDevice *dev, uint16_t slot)
dev->exp.hpev_notified = false;
- pci_bus_hotplug(pci_bridge_get_sec_bus(PCI_BRIDGE(dev)),
- pcie_cap_slot_hotplug, &dev->qdev);
+ bus->allow_hotplug = 1;
+ object_property_set_link(OBJECT(bus), OBJECT(dev),
+ QDEV_HOTPLUG_HANDLER_PROPERTY, NULL);
}
void pcie_cap_slot_reset(PCIDevice *dev)
diff --git a/hw/pci/pcie_port.c b/hw/pci/pcie_port.c
index 2adb030..fa24877 100644
--- a/hw/pci/pcie_port.c
+++ b/hw/pci/pcie_port.c
@@ -19,6 +19,7 @@
*/
#include "hw/pci/pcie_port.h"
+#include "hw/hotplug.h"
void pcie_port_init_reg(PCIDevice *d)
{
@@ -149,8 +150,11 @@ static Property pcie_slot_props[] = {
static void pcie_slot_class_init(ObjectClass *oc, void *data)
{
DeviceClass *dc = DEVICE_CLASS(oc);
+ HotplugHandlerClass *hc = HOTPLUG_HANDLER_CLASS(oc);
dc->props = pcie_slot_props;
+ hc->plug = pcie_cap_slot_hotplug_cb;
+ hc->unplug = pcie_cap_slot_hot_unplug_cb;
}
static const TypeInfo pcie_slot_type_info = {
@@ -159,6 +163,10 @@ static const TypeInfo pcie_slot_type_info = {
.instance_size = sizeof(PCIESlot),
.abstract = true,
.class_init = pcie_slot_class_init,
+ .interfaces = (InterfaceInfo[]) {
+ { TYPE_HOTPLUG_HANDLER },
+ { }
+ }
};
static void pcie_port_register_types(void)
diff --git a/include/hw/pci/pcie.h b/include/hw/pci/pcie.h
index 1966169..b0bf7e3 100644
--- a/include/hw/pci/pcie.h
+++ b/include/hw/pci/pcie.h
@@ -25,6 +25,7 @@
#include "hw/pci/pci_regs.h"
#include "hw/pci/pcie_regs.h"
#include "hw/pci/pcie_aer.h"
+#include "hw/hotplug.h"
typedef enum {
/* for attention and power indicator */
@@ -122,4 +123,8 @@ extern const VMStateDescription vmstate_pcie_device;
.offset = vmstate_offset_value(_state, _field, PCIDevice), \
}
+void pcie_cap_slot_hotplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
+ Error **errp);
+void pcie_cap_slot_hot_unplug_cb(HotplugHandler *hotplug_dev, DeviceState *dev,
+ Error **errp);
#endif /* QEMU_PCIE_H */
--
1.7.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Qemu-devel] [PATCH 9/9] hw/pci: switch to a generic hotplug handling for PCIDevice
2014-01-14 16:55 [Qemu-devel] [PATCH 0/9 v4] Refactor PCI/SHPC/PCIE hotplug to use a more generic hotplug API Igor Mammedov
` (7 preceding siblings ...)
2014-01-14 16:55 ` [Qemu-devel] [PATCH 8/9] pci/pcie: convert PCIE " Igor Mammedov
@ 2014-01-14 16:55 ` Igor Mammedov
2014-01-20 11:36 ` Michael S. Tsirkin
2014-01-16 9:33 ` [Qemu-devel] [PATCH 0/9 v4] Refactor PCI/SHPC/PCIE hotplug to use a more generic hotplug API Michael S. Tsirkin
9 siblings, 1 reply; 15+ messages in thread
From: Igor Mammedov @ 2014-01-14 16:55 UTC (permalink / raw)
To: qemu-devel; +Cc: peter.crosthwaite, marcel.a, mst, aliguori, pbonzini, afaerber
make qdev_unplug()/device_set_realized() to call hotplug handler's
plug/unplug methods if available and remove not needed anymore
hot(un)plug handling from PCIDevice.
In case if hotplug handler is not available, revert to the legacy
hotplug method.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
v2:
* fix test-qdev-global-props build failure during "make check"
---
hw/core/qdev.c | 17 +++++++++++++----
hw/pci/pci.c | 29 +----------------------------
include/hw/pci/pci.h | 10 ----------
include/hw/pci/pci_bus.h | 2 --
tests/Makefile | 2 +-
5 files changed, 15 insertions(+), 45 deletions(-)
diff --git a/hw/core/qdev.c b/hw/core/qdev.c
index d8b83f1..3486e5d 100644
--- a/hw/core/qdev.c
+++ b/hw/core/qdev.c
@@ -213,7 +213,6 @@ void qdev_unplug(DeviceState *dev, Error **errp)
error_set(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name);
return;
}
- assert(dc->unplug != NULL);
if (!dc->hotpluggable) {
error_set(errp, QERR_DEVICE_NO_HOTPLUG,
@@ -223,9 +222,13 @@ void qdev_unplug(DeviceState *dev, Error **errp)
qdev_hot_removed = true;
- if (dc->unplug(dev) < 0) {
- error_set(errp, QERR_UNDEFINED_ERROR);
- return;
+ if (dev->parent_bus && dev->parent_bus->hotplug_handler) {
+ hotplug_handler_unplug(dev->parent_bus->hotplug_handler, dev, errp);
+ } else {
+ assert(dc->unplug != NULL);
+ if (dc->unplug(dev) < 0) { /* legacy handler */
+ error_set(errp, QERR_UNDEFINED_ERROR);
+ }
}
}
@@ -720,6 +723,12 @@ static void device_set_realized(Object *obj, bool value, Error **err)
dc->realize(dev, &local_err);
}
+ if (dev->parent_bus && dev->parent_bus->hotplug_handler &&
+ local_err == NULL) {
+ hotplug_handler_plug(dev->parent_bus->hotplug_handler,
+ dev, &local_err);
+ }
+
if (qdev_get_vmsd(dev) && local_err == NULL) {
vmstate_register_with_alias_id(dev, -1, qdev_get_vmsd(dev), dev,
dev->instance_id_alias,
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index b8770ef..7e36f29 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -35,6 +35,7 @@
#include "hw/pci/msi.h"
#include "hw/pci/msix.h"
#include "exec/address-spaces.h"
+#include "hw/hotplug.h"
//#define DEBUG_PCI
#ifdef DEBUG_PCI
@@ -346,13 +347,6 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
bus->irq_count = g_malloc0(nirq * sizeof(bus->irq_count[0]));
}
-void pci_bus_hotplug(PCIBus *bus, pci_hotplug_fn hotplug, DeviceState *qdev)
-{
- bus->qbus.allow_hotplug = 1;
- bus->hotplug = hotplug;
- bus->hotplug_qdev = qdev;
-}
-
PCIBus *pci_register_bus(DeviceState *parent, const char *name,
pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
void *irq_opaque,
@@ -1750,29 +1744,9 @@ static int pci_qdev_init(DeviceState *qdev)
}
pci_add_option_rom(pci_dev, is_default_rom);
- if (bus->hotplug) {
- /* Let buses differentiate between hotplug and when device is
- * enabled during qemu machine creation. */
- rc = bus->hotplug(bus->hotplug_qdev, pci_dev,
- qdev->hotplugged ? PCI_HOTPLUG_ENABLED:
- PCI_COLDPLUG_ENABLED);
- if (rc != 0) {
- int r = pci_unregister_device(&pci_dev->qdev);
- assert(!r);
- return rc;
- }
- }
return 0;
}
-static int pci_unplug_device(DeviceState *qdev)
-{
- PCIDevice *dev = PCI_DEVICE(qdev);
-
- return dev->bus->hotplug(dev->bus->hotplug_qdev, dev,
- PCI_HOTPLUG_DISABLED);
-}
-
PCIDevice *pci_create_multifunction(PCIBus *bus, int devfn, bool multifunction,
const char *name)
{
@@ -2243,7 +2217,6 @@ static void pci_device_class_init(ObjectClass *klass, void *data)
{
DeviceClass *k = DEVICE_CLASS(klass);
k->init = pci_qdev_init;
- k->unplug = pci_unplug_device;
k->exit = pci_unregister_device;
k->bus_type = TYPE_PCI_BUS;
k->props = pci_props;
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 03d4bee..c709d62 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -327,15 +327,6 @@ typedef void (*pci_set_irq_fn)(void *opaque, int irq_num, int level);
typedef int (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num);
typedef PCIINTxRoute (*pci_route_irq_fn)(void *opaque, int pin);
-typedef enum {
- PCI_HOTPLUG_DISABLED,
- PCI_HOTPLUG_ENABLED,
- PCI_COLDPLUG_ENABLED,
-} PCIHotplugState;
-
-typedef int (*pci_hotplug_fn)(DeviceState *qdev, PCIDevice *pci_dev,
- PCIHotplugState state);
-
#define TYPE_PCI_BUS "PCI"
#define PCI_BUS(obj) OBJECT_CHECK(PCIBus, (obj), TYPE_PCI_BUS)
#define TYPE_PCIE_BUS "PCIE"
@@ -354,7 +345,6 @@ PCIBus *pci_bus_new(DeviceState *parent, const char *name,
void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
void *irq_opaque, int nirq);
int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
-void pci_bus_hotplug(PCIBus *bus, pci_hotplug_fn hotplug, DeviceState *dev);
/* 0 <= pin <= 3 0 = INTA, 1 = INTB, 2 = INTC, 3 = INTD */
int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin);
PCIBus *pci_register_bus(DeviceState *parent, const char *name,
diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h
index 9df1788..fabaeee 100644
--- a/include/hw/pci/pci_bus.h
+++ b/include/hw/pci/pci_bus.h
@@ -16,8 +16,6 @@ struct PCIBus {
pci_set_irq_fn set_irq;
pci_map_irq_fn map_irq;
pci_route_irq_fn route_intx_to_irq;
- pci_hotplug_fn hotplug;
- DeviceState *hotplug_qdev;
void *irq_opaque;
PCIDevice *devices[PCI_SLOT_MAX * PCI_FUNC_MAX];
PCIDevice *parent_dev;
diff --git a/tests/Makefile b/tests/Makefile
index 0aaf657..0bbdecb 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -163,7 +163,7 @@ tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o xbzrle.o page_cache.o libqemuuti
tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o
tests/test-int128$(EXESUF): tests/test-int128.o
tests/test-qdev-global-props$(EXESUF): tests/test-qdev-global-props.o \
- hw/core/qdev.o hw/core/qdev-properties.o \
+ hw/core/qdev.o hw/core/qdev-properties.o hw/core/hotplug.o\
hw/core/irq.o \
$(qom-core-obj) \
$(test-qapi-obj-y) \
--
1.7.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH 9/9] hw/pci: switch to a generic hotplug handling for PCIDevice
2014-01-14 16:55 ` [Qemu-devel] [PATCH 9/9] hw/pci: switch to a generic hotplug handling for PCIDevice Igor Mammedov
@ 2014-01-20 11:36 ` Michael S. Tsirkin
2014-01-20 12:45 ` Igor Mammedov
0 siblings, 1 reply; 15+ messages in thread
From: Michael S. Tsirkin @ 2014-01-20 11:36 UTC (permalink / raw)
To: Igor Mammedov
Cc: peter.crosthwaite, marcel.a, qemu-devel, aliguori, pbonzini,
afaerber
On Tue, Jan 14, 2014 at 05:55:54PM +0100, Igor Mammedov wrote:
> make qdev_unplug()/device_set_realized() to call hotplug handler's
> plug/unplug methods if available and remove not needed anymore
> hot(un)plug handling from PCIDevice.
>
> In case if hotplug handler is not available, revert to the legacy
> hotplug method.
When isn't it available?
For buses other than PCI?
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
> v2:
> * fix test-qdev-global-props build failure during "make check"
> ---
> hw/core/qdev.c | 17 +++++++++++++----
> hw/pci/pci.c | 29 +----------------------------
> include/hw/pci/pci.h | 10 ----------
> include/hw/pci/pci_bus.h | 2 --
> tests/Makefile | 2 +-
> 5 files changed, 15 insertions(+), 45 deletions(-)
>
> diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> index d8b83f1..3486e5d 100644
> --- a/hw/core/qdev.c
> +++ b/hw/core/qdev.c
> @@ -213,7 +213,6 @@ void qdev_unplug(DeviceState *dev, Error **errp)
> error_set(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name);
> return;
> }
> - assert(dc->unplug != NULL);
>
> if (!dc->hotpluggable) {
> error_set(errp, QERR_DEVICE_NO_HOTPLUG,
> @@ -223,9 +222,13 @@ void qdev_unplug(DeviceState *dev, Error **errp)
>
> qdev_hot_removed = true;
>
> - if (dc->unplug(dev) < 0) {
> - error_set(errp, QERR_UNDEFINED_ERROR);
> - return;
> + if (dev->parent_bus && dev->parent_bus->hotplug_handler) {
> + hotplug_handler_unplug(dev->parent_bus->hotplug_handler, dev, errp);
> + } else {
> + assert(dc->unplug != NULL);
> + if (dc->unplug(dev) < 0) { /* legacy handler */
> + error_set(errp, QERR_UNDEFINED_ERROR);
> + }
> }
> }
>
> @@ -720,6 +723,12 @@ static void device_set_realized(Object *obj, bool value, Error **err)
> dc->realize(dev, &local_err);
> }
>
> + if (dev->parent_bus && dev->parent_bus->hotplug_handler &&
> + local_err == NULL) {
> + hotplug_handler_plug(dev->parent_bus->hotplug_handler,
> + dev, &local_err);
> + }
> +
> if (qdev_get_vmsd(dev) && local_err == NULL) {
> vmstate_register_with_alias_id(dev, -1, qdev_get_vmsd(dev), dev,
> dev->instance_id_alias,
> diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> index b8770ef..7e36f29 100644
> --- a/hw/pci/pci.c
> +++ b/hw/pci/pci.c
> @@ -35,6 +35,7 @@
> #include "hw/pci/msi.h"
> #include "hw/pci/msix.h"
> #include "exec/address-spaces.h"
> +#include "hw/hotplug.h"
>
> //#define DEBUG_PCI
> #ifdef DEBUG_PCI
> @@ -346,13 +347,6 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> bus->irq_count = g_malloc0(nirq * sizeof(bus->irq_count[0]));
> }
>
> -void pci_bus_hotplug(PCIBus *bus, pci_hotplug_fn hotplug, DeviceState *qdev)
> -{
> - bus->qbus.allow_hotplug = 1;
> - bus->hotplug = hotplug;
> - bus->hotplug_qdev = qdev;
> -}
> -
> PCIBus *pci_register_bus(DeviceState *parent, const char *name,
> pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> void *irq_opaque,
> @@ -1750,29 +1744,9 @@ static int pci_qdev_init(DeviceState *qdev)
> }
> pci_add_option_rom(pci_dev, is_default_rom);
>
> - if (bus->hotplug) {
> - /* Let buses differentiate between hotplug and when device is
> - * enabled during qemu machine creation. */
> - rc = bus->hotplug(bus->hotplug_qdev, pci_dev,
> - qdev->hotplugged ? PCI_HOTPLUG_ENABLED:
> - PCI_COLDPLUG_ENABLED);
> - if (rc != 0) {
> - int r = pci_unregister_device(&pci_dev->qdev);
> - assert(!r);
> - return rc;
> - }
> - }
> return 0;
> }
>
> -static int pci_unplug_device(DeviceState *qdev)
> -{
> - PCIDevice *dev = PCI_DEVICE(qdev);
> -
> - return dev->bus->hotplug(dev->bus->hotplug_qdev, dev,
> - PCI_HOTPLUG_DISABLED);
> -}
> -
> PCIDevice *pci_create_multifunction(PCIBus *bus, int devfn, bool multifunction,
> const char *name)
> {
> @@ -2243,7 +2217,6 @@ static void pci_device_class_init(ObjectClass *klass, void *data)
> {
> DeviceClass *k = DEVICE_CLASS(klass);
> k->init = pci_qdev_init;
> - k->unplug = pci_unplug_device;
> k->exit = pci_unregister_device;
> k->bus_type = TYPE_PCI_BUS;
> k->props = pci_props;
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index 03d4bee..c709d62 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -327,15 +327,6 @@ typedef void (*pci_set_irq_fn)(void *opaque, int irq_num, int level);
> typedef int (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num);
> typedef PCIINTxRoute (*pci_route_irq_fn)(void *opaque, int pin);
>
> -typedef enum {
> - PCI_HOTPLUG_DISABLED,
> - PCI_HOTPLUG_ENABLED,
> - PCI_COLDPLUG_ENABLED,
> -} PCIHotplugState;
> -
> -typedef int (*pci_hotplug_fn)(DeviceState *qdev, PCIDevice *pci_dev,
> - PCIHotplugState state);
> -
> #define TYPE_PCI_BUS "PCI"
> #define PCI_BUS(obj) OBJECT_CHECK(PCIBus, (obj), TYPE_PCI_BUS)
> #define TYPE_PCIE_BUS "PCIE"
> @@ -354,7 +345,6 @@ PCIBus *pci_bus_new(DeviceState *parent, const char *name,
> void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> void *irq_opaque, int nirq);
> int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
> -void pci_bus_hotplug(PCIBus *bus, pci_hotplug_fn hotplug, DeviceState *dev);
> /* 0 <= pin <= 3 0 = INTA, 1 = INTB, 2 = INTC, 3 = INTD */
> int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin);
> PCIBus *pci_register_bus(DeviceState *parent, const char *name,
> diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h
> index 9df1788..fabaeee 100644
> --- a/include/hw/pci/pci_bus.h
> +++ b/include/hw/pci/pci_bus.h
> @@ -16,8 +16,6 @@ struct PCIBus {
> pci_set_irq_fn set_irq;
> pci_map_irq_fn map_irq;
> pci_route_irq_fn route_intx_to_irq;
> - pci_hotplug_fn hotplug;
> - DeviceState *hotplug_qdev;
> void *irq_opaque;
> PCIDevice *devices[PCI_SLOT_MAX * PCI_FUNC_MAX];
> PCIDevice *parent_dev;
> diff --git a/tests/Makefile b/tests/Makefile
> index 0aaf657..0bbdecb 100644
> --- a/tests/Makefile
> +++ b/tests/Makefile
> @@ -163,7 +163,7 @@ tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o xbzrle.o page_cache.o libqemuuti
> tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o
> tests/test-int128$(EXESUF): tests/test-int128.o
> tests/test-qdev-global-props$(EXESUF): tests/test-qdev-global-props.o \
> - hw/core/qdev.o hw/core/qdev-properties.o \
> + hw/core/qdev.o hw/core/qdev-properties.o hw/core/hotplug.o\
> hw/core/irq.o \
> $(qom-core-obj) \
> $(test-qapi-obj-y) \
> --
> 1.7.1
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH 9/9] hw/pci: switch to a generic hotplug handling for PCIDevice
2014-01-20 11:36 ` Michael S. Tsirkin
@ 2014-01-20 12:45 ` Igor Mammedov
2014-01-20 12:57 ` Paolo Bonzini
0 siblings, 1 reply; 15+ messages in thread
From: Igor Mammedov @ 2014-01-20 12:45 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: peter.crosthwaite, marcel.a, qemu-devel, aliguori, pbonzini,
afaerber
On Mon, 20 Jan 2014 13:36:39 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Tue, Jan 14, 2014 at 05:55:54PM +0100, Igor Mammedov wrote:
> > make qdev_unplug()/device_set_realized() to call hotplug handler's
> > plug/unplug methods if available and remove not needed anymore
> > hot(un)plug handling from PCIDevice.
> >
> > In case if hotplug handler is not available, revert to the legacy
> > hotplug method.
>
> When isn't it available?
> For buses other than PCI?
For example scsi virtio usb buses.
Paolo said he would convert scsi bus to this interface once series is in.
Eventually all hotluggable buses will/should be converted to new interface
so that we could get rid of legacy implementation.
>
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> > v2:
> > * fix test-qdev-global-props build failure during "make check"
> > ---
> > hw/core/qdev.c | 17 +++++++++++++----
> > hw/pci/pci.c | 29 +----------------------------
> > include/hw/pci/pci.h | 10 ----------
> > include/hw/pci/pci_bus.h | 2 --
> > tests/Makefile | 2 +-
> > 5 files changed, 15 insertions(+), 45 deletions(-)
> >
> > diff --git a/hw/core/qdev.c b/hw/core/qdev.c
> > index d8b83f1..3486e5d 100644
> > --- a/hw/core/qdev.c
> > +++ b/hw/core/qdev.c
> > @@ -213,7 +213,6 @@ void qdev_unplug(DeviceState *dev, Error **errp)
> > error_set(errp, QERR_BUS_NO_HOTPLUG, dev->parent_bus->name);
> > return;
> > }
> > - assert(dc->unplug != NULL);
> >
> > if (!dc->hotpluggable) {
> > error_set(errp, QERR_DEVICE_NO_HOTPLUG,
> > @@ -223,9 +222,13 @@ void qdev_unplug(DeviceState *dev, Error **errp)
> >
> > qdev_hot_removed = true;
> >
> > - if (dc->unplug(dev) < 0) {
> > - error_set(errp, QERR_UNDEFINED_ERROR);
> > - return;
> > + if (dev->parent_bus && dev->parent_bus->hotplug_handler) {
> > + hotplug_handler_unplug(dev->parent_bus->hotplug_handler, dev, errp);
> > + } else {
> > + assert(dc->unplug != NULL);
> > + if (dc->unplug(dev) < 0) { /* legacy handler */
> > + error_set(errp, QERR_UNDEFINED_ERROR);
> > + }
> > }
> > }
> >
> > @@ -720,6 +723,12 @@ static void device_set_realized(Object *obj, bool value, Error **err)
> > dc->realize(dev, &local_err);
> > }
> >
> > + if (dev->parent_bus && dev->parent_bus->hotplug_handler &&
> > + local_err == NULL) {
> > + hotplug_handler_plug(dev->parent_bus->hotplug_handler,
> > + dev, &local_err);
> > + }
> > +
> > if (qdev_get_vmsd(dev) && local_err == NULL) {
> > vmstate_register_with_alias_id(dev, -1, qdev_get_vmsd(dev), dev,
> > dev->instance_id_alias,
> > diff --git a/hw/pci/pci.c b/hw/pci/pci.c
> > index b8770ef..7e36f29 100644
> > --- a/hw/pci/pci.c
> > +++ b/hw/pci/pci.c
> > @@ -35,6 +35,7 @@
> > #include "hw/pci/msi.h"
> > #include "hw/pci/msix.h"
> > #include "exec/address-spaces.h"
> > +#include "hw/hotplug.h"
> >
> > //#define DEBUG_PCI
> > #ifdef DEBUG_PCI
> > @@ -346,13 +347,6 @@ void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> > bus->irq_count = g_malloc0(nirq * sizeof(bus->irq_count[0]));
> > }
> >
> > -void pci_bus_hotplug(PCIBus *bus, pci_hotplug_fn hotplug, DeviceState *qdev)
> > -{
> > - bus->qbus.allow_hotplug = 1;
> > - bus->hotplug = hotplug;
> > - bus->hotplug_qdev = qdev;
> > -}
> > -
> > PCIBus *pci_register_bus(DeviceState *parent, const char *name,
> > pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> > void *irq_opaque,
> > @@ -1750,29 +1744,9 @@ static int pci_qdev_init(DeviceState *qdev)
> > }
> > pci_add_option_rom(pci_dev, is_default_rom);
> >
> > - if (bus->hotplug) {
> > - /* Let buses differentiate between hotplug and when device is
> > - * enabled during qemu machine creation. */
> > - rc = bus->hotplug(bus->hotplug_qdev, pci_dev,
> > - qdev->hotplugged ? PCI_HOTPLUG_ENABLED:
> > - PCI_COLDPLUG_ENABLED);
> > - if (rc != 0) {
> > - int r = pci_unregister_device(&pci_dev->qdev);
> > - assert(!r);
> > - return rc;
> > - }
> > - }
> > return 0;
> > }
> >
> > -static int pci_unplug_device(DeviceState *qdev)
> > -{
> > - PCIDevice *dev = PCI_DEVICE(qdev);
> > -
> > - return dev->bus->hotplug(dev->bus->hotplug_qdev, dev,
> > - PCI_HOTPLUG_DISABLED);
> > -}
> > -
> > PCIDevice *pci_create_multifunction(PCIBus *bus, int devfn, bool multifunction,
> > const char *name)
> > {
> > @@ -2243,7 +2217,6 @@ static void pci_device_class_init(ObjectClass *klass, void *data)
> > {
> > DeviceClass *k = DEVICE_CLASS(klass);
> > k->init = pci_qdev_init;
> > - k->unplug = pci_unplug_device;
> > k->exit = pci_unregister_device;
> > k->bus_type = TYPE_PCI_BUS;
> > k->props = pci_props;
> > diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> > index 03d4bee..c709d62 100644
> > --- a/include/hw/pci/pci.h
> > +++ b/include/hw/pci/pci.h
> > @@ -327,15 +327,6 @@ typedef void (*pci_set_irq_fn)(void *opaque, int irq_num, int level);
> > typedef int (*pci_map_irq_fn)(PCIDevice *pci_dev, int irq_num);
> > typedef PCIINTxRoute (*pci_route_irq_fn)(void *opaque, int pin);
> >
> > -typedef enum {
> > - PCI_HOTPLUG_DISABLED,
> > - PCI_HOTPLUG_ENABLED,
> > - PCI_COLDPLUG_ENABLED,
> > -} PCIHotplugState;
> > -
> > -typedef int (*pci_hotplug_fn)(DeviceState *qdev, PCIDevice *pci_dev,
> > - PCIHotplugState state);
> > -
> > #define TYPE_PCI_BUS "PCI"
> > #define PCI_BUS(obj) OBJECT_CHECK(PCIBus, (obj), TYPE_PCI_BUS)
> > #define TYPE_PCIE_BUS "PCIE"
> > @@ -354,7 +345,6 @@ PCIBus *pci_bus_new(DeviceState *parent, const char *name,
> > void pci_bus_irqs(PCIBus *bus, pci_set_irq_fn set_irq, pci_map_irq_fn map_irq,
> > void *irq_opaque, int nirq);
> > int pci_bus_get_irq_level(PCIBus *bus, int irq_num);
> > -void pci_bus_hotplug(PCIBus *bus, pci_hotplug_fn hotplug, DeviceState *dev);
> > /* 0 <= pin <= 3 0 = INTA, 1 = INTB, 2 = INTC, 3 = INTD */
> > int pci_swizzle_map_irq_fn(PCIDevice *pci_dev, int pin);
> > PCIBus *pci_register_bus(DeviceState *parent, const char *name,
> > diff --git a/include/hw/pci/pci_bus.h b/include/hw/pci/pci_bus.h
> > index 9df1788..fabaeee 100644
> > --- a/include/hw/pci/pci_bus.h
> > +++ b/include/hw/pci/pci_bus.h
> > @@ -16,8 +16,6 @@ struct PCIBus {
> > pci_set_irq_fn set_irq;
> > pci_map_irq_fn map_irq;
> > pci_route_irq_fn route_intx_to_irq;
> > - pci_hotplug_fn hotplug;
> > - DeviceState *hotplug_qdev;
> > void *irq_opaque;
> > PCIDevice *devices[PCI_SLOT_MAX * PCI_FUNC_MAX];
> > PCIDevice *parent_dev;
> > diff --git a/tests/Makefile b/tests/Makefile
> > index 0aaf657..0bbdecb 100644
> > --- a/tests/Makefile
> > +++ b/tests/Makefile
> > @@ -163,7 +163,7 @@ tests/test-xbzrle$(EXESUF): tests/test-xbzrle.o xbzrle.o page_cache.o libqemuuti
> > tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o
> > tests/test-int128$(EXESUF): tests/test-int128.o
> > tests/test-qdev-global-props$(EXESUF): tests/test-qdev-global-props.o \
> > - hw/core/qdev.o hw/core/qdev-properties.o \
> > + hw/core/qdev.o hw/core/qdev-properties.o hw/core/hotplug.o\
> > hw/core/irq.o \
> > $(qom-core-obj) \
> > $(test-qapi-obj-y) \
> > --
> > 1.7.1
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH 9/9] hw/pci: switch to a generic hotplug handling for PCIDevice
2014-01-20 12:45 ` Igor Mammedov
@ 2014-01-20 12:57 ` Paolo Bonzini
0 siblings, 0 replies; 15+ messages in thread
From: Paolo Bonzini @ 2014-01-20 12:57 UTC (permalink / raw)
To: Igor Mammedov
Cc: peter.crosthwaite, marcel.a, Michael S. Tsirkin, qemu-devel,
aliguori, afaerber
Il 20/01/2014 13:45, Igor Mammedov ha scritto:
> On Mon, 20 Jan 2014 13:36:39 +0200
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
>
>> On Tue, Jan 14, 2014 at 05:55:54PM +0100, Igor Mammedov wrote:
>>> make qdev_unplug()/device_set_realized() to call hotplug handler's
>>> plug/unplug methods if available and remove not needed anymore
>>> hot(un)plug handling from PCIDevice.
>>>
>>> In case if hotplug handler is not available, revert to the legacy
>>> hotplug method.
>>
>> When isn't it available?
>> For buses other than PCI?
> For example scsi virtio usb buses.
> Paolo said he would convert scsi bus to this interface once series is in.
>
> Eventually all hotluggable buses will/should be converted to new interface
> so that we could get rid of legacy implementation.
There are just 2 interesting implementations apart from PCI and SCSI:
hw/ide/piix.c: dc->unplug = pci_piix3_xen_ide_unplug;
hw/pci/pci.c: k->unplug = pci_unplug_device;
hw/s390x/virtio-ccw.c: dc->unplug = virtio_ccw_busdev_unplug;
hw/scsi/scsi-bus.c: k->unplug = scsi_qdev_unplug;
Then, all that remains is these four:
hw/char/virtio-serial-bus.c: k->unplug = qdev_simple_unplug_cb;
hw/s390x/event-facility.c: dc->unplug = qdev_simple_unplug_cb;
hw/s390x/s390-virtio-bus.c: dc->unplug = qdev_simple_unplug_cb;
hw/usb/bus.c: k->unplug = qdev_simple_unplug_cb;
but we can just inline qdev_simple_unplug_cb in the caller.
Paolo
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 0/9 v4] Refactor PCI/SHPC/PCIE hotplug to use a more generic hotplug API
2014-01-14 16:55 [Qemu-devel] [PATCH 0/9 v4] Refactor PCI/SHPC/PCIE hotplug to use a more generic hotplug API Igor Mammedov
` (8 preceding siblings ...)
2014-01-14 16:55 ` [Qemu-devel] [PATCH 9/9] hw/pci: switch to a generic hotplug handling for PCIDevice Igor Mammedov
@ 2014-01-16 9:33 ` Michael S. Tsirkin
2014-01-16 9:38 ` Igor Mammedov
9 siblings, 1 reply; 15+ messages in thread
From: Michael S. Tsirkin @ 2014-01-16 9:33 UTC (permalink / raw)
To: Igor Mammedov
Cc: peter.crosthwaite, marcel.a, qemu-devel, aliguori, pbonzini,
afaerber
On Tue, Jan 14, 2014 at 05:55:45PM +0100, Igor Mammedov wrote:
> changes since v3:
> - fixup/add comments as reqused by Peter Crosthwaite
> - use error_abort to reduce error handling verbosity
> - fix tests/test-qdev-global-props build failure on make check
> - rebase on top of current master:133fe7743 (with interface fixes)
>
> Reference to previous version:
> http://lists.gnu.org/archive/html/qemu-devel/2013-12/msg02461.html
Please add actual content to cover letters.
I see it was there in the previous version.
Also pl use --subject-prefix or (with recent git) -v flag so that all
patches are versioned, not just the cover letter.
> git tree for testing:
> https://github.com/imammedo/qemu/commits/hotplug_dev_inf_v4
>
> Igor Mammedov (9):
> define hotplug interface
> qdev: add to BusState "hotplug-handler" link
> qdev: add "hotpluggable" property to Device
> hw/acpi: move typeinfo to the file end
> qdev:pci: refactor PCIDevice to use generic "hotpluggable" property
> acpi/piix4pm: convert ACPI PCI hotplug to use hotplug-handler API
> pci/shpc: convert SHPC hotplug to use hotplug-handler API
> pci/pcie: convert PCIE hotplug to use hotplug-handler API
> hw/pci: switch to a generic hotplug handling for PCIDevice
>
> hw/acpi/piix4.c | 156 ++++++++++++++++++++--------------------
> hw/core/Makefile.objs | 1 +
> hw/core/hotplug.c | 48 ++++++++++++
> hw/core/qdev.c | 50 ++++++++++++-
> hw/display/cirrus_vga.c | 2 +-
> hw/display/qxl.c | 2 +-
> hw/display/vga-pci.c | 2 +-
> hw/display/vmware_vga.c | 2 +-
> hw/i386/acpi-build.c | 6 +-
> hw/ide/piix.c | 4 +-
> hw/isa/piix4.c | 2 +-
> hw/pci-bridge/pci_bridge_dev.c | 9 +++
> hw/pci-host/piix.c | 6 +-
> hw/pci/pci.c | 40 +----------
> hw/pci/pcie.c | 67 +++++++++++-------
> hw/pci/pcie_port.c | 8 ++
> hw/pci/shpc.c | 127 ++++++++++++++++++++------------
> hw/usb/hcd-ehci-pci.c | 2 +-
> hw/usb/hcd-ohci.c | 2 +-
> hw/usb/hcd-uhci.c | 2 +-
> hw/usb/hcd-xhci.c | 2 +-
> include/hw/hotplug.h | 78 ++++++++++++++++++++
> include/hw/pci/pci.h | 13 ----
> include/hw/pci/pci_bus.h | 2 -
> include/hw/pci/pcie.h | 5 ++
> include/hw/pci/shpc.h | 8 ++
> include/hw/qdev-core.h | 8 ++
> tests/Makefile | 2 +-
> 28 files changed, 432 insertions(+), 224 deletions(-)
> create mode 100644 hw/core/hotplug.c
> create mode 100644 include/hw/hotplug.h
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [Qemu-devel] [PATCH 0/9 v4] Refactor PCI/SHPC/PCIE hotplug to use a more generic hotplug API
2014-01-16 9:33 ` [Qemu-devel] [PATCH 0/9 v4] Refactor PCI/SHPC/PCIE hotplug to use a more generic hotplug API Michael S. Tsirkin
@ 2014-01-16 9:38 ` Igor Mammedov
0 siblings, 0 replies; 15+ messages in thread
From: Igor Mammedov @ 2014-01-16 9:38 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: peter.crosthwaite, marcel.a, qemu-devel, aliguori, pbonzini,
afaerber
On Thu, 16 Jan 2014 11:33:37 +0200
"Michael S. Tsirkin" <mst@redhat.com> wrote:
> On Tue, Jan 14, 2014 at 05:55:45PM +0100, Igor Mammedov wrote:
> > changes since v3:
> > - fixup/add comments as reqused by Peter Crosthwaite
> > - use error_abort to reduce error handling verbosity
> > - fix tests/test-qdev-global-props build failure on make check
> > - rebase on top of current master:133fe7743 (with interface fixes)
> >
> > Reference to previous version:
> > http://lists.gnu.org/archive/html/qemu-devel/2013-12/msg02461.html
>
> Please add actual content to cover letters.
> I see it was there in the previous version.
I'll do it further on.
> Also pl use --subject-prefix or (with recent git) -v flag so that all
> patches are versioned, not just the cover letter.
ok, thanks for a tip.
>
> > git tree for testing:
> > https://github.com/imammedo/qemu/commits/hotplug_dev_inf_v4
> >
> > Igor Mammedov (9):
> > define hotplug interface
> > qdev: add to BusState "hotplug-handler" link
> > qdev: add "hotpluggable" property to Device
> > hw/acpi: move typeinfo to the file end
> > qdev:pci: refactor PCIDevice to use generic "hotpluggable" property
> > acpi/piix4pm: convert ACPI PCI hotplug to use hotplug-handler API
> > pci/shpc: convert SHPC hotplug to use hotplug-handler API
> > pci/pcie: convert PCIE hotplug to use hotplug-handler API
> > hw/pci: switch to a generic hotplug handling for PCIDevice
> >
> > hw/acpi/piix4.c | 156 ++++++++++++++++++++--------------------
> > hw/core/Makefile.objs | 1 +
> > hw/core/hotplug.c | 48 ++++++++++++
> > hw/core/qdev.c | 50 ++++++++++++-
> > hw/display/cirrus_vga.c | 2 +-
> > hw/display/qxl.c | 2 +-
> > hw/display/vga-pci.c | 2 +-
> > hw/display/vmware_vga.c | 2 +-
> > hw/i386/acpi-build.c | 6 +-
> > hw/ide/piix.c | 4 +-
> > hw/isa/piix4.c | 2 +-
> > hw/pci-bridge/pci_bridge_dev.c | 9 +++
> > hw/pci-host/piix.c | 6 +-
> > hw/pci/pci.c | 40 +----------
> > hw/pci/pcie.c | 67 +++++++++++-------
> > hw/pci/pcie_port.c | 8 ++
> > hw/pci/shpc.c | 127 ++++++++++++++++++++------------
> > hw/usb/hcd-ehci-pci.c | 2 +-
> > hw/usb/hcd-ohci.c | 2 +-
> > hw/usb/hcd-uhci.c | 2 +-
> > hw/usb/hcd-xhci.c | 2 +-
> > include/hw/hotplug.h | 78 ++++++++++++++++++++
> > include/hw/pci/pci.h | 13 ----
> > include/hw/pci/pci_bus.h | 2 -
> > include/hw/pci/pcie.h | 5 ++
> > include/hw/pci/shpc.h | 8 ++
> > include/hw/qdev-core.h | 8 ++
> > tests/Makefile | 2 +-
> > 28 files changed, 432 insertions(+), 224 deletions(-)
> > create mode 100644 hw/core/hotplug.c
> > create mode 100644 include/hw/hotplug.h
^ permalink raw reply [flat|nested] 15+ messages in thread