Currently the property may flip its state
during VM bring up or just doesn't work as
the name implies.
In particular with PCIE root port that has
'hotplug={on|off}' property, and when it's
turned off, one would expect
'hotpluggable' == false
for any devices attached to it.
Which is not the case since qbus_is_hotpluggable()
used by the property just checks for presence
of any hotplug_handler set on bus.
The problem is that name BusState::hotplug_handler
from its inception is misnomer, as it handles
not only hotplug but also in many cases coldplug
as well (i.e. generic wiring interface), and
it's fine to have hotplug_handler set on bus
while it doesn't support hotplug (ex. pcie-slot
with hotplug=off).
Another case of root port flipping 'hotpluggable'
state when ACPI PCI hotplug is enabled in this
case root port with 'hotplug=off' starts as
hotpluggable and then later on, pcihp
hotplug_handler clears hotplug_handler
explicitly after checking root port's 'hotplug'
property.
So root-port hotpluggablity check sort of works
if pcihp is enabled but is broken if pcihp is
disabled.
One way to deal with the issue is to ask
hotplug_handler if bus it controls is hotpluggable
or not. To do that add is_hotpluggable_bus()
hook to HotplugHandler interface and use it in
'hotpluggable' property + teach pcie-slot to
actually look into 'hotplug' property state
before deciding if bus is hotpluggable.
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
include/hw/hotplug.h | 2 ++
include/hw/qdev-core.h | 13 ++++++++++++-
hw/pci/pcie_port.c | 8 ++++++++
3 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/include/hw/hotplug.h b/include/hw/hotplug.h
index e15f59c8b3..a9840ed485 100644
--- a/include/hw/hotplug.h
+++ b/include/hw/hotplug.h
@@ -48,6 +48,7 @@ typedef void (*hotplug_fn)(HotplugHandler *plug_handler,
* @unplug: unplug callback.
* Used for device removal with devices that implement
* asynchronous and synchronous (surprise) removal.
+ * @is_hotpluggable_bus: called to check if bus/its parent allow hotplug on bus
*/
struct HotplugHandlerClass {
/* <private> */
@@ -58,6 +59,7 @@ struct HotplugHandlerClass {
hotplug_fn plug;
hotplug_fn unplug_request;
hotplug_fn unplug;
+ bool (*is_hotpluggable_bus)(HotplugHandler *plug_handler, BusState *bus);
};
/**
diff --git a/include/hw/qdev-core.h b/include/hw/qdev-core.h
index 35fddb19a6..3b3518146b 100644
--- a/include/hw/qdev-core.h
+++ b/include/hw/qdev-core.h
@@ -812,7 +812,18 @@ void qbus_set_bus_hotplug_handler(BusState *bus);
static inline bool qbus_is_hotpluggable(BusState *bus)
{
- return bus->hotplug_handler;
+ HotplugHandler *plug_handler = bus->hotplug_handler;
+ bool ret = !!plug_handler;
+
+ if (plug_handler) {
+ HotplugHandlerClass *hdc;
+
+ hdc = HOTPLUG_HANDLER_GET_CLASS(plug_handler);
+ if (hdc->is_hotpluggable_bus) {
+ ret = hdc->is_hotpluggable_bus(plug_handler, bus);
+ }
+ }
+ return ret;
}
/**
diff --git a/hw/pci/pcie_port.c b/hw/pci/pcie_port.c
index 65a397ad23..000633fec1 100644
--- a/hw/pci/pcie_port.c
+++ b/hw/pci/pcie_port.c
@@ -161,6 +161,13 @@ PCIDevice *pcie_find_port_by_pn(PCIBus *bus, uint8_t pn)
return NULL;
}
+static bool pcie_slot_is_hotpluggbale_bus(HotplugHandler *plug_handler,
+ BusState *bus)
+{
+ PCIESlot *s = PCIE_SLOT(bus->parent);
+ return s->hotplug;
+}
+
static const TypeInfo pcie_port_type_info = {
.name = TYPE_PCIE_PORT,
.parent = TYPE_PCI_BRIDGE,
@@ -188,6 +195,7 @@ static void pcie_slot_class_init(ObjectClass *oc, void *data)
hc->plug = pcie_cap_slot_plug_cb;
hc->unplug = pcie_cap_slot_unplug_cb;
hc->unplug_request = pcie_cap_slot_unplug_request_cb;
+ hc->is_hotpluggable_bus = pcie_slot_is_hotpluggbale_bus;
}
static const TypeInfo pcie_slot_type_info = {
--
2.39.1