[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PULL 38/73] pci: fix 'hotplugglable' property behavior
From: |
Michael S. Tsirkin |
Subject: |
[PULL 38/73] pci: fix 'hotplugglable' property behavior |
Date: |
Tue, 7 Mar 2023 20:12:43 -0500 |
From: Igor Mammedov <imammedo@redhat.com>
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>
Message-Id: <20230302161543.286002-13-imammedo@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@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 f5b3b2f89a..bd50ad5ee1 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 = {
--
MST
- [PULL 30/73] tests: acpi: update expected blobs, (continued)
- [PULL 30/73] tests: acpi: update expected blobs, Michael S. Tsirkin, 2023/03/07
- [PULL 25/73] vdpa net: allow VHOST_F_LOG_ALL, Michael S. Tsirkin, 2023/03/07
- [PULL 27/73] Revert "tests/qtest: Check for devices in bios-tables-test", Michael S. Tsirkin, 2023/03/07
- [PULL 31/73] tests: acpi: whitelist q35/DSDT.multi-bridge before extending testcase, Michael S. Tsirkin, 2023/03/07
- [PULL 33/73] x86: pcihp: fix missing PCNT callchain when intermediate root-port has 'hotplug=off' set, Michael S. Tsirkin, 2023/03/07
- [PULL 32/73] tests: acpi: extend multi-bridge case with case 'root-port,id=HOHP,hotplug=off root-port,bus=NOHP', Michael S. Tsirkin, 2023/03/07
- [PULL 34/73] tests: acpi: whitelist pc/DSDT.hpbrroot and pc/DSDT.hpbridge tests, Michael S. Tsirkin, 2023/03/07
- [PULL 35/73] x86: pcihp: fix missing bridge AML when intermediate root-port has 'hotplug=off' set, Michael S. Tsirkin, 2023/03/07
- [PULL 36/73] tests: acpi: update expected blobs, Michael S. Tsirkin, 2023/03/07
- [PULL 37/73] pcihp: piix4: do not redirect hotplug controller to piix4 when ACPI hotplug is disabled, Michael S. Tsirkin, 2023/03/07
- [PULL 38/73] pci: fix 'hotplugglable' property behavior,
Michael S. Tsirkin <=
- [PULL 40/73] pcihp: move PCI _DSM function 0 prolog into separate function, Michael S. Tsirkin, 2023/03/07
- [PULL 39/73] tests: acpi: whitelist DSDT blobs before isolating PCI _DSM func 0 prolog, Michael S. Tsirkin, 2023/03/07
- [PULL 41/73] tests: acpi: update expected blobs, Michael S. Tsirkin, 2023/03/07
- [PULL 42/73] tests: acpi: whitelist DSDT before adding EDSM method, Michael S. Tsirkin, 2023/03/07
- [PULL 43/73] acpi: pci: add EDSM method to DSDT, Michael S. Tsirkin, 2023/03/07
- [PULL 44/73] tests: acpi: update expected blobs, Michael S. Tsirkin, 2023/03/07
- [PULL 45/73] tests: acpi: whitelist DSDT before adding device with acpi-index to testcases, Michael S. Tsirkin, 2023/03/07
- [PULL 46/73] tests: acpi: add device with acpi-index on non-hotpluggble bus, Michael S. Tsirkin, 2023/03/07
- [PULL 47/73] acpi: pci: support acpi-index for non-hotpluggable devices, Michael S. Tsirkin, 2023/03/07
- [PULL 48/73] tests: acpi: update expected blobs, Michael S. Tsirkin, 2023/03/07